[RFC,v3,00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs
mbox series

Message ID 20200127200350.24465-1-sibis@codeaurora.org
Headers show
Series
  • DDR/L3 Scaling support on SDM845 and SC7180 SoCs
Related show

Message

Sibi Sankar Jan. 27, 2020, 8:03 p.m. UTC
This RFC series aims to extend cpu based scaling support to L3/DDR on
SDM845 and SC7180 SoCs.

Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
Patches [5-7] - Hack in a way to add/remove multiple opp tables to
                a single device. I am yet to fix the debugfs to
		support multiple opp_tables per device but wanted to
		send what was working upstream to get an idea if multiple
		opp tables per device is a feature that will be useful
		upstream.
Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
                 and SC7180 SoCs.

v3:
 * Migrated to using Saravana's opp-kBps bindings [1]
 * Fixed some misc comments from Rajendra
 * Added support for SC7180

v2:
 * Incorporated Viresh's comments from:
 https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
 https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
 * Dropped cpufreq-map passive governor

Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420

Some alternate ways of hosting the opp-tables:
https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3
https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98
Viresh didn't really like ^^ bindings and they dont really scale well. Just
including them here for completeness.

Depends on the following series:
[1] https://patchwork.kernel.org/cover/11277199/
[2] https://patchwork.kernel.org/cover/11055499/ 
[3] https://patchwork.kernel.org/cover/11326381/

Sibi Sankar (10):
  arm64: dts: qcom: sdm845: Add SoC compatible to MTP
  cpufreq: blacklist SDM845 in cpufreq-dt-platdev
  cpufreq: blacklist SC7180 in cpufreq-dt-platdev
  OPP: Add and export helper to update voltage
  opp: of: export _opp_of_get_opp_desc_node
  opp: Allow multiple opp_tables to be mapped to a single device
  opp: Remove multiple attached opp tables from a device
  cpufreq: qcom: Update the bandwidth levels on frequency change
  arm64: dts: qcom: sdm845: Add cpu OPP tables
  arm64: dts: qcom: sc7180: Add cpu OPP tables

 arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
 arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 ++++++++++++++++++++++++
 drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
 drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
 drivers/opp/core.c                      | 111 +++++-
 drivers/opp/of.c                        |   3 +-
 drivers/opp/opp.h                       |   2 +
 include/linux/pm_opp.h                  |  10 +
 9 files changed, 1083 insertions(+), 33 deletions(-)

Comments

Rob Clark Jan. 29, 2020, 2:54 a.m. UTC | #1
On Mon, Jan 27, 2020 at 12:05 PM Sibi Sankar <sibis@codeaurora.org> wrote:
>
> This RFC series aims to extend cpu based scaling support to L3/DDR on
> SDM845 and SC7180 SoCs.
>
> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
> Patches [5-7] - Hack in a way to add/remove multiple opp tables to
>                 a single device. I am yet to fix the debugfs to
>                 support multiple opp_tables per device but wanted to
>                 send what was working upstream to get an idea if multiple
>                 opp tables per device is a feature that will be useful
>                 upstream.
> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
>                  and SC7180 SoCs.
>
> v3:
>  * Migrated to using Saravana's opp-kBps bindings [1]
>  * Fixed some misc comments from Rajendra
>  * Added support for SC7180
>
> v2:
>  * Incorporated Viresh's comments from:
>  https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
>  https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
>  * Dropped cpufreq-map passive governor
>
> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420
>
> Some alternate ways of hosting the opp-tables:
> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3
> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98
> Viresh didn't really like ^^ bindings and they dont really scale well. Just
> including them here for completeness.
>
> Depends on the following series:
> [1] https://patchwork.kernel.org/cover/11277199/
> [2] https://patchwork.kernel.org/cover/11055499/
> [3] https://patchwork.kernel.org/cover/11326381/

So drive-by question, from the perspective of someone who cares about
a non-cpu device that frequently enjoys a lot of bandwidth (ie. the
GPU).. any thoughts on how ddr scaling would/should work for workloads
where the CPU is not particularly busy?

BR,
-R

>
> Sibi Sankar (10):
>   arm64: dts: qcom: sdm845: Add SoC compatible to MTP
>   cpufreq: blacklist SDM845 in cpufreq-dt-platdev
>   cpufreq: blacklist SC7180 in cpufreq-dt-platdev
>   OPP: Add and export helper to update voltage
>   opp: of: export _opp_of_get_opp_desc_node
>   opp: Allow multiple opp_tables to be mapped to a single device
>   opp: Remove multiple attached opp tables from a device
>   cpufreq: qcom: Update the bandwidth levels on frequency change
>   arm64: dts: qcom: sdm845: Add cpu OPP tables
>   arm64: dts: qcom: sc7180: Add cpu OPP tables
>
>  arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
>  arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 ++++++++++++++++++++++++
>  drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
>  drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
>  drivers/opp/core.c                      | 111 +++++-
>  drivers/opp/of.c                        |   3 +-
>  drivers/opp/opp.h                       |   2 +
>  include/linux/pm_opp.h                  |  10 +
>  9 files changed, 1083 insertions(+), 33 deletions(-)
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
Lukasz Luba Jan. 29, 2020, 9:46 a.m. UTC | #2
Hi Sibi,

In my opinion this solution depends on not always true assumption that
CPUFreq notification chain will be triggered when there is a frequency
switch. Extending devfreq governor (as in one of the dependent patch
series that you have referred) by attaching to this notification
chain makes sense only when the SchedUtil and fast_switch is not in use.
The Schedutil CPUFreq governor might use the fast_switch from this
driver and the notifications will not be triggered. I have also
commented patch 08/10 which tries to disable it.

Regards,
Lukasz

On 1/27/20 8:03 PM, Sibi Sankar wrote:
> This RFC series aims to extend cpu based scaling support to L3/DDR on
> SDM845 and SC7180 SoCs.
> 
> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
> Patches [5-7] - Hack in a way to add/remove multiple opp tables to
>                  a single device. I am yet to fix the debugfs to
> 		support multiple opp_tables per device but wanted to
> 		send what was working upstream to get an idea if multiple
> 		opp tables per device is a feature that will be useful
> 		upstream.
> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
>                   and SC7180 SoCs.
> 
> v3:
>   * Migrated to using Saravana's opp-kBps bindings [1]
>   * Fixed some misc comments from Rajendra
>   * Added support for SC7180
> 
> v2:
>   * Incorporated Viresh's comments from:
>   https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
>   https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
>   * Dropped cpufreq-map passive governor
> 
> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420
> 
> Some alternate ways of hosting the opp-tables:
> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3
> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98
> Viresh didn't really like ^^ bindings and they dont really scale well. Just
> including them here for completeness.
> 
> Depends on the following series:
> [1] https://patchwork.kernel.org/cover/11277199/
> [2] https://patchwork.kernel.org/cover/11055499/
> [3] https://patchwork.kernel.org/cover/11326381/
> 
> Sibi Sankar (10):
>    arm64: dts: qcom: sdm845: Add SoC compatible to MTP
>    cpufreq: blacklist SDM845 in cpufreq-dt-platdev
>    cpufreq: blacklist SC7180 in cpufreq-dt-platdev
>    OPP: Add and export helper to update voltage
>    opp: of: export _opp_of_get_opp_desc_node
>    opp: Allow multiple opp_tables to be mapped to a single device
>    opp: Remove multiple attached opp tables from a device
>    cpufreq: qcom: Update the bandwidth levels on frequency change
>    arm64: dts: qcom: sdm845: Add cpu OPP tables
>    arm64: dts: qcom: sc7180: Add cpu OPP tables
> 
>   arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
>   arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
>   arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 ++++++++++++++++++++++++
>   drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
>   drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
>   drivers/opp/core.c                      | 111 +++++-
>   drivers/opp/of.c                        |   3 +-
>   drivers/opp/opp.h                       |   2 +
>   include/linux/pm_opp.h                  |  10 +
>   9 files changed, 1083 insertions(+), 33 deletions(-)
>
Sibi Sankar Jan. 29, 2020, 2:21 p.m. UTC | #3
Hey Rob,

On 2020-01-29 08:24, Rob Clark wrote:
> On Mon, Jan 27, 2020 at 12:05 PM Sibi Sankar <sibis@codeaurora.org> 
> wrote:
>> 
>> This RFC series aims to extend cpu based scaling support to L3/DDR on
>> SDM845 and SC7180 SoCs.
>> 
>> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
>> Patches [5-7] - Hack in a way to add/remove multiple opp tables to
>>                 a single device. I am yet to fix the debugfs to
>>                 support multiple opp_tables per device but wanted to
>>                 send what was working upstream to get an idea if 
>> multiple
>>                 opp tables per device is a feature that will be useful
>>                 upstream.
>> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
>>                  and SC7180 SoCs.
>> 
>> v3:
>>  * Migrated to using Saravana's opp-kBps bindings [1]
>>  * Fixed some misc comments from Rajendra
>>  * Added support for SC7180
>> 
>> v2:
>>  * Incorporated Viresh's comments from:
>>  
>> https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
>>  
>> https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
>>  * Dropped cpufreq-map passive governor
>> 
>> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420
>> 
>> Some alternate ways of hosting the opp-tables:
>> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3
>> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98
>> Viresh didn't really like ^^ bindings and they dont really scale well. 
>> Just
>> including them here for completeness.
>> 
>> Depends on the following series:
>> [1] https://patchwork.kernel.org/cover/11277199/
>> [2] https://patchwork.kernel.org/cover/11055499/
>> [3] https://patchwork.kernel.org/cover/11326381/
> 
> So drive-by question, from the perspective of someone who cares about
> a non-cpu device that frequently enjoys a lot of bandwidth (ie. the
> GPU).. any thoughts on how ddr scaling would/should work for workloads
> where the CPU is not particularly busy?

I guess Jordan would have more info
about ^^. From what I can see from
the CAF trees there is a similar
device freq -> DDR Freq mapping that
you can achieve with required-opps
bindings and a custom governor ;)

