[Skiboot] [PATCH v2] SLW: Remove stop1_lite and stop2_lite

[Stewart Smith]

Balbir Singh <bsingharora at gmail.com> writes:
> On Mon, May 28, 2018 at 10:35 AM, Balbir Singh <bsingharora at gmail.com> wrote:

>> On 25/05/18 14:36, Vaidyanathan Srinivasan wrote:
>> > * Balbir Singh <bsingharora at gmail.com> [2018-05-23 21:37:44]:
>> >
>> >> On Wed, May 23, 2018 at 6:17 PM, Akshay Adiga
>> >> <akshay.adiga at linux.vnet.ibm.com> wrote:
>> >>> stop1_lite has been removed since it adds no additional benefit
>> >>> over stop0_lite. stop2_lite has been removed since currently it adds
>> >>> minimal benefit over stop2. However, the benefit is eclipsed by the time
>> >>> required to ungate the clocks
>> >>>
>> >>> Moreover, Lite states don't give up the SMT resources, can potentially
>> >>> have a performance impact on sibling threads.
>> >>>
>> >>> Signed-off-by: Akshay Adiga <akshay.adiga at linux.vnet.ibm.com>
>> >>> ---
>> >>>  hw/slw.c | 36 ++++++++----------------------------
>> >>>  1 file changed, 8 insertions(+), 28 deletions(-)
>> >>>
>> >>> diff --git a/hw/slw.c b/hw/slw.c
>> >>> index 2b305db..ff6e2a4 100644
>> >>> --- a/hw/slw.c
>> >>> +++ b/hw/slw.c
>> >>> @@ -529,20 +529,9 @@ static struct cpu_idle_states power9_cpu_idle_states[] = {
>> >>>                                  | OPAL_PM_PSSCR_ESL \
>> >>>                                  | OPAL_PM_PSSCR_EC,
>> >>>                 .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK },
>> >>> -       {
>> >>> -               .name = "stop1_lite", /* Enter stop1 with no state loss */
>> >>> -               .latency_ns = 4900,
>> >>> -               .residency_ns = 49000,
>> >>
>> >> How did we come up with the latency and residency values for
>> >> stop1_lite, stop2_lite? It sounds like the values are roughtly 2% of
>> >> the corresponding non lite states? Are these values empirical,
>> >> guesses, retrofitted to the OS?
>> >
>> > These values are based on real latency, but bumped up to fit into
>> > kernel with sufficient space among other stop states to make an
>> > independent selection. It is a mix of measurements and heuristics.
>> > This patch reduces the number of stop states so that we keep the ones
>> > that make a difference. This will help us tune these number better.
>> >
>>
>> OK, so the next question is why did we take out the stop lite states and not
>> the stop states? Is there a huge saving with the actual stop states compared
>> to the lite states? IOW, can we get similar numbers without the state loss?
>
> Never mind, I just re-read the changelog about SMT resources

Yeah, and it turns out we should both inform the kernel about that *and*
fix the kernel to take it into consideration.

-- 
Stewart Smith
OPAL Architect, IBM.