Re: [PATCH V3] SKL intel_pstate update MSR values when changing governors

[Rafael J. Wysocki]

Hi,

On Sat, Nov 21, 2015 at 1:16 AM, Srinivas Pandruvada
<srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote:
>
>
> On 11/18/2015 02:58 PM, Alexandra Yates wrote:
>>
>> When changing from powersave to performance governors
>> Intel_pstate fails to update the MSR values that reflect the
>> max_perf_pct to 100%. For instance in SKL reading rdmsr 0x774:
>>
>> Governor        MSR                     max_perf_pct
>> =========       ========                ============
>> Powersave       80002808                100%
>> Powersave       80002008                80%
>> Performance     80002028 [error]        100%
>> Performance     80002828 [expected]     100%
>>
>> The line label [error] shows the culprit. At this point the MSR
>> should reflect the max_perf_pct that is 100%, that corresponds
>> to MSR 80002828 as shown on the next line of the example. Which
>> is the maximum performance for the Performance governor.
>> Instead it holds back the MSR value previously set by the
>> Powersave, in this case 80002028.
>>
>> This patch allows the system to print the correct MSR value
>> 80002828 that corresponds to the 100% max_perf_pct when changing
>> from powersave to performance governors.

Is this only about what is printed or does it mean that the driver
actually uses an incorrect MSR value?

>> For more information on the MSR values for SKL please visit
>> ISDM under Managing HWP.
>>
>> Signed-off-by: Alexandra Yates <alexandra.yates@xxxxxxxxxxxxxxx>
>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
>>
>> ---
>>   drivers/cpufreq/intel_pstate.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/cpufreq/intel_pstate.c
>> b/drivers/cpufreq/intel_pstate.c
>> index 2e31d09..0eeb7da 100644
>> --- a/drivers/cpufreq/intel_pstate.c
>> +++ b/drivers/cpufreq/intel_pstate.c
>> @@ -1242,6 +1242,8 @@ static int intel_pstate_set_policy(struct
>> cpufreq_policy *policy)
>>             policy->max >= policy->cpuinfo.max_freq) {
>>                 pr_debug("intel_pstate: set performance\n");
>>                 limits = &performance_limits;
>> +               if (hwp_active)
>> +                       intel_pstate_hwp_set();

Honestly, I'm not really sure how this is matching the changelog.

What it does is to ensure that the correct limits are used when in the
HWP mode too as far as I can say.  Is my understanding correct here?

>>                 return 0;
>>         }
>>
>>
> --

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/