> 
> BR,
> -R
> 
>> 
>> Sibi Sankar (10):
>>   arm64: dts: qcom: sdm845: Add SoC compatible to MTP
>>   cpufreq: blacklist SDM845 in cpufreq-dt-platdev
>>   cpufreq: blacklist SC7180 in cpufreq-dt-platdev
>>   OPP: Add and export helper to update voltage
>>   opp: of: export _opp_of_get_opp_desc_node
>>   opp: Allow multiple opp_tables to be mapped to a single device
>>   opp: Remove multiple attached opp tables from a device
>>   cpufreq: qcom: Update the bandwidth levels on frequency change
>>   arm64: dts: qcom: sdm845: Add cpu OPP tables
>>   arm64: dts: qcom: sc7180: Add cpu OPP tables
>> 
>>  arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
>>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
>>  arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 
>> ++++++++++++++++++++++++
>>  drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
>>  drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
>>  drivers/opp/core.c                      | 111 +++++-
>>  drivers/opp/of.c                        |   3 +-
>>  drivers/opp/opp.h                       |   2 +
>>  include/linux/pm_opp.h                  |  10 +
>>  9 files changed, 1083 insertions(+), 33 deletions(-)
>> 
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>> a Linux Foundation Collaborative Project
Sibi Sankar Jan. 29, 2020, 2:37 p.m. UTC | #4
Hey Lukasz,
Thanks for taking time to review
the series!

