Message ID | cover.1715356532.git.perry.yuan@amd.com |
---|---|
Headers | show |
Series | AMD Pstate Driver Fixes and Improvements | expand |
Hello Perry, On 5/13/2024 7:37 AM, Perry Yuan wrote: > To enhance the debugging capability of the driver loading failure for > broken CPPC ACPI tables, it can optimize the expression by moving the > verification of `min_freq`, `nominal_freq`, and other dependency values > to the `amd_pstate_init_freq()` function where they are initialized. > If any of these values are incorrect, the `amd-pstate` driver will not be registered. > > By ensuring that these values are correct before they are used, it will facilitate > the debugging process when encountering driver loading failures due to faulty CPPC > ACPI tables from BIOS > > Signed-off-by: Perry Yuan <perry.yuan@amd.com> > --- > drivers/cpufreq/amd-pstate.c | 35 ++++++++++++++++++----------------- > 1 file changed, 18 insertions(+), 17 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index 6a342b0c0140..614f6fac0764 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -889,6 +889,24 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata) > WRITE_ONCE(cpudata->nominal_freq, nominal_freq); > WRITE_ONCE(cpudata->max_freq, max_freq); > > + /** > + * Below values need to be initialized correctly, otherwise driver will fail to load > + * max_freq is calculated according to (nominal_freq * highest_perf)/nominal_perf > + * lowest_nonlinear_freq is a value between [min_freq, nominal_freq] > + * Check _CPC in ACPI table objects if any values are incorrect > + */ > + if (min_freq <= 0 || max_freq <= 0 || nominal_freq <= 0 || min_freq > max_freq) { > + pr_err("min_freq(%d) or max_freq(%d) or nominal_freq(%d) value is incorrect\n", > + min_freq, max_freq, nominal_freq); > + return -EINVAL; > + } > + > + if (lowest_nonlinear_freq <= min_freq || lowest_nonlinear_freq > nominal_freq * 1000) { > + pr_err("lowest_nonlinear_freq(%d) value is out of range [min_freq(%d), nominal_freq(%d)]\n", > + lowest_nonlinear_freq, min_freq, nominal_freq * 1000); A reminder, we should fix the below code section (due to only nominal freq being in MHz), 685 static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state) 686 { 687 struct amd_cpudata *cpudata = policy->driver_data; 688 int ret; 689 690 if (!cpudata->boost_supported) { 691 pr_err("Boost mode is not supported by this processor or SBIOS\n"); 692 return -EINVAL; 693 } 694 695 if (state) 696 policy->cpuinfo.max_freq = cpudata->max_freq; 697 else 698 policy->cpuinfo.max_freq = cpudata->nominal_freq; <--- mismatch in left and right hand side units To avoid below situation(from a Zen4 AMD EPYC system with boost disabled), /sys/devices/system/cpu/cpu0/cpufreq# cat scaling_max_freq 2151 <--- MHz /sys/devices/system/cpu/cpu0/cpufreq# cat amd_pstate_max_freq 2287000 <--- KHz Thanks, Dhananjay > + return -EINVAL; > + } > + > return 0; > } > > @@ -927,15 +945,6 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy) > max_freq = READ_ONCE(cpudata->max_freq); > nominal_freq = READ_ONCE(cpudata->nominal_freq); > > - if (min_freq <= 0 || max_freq <= 0 || > - nominal_freq <= 0 || min_freq > max_freq) { > - dev_err(dev, > - "min_freq(%d) or max_freq(%d) or nominal_freq (%d) value is incorrect, check _CPC in ACPI tables\n", > - min_freq, max_freq, nominal_freq); > - ret = -EINVAL; > - goto free_cpudata1; > - } > - > policy->cpuinfo.transition_latency = amd_pstate_get_transition_latency(policy->cpu); > policy->transition_delay_us = amd_pstate_get_transition_delay_us(policy->cpu); > > @@ -1388,14 +1397,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy) > min_freq = READ_ONCE(cpudata->min_freq); > max_freq = READ_ONCE(cpudata->max_freq); > nominal_freq = READ_ONCE(cpudata->nominal_freq); > - if (min_freq <= 0 || max_freq <= 0 || > - nominal_freq <= 0 || min_freq > max_freq) { > - dev_err(dev, > - "min_freq(%d) or max_freq(%d) or nominal_freq(%d) value is incorrect, check _CPC in ACPI tables\n", > - min_freq, max_freq, nominal_freq); > - ret = -EINVAL; > - goto free_cpudata1; > - } > > policy->cpuinfo.min_freq = min_freq; > policy->cpuinfo.max_freq = max_freq;