Patchwork [RFC] ACPI: add "processor.broadcast_ppc" hook to broadcast _PPC to all online CPUs

login
register
mail settings
Submitter Chen, Yu C
Date Feb. 9, 2019, 12:02 p.m.
Message ID <20190209120232.21582-1-yu.c.chen@intel.com>
Download mbox | patch
Permalink /patch/722219/
State New
Headers show

Comments

Chen, Yu C - Feb. 9, 2019, 12:02 p.m.
On Dell Inc. XPS13 9333, the BIOS changes the value of
MSR_IA32_MISC_ENABLE_TURBO_DISABLE at runtime (e.g., when
the power source changes), the maximum frequency of the
CPU is not updated accordingly. This is due to the policy's
cpuinfo.max is not updated when _PPC notifier fires.

Fix this problem by updating the policy's cpuinfo.max
and broadcast the _PPC notifier to all online CPUs.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=200759
Reported-and-tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
Originally-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 drivers/acpi/processor_perflib.c | 16 ++++++++++++++--
 drivers/cpufreq/cpufreq.c        |  2 ++
 drivers/cpufreq/intel_pstate.c   | 15 ++++++++++++++-
 3 files changed, 30 insertions(+), 3 deletions(-)
Viresh Kumar - Feb. 11, 2019, 9:16 a.m.
On 09-02-19, 20:02, Chen Yu wrote:
> On Dell Inc. XPS13 9333, the BIOS changes the value of
> MSR_IA32_MISC_ENABLE_TURBO_DISABLE at runtime (e.g., when
> the power source changes), the maximum frequency of the
> CPU is not updated accordingly. This is due to the policy's
> cpuinfo.max is not updated when _PPC notifier fires.
> 
> Fix this problem by updating the policy's cpuinfo.max
> and broadcast the _PPC notifier to all online CPUs.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=200759
> Reported-and-tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> Originally-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
>  drivers/acpi/processor_perflib.c | 16 ++++++++++++++--
>  drivers/cpufreq/cpufreq.c        |  2 ++
>  drivers/cpufreq/intel_pstate.c   | 15 ++++++++++++++-
>  3 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index a303fd0e108c..737dbf5aa7f7 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -63,6 +63,10 @@ module_param(ignore_ppc, int, 0644);
>  MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
>  		 "limited by BIOS, this should help");
>  
> +static int broadcast_ppc;
> +module_param(broadcast_ppc, int, 0644);
> +MODULE_PARM_DESC(broadcast_ppc, "Broadcast the ppc to all online CPUs");
> +
>  #define PPC_REGISTERED   1
>  #define PPC_IN_USE       2
>  
> @@ -180,8 +184,16 @@ void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
>  		else
>  			acpi_processor_ppc_ost(pr->handle, 0);
>  	}
> -	if (ret >= 0)
> -		cpufreq_update_policy(pr->id);
> +	if (ret >= 0) {
> +		if (broadcast_ppc) {
> +			int cpu;
> +
> +			for_each_possible_cpu(cpu)
> +				cpufreq_update_policy(cpu);
> +		} else {
> +			cpufreq_update_policy(pr->id);
> +		}
> +	}
>  }
>  
>  int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index e35a886e00bc..95e08816b512 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2237,6 +2237,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
>  
>  	policy->min = new_policy->min;
>  	policy->max = new_policy->max;
> +	policy->cpuinfo.max_freq = new_policy->cpuinfo.max_freq;
> +	policy->cpuinfo.min_freq = new_policy->cpuinfo.min_freq;
>  	trace_cpu_frequency_limits(policy);
>  
>  	policy->cached_target_freq = UINT_MAX;
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index dd66decf2087..e1881313c396 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2081,11 +2081,24 @@ static void intel_pstate_adjust_policy_max(struct cpufreq_policy *policy,
>  
>  static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
>  {
> +	int max_freq;
>  	struct cpudata *cpu = all_cpu_data[policy->cpu];
>  
>  	update_turbo_state();
> +	max_freq = intel_pstate_get_max_freq(cpu);
> +
> +	if (acpi_ppc && policy->max == policy->cpuinfo.max_freq &&
> +	    max_freq != policy->cpuinfo.max_freq) {
> +		/*
> +		 * System was not running under any constraints, but the
> +		 * current max possible frequency is changed. So reset
> +		 * policy limits.
> +		 */
> +		policy->cpuinfo.max_freq = policy->max = max_freq;
> +	}
> +
>  	cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
> -				     intel_pstate_get_max_freq(cpu));
> +				     max_freq);
>  
>  	if (policy->policy != CPUFREQ_POLICY_POWERSAVE &&
>  	    policy->policy != CPUFREQ_POLICY_PERFORMANCE)