On 2020-01-29 15:16, Lukasz Luba wrote:
> Hi Sibi,
> 
> In my opinion this solution depends on not always true assumption that
> CPUFreq notification chain will be triggered when there is a frequency

This series does not create any
devfreq devices nor use the cpufreq
notification chain. It just relies
on the opening up of required-opps
from being exclusive to gen-pd using
patches 1,2,3 from patch series[1].
With the fast path disabled and
schedutil enabled, this series will
not miss any cpufreq changes.

[1] https://patchwork.kernel.org/cover/11055499/

> switch. Extending devfreq governor (as in one of the dependent patch
> series that you have referred) by attaching to this notification
> chain makes sense only when the SchedUtil and fast_switch is not in 
> use.

fast_switch and cpu notifier chains
are mutually exclusive but schedutil
will still operate in the slow path
IIRC.

> The Schedutil CPUFreq governor might use the fast_switch from this
> driver and the notifications will not be triggered. I have also
> commented patch 08/10 which tries to disable it.
> 
> Regards,
> Lukasz
> 
> On 1/27/20 8:03 PM, Sibi Sankar wrote:
>> This RFC series aims to extend cpu based scaling support to L3/DDR on
>> SDM845 and SC7180 SoCs.
>> 
>> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
>> Patches [5-7] - Hack in a way to add/remove multiple opp tables to
>>                  a single device. I am yet to fix the debugfs to
>> 		support multiple opp_tables per device but wanted to
>> 		send what was working upstream to get an idea if multiple
>> 		opp tables per device is a feature that will be useful
>> 		upstream.
>> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
>>                   and SC7180 SoCs.
>> 
>> v3:
>>   * Migrated to using Saravana's opp-kBps bindings [1]
>>   * Fixed some misc comments from Rajendra
>>   * Added support for SC7180
>> 
>> v2:
>>   * Incorporated Viresh's comments from:
>>   
>> https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
>>   
>> https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
>>   * Dropped cpufreq-map passive governor
>> 
>> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420
>> 
>> Some alternate ways of hosting the opp-tables:
>> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3
>> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98
>> Viresh didn't really like ^^ bindings and they dont really scale well. 
>> Just
>> including them here for completeness.
>> 
>> Depends on the following series:
>> [1] https://patchwork.kernel.org/cover/11277199/
>> [2] https://patchwork.kernel.org/cover/11055499/
>> [3] https://patchwork.kernel.org/cover/11326381/
>> 
>> Sibi Sankar (10):
>>    arm64: dts: qcom: sdm845: Add SoC compatible to MTP
>>    cpufreq: blacklist SDM845 in cpufreq-dt-platdev
>>    cpufreq: blacklist SC7180 in cpufreq-dt-platdev
>>    OPP: Add and export helper to update voltage
>>    opp: of: export _opp_of_get_opp_desc_node
>>    opp: Allow multiple opp_tables to be mapped to a single device
>>    opp: Remove multiple attached opp tables from a device
>>    cpufreq: qcom: Update the bandwidth levels on frequency change
>>    arm64: dts: qcom: sdm845: Add cpu OPP tables
>>    arm64: dts: qcom: sc7180: Add cpu OPP tables
>> 
>>   arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
>>   arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
>>   arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 
>> ++++++++++++++++++++++++
>>   drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
>>   drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
>>   drivers/opp/core.c                      | 111 +++++-
>>   drivers/opp/of.c                        |   3 +-
>>   drivers/opp/opp.h                       |   2 +
>>   include/linux/pm_opp.h                  |  10 +
>>   9 files changed, 1083 insertions(+), 33 deletions(-)
>>
Lukasz Luba Jan. 29, 2020, 3:47 p.m. UTC | #5
On 1/29/20 2:37 PM, Sibi Sankar wrote:
> Hey Lukasz,
> Thanks for taking time to review
> the series!
> 
> On 2020-01-29 15:16, Lukasz Luba wrote:
>> Hi Sibi,
>>
>> In my opinion this solution depends on not always true assumption that
>> CPUFreq notification chain will be triggered when there is a frequency
> 
> This series does not create any
> devfreq devices nor use the cpufreq
> notification chain. It just relies
> on the opening up of required-opps
> from being exclusive to gen-pd using
> patches 1,2,3 from patch series[1].
> With the fast path disabled and
> schedutil enabled, this series will
> not miss any cpufreq changes.

