From: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
To: xry111@mengyan1223.wang, rui.zhang@intel.com,
daniel.lezcano@linaro.org, rjw@rjwysocki.net,
viresh.kumar@linaro.org, lenb@kernel.org
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] cpufreq: intel_pstate: Process HWP Guaranteed change notification
Date: Tue, 07 Sep 2021 18:54:05 -0700 [thread overview]
Message-ID: <7fa77e2239dd87004319a799dbea39988b718233.camel@linux.intel.com> (raw)
In-Reply-To: <00450b0a9efbfde513ea8b445d31657ce5ac2f37.camel@mengyan1223.wang>
Hi Wang,
I think this is Lenovo system. The FW sends some interrupts prematurely
before driver is ready,
There is a fix available but We have decided to revert the commit.
Please try the patch attached here.
https://bugzilla.kernel.org/show_bug.cgi?id=214329
Thanks,
Srinivas
On Wed, 2021-09-08 at 02:13 +0800, Xi Ruoyao wrote:
> Hi Srinivas,
>
> Sorry for distrubing, but the mainline kernel panics on my system
> during
> boot. By reverting this commit the problem seems fixed.
>
> I don't have kdump set up on the system, so I could only took a photo
> containing (a part of :( ) panic message. The system is a laptop with
> Core i7-1065G7. My kernel config is also attached.
>
> Not sure if there is something wrong in this commit, or there is a
> firmware bug from the vendor of my laptop. In the latter case, can we
> have something in kernel config or cmdline to disable HWP as a
> workaround?
>
> On Thu, 2021-08-19 at 19:40 -0700, Srinivas Pandruvada wrote:
> > It is possible that HWP guaranteed ratio is changed in response to
> > change in power and thermal limits. For example when Intel Speed
> > Select
> > performance profile is changed or there is change in TDP, hardware
> > can
> > send notifications. It is possible that the guaranteed ratio is
> > increased. This creates an issue when turbo is disabled, as the old
> > limits set in MSR_HWP_REQUEST are still lower and hardware will clip
> > to older limits.
> >
> > This change enables HWP interrupt and process HWP interrupts. When
> > guaranteed is changed, calls cpufreq_update_policy() so that driver
> > callbacks are called to update to new HWP limits. This callback
> > is called from a delayed workqueue of 10ms to avoid frequent updates.
> >
> > Signed-off-by: Srinivas Pandruvada
> > <srinivas.pandruvada@linux.intel.com>
> > ---
> > drivers/cpufreq/intel_pstate.c | 39
> > ++++++++++++++++++++++++++++++++++
> > 1 file changed, 39 insertions(+)
> >
> > diff --git a/drivers/cpufreq/intel_pstate.c
> > b/drivers/cpufreq/intel_pstate.c
> > index bb4549959b11..0fd2375c1f1e 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -32,6 +32,7 @@
> > #include <asm/cpu_device_id.h>
> > #include <asm/cpufeature.h>
> > #include <asm/intel-family.h>
> > +#include "../drivers/thermal/intel/thermal_interrupt.h"
> >
> > #define INTEL_PSTATE_SAMPLING_INTERVAL (10 * NSEC_PER_MSEC)
> >
> > @@ -219,6 +220,7 @@ struct global_params {
> > * @sched_flags: Store scheduler flags for possible cross CPU
> > update
> > * @hwp_boost_min: Last HWP boosted min performance
> > * @suspended: Whether or not the driver has been suspended.
> > + * @hwp_notify_work: workqueue for HWP notifications.
> > *
> > * This structure stores per CPU instance data for all CPUs.
> > */
> > @@ -257,6 +259,7 @@ struct cpudata {
> > unsigned int sched_flags;
> > u32 hwp_boost_min;
> > bool suspended;
> > + struct delayed_work hwp_notify_work;
> > };
> >
> > static struct cpudata **all_cpu_data;
> > @@ -1625,6 +1628,40 @@ static void
> > intel_pstate_sysfs_hide_hwp_dynamic_boost(void)
> >
> > /************************** sysfs end ************************/
> >
> > +static void intel_pstate_notify_work(struct work_struct *work)
> > +{
> > + mutex_lock(&intel_pstate_driver_lock);
> > + cpufreq_update_policy(smp_processor_id());
> > + wrmsrl(MSR_HWP_STATUS, 0);
> > + mutex_unlock(&intel_pstate_driver_lock);
> > +}
> > +
> > +void notify_hwp_interrupt(void)
> > +{
> > + unsigned int this_cpu = smp_processor_id();
> > + struct cpudata *cpudata;
> > + u64 value;
> > +
> > + if (!hwp_active || !boot_cpu_has(X86_FEATURE_HWP_NOTIFY))
> > + return;
> > +
> > + rdmsrl(MSR_HWP_STATUS, value);
> > + if (!(value & 0x01))
> > + return;
> > +
> > + cpudata = all_cpu_data[this_cpu];
> > + schedule_delayed_work_on(this_cpu, &cpudata->hwp_notify_work,
> > msecs_to_jiffies(10));
> > +}
> > +
> > +static void intel_pstate_enable_hwp_interrupt(struct cpudata
> > *cpudata)
> > +{
> > + /* Enable HWP notification interrupt for guaranteed
> > performance change */
> > + if (boot_cpu_has(X86_FEATURE_HWP_NOTIFY)) {
> > + INIT_DELAYED_WORK(&cpudata->hwp_notify_work,
> > intel_pstate_notify_work);
> > + wrmsrl_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, 0x01);
> > + }
> > +}
> > +
> > static void intel_pstate_hwp_enable(struct cpudata *cpudata)
> > {
> > /* First disable HWP notification interrupt as we don't
> > process them */
> > @@ -1634,6 +1671,8 @@ static void intel_pstate_hwp_enable(struct
> > cpudata *cpudata)
> > wrmsrl_on_cpu(cpudata->cpu, MSR_PM_ENABLE, 0x1);
> > if (cpudata->epp_default == -EINVAL)
> > cpudata->epp_default = intel_pstate_get_epp(cpudata,
> > 0);
> > +
> > + intel_pstate_enable_hwp_interrupt(cpudata);
> > }
> >
> > static int atom_get_min_pstate(void)
>
next prev parent reply other threads:[~2021-09-08 1:54 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-20 2:40 [PATCH 1/2] thermal: intel: Allow processing of HWP interrupt Srinivas Pandruvada
2021-08-20 2:40 ` [PATCH 2/2] cpufreq: intel_pstate: Process HWP Guaranteed change notification Srinivas Pandruvada
2021-08-20 13:10 ` Daniel Lezcano
2021-08-25 18:11 ` Rafael J. Wysocki
[not found] ` <00450b0a9efbfde513ea8b445d31657ce5ac2f37.camel@mengyan1223.wang>
2021-09-08 1:54 ` Srinivas Pandruvada [this message]
2021-09-08 5:45 ` Xi Ruoyao
2021-08-20 3:35 ` [PATCH 1/2] thermal: intel: Allow processing of HWP interrupt Zhang Rui
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7fa77e2239dd87004319a799dbea39988b718233.camel@linux.intel.com \
--to=srinivas.pandruvada@linux.intel.com \
--cc=daniel.lezcano@linaro.org \
--cc=lenb@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rjw@rjwysocki.net \
--cc=rui.zhang@intel.com \
--cc=viresh.kumar@linaro.org \
--cc=xry111@mengyan1223.wang \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).