Adding support for cpu intel_pstate scaling driver

[CZerta]

Resolves: rhbz#2095829

Signed-off-by: Jan Zerdik [email protected]

[CZerta]

@joemario

I suspect you need both a

def intel_energy_performance_available_preferences(...) and a def intel_energy_performance_preference(...)

The method _intel_preference_path is used for both of them depanding on available argument, so I don't need to hard code two basically identical path.

You might want to remove the "pstate" from the "intel_pstate_preference_path name(s). That's because with the upcoming kernel patch discussed in the BZ should make the energy_performance* files available whether intel_pstate is enabled or not.

Removed.

The comments in this section of new code (lines 539-555) mention "energy_perf_bias". Unless I'm missing something, those comments should be mentioning energy_performance_preference.

This is because it is set through "energy_perf_bias" option. We can add new one I don't think it makes sense as it is setting same thing and renaming would break already written profiles.

[joemario]

Jan: Before you complete this work:

1. We need to have @prarit respond with confirmation that the algorithm he said TuneD needs to use is indeed valid.

2. The perf team and Jirka Hladly's team needs to finish its perf testing to show that not setting energy_perf_bias (e.g. to 0) and letting it remain with the kernel default of 6, does not hurt performance. We've done a lot of testing so far which shows it will be fine to make this change. But hold off for a final ack from us.

Thank you. Joe

[prarit]

The algorithm described in the BZ is correct.

[joemario]

Hi Jan:

Joe wrote:

The comments in this section of new code (lines 539-555) mention "energy_perf_bias". Unless I'm missing something, those comments should be mentioning energy_performance_preference.

Jan replied: This is because it is set through "energy_perf_bias" option. We can add new one I don't think it makes sense as it is setting same thing and renaming would break already written profiles.

I must be missing something. Energy_perf_bias and energy_performance_preference are two separate flags, with different meanings. I'm wondering if my confusion stems from TuneD treating them together from a single line in a profile.

Is it true that if the user profile states: energy_perf_bias=performance that TuneD would previously set both energy_perf_bias and energy_performance_preference to "performance"?

If that's true, then I can see where the new code could cause confusion:

For example, look at the "_get_energy_perf_bias() definition. The newly added lines will first look to see if energy_performance_available_preferences exists, and if it does, then return the value in energy_performance_preference. If the request is to return the value of energy_perf_bias, shouldn't the call be returning it and not returning energy_performance_preference?
Just because TuneD may not have set energy_perf_bias earlier, it still is a valid kernel value that is different than energy_performance_preferrence.

And earlier on in the _set_energy_perf_bias() definition, there's more opportunity for confusion. For example, if the user asks for a bogus energy_performance_preference value, then that routine will return with warning messages about how it couldn't set "energy_perf_bias", when it was really "energy_performance_preference" that it could not set.

Does this make sense? Joe

[CZerta]

@joemario Hi. I pushed changed version. Can you check if it's like you expected it?