Thank you Sibi for clarifying this. I spotted in patch 08/10
that the fast_switch is removed and the normal path is in use,
which also triggers the notification chain in CPUFreq. Then maybe other
machinery which you have put in your 'depends on' list accidentally
works thanks to this.

> 
> [1] https://patchwork.kernel.org/cover/11055499/
> 
>> switch. Extending devfreq governor (as in one of the dependent patch
>> series that you have referred) by attaching to this notification
>> chain makes sense only when the SchedUtil and fast_switch is not in use.
> 
> fast_switch and cpu notifier chains
> are mutually exclusive but schedutil
> will still operate in the slow path
> IIRC.

True, SchedUtil would work in slow_path. The driver and SoC support
'fast_switch', your solution when is properly initialized, disables it.
I would suggest to put this information in the commit message.
I don't know the side effects on the performance, though. The other
side effect would be: the CPUFreq notifications will be triggered.

The fast_switch is now the preferred way, any new ideas should
consider also this path.

> 
>> The Schedutil CPUFreq governor might use the fast_switch from this
>> driver and the notifications will not be triggered. I have also
>> commented patch 08/10 which tries to disable it.
>>
>> Regards,
>> Lukasz
>>
>> On 1/27/20 8:03 PM, Sibi Sankar wrote:
>>> This RFC series aims to extend cpu based scaling support to L3/DDR on
>>> SDM845 and SC7180 SoCs.
>>>
>>> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
>>> Patches [5-7] - Hack in a way to add/remove multiple opp tables to
>>>                  a single device. I am yet to fix the debugfs to
>>>         support multiple opp_tables per device but wanted to
>>>         send what was working upstream to get an idea if multiple
>>>         opp tables per device is a feature that will be useful
>>>         upstream.
>>> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
>>>                   and SC7180 SoCs.
>>>
>>> v3:
>>>   * Migrated to using Saravana's opp-kBps bindings [1]
>>>   * Fixed some misc comments from Rajendra
>>>   * Added support for SC7180
>>>
>>> v2:
>>>   * Incorporated Viresh's comments from:
>>> https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
>>> https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
>>>   * Dropped cpufreq-map passive governor
>>>
>>> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420
>>>
>>> Some alternate ways of hosting the opp-tables:
>>> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3 
>>>
>>> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98 
>>>
>>> Viresh didn't really like ^^ bindings and they dont really scale 
>>> well. Just
>>> including them here for completeness.
>>>
>>> Depends on the following series:
>>> [1] https://patchwork.kernel.org/cover/11277199/
>>> [2] https://patchwork.kernel.org/cover/11055499/
>>> [3] https://patchwork.kernel.org/cover/11326381/
>>>
>>> Sibi Sankar (10):
>>>    arm64: dts: qcom: sdm845: Add SoC compatible to MTP
>>>    cpufreq: blacklist SDM845 in cpufreq-dt-platdev
>>>    cpufreq: blacklist SC7180 in cpufreq-dt-platdev
>>>    OPP: Add and export helper to update voltage
>>>    opp: of: export _opp_of_get_opp_desc_node
>>>    opp: Allow multiple opp_tables to be mapped to a single device
>>>    opp: Remove multiple attached opp tables from a device
>>>    cpufreq: qcom: Update the bandwidth levels on frequency change
>>>    arm64: dts: qcom: sdm845: Add cpu OPP tables
>>>    arm64: dts: qcom: sc7180: Add cpu OPP tables
>>>
>>>   arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
>>>   arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
>>>   arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 ++++++++++++++++++++++++
>>>   drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
>>>   drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
>>>   drivers/opp/core.c                      | 111 +++++-
>>>   drivers/opp/of.c                        |   3 +-
>>>   drivers/opp/opp.h                       |   2 +
>>>   include/linux/pm_opp.h                  |  10 +
>>>   9 files changed, 1083 insertions(+), 33 deletions(-)
>>>
>
Sibi Sankar March 17, 2020, 8:43 p.m. UTC | #6
On 2020-01-28 01:33, Sibi Sankar wrote:
> This RFC series aims to extend cpu based scaling support to L3/DDR on
> SDM845 and SC7180 SoCs.
> 

