Re: [Xen-devel] [RFC PATCH 04/31] cpufreq: make turbo settings to be configurable

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

Hi, Stefano.

On Tue, Dec 5, 2017 at 9:24 PM, Stefano Stabellini
<sstabellini@xxxxxxxxxx> wrote:
> On Tue, 5 Dec 2017, Oleksandr Tyshchenko wrote:
>> >> Another question is second_max_freq. As I understand, it is highest
>> >> non-turbo frequency calculated by framework to limit target frequency
>> >> when
>> >> turbo mode "is disabled". And Xen assumes that second_max_freq is
>> >> always P1 if turbo mode is on.
>> >> But, there might be a case when a few highest frequencies are
>> >> turbo-frequencies. So, I propose to add an extra flag for handling
>> >> that.
>> >> So, each CPUFreq driver responsibility will be to mark
>> >> turbo-frequency(ies) for the framework to properly calculate
>> >> second_max_freq.
>> >
>> > As Andre wrote, we can start simply assuming that ARM doesn't have
>> > turbo. If turbo mode is assumed to be off, I don't think we need the
>> > patch below and the new flag, because second_max_freq == max_freq.
>>
>> I just want to show you real example, where we have ARM SoC +
>> turbo-mode + > 1 turbo freq
>> https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/tree/arch/arm64/boot/dts/renesas/r8a7795.dtsi?h=v4.9/rcar-3.5.9#n197
>> As you can see, there are two freqs marked as turbo-freqs: 1600000000
>> Hz and 1700000000 Hz
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git/tree/arch/arm64/boot/dts/renesas/r8a7796.dtsi?h=v4.9/rcar-3.5.9#n166
>> For M3 SoC three turbo-freqs are used: 1600000000 Hz, 1700000000 Hz
>> and 1800000000 Hz
>
> Oh well, I take that back then :-)
>
>
>> If a proposed below patch is not an option then we should find another
>> way to clarify second_max_freq.
>
> Yes, it looks like there must be better ways to define second_max_freq.
> Taking the first frequency below the max seems a bit crude to me.
>
>
>> >
>> >> Something like that:
>> >>
>> >> diff --git a/xen/drivers/cpufreq/utility.c b/xen/drivers/cpufreq/utility.c
>> >> index 25bf983..122a88b 100644
>> >> --- a/xen/drivers/cpufreq/utility.c
>> >> +++ b/xen/drivers/cpufreq/utility.c
>> >> @@ -226,7 +226,8 @@ int cpufreq_frequency_table_cpuinfo(struct
>> >> cpufreq_policy *policy,
>> >>  #ifdef CONFIG_HAS_CPU_TURBO
>> >>      for (i=0; (table[i].frequency != CPUFREQ_TABLE_END); i++) {
>> >>          unsigned int freq = table[i].frequency;
>> >> -        if (freq == CPUFREQ_ENTRY_INVALID || freq == max_freq)
>> >> +        if ((freq == CPUFREQ_ENTRY_INVALID) ||
>> >> +            (table[i].flags & CPUFREQ_BOOST_FREQ))
>> >>              continue;
>> >>          if (freq > second_max_freq)
>> >>              second_max_freq = freq;
>> >> diff --git a/xen/include/xen/cpufreq.h b/xen/include/xen/cpufreq.h
>> >> index 2e0c16a..77b29da 100644
>> >> --- a/xen/include/xen/cpufreq.h
>> >> +++ b/xen/include/xen/cpufreq.h
>> >> @@ -204,7 +204,11 @@ void cpufreq_verify_within_limits(struct
>> >> cpufreq_policy *policy,
>> >>  #define CPUFREQ_ENTRY_INVALID ~0
>> >>  #define CPUFREQ_TABLE_END     ~1
>> >>
>> >> +/* Special Values of .flags field */
>> >> +#define CPUFREQ_BOOST_FREQ    (1 << 0)
>> >> +
>> >>  struct cpufreq_frequency_table {
>> >> +       unsigned int    flags;
>> >>      unsigned int    index;     /* any */
>> >>      unsigned int    frequency; /* kHz - doesn't need to be in ascending
>> >>                                  * order */
>> >>
>> >> Both existing on x86 CPUFreq drivers just need to mark P0 frequency as
>> >> a turbo-frequency if turbo mode "is supported". Am I correct?
>
> Yes, I think it is a better approach than what we have today, even for
> x86.

OK, I will prepare patches which will include these changes to common part and
changes to the existing on x86 CPUFreq drivers (to mark P0 frequency as
a turbo-frequency if turbo mode "is supported") if nobody mind.

>
>
>> >> And the most important question is how to recognize in Xen on ARM
>> >> (using SCPI protocol) which frequencies are turbo-frequencies
>> >> actually? I couldn't find any information regarding that in protocol
>> >> description.
>> >> For DT-based CPUFreq it is not an issue, since there is a specific
>> >> property "turbo-mode" to mark corresponding OPPs. [1].
>> >> But neither SCPI DT bindings [2] nor the SCPI protocol itself [3]
>> >> mentions about it. Perhaps, additional command should be added to pass
>> >> such info.
>> >>
>> >> [1] 
>> >> https://www.kernel.org/doc/Documentation/devicetree/bindings/opp/opp.txt
>> >> [2] 
>> >> http://elixir.free-electrons.com/linux/v4.15-rc1/source/Documentation/devicetree/bindings/arm/arm,scpi.txt
>> >> [3] 
>> >> http://infocenter.arm.com/help/topic/com.arm.doc.dui0922g/scp_message_interface_v1_2_DUI0922G_en.pdf
>
> If there are no mentions of them, then I would assume that none of the
> available frequencies are turbo frequencies.



-- 
Regards,

Oleksandr Tyshchenko

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel