Re: [PATCH v1] platform/x86: intel_pmc_core: promote S0ix failure warn() to WARN()

["Limonciello, Mario" <mario.limonciello@amd.com>]

On 10/31/2022 15:47, Rajat Jain wrote:
> Hello,
> 
> On Mon, Oct 31, 2022 at 12:39 PM Limonciello, Mario
> <Mario.Limonciello@amd.com <mailto:Mario.Limonciello@amd.com>> wrote:
>  >
>  > [Public]
>  >
>  > > -----Original Message-----
>  > > From: Sven van Ashbrook <svenva@chromium.org 
> <mailto:svenva@chromium.org>>
>  > > Sent: Friday, October 28, 2022 23:12
>  > > To: Rajneesh Bhardwaj <irenic.rajneesh@gmail.com 
> <mailto:irenic.rajneesh@gmail.com>>; Hans de Goede
>  > > <hdegoede@redhat.com <mailto:hdegoede@redhat.com>>
>  > > Cc: Limonciello, Mario <Mario.Limonciello@amd.com 
> <mailto:Mario.Limonciello@amd.com>>; LKML <linux-
>  > > kernel@vger.kernel.org <mailto:kernel@vger.kernel.org>>; S-k, 
> Shyam-sundar <Shyam-sundar.S-
>  > > k@amd.com <mailto:k@amd.com>>; rrangel@chromium.org 
> <mailto:rrangel@chromium.org>; platform-driver-
>  > > x86@vger.kernel.org <mailto:x86@vger.kernel.org>; Rajneesh Bhardwaj 
> <rajneesh.bhardwaj@intel.com <mailto:rajneesh.bhardwaj@intel.com>>;
>  > > Rafael J Wysocki <rjw@rjwysocki.net <mailto:rjw@rjwysocki.net>>; 
> Rajat Jain <rajatja@google.com <mailto:rajatja@google.com>>;
>  > > David E Box <david.e.box@intel.com <mailto:david.e.box@intel.com>>; 
> Mark Gross <markgross@kernel.org <mailto:markgross@kernel.org>>
>  > > Subject: Re: [PATCH v1] platform/x86: intel_pmc_core: promote S0ix 
> failure
>  > > warn() to WARN()
>  > >
>  > > On Thu, Oct 27, 2022 at 12:02 PM Rajneesh Bhardwaj
>  > > <irenic.rajneesh@gmail.com <mailto:irenic.rajneesh@gmail.com>> wrote:
>  > > > I'd advise against this promotion based on my experience with 
> S0ix entry
>  > > failures.
>  > >
>  > > On Thu, Oct 27, 2022 at 11:40 AM Hans de Goede <hdegoede@redhat.com 
> <mailto:hdegoede@redhat.com>>
>  > > wrote:
>  > > > I'm not a fan of the change you are suggesting here.
>  > >
>  > > Thanks everyone for the feedback. Looks like there is consensus 
> that it's
>  > > not advisable to promote the warning. We will move forward with 
> changes to
>  > > our monitoring infrastructure instead.
>  >
>  > Did you see the idea proposed by David Box to introduce some 
> infrastructure in
>  > the kernel for this?
>  >
>  > Just thinking about it a little bit more, it could be a lot nicer to 
> have something like:
>  >
>  > /sys/power/suspend_stats/last_hw_deepest_state
>  >
>  > During the resume process drivers such as amd_pmc and intel_pmc_core 
> could
>  > read the appropriate values for the hardware and call a function that 
> would
>  > populate it with either a "0" or "1" or maybe even the amount of time 
> spent in
>  > that state.
>  >
>  > We could then retire the debugging messages from both drivers
> 
> I do not think we should retire the debug messages. The sysfs
> attribute could help us *trigger* a failure detection, but we would
> still need these debug logs to actually determine why exactly we did
> not go into the S0ix / deepest power state (And the debug messages
> print out the debug register bit fields that let us know that).
> 
> Thanks,
> 

I just spun together an RFC series for this idea and while doing it I 
had the same realization.  So I left the warning messages in place for 
both drivers.

You can take a look at the series here:

https://lore.kernel.org/platform-driver-x86/20221031204320.22464-1-mario.limonciello@amd.com/T/#m6c7db55c98b8a3ce8c48d451fc01c1d9b0df37fb

Thanks,