[Skiboot] [PATCH] occ: Poll OCC throttle status and queue OCC events to host

[Preeti U Murthy]

Hi Shilpa,

On 04/22/2015 10:04 PM, Shilpasri G Bhat wrote:
> Add a new class of message definition OPAL_MSG_OCC to
> opal_message_type to notify the following OCC events to host:
> 1) OCC Reset
> 2) OCC Load
> 3) OCC Throttle Status Change
> 
> Add an opal poller to periodically read throttle status updated by OCC
> for each chip and notify any change in throttle status to host. The
> throttle status indicates the reason why OCC may have limited the max
> Pstate of the chip.
> 
> Signed-off-by: Shilpasri G Bhat <shilpa.bhat at linux.vnet.ibm.com>
> ---
>  hw/occ.c           | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/chip.h     |  1 +
>  include/opal-api.h |  7 +++++
>  3 files changed, 93 insertions(+)
> 
> diff --git a/hw/occ.c b/hw/occ.c
> index 34d6de5..d3711b2 100644
> --- a/hw/occ.c
> +++ b/hw/occ.c
> @@ -24,6 +24,8 @@
>  #include <timebase.h>
>  #include <hostservices.h>
>  #include <errorlog.h>
> +#include <opal-api.h>
> +#include <opal-msg.h>
> 
>  /* OCC Communication Area for PStates */
> 
> @@ -31,6 +33,16 @@
> 
>  #define MAX_PSTATES 256
> 
> +#define OCC_RESET	0
> +#define OCC_LOAD	1
> +#define OCC_THROTTLE	2
> +
> +#define chip_occ_data(chip) \
> +		((struct occ_pstate_table *)(chip->homer_base + \
> +				P8_HOMER_SAPPHIRE_DATA_OFFSET))
> +
> +static bool occ_reset;
> +
>  struct occ_pstate_entry {
>  	s8 id;
>  	u8 flags;
> @@ -302,6 +314,58 @@ static bool cpu_pstates_prepare_core(struct proc_chip *chip, struct cpu_thread *
>  	return true;
>  }
> 
> +/* occ_throttle_poll: This function will queue a meassage of type
> + * OPAL_MSG_OCC to notify any change in the throttle status of the
> + * chip. Throttle status indicates the reason why OCC may have limited
> + * the max Pstate of the chip.
> + * 0x00 = No throttle
> + * 0x01 = Power Cap
> + * 0x02 = Processor Over Temperature
> + * 0x03 = Power Supply Failure (currently not used)
> + * 0x04 = Over current (currently not used)
> + * 0x05 = OCC Reset (not reliable as some failures will not allow for
> + * OCC to update throttle status, so use 'occ_reset')
> + */
> +static void occ_throttle_poll(void *data __unused)
> +{
> +	struct proc_chip *chip;
> +	struct occ_pstate_table *occ_data;
> +
> +	if (occ_reset) {
> +		int inactive = 0;
> +
> +		for_each_chip(chip) {
> +			occ_data = chip_occ_data(chip);
> +			if (occ_data->valid != 1) {
> +				inactive = 1;
> +				break;
> +			} else if (!occ_data->throttle) {
> +				chip->prev_throttle = occ_data->throttle;

Why do you do this ?                 ^^^^

Why cant we initialize prev_throttle to 0x00 - No throttle at boot and
on each OCC reset ? I did not see the point of introducing another value
to initialize prev_throttle.

> +			}
> +		}
> +		if (!inactive) {
> +			occ_reset = false;
> +			/*
> +			 * Queue OCC_THROTTLE with throttle status as 0 to
> +			 * indicate all OCCs are active after a reset.
> +			 */
> +			opal_queue_msg(OPAL_MSG_OCC, NULL, NULL,
> +						OCC_THROTTLE, 0, 0);

Note that opal_queue_msg() is expected to fail. So we should handle it.
Refer to the other call sites to get an idea. Should we do something
like cancel the poller if the queuing of messages fail ?

Regards
Preeti U Murthy