Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
MIME-Version: 1.0
References: <20231022055221.569634-1-yu.c.chen@intel.com>
In-Reply-To: <20231022055221.569634-1-yu.c.chen@intel.com>
From:   Len Brown <lenb@kernel.org>
Date:   Mon, 27 Nov 2023 20:10:50 -0500
Message-ID: <CAJvTdKkFLTzWdU7bssUSJkTW4ocfKcFEu5c+8m=EVNZ44+iXug@mail.gmail.com>
Subject: Re: [RFC PATCH] tools/power turbostat: Do not print negative LPI residency
To:     Chen Yu <yu.c.chen@intel.com>
Cc:     linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        Todd Brandt <todd.e.brandt@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

BIOS bugs:-(

I agree that printing 0 is an improvement over printing an insane
negative number.

But printing 0 suggests that there was no residency, and that could be
misleading...

Maybe we should output some kind of warning about the broken BIOS?

On Sun, Oct 22, 2023 at 1:53 AM Chen Yu <yu.c.chen@intel.com> wrote:
>
> turbostat prints the abnormal SYS%LPI across suspend-to-idle:
> SYS%LPI = 114479815993277.50
>
> This is reproduced by:
> Run a freeze cycle, e.g. "sleepgraph -m freeze -rtcwake 15".
> Then do a reboot. After boot up, launch the suspend-idle-idle
> and check the SYS%LPI field.
>
> The slp_so residence counter is in LPIT table, and BIOS does not
> clears this register across reset. The PMC expects the OS to calculate
> the LPI residency based on the delta. However, there is an firmware
> issue that the LPIT gets cleared to 0 during the second suspend
> to idle after the reboot, which brings negative delta value.
>
> Prints a simple 0 to indicate this error to not confuse the user.
>
> Reported-by: Todd Brandt <todd.e.brandt@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
>  tools/power/x86/turbostat/turbostat.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> index 9a10512e3407..3fa5f9a0218a 100644
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -1472,8 +1472,16 @@ int delta_package(struct pkg_data *new, struct pkg_data *old)
>         old->pc8 = new->pc8 - old->pc8;
>         old->pc9 = new->pc9 - old->pc9;
>         old->pc10 = new->pc10 - old->pc10;
> -       old->cpu_lpi = new->cpu_lpi - old->cpu_lpi;
> -       old->sys_lpi = new->sys_lpi - old->sys_lpi;
> +       if (new->cpu_lpi > old->cpu_lpi) {
> +               old->cpu_lpi = new->cpu_lpi - old->cpu_lpi;
> +       } else {
> +               old->cpu_lpi = 0;
> +       }
> +       if (new->sys_lpi > old->sys_lpi) {
> +               old->sys_lpi = new->sys_lpi - old->sys_lpi;
> +       } else {
> +               old->sys_lpi = 0;
> +       }
>         old->pkg_temp_c = new->pkg_temp_c;
>
>         /* flag an error when rc6 counter resets/wraps */
> --
> 2.25.1
>


-- 
Len Brown, Intel