Linux-ARM-MSM Archive on lore.kernel.org
 help / color / Atom feed
From: Viresh Kumar <viresh.kumar@linaro.org>
To: swboyd@chromium.org, Rajendra Nayak <rnayak@codeaurora.org>
Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-pm@vger.kernel.org, linux-serial@vger.kernel.org,
	linux-spi@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-scsi@vger.kernel.org, ulf.hansson@linaro.org,
	dianders@chromium.org, rafael@kernel.org
Subject: Re: [RFC v2 01/11] OPP: Don't overwrite rounded clk rate
Date: Tue, 11 Jun 2019 16:24:32 +0530
Message-ID: <20190611105432.x3nzqiib35t6mvyg@vireshk-i7> (raw)
In-Reply-To: <20190320094918.20234-2-rnayak@codeaurora.org>

On 20-03-19, 15:19, Rajendra Nayak wrote:
> From: Stephen Boyd <swboyd@chromium.org>
> 
> Doing this allows us to call this API with any rate requested and have
> it not need to match in the OPP table. Instead, we'll round the rate up
> to the nearest OPP that we see so that we can get the voltage or level
> that's required for that OPP. This supports users of OPP that want to
> specify the 'fmax' tables of a device instead of every single frequency
> that they need. And for devices that required the exact frequency, we
> can rely on the clk framework to round the rate to the nearest supported
> frequency instead of the OPP framework to do so.
> 
> Note that this may affect drivers that don't want the clk framework to
> do rounding, but instead want the OPP table to do the rounding for them.
> Do we have that case? Should we add some flag to the OPP table to
> indicate this and then not have that flag set when there isn't an OPP
> table for the device and also introduce a property like 'opp-use-clk' to
> tell the table that it should use the clk APIs to round rates instead of
> OPP?
> 
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
> ---
>  drivers/opp/core.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 0420f7e8ad5b..bc9a7762dd4c 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -703,7 +703,7 @@ static int _set_required_opps(struct device *dev,
>  int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
>  {
>  	struct opp_table *opp_table;
> -	unsigned long freq, old_freq;
> +	unsigned long freq, opp_freq, old_freq, old_opp_freq;
>  	struct dev_pm_opp *old_opp, *opp;
>  	struct clk *clk;
>  	int ret;
> @@ -742,13 +742,15 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
>  		goto put_opp_table;
>  	}
>  
> -	old_opp = _find_freq_ceil(opp_table, &old_freq);
> +	old_opp_freq = old_freq;
> +	old_opp = _find_freq_ceil(opp_table, &old_opp_freq);
>  	if (IS_ERR(old_opp)) {
>  		dev_err(dev, "%s: failed to find current OPP for freq %lu (%ld)\n",
>  			__func__, old_freq, PTR_ERR(old_opp));
>  	}
>  
> -	opp = _find_freq_ceil(opp_table, &freq);
> +	opp_freq = freq;
> +	opp = _find_freq_ceil(opp_table, &opp_freq);
>  	if (IS_ERR(opp)) {
>  		ret = PTR_ERR(opp);
>  		dev_err(dev, "%s: failed to find OPP for freq %lu (%d)\n",

I see a logical problem with this patch.

Suppose the clock driver supports following frequencies: 500M, 800M,
1G, 1.2G and the OPP table contains following list: 500M, 1G, 1.2G
(i.e. missing 800M).

Now 800M should never get programmed as it isn't part of the OPP
table. But if you pass 600M to opp-set-rate, then it will end up
selecting 800M as clock driver will round up to the closest value.

Even if no one is doing this right now, it is a sensible usecase,
specially during testing of patches and I don't think we should avoid
it.

What exactly is the use case for which we need this patch ? What kind
of driver ? Some detail can be helpful to find another solution that
fixes this problem.

-- 
viresh

  reply index

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-20  9:49 [RFC v2 00/11] DVFS in the OPP core Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 01/11] OPP: Don't overwrite rounded clk rate Rajendra Nayak
2019-06-11 10:54   ` Viresh Kumar [this message]
2019-06-12  7:42     ` Rajendra Nayak
2019-06-12  8:25       ` Viresh Kumar
2019-06-13  9:54         ` Viresh Kumar
2019-06-14  5:27           ` Viresh Kumar
2019-06-17  3:50             ` Viresh Kumar
2019-06-17  4:07               ` Rajendra Nayak
2019-06-17  4:17                 ` Viresh Kumar
2019-06-17  4:25                   ` Rajendra Nayak
2019-06-14  5:54           ` Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 02/11] OPP: Make dev_pm_opp_set_rate() with freq=0 as valid Rajendra Nayak
2019-06-14  6:32   ` Viresh Kumar
2019-06-17  4:04     ` Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 03/11] tty: serial: qcom_geni_serial: Use OPP API to set clk/perf state Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 04/11] spi: spi-geni-qcom: " Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 05/11] arm64: dts: sdm845: Add OPP table for all qup devices Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 06/11] scsi: ufs: Add support to manage multiple power domains in ufshcd-pltfrm Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 07/11] scsi: ufs: Add support for specifying OPP tables in DT Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 08/11] arm64: dts: sdm845: Add ufs opps and power-domains Rajendra Nayak
2019-05-14  7:53   ` Ulf Hansson
2019-03-20  9:49 ` [RFC v2 09/11] drm/msm/dpu: Use OPP API to set clk/perf state Rajendra Nayak
2019-04-10  3:49   ` Viresh Kumar
2019-04-10  3:49     ` Viresh Kumar
2019-03-20  9:49 ` [RFC v2 10/11] drm/msm: dsi: " Rajendra Nayak
2019-03-20  9:49 ` [RFC v2 11/11] arm64: dts: sdm845: Add DSI and MDP OPP tables and power-domains Rajendra Nayak
2019-04-10  3:51 ` [RFC v2 00/11] DVFS in the OPP core Viresh Kumar
2019-04-10  3:51   ` Viresh Kumar
2019-05-21  6:22 ` Viresh Kumar
2019-05-24  6:03   ` Rajendra Nayak
2019-06-17  4:26 ` Viresh Kumar

Reply instructions:

You may reply publically 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=20190611105432.x3nzqiib35t6mvyg@vireshk-i7 \
    --to=viresh.kumar@linaro.org \
    --cc=dianders@chromium.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=rnayak@codeaurora.org \
    --cc=swboyd@chromium.org \
    --cc=ulf.hansson@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

Linux-ARM-MSM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-msm/0 linux-arm-msm/git/0.git

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


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


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