Hey Viresh/Saravana,

Ping! Can you take a stab at reviewing
the series, it has been on the list for
a while now.

> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
> Patches [5-7] - Hack in a way to add/remove multiple opp tables to
>                 a single device. I am yet to fix the debugfs to
> 		support multiple opp_tables per device but wanted to
> 		send what was working upstream to get an idea if multiple
> 		opp tables per device is a feature that will be useful
> 		upstream.
> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
>                  and SC7180 SoCs.
> 
> v3:
>  * Migrated to using Saravana's opp-kBps bindings [1]
>  * Fixed some misc comments from Rajendra
>  * Added support for SC7180
> 
> v2:
>  * Incorporated Viresh's comments from:
>  
> https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
>  
> https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
>  * Dropped cpufreq-map passive governor
> 
> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420
> 
> Some alternate ways of hosting the opp-tables:
> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3
> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98
> Viresh didn't really like ^^ bindings and they dont really scale well. 
> Just
> including them here for completeness.
> 
> Depends on the following series:
> [1] https://patchwork.kernel.org/cover/11277199/
> [2] https://patchwork.kernel.org/cover/11055499/
> [3] https://patchwork.kernel.org/cover/11326381/
> 
> Sibi Sankar (10):
>   arm64: dts: qcom: sdm845: Add SoC compatible to MTP
>   cpufreq: blacklist SDM845 in cpufreq-dt-platdev
>   cpufreq: blacklist SC7180 in cpufreq-dt-platdev
>   OPP: Add and export helper to update voltage
>   opp: of: export _opp_of_get_opp_desc_node
>   opp: Allow multiple opp_tables to be mapped to a single device
>   opp: Remove multiple attached opp tables from a device
>   cpufreq: qcom: Update the bandwidth levels on frequency change
>   arm64: dts: qcom: sdm845: Add cpu OPP tables
>   arm64: dts: qcom: sc7180: Add cpu OPP tables
> 
>  arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
>  arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 ++++++++++++++++++++++++
>  drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
>  drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
>  drivers/opp/core.c                      | 111 +++++-
>  drivers/opp/of.c                        |   3 +-
>  drivers/opp/opp.h                       |   2 +
>  include/linux/pm_opp.h                  |  10 +
>  9 files changed, 1083 insertions(+), 33 deletions(-)
Viresh Kumar March 18, 2020, 3:42 a.m. UTC | #7
On 18-03-20, 02:13, Sibi Sankar wrote:
> On 2020-01-28 01:33, Sibi Sankar wrote:
> > This RFC series aims to extend cpu based scaling support to L3/DDR on
> > SDM845 and SC7180 SoCs.
> > 
> 
> Hey Viresh/Saravana,
> 
> Ping! Can you take a stab at reviewing
> the series, it has been on the list for
> a while now.