By TURBO I believe this is about boost-frequencies and you should use
that infrastructure to make it work, isn't it ?
Rafael J. Wysocki - Feb. 11, 2019, 10:20 a.m.
On Mon, Feb 11, 2019 at 10:16 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 09-02-19, 20:02, Chen Yu wrote:
> > On Dell Inc. XPS13 9333, the BIOS changes the value of
> > MSR_IA32_MISC_ENABLE_TURBO_DISABLE at runtime (e.g., when
> > the power source changes), the maximum frequency of the
> > CPU is not updated accordingly. This is due to the policy's
> > cpuinfo.max is not updated when _PPC notifier fires.
> >
> > Fix this problem by updating the policy's cpuinfo.max
> > and broadcast the _PPC notifier to all online CPUs.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=200759
> > Reported-and-tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > Originally-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > ---
> >  drivers/acpi/processor_perflib.c | 16 ++++++++++++++--
> >  drivers/cpufreq/cpufreq.c        |  2 ++
> >  drivers/cpufreq/intel_pstate.c   | 15 ++++++++++++++-
> >  3 files changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> > index a303fd0e108c..737dbf5aa7f7 100644
> > --- a/drivers/acpi/processor_perflib.c
> > +++ b/drivers/acpi/processor_perflib.c
> > @@ -63,6 +63,10 @@ module_param(ignore_ppc, int, 0644);
> >  MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
> >                "limited by BIOS, this should help");
> >
> > +static int broadcast_ppc;
> > +module_param(broadcast_ppc, int, 0644);
> > +MODULE_PARM_DESC(broadcast_ppc, "Broadcast the ppc to all online CPUs");
> > +
> >  #define PPC_REGISTERED   1
> >  #define PPC_IN_USE       2
> >
> > @@ -180,8 +184,16 @@ void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
> >               else
> >                       acpi_processor_ppc_ost(pr->handle, 0);
> >       }
> > -     if (ret >= 0)
> > -             cpufreq_update_policy(pr->id);
> > +     if (ret >= 0) {
> > +             if (broadcast_ppc) {
> > +                     int cpu;
> > +
> > +                     for_each_possible_cpu(cpu)
> > +                             cpufreq_update_policy(cpu);
> > +             } else {
> > +                     cpufreq_update_policy(pr->id);
> > +             }
> > +     }
> >  }
> >
> >  int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index e35a886e00bc..95e08816b512 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -2237,6 +2237,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
> >
> >       policy->min = new_policy->min;
> >       policy->max = new_policy->max;
> > +     policy->cpuinfo.max_freq = new_policy->cpuinfo.max_freq;
> > +     policy->cpuinfo.min_freq = new_policy->cpuinfo.min_freq;
> >       trace_cpu_frequency_limits(policy);
> >
> >       policy->cached_target_freq = UINT_MAX;
> > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > index dd66decf2087..e1881313c396 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -2081,11 +2081,24 @@ static void intel_pstate_adjust_policy_max(struct cpufreq_policy *policy,
> >
> >  static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
> >  {
> > +     int max_freq;
> >       struct cpudata *cpu = all_cpu_data[policy->cpu];
> >
> >       update_turbo_state();
> > +     max_freq = intel_pstate_get_max_freq(cpu);
> > +
> > +     if (acpi_ppc && policy->max == policy->cpuinfo.max_freq &&
> > +         max_freq != policy->cpuinfo.max_freq) {
> > +             /*
> > +              * System was not running under any constraints, but the
> > +              * current max possible frequency is changed. So reset
> > +              * policy limits.
> > +              */
> > +             policy->cpuinfo.max_freq = policy->max = max_freq;
> > +     }
> > +
> >       cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
> > -                                  intel_pstate_get_max_freq(cpu));
> > +                                  max_freq);
> >
> >       if (policy->policy != CPUFREQ_POLICY_POWERSAVE &&
> >           policy->policy != CPUFREQ_POLICY_PERFORMANCE)
>
> By TURBO I believe this is about boost-frequencies and you should use
> that infrastructure to make it work, isn't it ?

I guess you mean the the "boost" attribute in the core, but that's not
applicable to intel_pstate.
Viresh Kumar - Feb. 11, 2019, 10:30 a.m.
On 11-02-19, 11:20, Rafael J. Wysocki wrote:
> On Mon, Feb 11, 2019 at 10:16 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 09-02-19, 20:02, Chen Yu wrote:
> > > On Dell Inc. XPS13 9333, the BIOS changes the value of
> > > MSR_IA32_MISC_ENABLE_TURBO_DISABLE at runtime (e.g., when
> > > the power source changes), the maximum frequency of the
> > > CPU is not updated accordingly. This is due to the policy's
> > > cpuinfo.max is not updated when _PPC notifier fires.
> > >
> > > Fix this problem by updating the policy's cpuinfo.max
> > > and broadcast the _PPC notifier to all online CPUs.
> > >
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=200759
> > > Reported-and-tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > > Originally-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > > ---
> > >  drivers/acpi/processor_perflib.c | 16 ++++++++++++++--
> > >  drivers/cpufreq/cpufreq.c        |  2 ++
> > >  drivers/cpufreq/intel_pstate.c   | 15 ++++++++++++++-
> > >  3 files changed, 30 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> > > index a303fd0e108c..737dbf5aa7f7 100644
> > > --- a/drivers/acpi/processor_perflib.c
> > > +++ b/drivers/acpi/processor_perflib.c
> > > @@ -63,6 +63,10 @@ module_param(ignore_ppc, int, 0644);
> > >  MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
> > >                "limited by BIOS, this should help");
> > >
> > > +static int broadcast_ppc;
> > > +module_param(broadcast_ppc, int, 0644);
> > > +MODULE_PARM_DESC(broadcast_ppc, "Broadcast the ppc to all online CPUs");
> > > +
> > >  #define PPC_REGISTERED   1
> > >  #define PPC_IN_USE       2
> > >
> > > @@ -180,8 +184,16 @@ void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
> > >               else
> > >                       acpi_processor_ppc_ost(pr->handle, 0);
> > >       }
> > > -     if (ret >= 0)
> > > -             cpufreq_update_policy(pr->id);
> > > +     if (ret >= 0) {
> > > +             if (broadcast_ppc) {
> > > +                     int cpu;
> > > +
> > > +                     for_each_possible_cpu(cpu)
> > > +                             cpufreq_update_policy(cpu);
> > > +             } else {
> > > +                     cpufreq_update_policy(pr->id);
> > > +             }
> > > +     }
> > >  }
> > >
> > >  int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > > index e35a886e00bc..95e08816b512 100644
> > > --- a/drivers/cpufreq/cpufreq.c
> > > +++ b/drivers/cpufreq/cpufreq.c
> > > @@ -2237,6 +2237,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
> > >
> > >       policy->min = new_policy->min;
> > >       policy->max = new_policy->max;
> > > +     policy->cpuinfo.max_freq = new_policy->cpuinfo.max_freq;
> > > +     policy->cpuinfo.min_freq = new_policy->cpuinfo.min_freq;
> > >       trace_cpu_frequency_limits(policy);
> > >
> > >       policy->cached_target_freq = UINT_MAX;
> > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > > index dd66decf2087..e1881313c396 100644
> > > --- a/drivers/cpufreq/intel_pstate.c
> > > +++ b/drivers/cpufreq/intel_pstate.c
> > > @@ -2081,11 +2081,24 @@ static void intel_pstate_adjust_policy_max(struct cpufreq_policy *policy,
> > >
> > >  static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
> > >  {
> > > +     int max_freq;
> > >       struct cpudata *cpu = all_cpu_data[policy->cpu];
> > >
> > >       update_turbo_state();
> > > +     max_freq = intel_pstate_get_max_freq(cpu);
> > > +
> > > +     if (acpi_ppc && policy->max == policy->cpuinfo.max_freq &&
> > > +         max_freq != policy->cpuinfo.max_freq) {
> > > +             /*
> > > +              * System was not running under any constraints, but the
> > > +              * current max possible frequency is changed. So reset
> > > +              * policy limits.
> > > +              */
> > > +             policy->cpuinfo.max_freq = policy->max = max_freq;
> > > +     }
> > > +
> > >       cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
> > > -                                  intel_pstate_get_max_freq(cpu));
> > > +                                  max_freq);
> > >
> > >       if (policy->policy != CPUFREQ_POLICY_POWERSAVE &&
> > >           policy->policy != CPUFREQ_POLICY_PERFORMANCE)
> >
> > By TURBO I believe this is about boost-frequencies and you should use
> > that infrastructure to make it work, isn't it ?
> 
> I guess you mean the the "boost" attribute in the core, but that's not
> applicable to intel_pstate.

