Re: [PATCH 2/2] powernv:idle:Implement lite variant of power_enter_stop

[Gautham R Shenoy <ego@linux.vnet.ibm.com>]

Hi Balbir,

On Tue, Sep 20, 2016 at 03:54:43PM +1000, Balbir Singh wrote:
> > diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> > index 479c256..c3d3fed 100644
> > --- a/arch/powerpc/platforms/powernv/idle.c
> > +++ b/arch/powerpc/platforms/powernv/idle.c
> > @@ -244,8 +244,15 @@ static DEVICE_ATTR(fastsleep_workaround_applyonce, 0600,
> >  static void power9_idle(void)
> >  {
> >  	/* Requesting stop state 0 */
> > -	power9_idle_stop(0);
> > +	power9_idle_stop(0, 0);
> >  }
> > +
> > +static void power9_idle_lite(void)
> > +{
> > +	/* Requesting stop state 0 with ESL=EC=0 */
> > +	power9_idle_stop(0, 1);
> > +}
> > +
> >  /*
> >   * First deep stop state. Used to figure out when to save/restore
> >   * hypervisor context.
> > @@ -414,8 +421,12 @@ static int __init pnv_init_idle_states(void)
> >  
> >  	if (supported_cpuidle_states & OPAL_PM_NAP_ENABLED)
> >  		ppc_md.power_save = power7_idle;
> > -	else if (supported_cpuidle_states & OPAL_PM_STOP_INST_FAST)
> > -		ppc_md.power_save = power9_idle;
> > +	else if (supported_cpuidle_states & OPAL_PM_STOP_INST_FAST) {
> > +		if (supported_cpuidle_states & OPAL_PM_WAKEUP_AT_NEXT_INST)
> > +			ppc_md.power_save = power9_idle_lite;
> > +		else
> > +			ppc_md.power_save = power9_idle;
> > +	}
> 
> If I am reading this correctly we decide at boot time whether we support
> wakeup at next instruction and make that the default sleep state.
> I am a little surprised that these are exclusive. I was expecting
> power9_idle_lite to be one of the states to go into before
> power9_idle

At boot time, we initialize ppc_md.power_save to
power9_idle/power9_idle_lite which ends up being the default idle
function in the absence of the cpuidle subsystem. When cpuidle is
available, idle code will call cpuidle governors which will determine
the appropriate idle state that can be entered into. Each of these
idle states has an associated callback function. In case of the
idle-states without OPAL_PM_STOP_INST_FAST associated with them, the
callback is stop_loop() and when the flag is set, the callback
function is stop_lite_loop().  So when cpuidle is present, these
states are not exclusive.

Note that both power9_idle() and power9_idle_lite() call stop0. Just
that the former executes stop0 with ESL=EC=1 and latter with ESL=EC=0.

That said, you're right that in the absence of the cpuidle governor,
if the lite variant of stop is advertised by the firmware through the
device-tree, we end up picking the liter version of stop0 as the
default idle state. Do you suggest that we retain power9_idle which
calls stop0 with ESL=EC=1 ? 

> Balbir Singh.
> 
--
Thanks and Regards
gautham.