I believe this depends on Saravana's series on which I have raised
some doubts few weeks back ? I am still waiting for them to get
clarified by him.
Rajendra Nayak March 19, 2020, 9:42 a.m. UTC | #8
On 3/18/2020 9:12 AM, Viresh Kumar wrote:
> On 18-03-20, 02:13, Sibi Sankar wrote:
>> On 2020-01-28 01:33, Sibi Sankar wrote:
>>> This RFC series aims to extend cpu based scaling support to L3/DDR on
>>> SDM845 and SC7180 SoCs.
>>>
>>
>> Hey Viresh/Saravana,
>>
>> Ping! Can you take a stab at reviewing
>> the series, it has been on the list for
>> a while now.
> 
> I believe this depends on Saravana's series on which I have raised
> some doubts few weeks back ? I am still waiting for them to get
> clarified by him.

Could you please post a link to the discussion that you are referring to here?
I looked at a few links posted in the cover letter as dependencies and it seems
like the discussions are pending for *months* and not weeks but I might have looked
at the wrong ones.
Sibi Sankar March 19, 2020, 10:11 a.m. UTC | #9
On 2020-03-19 15:12, Rajendra Nayak wrote:
> On 3/18/2020 9:12 AM, Viresh Kumar wrote:
>> On 18-03-20, 02:13, Sibi Sankar wrote:
>>> On 2020-01-28 01:33, Sibi Sankar wrote:
>>>> This RFC series aims to extend cpu based scaling support to L3/DDR 
>>>> on
>>>> SDM845 and SC7180 SoCs.
>>>> 
>>> 
>>> Hey Viresh/Saravana,
>>> 
>>> Ping! Can you take a stab at reviewing
>>> the series, it has been on the list for
>>> a while now.
>> 
>> I believe this depends on Saravana's series on which I have raised
>> some doubts few weeks back ? I am still waiting for them to get
>> clarified by him.

Viresh,
Saravana's example does show a device
with multiple opp tables but doesn't
need multiple opp table support to
land though (since it works fine with
the current implementation). I am more
interested  in understanding your/
Stephen's/Saravana's stance on adding
multiple opp-table support. Personally
I feel its inevitable, since multiple
qc drivers using interconnect opp-tables,
routinely need vote on multiple paths in
a non-trivial manner.

> 
> Could you please post a link to the discussion that you are referring 
> to here?
> I looked at a few links posted in the cover letter as dependencies and 
> it seems
> like the discussions are pending for *months* and not weeks but I
> might have looked
> at the wrong ones.

https://lore.kernel.org/lkml/20200114103448.odnvqawnqb3twst5@vireshk-i7/

