Re: [PATCH v1] cpufreq: intel_pstate: Skip HWP_CAP reads in Broadwell mode

[Rafael J. Wysocki]

Hi,

On Mon, Jun 22, 2026 at 5:16 PM Chen, Yu C <yu.c.chen@xxxxxxxxx> wrote:
>
> Hi Rafael,
>
> On 6/20/2026 3:24 AM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> >
> > When running in the Broadwell HWP mode, the turbo, max and min
> > P-state values are retrieved from MSR_PLATFORM_INFO, so reading
> > MSR_HWP_CAPABILITIES in that mode in order to update those values
> > is pointless.  Moreover, using the MSR_HWP_CAPABILITIES value for
> > updating them in the Broadwell mode may be harmful, so avoid doing
> > that altogether.
> >
> > Fixes: de5bcf404ace ("cpufreq: intel_pstate: Clean up frequency computations")
>
> sashiko[1] reports an issue that hwp_cap_cached remains 0.

Right, it will.  It is used in a few places that I have overlooked though.

> Maybe still use the value from MSR_HWP_CAPABILITIES for cached value,
> which was what we did before commit de5bcf404ace? I fed this
> to AI and was given the following:
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 5a0eeb84d382..a373a199c9c6 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1189,6 +1189,8 @@ static void __intel_pstate_get_hwp_cap(struct
> cpudata *cpu)
>
>       rdmsrq_on_cpu(cpu->cpu, MSR_HWP_CAPABILITIES, &cap);
>       WRITE_ONCE(cpu->hwp_cap_cached, cap);
> +       if (hwp_mode_bdw)
> +               return;
>       cpu->pstate.max_pstate = HWP_GUARANTEED_PERF(cap);
>       cpu->pstate.turbo_pstate = HWP_HIGHEST_PERF(cap);
>   }
> @@ -1199,6 +1201,9 @@ static void intel_pstate_get_hwp_cap(struct
> cpudata *cpu)
>
>       __intel_pstate_get_hwp_cap(cpu);
>
> +       if (hwp_mode_bdw)
> +               return;
> +
>       cpu->pstate.max_freq = cpu->pstate.max_pstate * scaling;
>       cpu->pstate.turbo_freq = cpu->pstate.turbo_pstate * scaling;
>       if (scaling != cpu->pstate.perf_ctl_scaling) {
>
>
> [1] https://sashiko.dev/#/patchset/12920597.O9o76ZdvQC%40rafael.j.wysocki

That gets a bit messy and also hwp_cap_cached is referred to directly
and used for programming HWP_REQUEST without checking the Broadwell
mode.  I'm not sure if I want to change that after 5 years though.

ATM the Broadwell mode only affects initialization and while it
prevents __intel_pstate_get_hwp_cap() from running in
intel_pstate_get_cpu_pstates(), it doesn't prevent that function from
running anywhere else and effectively it only really prevents the
hybrid initialization from being done, so after all I'm not sure if
it's worth retaining.  It might as well be dropped altogether AFAIAC.