LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Lukasz Luba <lukasz.luba@arm.com>
To: Sibi Sankar <sibis@codeaurora.org>
Cc: viresh.kumar@linaro.org, sboyd@kernel.org,
	georgi.djakov@linaro.org, saravanak@google.com, nm@ti.com,
	bjorn.andersson@linaro.org, agross@kernel.org,
	david.brown@linaro.org, robh+dt@kernel.org, mark.rutland@arm.com,
	rjw@rjwysocki.net, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-pm@vger.kernel.org, dianders@chromium.org,
	mka@chromium.org, vincent.guittot@linaro.org,
	amit.kucheria@linaro.org, ulf.hansson@linaro.org,
	linux-arm-msm-owner@vger.kernel.org
Subject: Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs
Date: Wed, 29 Jan 2020 15:47:40 +0000
Message-ID: <a87feb0c-3a34-9070-0b4d-ce31a41136b4@arm.com> (raw)
In-Reply-To: <57f9a785d93193719ee0b91e43d0922f@codeaurora.org>



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(-)
>>>
> 

  reply index

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-27 20:03 Sibi Sankar
2020-01-27 20:03 ` [RFC v3 01/10] arm64: dts: qcom: sdm845: Add SoC compatible to MTP Sibi Sankar
2020-01-28 20:40   ` Matthias Kaehlcke
2020-01-29 13:45     ` Sibi Sankar
2020-01-27 20:03 ` [RFC v3 02/10] cpufreq: blacklist SDM845 in cpufreq-dt-platdev Sibi Sankar
2020-01-28 20:44   ` Matthias Kaehlcke
2020-01-29 13:46     ` Sibi Sankar
2020-01-30 11:40     ` Sudeep Holla
2020-02-01 12:21       ` Sibi Sankar
2020-01-27 20:03 ` [RFC v3 03/10] cpufreq: blacklist SC7180 " Sibi Sankar
2020-01-28 20:45   ` Matthias Kaehlcke
2020-01-27 20:03 ` [RFC v3 04/10] OPP: Add and export helper to update voltage Sibi Sankar
2020-01-28 21:33   ` Matthias Kaehlcke
2020-01-29 13:49     ` Sibi Sankar
2020-01-27 20:03 ` [RFC v3 05/10] opp: of: export _opp_of_get_opp_desc_node Sibi Sankar
2020-01-27 20:03 ` [RFC v3 06/10] opp: Allow multiple opp_tables to be mapped to a single device Sibi Sankar
2020-01-27 20:03 ` [RFC v3 07/10] opp: Remove multiple attached opp tables from a device Sibi Sankar
2020-01-27 20:03 ` [RFC v3 08/10] cpufreq: qcom: Update the bandwidth levels on frequency change Sibi Sankar
2020-01-29  9:35   ` Lukasz Luba
2020-01-29 14:27     ` Sibi Sankar
2020-01-27 20:03 ` [RFC v3 09/10] arm64: dts: qcom: sdm845: Add cpu OPP tables Sibi Sankar
2020-01-29  1:24   ` Matthias Kaehlcke
2020-01-29 14:05     ` Sibi Sankar
2020-01-29 18:18       ` Matthias Kaehlcke
2020-01-27 20:03 ` [RFC v3 10/10] arm64: dts: qcom: sc7180: " Sibi Sankar
2020-01-29  2:54 ` [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs Rob Clark
2020-01-29 14:21   ` Sibi Sankar
2020-01-29  9:46 ` Lukasz Luba
2020-01-29 14:37   ` Sibi Sankar
2020-01-29 15:47     ` Lukasz Luba [this message]
2020-03-17 20:43 ` Sibi Sankar
2020-03-18  3:42   ` Viresh Kumar
2020-03-19  9:42     ` Rajendra Nayak
2020-03-19 10:11       ` Sibi Sankar
2020-03-19 10:24         ` Viresh Kumar
2020-03-19 10:53           ` Rajendra Nayak
2020-03-19 11:08             ` Viresh Kumar
2020-03-19 11:33               ` Rajendra Nayak
2020-03-20  8:01                 ` Viresh Kumar
2020-03-19 10:57           ` Sibi Sankar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a87feb0c-3a34-9070-0b4d-ce31a41136b4@arm.com \
    --to=lukasz.luba@arm.com \
    --cc=agross@kernel.org \
    --cc=amit.kucheria@linaro.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=david.brown@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=georgi.djakov@linaro.org \
    --cc=linux-arm-msm-owner@vger.kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mka@chromium.org \
    --cc=nm@ti.com \
    --cc=rjw@rjwysocki.net \
    --cc=robh+dt@kernel.org \
    --cc=saravanak@google.com \
    --cc=sboyd@kernel.org \
    --cc=sibis@codeaurora.org \
    --cc=ulf.hansson@linaro.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git