Rajendra,
Viresh is referring to ^^ one
Viresh Kumar March 19, 2020, 10:24 a.m. UTC | #10
On 19-03-20, 15:41, Sibi Sankar wrote:
> Viresh,
> Saravana's example does show a device
> with multiple opp tables but doesn't
> need multiple opp table support to
> land though (since it works fine with
> the current implementation). I am more
> interested  in understanding your/
> Stephen's/Saravana's stance on adding
> multiple opp-table support. Personally
> I feel its inevitable, since multiple
> qc drivers using interconnect opp-tables,
> routinely need vote on multiple paths in
> a non-trivial manner.

The OPP core doesn't support multiple OPP tables for a device and I
don't understand how it will. And so I have been waiting for a reply.

> > 
> > Could you please post a link to the discussion that you are referring to
> > here?
> > I looked at a few links posted in the cover letter as dependencies and
> > it seems
> > like the discussions are pending for *months* and not weeks but I
> > might have looked
> > at the wrong ones.
> 
> https://lore.kernel.org/lkml/20200114103448.odnvqawnqb3twst5@vireshk-i7/
> 
> Rajendra,
> Viresh is referring to ^^ one

Right, thanks.
Rajendra Nayak March 19, 2020, 10:53 a.m. UTC | #11
On 3/19/2020 3:54 PM, Viresh Kumar wrote:
> On 19-03-20, 15:41, Sibi Sankar wrote:
>> Viresh,
>> Saravana's example does show a device
>> with multiple opp tables but doesn't
>> need multiple opp table support to
>> land though (since it works fine with
>> the current implementation). I am more
>> interested  in understanding your/
>> Stephen's/Saravana's stance on adding
>> multiple opp-table support. Personally
>> I feel its inevitable, since multiple
>> qc drivers using interconnect opp-tables,
>> routinely need vote on multiple paths in
>> a non-trivial manner.
> 
> The OPP core doesn't support multiple OPP tables for a device and I
> don't understand how it will. And so I have been waiting for a reply.

I thought this series indeed is proposing to add that support in OPP core?
a.k.a "[RFC v3 06/10] opp: Allow multiple opp_tables to be mapped to a single device"

> 
>>>
>>> Could you please post a link to the discussion that you are referring to
>>> here?
>>> I looked at a few links posted in the cover letter as dependencies and
>>> it seems
>>> like the discussions are pending for *months* and not weeks but I
>>> might have looked
>>> at the wrong ones.
>>
>> https://lore.kernel.org/lkml/20200114103448.odnvqawnqb3twst5@vireshk-i7/
>>
>> Rajendra,
>> Viresh is referring to ^^ one
> 
> Right, thanks.

These discussions are stalled for over 2 months now waiting on a response from Saravana.
Viresh, whats the way forward here and how long do we plan on waiting for Saravanas response?
Sibi Sankar March 19, 2020, 10:57 a.m. UTC | #12
On 2020-03-19 15:54, Viresh Kumar wrote:
> On 19-03-20, 15:41, Sibi Sankar wrote:
>> Viresh,
>> Saravana's example does show a device
>> with multiple opp tables but doesn't
>> need multiple opp table support to
>> land though (since it works fine with
>> the current implementation). I am more
>> interested  in understanding your/
>> Stephen's/Saravana's stance on adding
>> multiple opp-table support. Personally
>> I feel its inevitable, since multiple
>> qc drivers using interconnect opp-tables,
>> routinely need vote on multiple paths in
>> a non-trivial manner.
> 
> The OPP core doesn't support multiple OPP tables for a device and I
> don't understand how it will. And so I have been waiting for a reply.

