| Message ID | c3e867be691b6fdb3d34e378199b297256aeebff.1718095377.git.perry.yuan@amd.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mario Limonciello |
| Headers | show |
| Series | AMD Pstate Driver Fixes and Improvements | expand |
On 6/11/2024 03:52, 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> > Acked-by: Gautham R. Shenoy <gautham.shenoy@amd.com> Acked-by: Mario Limonciello <mario.limonciello@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 9ad62dbe8bfb..37fce0569d06 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -921,6 +921,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 * 1000); > + 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); > + return -EINVAL; > + } > + > return 0; > } > > @@ -959,15 +977,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); > > @@ -1420,14 +1429,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;
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index 9ad62dbe8bfb..37fce0569d06 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -921,6 +921,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 * 1000); + 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); + return -EINVAL; + } + return 0; } @@ -959,15 +977,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); @@ -1420,14 +1429,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;