From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932404AbcITIcx (ORCPT ); Tue, 20 Sep 2016 04:32:53 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:39353 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750730AbcITIct (ORCPT ); Tue, 20 Sep 2016 04:32:49 -0400 Date: Tue, 20 Sep 2016 14:02:36 +0530 From: Gautham R Shenoy To: Balbir Singh Cc: "Gautham R. Shenoy" , linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, Vaidyanathan Srinivasan , Michael Ellerman , "Shreyas B. Prabhu" , Michael Neuling , Shilpasri G Bhat Subject: Re: [PATCH 2/2] powernv:idle:Implement lite variant of power_enter_stop Reply-To: ego@linux.vnet.ibm.com References: <86d28da16d5bf66f111cc93eea3f000e41faff57.1474015799.git.ego@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16092008-0012-0000-0000-000010B2BF55 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00005790; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000185; SDB=6.00759412; UDB=6.00360975; IPR=6.00533707; BA=6.00004739; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00012723; XFM=3.00000011; UTC=2016-09-20 08:32:46 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 16092008-0013-0000-0000-0000459F9650 Message-Id: <20160920083236.GA22328@in.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-09-20_03:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1609020000 definitions=main-1609200109 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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.