This series tries to add minimal support
for multiple opp-tables per device and
can be tested on db845c/sdm845mtp/sc7180.
Debugfs still needs to be fixed though and
fixing it did feel non-trivial (I can get
to it if we reach a consensus). Perhaps we
can fork out icc oppp-tables from regular
opp-tables and allow only multiple instances
of icc opp-tables per device(though Rob
didn't like the bindings associated with it)
that way all the current users wont be
affected but from what I've tested the
series shouldn't cause any regression.

> 
>> >
>> > Could you please post a link to the discussion that you are referring to
>> > here?
>> > I looked at a few links posted in the cover letter as dependencies and
>> > it seems
>> > like the discussions are pending for *months* and not weeks but I
>> > might have looked
>> > at the wrong ones.
>> 
>> https://lore.kernel.org/lkml/20200114103448.odnvqawnqb3twst5@vireshk-i7/
>> 
>> Rajendra,
>> Viresh is referring to ^^ one
> 
> Right, thanks.
Viresh Kumar March 19, 2020, 11:08 a.m. UTC | #13
On 19-03-20, 16:23, Rajendra Nayak wrote:
> 
> 
> On 3/19/2020 3:54 PM, Viresh Kumar wrote:
> I thought this series indeed is proposing to add that support in OPP core?
> a.k.a "[RFC v3 06/10] opp: Allow multiple opp_tables to be mapped to a single device"
> 
> These discussions are stalled for over 2 months now waiting on a response from Saravana.
> Viresh, whats the way forward here and how long do we plan on waiting for Saravanas response?

I agree and I am equally worried about it. So lets clear the air a bit
first. Can someone answer following :

- This series depends on the series from Saravana ? Right, so that
  needs to get merged/accepted first ?

- If yes, then what is the way forward as Saravana isn't responding
  right now ..
Rajendra Nayak March 19, 2020, 11:33 a.m. UTC | #14
On 3/19/2020 4:38 PM, Viresh Kumar wrote:
> On 19-03-20, 16:23, Rajendra Nayak wrote:
>>
>>
>> On 3/19/2020 3:54 PM, Viresh Kumar wrote:
>> I thought this series indeed is proposing to add that support in OPP core?
>> a.k.a "[RFC v3 06/10] opp: Allow multiple opp_tables to be mapped to a single device"
>>
>> These discussions are stalled for over 2 months now waiting on a response from Saravana.
>> Viresh, whats the way forward here and how long do we plan on waiting for Saravanas response?
> 
> I agree and I am equally worried about it. So lets clear the air a bit
> first. Can someone answer following :
> 
> - This series depends on the series from Saravana ? Right, so that
>    needs to get merged/accepted first ?
> 
> - If yes, then what is the way forward as Saravana isn't responding
>    right now ..

sure, I understand there is a dependency, however refusing to review the approach
(to add multiple OPPS tables per device) that this series is taking because of an outstanding
question which, if I read it right is "We can not add multiple OPP tables for a single device right now"
seems odd.

Its fine if you are not happy with the approach taken here and you can propose something else,
but it looks inevitable that we would need something like this to be supported (multiple OPP tables per device)
and hence the request to review the patches.
Viresh Kumar March 20, 2020, 8:01 a.m. UTC | #15
On 19-03-20, 17:03, Rajendra Nayak wrote:
> sure, I understand there is a dependency, however refusing to review the approach
> (to add multiple OPPS tables per device) that this series is taking because of an outstanding
> question which, if I read it right is "We can not add multiple OPP tables for a single device right now"
> seems odd.
> 
> Its fine if you are not happy with the approach taken here and you can propose something else,
> but it looks inevitable that we would need something like this to be supported (multiple OPP tables per device)
> and hence the request to review the patches.

I understand the frustration you have right now, but honestly most of
the delay is not from my part. I normally try to be very quick in
giving reviews or even accepting material for next cycle. Still, I am
part of the chain which has blocked this series and I am really sorry
for that.

Coming back to reviews. I never liked the idea of keeping separate OPP
tables just for the relation that a peripheral share with the
interconnect, i.e. the b/w tables getting discussed right now.

And I am not at all sure if such tables should be added as device
tables anyway, it was present in cover letter of Saravana for sometime
but not in the real patches and so I missed it until the time Sibi
asked me something on IRC.

To be clear, I don't think we will allow multiple active OPP tables
for a device for now, unless we have a very good reason to do that. It
just doesn't make sense to me to have one table for keeping
frequency/voltage thing, and another set of tables for b/w
requirements from interconnect.

So, the changes proposed by this patchset even doesn't matter as I am
not agreeing on the binding itself. And we need a binding change for
that, which Saravana never sent, i.e. to have multiple active tables
for a device.

Yes, we need a solution for this and a quick one. I am fine to discuss
this all on a hangout session (anyway, everyone is working from home
now anyways) anytime next week with the interested parties. I think
/me and Vincent can join it.