Ahh, I missed the part that this is for the !has_target() platforms :(
Viresh Kumar - Feb. 11, 2019, 10:33 a.m.
On 09-02-19, 20:02, Chen Yu wrote:
> On Dell Inc. XPS13 9333, the BIOS changes the value of
> MSR_IA32_MISC_ENABLE_TURBO_DISABLE at runtime (e.g., when
> the power source changes), the maximum frequency of the
> CPU is not updated accordingly. This is due to the policy's
> cpuinfo.max is not updated when _PPC notifier fires.
> 
> Fix this problem by updating the policy's cpuinfo.max
> and broadcast the _PPC notifier to all online CPUs.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=200759
> Reported-and-tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> Originally-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
>  drivers/acpi/processor_perflib.c | 16 ++++++++++++++--
>  drivers/cpufreq/cpufreq.c        |  2 ++
>  drivers/cpufreq/intel_pstate.c   | 15 ++++++++++++++-
>  3 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index a303fd0e108c..737dbf5aa7f7 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -63,6 +63,10 @@ module_param(ignore_ppc, int, 0644);
>  MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
>  		 "limited by BIOS, this should help");
>  
> +static int broadcast_ppc;
> +module_param(broadcast_ppc, int, 0644);
> +MODULE_PARM_DESC(broadcast_ppc, "Broadcast the ppc to all online CPUs");
> +
>  #define PPC_REGISTERED   1
>  #define PPC_IN_USE       2
>  
> @@ -180,8 +184,16 @@ void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
>  		else
>  			acpi_processor_ppc_ost(pr->handle, 0);
>  	}
> -	if (ret >= 0)
> -		cpufreq_update_policy(pr->id);
> +	if (ret >= 0) {
> +		if (broadcast_ppc) {
> +			int cpu;
> +
> +			for_each_possible_cpu(cpu)
> +				cpufreq_update_policy(cpu);
> +		} else {
> +			cpufreq_update_policy(pr->id);
> +		}
> +	}
>  }
>  
>  int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index e35a886e00bc..95e08816b512 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2237,6 +2237,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
>  
>  	policy->min = new_policy->min;
>  	policy->max = new_policy->max;
> +	policy->cpuinfo.max_freq = new_policy->cpuinfo.max_freq;
> +	policy->cpuinfo.min_freq = new_policy->cpuinfo.min_freq;
>  	trace_cpu_frequency_limits(policy);
>  
>  	policy->cached_target_freq = UINT_MAX;
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index dd66decf2087..e1881313c396 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2081,11 +2081,24 @@ static void intel_pstate_adjust_policy_max(struct cpufreq_policy *policy,
>  
>  static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
>  {
> +	int max_freq;
>  	struct cpudata *cpu = all_cpu_data[policy->cpu];
>  
>  	update_turbo_state();
> +	max_freq = intel_pstate_get_max_freq(cpu);
> +
> +	if (acpi_ppc && policy->max == policy->cpuinfo.max_freq &&

Why do have this check for policy->max here ?

> +	    max_freq != policy->cpuinfo.max_freq) {
> +		/*
> +		 * System was not running under any constraints, but the
> +		 * current max possible frequency is changed. So reset
> +		 * policy limits.
> +		 */
> +		policy->cpuinfo.max_freq = policy->max = max_freq;
> +	}
> +
>  	cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
> -				     intel_pstate_get_max_freq(cpu));
> +				     max_freq);
>  
>  	if (policy->policy != CPUFREQ_POLICY_POWERSAVE &&
>  	    policy->policy != CPUFREQ_POLICY_PERFORMANCE)
> -- 
> 2.17.1
Rafael J. Wysocki - Feb. 11, 2019, 10:41 a.m.
On Sat, Feb 9, 2019 at 12:54 PM Chen Yu <yu.c.chen@intel.com> wrote:
>
> On Dell Inc. XPS13 9333, the BIOS changes the value of
> MSR_IA32_MISC_ENABLE_TURBO_DISABLE at runtime (e.g., when
> the power source changes), the maximum frequency of the
> CPU is not updated accordingly. This is due to the policy's
> cpuinfo.max is not updated when _PPC notifier fires.
>
> Fix this problem by updating the policy's cpuinfo.max
> and broadcast the _PPC notifier to all online CPUs.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=200759
> Reported-and-tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> Originally-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
>  drivers/acpi/processor_perflib.c | 16 ++++++++++++++--
>  drivers/cpufreq/cpufreq.c        |  2 ++
>  drivers/cpufreq/intel_pstate.c   | 15 ++++++++++++++-
>  3 files changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index a303fd0e108c..737dbf5aa7f7 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -63,6 +63,10 @@ module_param(ignore_ppc, int, 0644);
>  MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
>                  "limited by BIOS, this should help");
>
> +static int broadcast_ppc;
> +module_param(broadcast_ppc, int, 0644);
> +MODULE_PARM_DESC(broadcast_ppc, "Broadcast the ppc to all online CPUs");
> +
>  #define PPC_REGISTERED   1
>  #define PPC_IN_USE       2
>
> @@ -180,8 +184,16 @@ void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
>                 else
>                         acpi_processor_ppc_ost(pr->handle, 0);
>         }
> -       if (ret >= 0)
> -               cpufreq_update_policy(pr->id);
> +       if (ret >= 0) {
> +               if (broadcast_ppc) {
> +                       int cpu;
> +
> +                       for_each_possible_cpu(cpu)
> +                               cpufreq_update_policy(cpu);
> +               } else {
> +                       cpufreq_update_policy(pr->id);
> +               }
> +       }
>  }
>
>  int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index e35a886e00bc..95e08816b512 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2237,6 +2237,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
>
>         policy->min = new_policy->min;
>         policy->max = new_policy->max;
> +       policy->cpuinfo.max_freq = new_policy->cpuinfo.max_freq;
> +       policy->cpuinfo.min_freq = new_policy->cpuinfo.min_freq;
>         trace_cpu_frequency_limits(policy);
>
>         policy->cached_target_freq = UINT_MAX;
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index dd66decf2087..e1881313c396 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -2081,11 +2081,24 @@ static void intel_pstate_adjust_policy_max(struct cpufreq_policy *policy,
>
>  static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
>  {
> +       int max_freq;
>         struct cpudata *cpu = all_cpu_data[policy->cpu];
>
>         update_turbo_state();

Well, update_turbo_state() should handle the case at hand already.

That's what it's for actually: It checks if
MSR_IA32_MISC_ENABLE_TURBO_DISABLE is set and sets
global.turbo_disabled is that's the case.

Why isn't that sufficient?

> +       max_freq = intel_pstate_get_max_freq(cpu);
> +
> +       if (acpi_ppc && policy->max == policy->cpuinfo.max_freq &&
> +           max_freq != policy->cpuinfo.max_freq) {
> +               /*
> +                * System was not running under any constraints, but the
> +                * current max possible frequency is changed. So reset
> +                * policy limits.
> +                */
> +               policy->cpuinfo.max_freq = policy->max = max_freq;
> +       }

Why does policy->cpuinfo.max_freq need to be updated?

> +
>         cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
> -                                    intel_pstate_get_max_freq(cpu));
> +                                    max_freq);
>
>         if (policy->policy != CPUFREQ_POLICY_POWERSAVE &&
>             policy->policy != CPUFREQ_POLICY_PERFORMANCE)
> --
Chen, Yu C - Feb. 13, 2019, 4:52 p.m.
Hi Rafael,
On Mon, Feb 11, 2019 at 11:41:26AM +0100, Rafael J. Wysocki wrote:
> On Sat, Feb 9, 2019 at 12:54 PM Chen Yu <yu.c.chen@intel.com> wrote:
> >
> > On Dell Inc. XPS13 9333, the BIOS changes the value of
> > MSR_IA32_MISC_ENABLE_TURBO_DISABLE at runtime (e.g., when
> > the power source changes), the maximum frequency of the
> > CPU is not updated accordingly. This is due to the policy's
> > cpuinfo.max is not updated when _PPC notifier fires.
> >
> > Fix this problem by updating the policy's cpuinfo.max
> > and broadcast the _PPC notifier to all online CPUs.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=200759
> > Reported-and-tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > Originally-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > ---
> >  drivers/acpi/processor_perflib.c | 16 ++++++++++++++--
> >  drivers/cpufreq/cpufreq.c        |  2 ++
> >  drivers/cpufreq/intel_pstate.c   | 15 ++++++++++++++-
> >  3 files changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> > index a303fd0e108c..737dbf5aa7f7 100644
> > --- a/drivers/acpi/processor_perflib.c
> > +++ b/drivers/acpi/processor_perflib.c
> > @@ -63,6 +63,10 @@ module_param(ignore_ppc, int, 0644);
> >  MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
> >                  "limited by BIOS, this should help");
> >
> > +static int broadcast_ppc;
> > +module_param(broadcast_ppc, int, 0644);
> > +MODULE_PARM_DESC(broadcast_ppc, "Broadcast the ppc to all online CPUs");
> > +
> >  #define PPC_REGISTERED   1
> >  #define PPC_IN_USE       2
> >
> > @@ -180,8 +184,16 @@ void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
> >                 else
> >                         acpi_processor_ppc_ost(pr->handle, 0);
> >         }
> > -       if (ret >= 0)
> > -               cpufreq_update_policy(pr->id);
> > +       if (ret >= 0) {
> > +               if (broadcast_ppc) {
> > +                       int cpu;
> > +
> > +                       for_each_possible_cpu(cpu)
> > +                               cpufreq_update_policy(cpu);
> > +               } else {
> > +                       cpufreq_update_policy(pr->id);
> > +               }
> > +       }
> >  }
> >
> >  int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index e35a886e00bc..95e08816b512 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -2237,6 +2237,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
> >
> >         policy->min = new_policy->min;
> >         policy->max = new_policy->max;
> > +       policy->cpuinfo.max_freq = new_policy->cpuinfo.max_freq;
> > +       policy->cpuinfo.min_freq = new_policy->cpuinfo.min_freq;
> >         trace_cpu_frequency_limits(policy);
> >
> >         policy->cached_target_freq = UINT_MAX;
> > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > index dd66decf2087..e1881313c396 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -2081,11 +2081,24 @@ static void intel_pstate_adjust_policy_max(struct cpufreq_policy *policy,
> >
> >  static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
> >  {
> > +       int max_freq;
> >         struct cpudata *cpu = all_cpu_data[policy->cpu];
> >
> >         update_turbo_state();
> 
> Well, update_turbo_state() should handle the case at hand already.
> 
> That's what it's for actually: It checks if
> MSR_IA32_MISC_ENABLE_TURBO_DISABLE is set and sets
> global.turbo_disabled is that's the case.
> 
> Why isn't that sufficient?
> 
update_turbo_state() changes the flag of global.turbo_diabled but we
need to also leverage it to adjust the policy.max accordingly. This is why
we add intel_pstate_get_max_freq() to get the updated max freq in
intel_pstate_verify_policy().
> > +       max_freq = intel_pstate_get_max_freq(cpu);
> > +
> > +       if (acpi_ppc && policy->max == policy->cpuinfo.max_freq &&
> > +           max_freq != policy->cpuinfo.max_freq) {
> > +               /*
> > +                * System was not running under any constraints, but the
> > +                * current max possible frequency is changed. So reset
> > +                * policy limits.
> > +                */
> > +               policy->cpuinfo.max_freq = policy->max = max_freq;
> > +       }
> 
> Why does policy->cpuinfo.max_freq need to be updated?
> 
This is my understanding:
There's a corner case that, if the system boots with battery,
the max cpu frequency will not scale up if we plug the AC later.
According to the log provided by Gabriele Mazzotta,  if the system
boot up with battery, then plug the AC after boot up, the max perf ratio
and policy->cpuinfo.max will remain 17 rather than increasing to
30(when AC plugged thus turbo enabled):

[   52.158810] CPU 0: _PPC is 6 - frequency  limited
[   52.158822] intel_pstate: set_policy cpuinfo.max 1700000 policy->max 1700000
[   52.158825] intel_pstate: cpu:0 max_state 30 min_policy_perf:8 max_policy_perf:17
[   52.158827] intel_pstate: cpu:0 global_min:8 global_max:30
[   52.158829] intel_pstate: cpu:0 max_perf_ratio:17 min_perf_ratio:8

This is caused by:
In current intel_pstate, there's only one chance for policy.cpuinfo.max to get updated
which is during boot up in __intel_pstate_cpu_init(). If the turbo status changes,
we might need to also update the policy->cpuinfo.max to tell user that the hardware
status has changed.

So we give it a chance to adjust the policy.cpuinfo.max and policy.max in
cpufreq_driver->verify()  according to turbo status, this is what this patch mainly
aims to do.

Besides, since on this platform there's only one _PPC notification for one CPU, it is
necessary to broadcast the notification to all CPUs on this package. And this patch
broadcast it to all online CPUs to make the change simpler.

Best,
Yu
Chen, Yu C - Feb. 13, 2019, 4:55 p.m.
Hi Viresh,
On Mon, Feb 11, 2019 at 04:03:07PM +0530, Viresh Kumar wrote:
> On 09-02-19, 20:02, Chen Yu wrote:
> > On Dell Inc. XPS13 9333, the BIOS changes the value of
> > MSR_IA32_MISC_ENABLE_TURBO_DISABLE at runtime (e.g., when
> > the power source changes), the maximum frequency of the
> > CPU is not updated accordingly. This is due to the policy's
> > cpuinfo.max is not updated when _PPC notifier fires.
> > 
> > Fix this problem by updating the policy's cpuinfo.max
> > and broadcast the _PPC notifier to all online CPUs.
> > 
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=200759
> > Reported-and-tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > Originally-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > ---
> >  drivers/acpi/processor_perflib.c | 16 ++++++++++++++--
> >  drivers/cpufreq/cpufreq.c        |  2 ++
> >  drivers/cpufreq/intel_pstate.c   | 15 ++++++++++++++-
> >  3 files changed, 30 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> > index a303fd0e108c..737dbf5aa7f7 100644
> > --- a/drivers/acpi/processor_perflib.c
> > +++ b/drivers/acpi/processor_perflib.c
> > @@ -63,6 +63,10 @@ module_param(ignore_ppc, int, 0644);
> >  MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
> >  		 "limited by BIOS, this should help");
> >  
> > +static int broadcast_ppc;
> > +module_param(broadcast_ppc, int, 0644);
> > +MODULE_PARM_DESC(broadcast_ppc, "Broadcast the ppc to all online CPUs");
> > +
> >  #define PPC_REGISTERED   1
> >  #define PPC_IN_USE       2
> >  
> > @@ -180,8 +184,16 @@ void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
> >  		else
> >  			acpi_processor_ppc_ost(pr->handle, 0);
> >  	}
> > -	if (ret >= 0)
> > -		cpufreq_update_policy(pr->id);
> > +	if (ret >= 0) {
> > +		if (broadcast_ppc) {
> > +			int cpu;
> > +
> > +			for_each_possible_cpu(cpu)
> > +				cpufreq_update_policy(cpu);
> > +		} else {
> > +			cpufreq_update_policy(pr->id);
> > +		}
> > +	}
> >  }
> >  
> >  int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index e35a886e00bc..95e08816b512 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -2237,6 +2237,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
> >  
> >  	policy->min = new_policy->min;
> >  	policy->max = new_policy->max;
> > +	policy->cpuinfo.max_freq = new_policy->cpuinfo.max_freq;
> > +	policy->cpuinfo.min_freq = new_policy->cpuinfo.min_freq;
> >  	trace_cpu_frequency_limits(policy);
> >  
> >  	policy->cached_target_freq = UINT_MAX;
> > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > index dd66decf2087..e1881313c396 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -2081,11 +2081,24 @@ static void intel_pstate_adjust_policy_max(struct cpufreq_policy *policy,
> >  
> >  static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
> >  {
> > +	int max_freq;
> >  	struct cpudata *cpu = all_cpu_data[policy->cpu];
> >  
> >  	update_turbo_state();
> > +	max_freq = intel_pstate_get_max_freq(cpu);
> > +
> > +	if (acpi_ppc && policy->max == policy->cpuinfo.max_freq &&
> 
> Why do have this check for policy->max here ?
>
Thanks for looking at this change, I've replied to another email in detail of
the scenario that, this is due to corner case that if the system boots
with battery and plug the AC after boot up, the cpufreq max limit will not
increase even the turbo has been enabled after the AC plugged.

Best,
Yu
Viresh Kumar - Feb. 14, 2019, 7:06 a.m.
On 14-02-19, 00:55, Yu Chen wrote:
> Hi Viresh,
> On Mon, Feb 11, 2019 at 04:03:07PM +0530, Viresh Kumar wrote:
> > On 09-02-19, 20:02, Chen Yu wrote:
> > > On Dell Inc. XPS13 9333, the BIOS changes the value of
> > > MSR_IA32_MISC_ENABLE_TURBO_DISABLE at runtime (e.g., when
> > > the power source changes), the maximum frequency of the
> > > CPU is not updated accordingly. This is due to the policy's
> > > cpuinfo.max is not updated when _PPC notifier fires.
> > > 
> > > Fix this problem by updating the policy's cpuinfo.max
> > > and broadcast the _PPC notifier to all online CPUs.
> > > 
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=200759
> > > Reported-and-tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > > Originally-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > > ---
> > >  drivers/acpi/processor_perflib.c | 16 ++++++++++++++--
> > >  drivers/cpufreq/cpufreq.c        |  2 ++
> > >  drivers/cpufreq/intel_pstate.c   | 15 ++++++++++++++-
> > >  3 files changed, 30 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> > > index a303fd0e108c..737dbf5aa7f7 100644
> > > --- a/drivers/acpi/processor_perflib.c
> > > +++ b/drivers/acpi/processor_perflib.c
> > > @@ -63,6 +63,10 @@ module_param(ignore_ppc, int, 0644);
> > >  MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
> > >  		 "limited by BIOS, this should help");
> > >  
> > > +static int broadcast_ppc;
> > > +module_param(broadcast_ppc, int, 0644);
> > > +MODULE_PARM_DESC(broadcast_ppc, "Broadcast the ppc to all online CPUs");
> > > +
> > >  #define PPC_REGISTERED   1
> > >  #define PPC_IN_USE       2
> > >  
> > > @@ -180,8 +184,16 @@ void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
> > >  		else
> > >  			acpi_processor_ppc_ost(pr->handle, 0);
> > >  	}
> > > -	if (ret >= 0)
> > > -		cpufreq_update_policy(pr->id);
> > > +	if (ret >= 0) {
> > > +		if (broadcast_ppc) {
> > > +			int cpu;
> > > +
> > > +			for_each_possible_cpu(cpu)
> > > +				cpufreq_update_policy(cpu);
> > > +		} else {
> > > +			cpufreq_update_policy(pr->id);
> > > +		}
> > > +	}
> > >  }
> > >  
> > >  int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > > index e35a886e00bc..95e08816b512 100644
> > > --- a/drivers/cpufreq/cpufreq.c
> > > +++ b/drivers/cpufreq/cpufreq.c
> > > @@ -2237,6 +2237,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
> > >  
> > >  	policy->min = new_policy->min;
> > >  	policy->max = new_policy->max;
> > > +	policy->cpuinfo.max_freq = new_policy->cpuinfo.max_freq;
> > > +	policy->cpuinfo.min_freq = new_policy->cpuinfo.min_freq;
> > >  	trace_cpu_frequency_limits(policy);
> > >  
> > >  	policy->cached_target_freq = UINT_MAX;
> > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > > index dd66decf2087..e1881313c396 100644
> > > --- a/drivers/cpufreq/intel_pstate.c
> > > +++ b/drivers/cpufreq/intel_pstate.c
> > > @@ -2081,11 +2081,24 @@ static void intel_pstate_adjust_policy_max(struct cpufreq_policy *policy,
> > >  
> > >  static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
> > >  {
> > > +	int max_freq;
> > >  	struct cpudata *cpu = all_cpu_data[policy->cpu];
> > >  
> > >  	update_turbo_state();
> > > +	max_freq = intel_pstate_get_max_freq(cpu);
> > > +
> > > +	if (acpi_ppc && policy->max == policy->cpuinfo.max_freq &&
> > 
> > Why do have this check for policy->max here ?
> >
> Thanks for looking at this change, I've replied to another email in detail of
> the scenario that, this is due to corner case that if the system boots
> with battery and plug the AC after boot up, the cpufreq max limit will not
> increase even the turbo has been enabled after the AC plugged.

Yeah, but I asked a different question I believe, why is this
comparison necessary ?

policy->max == policy->cpuinfo.max_freq
Rafael J. Wysocki - Feb. 14, 2019, 10:21 a.m.
On Wed, Feb 13, 2019 at 5:44 PM Yu Chen <yu.c.chen@intel.com> wrote:
>
> Hi Rafael,
> On Mon, Feb 11, 2019 at 11:41:26AM +0100, Rafael J. Wysocki wrote:
> > On Sat, Feb 9, 2019 at 12:54 PM Chen Yu <yu.c.chen@intel.com> wrote:
> > >
> > > On Dell Inc. XPS13 9333, the BIOS changes the value of
> > > MSR_IA32_MISC_ENABLE_TURBO_DISABLE at runtime (e.g., when
> > > the power source changes), the maximum frequency of the
> > > CPU is not updated accordingly. This is due to the policy's
> > > cpuinfo.max is not updated when _PPC notifier fires.
> > >
> > > Fix this problem by updating the policy's cpuinfo.max
> > > and broadcast the _PPC notifier to all online CPUs.
> > >
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=200759
> > > Reported-and-tested-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > > Originally-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > > ---
> > >  drivers/acpi/processor_perflib.c | 16 ++++++++++++++--
> > >  drivers/cpufreq/cpufreq.c        |  2 ++
> > >  drivers/cpufreq/intel_pstate.c   | 15 ++++++++++++++-
> > >  3 files changed, 30 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> > > index a303fd0e108c..737dbf5aa7f7 100644
> > > --- a/drivers/acpi/processor_perflib.c
> > > +++ b/drivers/acpi/processor_perflib.c
> > > @@ -63,6 +63,10 @@ module_param(ignore_ppc, int, 0644);
> > >  MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
> > >                  "limited by BIOS, this should help");
> > >
> > > +static int broadcast_ppc;
> > > +module_param(broadcast_ppc, int, 0644);
> > > +MODULE_PARM_DESC(broadcast_ppc, "Broadcast the ppc to all online CPUs");
> > > +
> > >  #define PPC_REGISTERED   1
> > >  #define PPC_IN_USE       2
> > >
> > > @@ -180,8 +184,16 @@ void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
> > >                 else
> > >                         acpi_processor_ppc_ost(pr->handle, 0);
> > >         }
> > > -       if (ret >= 0)
> > > -               cpufreq_update_policy(pr->id);
> > > +       if (ret >= 0) {
> > > +               if (broadcast_ppc) {
> > > +                       int cpu;
> > > +
> > > +                       for_each_possible_cpu(cpu)
> > > +                               cpufreq_update_policy(cpu);
> > > +               } else {
> > > +                       cpufreq_update_policy(pr->id);
> > > +               }
> > > +       }
> > >  }
> > >
> > >  int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > > index e35a886e00bc..95e08816b512 100644
> > > --- a/drivers/cpufreq/cpufreq.c
> > > +++ b/drivers/cpufreq/cpufreq.c
> > > @@ -2237,6 +2237,8 @@ static int cpufreq_set_policy(struct cpufreq_policy *policy,
> > >
> > >         policy->min = new_policy->min;
> > >         policy->max = new_policy->max;
> > > +       policy->cpuinfo.max_freq = new_policy->cpuinfo.max_freq;
> > > +       policy->cpuinfo.min_freq = new_policy->cpuinfo.min_freq;
> > >         trace_cpu_frequency_limits(policy);
> > >
> > >         policy->cached_target_freq = UINT_MAX;
> > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > > index dd66decf2087..e1881313c396 100644
> > > --- a/drivers/cpufreq/intel_pstate.c
> > > +++ b/drivers/cpufreq/intel_pstate.c
> > > @@ -2081,11 +2081,24 @@ static void intel_pstate_adjust_policy_max(struct cpufreq_policy *policy,
> > >
> > >  static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
> > >  {
> > > +       int max_freq;
> > >         struct cpudata *cpu = all_cpu_data[policy->cpu];
> > >
> > >         update_turbo_state();
> >
> > Well, update_turbo_state() should handle the case at hand already.
> >
> > That's what it's for actually: It checks if
> > MSR_IA32_MISC_ENABLE_TURBO_DISABLE is set and sets
> > global.turbo_disabled is that's the case.
> >
> > Why isn't that sufficient?
> >
> update_turbo_state() changes the flag of global.turbo_diabled but we
> need to also leverage it to adjust the policy.max accordingly. This is why
> we add intel_pstate_get_max_freq() to get the updated max freq in
> intel_pstate_verify_policy().

Yes, that's why intel_pstate_verify_policy() passes the return value
of intel_pstate_get_max_freq() as the second arg
cpufreq_verify_within_limits(), so really my question was about why
cpuinfo.max_freq needed to be updated (below).

> > > +       max_freq = intel_pstate_get_max_freq(cpu);
> > > +
> > > +       if (acpi_ppc && policy->max == policy->cpuinfo.max_freq &&
> > > +           max_freq != policy->cpuinfo.max_freq) {
> > > +               /*
> > > +                * System was not running under any constraints, but the
> > > +                * current max possible frequency is changed. So reset
> > > +                * policy limits.
> > > +                */
> > > +               policy->cpuinfo.max_freq = policy->max = max_freq;
> > > +       }
> >
> > Why does policy->cpuinfo.max_freq need to be updated?
> >
> This is my understanding:
> There's a corner case that, if the system boots with battery,
> the max cpu frequency will not scale up if we plug the AC later.

I see.  The *initial* cpuinfo.max_freq may be too low.  This part is
missing from your patch changelog.

The driver is not expected to update cpuinfo.max_freq after init.
That may not actually break anything, even though it is racy in
principle, but if it is done, it needs to be done in the "passive"
mode too and that may be more problematic.

Anyway, this is more fundamental than you seem to be thinking.

> According to the log provided by Gabriele Mazzotta,  if the system
> boot up with battery, then plug the AC after boot up, the max perf ratio
> and policy->cpuinfo.max will remain 17 rather than increasing to
> 30(when AC plugged thus turbo enabled):
>
> [   52.158810] CPU 0: _PPC is 6 - frequency  limited
> [   52.158822] intel_pstate: set_policy cpuinfo.max 1700000 policy->max 1700000
> [   52.158825] intel_pstate: cpu:0 max_state 30 min_policy_perf:8 max_policy_perf:17
> [   52.158827] intel_pstate: cpu:0 global_min:8 global_max:30
> [   52.158829] intel_pstate: cpu:0 max_perf_ratio:17 min_perf_ratio:8
>
> This is caused by:
> In current intel_pstate, there's only one chance for policy.cpuinfo.max to get updated
> which is during boot up in __intel_pstate_cpu_init(). If the turbo status changes,
> we might need to also update the policy->cpuinfo.max to tell user that the hardware
> status has changed.
>
> So we give it a chance to adjust the policy.cpuinfo.max and policy.max in
> cpufreq_driver->verify()  according to turbo status, this is what this patch mainly
> aims to do.
>
> Besides, since on this platform there's only one _PPC notification for one CPU, it is
> necessary to broadcast the notification to all CPUs on this package. And this patch
> broadcast it to all online CPUs to make the change simpler.

You're trying to make two substantial changes in one go, broadcasting
_PPC and updating cpuinfo.max_freq.  Don't do that, they need to be
separate changes.

Moreover, we may want to address the initial cpuinfo.max_freq issue in
a different way.

Thanks,
Rafael

Patch

diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index a303fd0e108c..737dbf5aa7f7 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -63,6 +63,10 @@  module_param(ignore_ppc, int, 0644);
 MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
 		 "limited by BIOS, this should help");
 
+static int broadcast_ppc;
+module_param(broadcast_ppc, int, 0644);
+MODULE_PARM_DESC(broadcast_ppc, "Broadcast the ppc to all online CPUs");
+
 #define PPC_REGISTERED   1
 #define PPC_IN_USE       2
 
@@ -180,8 +184,16 @@  void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag)
 		else
 			acpi_processor_ppc_ost(pr->handle, 0);
 	}
