On Tue, Aug 27, 2024 at 01:36:47PM -0500, Mario Limonciello wrote: > On 8/27/2024 11:52, Gautham R. Shenoy wrote: [..snip..] > > > > > > So henceforth, cpudata->highest_perf is expected to cache the value of > > CPPC.highest_perf and not the boost_ratio_numerator. There are couple > > of user-visible changes due to this. > > > > > > 1. On platforms where preferred-core is supported, previously the > > sysfs file > > /sys/devices/system/cpu/cpuX/cpufreq/amd_pstate_highest_perf would > > report the boost_ratio_numerator. Henceforth it will report > > CPPC.highest_perf. One other side effect is that the highest_perf sysfs file will now reveal the differential highest_perf, even when "amd_prefcore=false", while earlier all the cores would report CPPC_HIGHEST_PERF_DEFAULT. I think we may be better off reporting the boost-numerator here, but that's really not the highest_perf :( > > > > I hope there are no userspace tools that compute the boost_ratio > > using the syfs amd_pstate_highest_perf/amd_pstate_nominal_perf. > > > > 2. The amd_pstate_prefcore_ranking and amd_pstate_highest_perf will > > show the same values on all platforms, and henceforth are > > redundant. > > > > Good observations here. I'm not aware of any tools trying to replicate this > calculation. > With the redundancy I would actually argue we should just drop the sysfs > file 'amd_pstate_prefcore_ranking'. > > Thoughts? Looking at the code again, I realize that I was wrong. cpudata->prefcore_ranking also gets updated in amd_pstate_update_min_max_limits() and reflects the dynamic preference. While cpudata->highest_perf value indicates the initial boot-time preference. Hence it makes sense to amd_pstate_prefcore_ranking. > > > > > Shouldn't this be documented? > > I noticed amd_pstate_prefcore_ranking wasn't properly documented in > amd-pstate.rst in the first place. If the decision is not to drop the sysfs > file, then I'll add a section for it. Thanks. > > > > > The rest of the patch looks good to me. > > > > > > -- Thanks and Regards gautham.