-	if (ret >= 0)
-		cpufreq_update_policy(pr->id);
+	if (ret >= 0) {
+		if (broadcast_ppc) {
+			int cpu;
+
+			for_each_possible_cpu(cpu)
+				cpufreq_update_policy(cpu);
+		} else {
+			cpufreq_update_policy(pr->id);
+		}
+	}
 }
 
 int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index e35a886e00bc..95e08816b512 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2237,6 +2237,8 @@  static int cpufreq_set_policy(struct cpufreq_policy *policy,
 
 	policy->min = new_policy->min;
 	policy->max = new_policy->max;
+	policy->cpuinfo.max_freq = new_policy->cpuinfo.max_freq;
+	policy->cpuinfo.min_freq = new_policy->cpuinfo.min_freq;
 	trace_cpu_frequency_limits(policy);
 
 	policy->cached_target_freq = UINT_MAX;
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index dd66decf2087..e1881313c396 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2081,11 +2081,24 @@  static void intel_pstate_adjust_policy_max(struct cpufreq_policy *policy,
 
 static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
 {
+	int max_freq;
 	struct cpudata *cpu = all_cpu_data[policy->cpu];
 
 	update_turbo_state();
+	max_freq = intel_pstate_get_max_freq(cpu);
+
+	if (acpi_ppc && policy->max == policy->cpuinfo.max_freq &&
+	    max_freq != policy->cpuinfo.max_freq) {
+		/*
+		 * System was not running under any constraints, but the
+		 * current max possible frequency is changed. So reset
+		 * policy limits.
+		 */
+		policy->cpuinfo.max_freq = policy->max = max_freq;
+	}
+
 	cpufreq_verify_within_limits(policy, policy->cpuinfo.min_freq,
-				     intel_pstate_get_max_freq(cpu));
+				     max_freq);
 
 	if (policy->policy != CPUFREQ_POLICY_POWERSAVE &&
 	    policy->policy != CPUFREQ_POLICY_PERFORMANCE)