[PATCH v1 0/5] NVIDIA Tegra devfreq drivers improvements

[PATCH v1 0/5] NVIDIA Tegra devfreq drivers improvements

[Dmitry Osipenko]

Hello,

This series contains minor patches that I was going to send out a month or
two ago, but completely forgot about. More importantly, it also contains new
patches that are needed in order to address an upcoming problem in regards
to adding interconnect API support for NVIDIA Tegra [1].

[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=167480

The problem lies in clk_round_rate(), which rounds clock rate based on the
min/max clk limits imposed by active clk users. This is not suitable for
the Tegra devfreq drivers because they use clk_round_rate() for building
OPP table, and thus, nothing should limit the clk rate, otherwise the OPP
table values are erroneously getting limited to the clk's limits.

Dmitry Osipenko (5):
  PM / devfreq: tegra: Add Dmitry as a maintainer
  clk: Introduce clk_round_rate_unboundly()
  PM / devfreq: tegra20: Use clk_round_rate_unboundly()
  PM / devfreq: tegra30: Use clk_round_rate_unboundly()
  PM / devfreq: tegra30: Make CPUFreq notifier to take into account
    boosting

 MAINTAINERS                       |  9 ++++++
 drivers/clk/clk.c                 | 49 ++++++++++++++++++++++++-------
 drivers/devfreq/tegra20-devfreq.c |  4 +--
 drivers/devfreq/tegra30-devfreq.c |  6 ++--
 include/linux/clk.h               | 18 ++++++++++++
 5 files changed, 70 insertions(+), 16 deletions(-)

-- 
2.25.1

[PATCH v1 1/5] PM / devfreq: tegra: Add Dmitry as a maintainer

[Dmitry Osipenko]

I was contributing to the NVIDIA Tegra20+ devfreq drivers recently and
want to help keep them working and evolving in the future.

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8b8abe756ae0..9e8b0779f6df 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10931,6 +10931,15 @@ F:	include/linux/memblock.h
 F:	mm/memblock.c
 F:	Documentation/core-api/boot-time-mm.rst
 
+MEMORY FREQUENCY SCALING DRIVERS FOR NVIDIA TEGRA
+M:	Dmitry Osipenko <digetx@gmail.com>
+L:	linux-pm@vger.kernel.org
+L:	linux-tegra@vger.kernel.org
+T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git
+S:	Maintained
+F:	drivers/devfreq/tegra20-devfreq.c
+F:	drivers/devfreq/tegra30-devfreq.c
+
 MEMORY MANAGEMENT
 M:	Andrew Morton <akpm@linux-foundation.org>
 L:	linux-mm@kvack.org
-- 
2.25.1

Re: [PATCH v1 1/5] PM / devfreq: tegra: Add Dmitry as a maintainer

[Chanwoo Choi]

On 3/31/20 8:16 AM, Dmitry Osipenko wrote:
> I was contributing to the NVIDIA Tegra20+ devfreq drivers recently and
> want to help keep them working and evolving in the future.
> 
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  MAINTAINERS | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8b8abe756ae0..9e8b0779f6df 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10931,6 +10931,15 @@ F:	include/linux/memblock.h
>  F:	mm/memblock.c
>  F:	Documentation/core-api/boot-time-mm.rst
>  
> +MEMORY FREQUENCY SCALING DRIVERS FOR NVIDIA TEGRA
> +M:	Dmitry Osipenko <digetx@gmail.com>
> +L:	linux-pm@vger.kernel.org
> +L:	linux-tegra@vger.kernel.org
> +T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git

Need to update it as following:
git git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git

> +S:	Maintained
> +F:	drivers/devfreq/tegra20-devfreq.c
> +F:	drivers/devfreq/tegra30-devfreq.c
> +
>  MEMORY MANAGEMENT
>  M:	Andrew Morton <akpm@linux-foundation.org>
>  L:	linux-mm@kvack.org
> 

I already agreed it. To prevent the merge conflict of MAINTAINERS,
better to be merged it to linux-pm.git maintainer. On next time,
please add Rafael J. Wysocki to To list.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v1 1/5] PM / devfreq: tegra: Add Dmitry as a maintainer

[Dmitry Osipenko]

01.04.2020 02:13, Chanwoo Choi пишет:
> On 3/31/20 8:16 AM, Dmitry Osipenko wrote:
>> I was contributing to the NVIDIA Tegra20+ devfreq drivers recently and
>> want to help keep them working and evolving in the future.
>>
>> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  MAINTAINERS | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 8b8abe756ae0..9e8b0779f6df 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -10931,6 +10931,15 @@ F:	include/linux/memblock.h
>>  F:	mm/memblock.c
>>  F:	Documentation/core-api/boot-time-mm.rst
>>  
>> +MEMORY FREQUENCY SCALING DRIVERS FOR NVIDIA TEGRA
>> +M:	Dmitry Osipenko <digetx@gmail.com>
>> +L:	linux-pm@vger.kernel.org
>> +L:	linux-tegra@vger.kernel.org
>> +T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git
> 
> Need to update it as following:
> git git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git

Indeed, good catch!

>> +S:	Maintained
>> +F:	drivers/devfreq/tegra20-devfreq.c
>> +F:	drivers/devfreq/tegra30-devfreq.c
>> +
>>  MEMORY MANAGEMENT
>>  M:	Andrew Morton <akpm@linux-foundation.org>
>>  L:	linux-mm@kvack.org
>>
> 
> I already agreed it. To prevent the merge conflict of MAINTAINERS,
> better to be merged it to linux-pm.git maintainer. On next time,
> please add Rafael J. Wysocki to To list.

I'll update and re-send this patch separately from the rest of the
patches, thank you.

[PATCH v1 2/5] clk: Introduce clk_round_rate_unboundly()

[Dmitry Osipenko]

In same cases it may be desired to round clock's rate without taking into
account current min/max requests made by the clock's users. One example is
building up OPP table based on a possible clock rates.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/clk/clk.c   | 49 +++++++++++++++++++++++++++++++++++----------
 include/linux/clk.h | 18 +++++++++++++++++
 2 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 39c59f063aa0..28c2e1699619 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1412,16 +1412,7 @@ unsigned long clk_hw_round_rate(struct clk_hw *hw, unsigned long rate)
 }
 EXPORT_SYMBOL_GPL(clk_hw_round_rate);
 
-/**
- * clk_round_rate - round the given rate for a clk
- * @clk: the clk for which we are rounding a rate
- * @rate: the rate which is to be rounded
- *
- * Takes in a rate as input and rounds it to a rate that the clk can actually
- * use which is then returned.  If clk doesn't support round_rate operation
- * then the parent rate is returned.
- */
-long clk_round_rate(struct clk *clk, unsigned long rate)
+static long __clk_round_rate(struct clk *clk, unsigned long rate, bool bound)
 {
 	struct clk_rate_request req;
 	int ret;
@@ -1434,7 +1425,13 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
 	if (clk->exclusive_count)
 		clk_core_rate_unprotect(clk->core);
 
-	clk_core_get_boundaries(clk->core, &req.min_rate, &req.max_rate);
+	if (bound) {
+		clk_core_get_boundaries(clk->core, &req.min_rate,
+					&req.max_rate);
+	} else {
+		req.min_rate = 0;
+		req.max_rate = ULONG_MAX;
+	}
 	req.rate = rate;
 
 	ret = clk_core_round_rate_nolock(clk->core, &req);
@@ -1449,8 +1446,38 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
 
 	return req.rate;
 }
+
+/**
+ * clk_round_rate - round the given rate for a clk
+ * @clk: the clk for which we are rounding a rate
+ * @rate: the rate which is to be rounded
+ *
+ * Takes in a rate as input and rounds it to a rate that the clk can actually
+ * use which is then returned.  If clk doesn't support round_rate operation
+ * then the parent rate is returned.
+ */
+long clk_round_rate(struct clk *clk, unsigned long rate)
+{
+	return __clk_round_rate(clk, rate, true);
+}
 EXPORT_SYMBOL_GPL(clk_round_rate);
 
+/**
+ * clk_round_rate_unboundly - unboundly round the given rate for a clk
+ * @clk: the clk for which we are rounding a rate
+ * @rate: the rate which is to be rounded
+ *
+ * Takes in a rate as input and rounds it to a rate that the clk can use
+ * which is then returned.  The given rate isn't bounded by clk users min-max
+ * rates, unlike in a case of clk_round_rate().  If clk doesn't support
+ * round_rate operation then the parent rate is returned.
+ */
+long clk_round_rate_unboundly(struct clk *clk, unsigned long rate)
+{
+	return __clk_round_rate(clk, rate, false);
+}
+EXPORT_SYMBOL_GPL(clk_round_rate_unboundly);
+
 /**
  * __clk_notify - call clk notifier chain
  * @core: clk that is changing rate
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 7fd6a1febcf4..b534643015f5 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -622,6 +622,19 @@ void devm_clk_put(struct device *dev, struct clk *clk);
  */
 long clk_round_rate(struct clk *clk, unsigned long rate);
 
+/**
+ * clk_round_rate_unboundly - adjust a rate to the rate a clock can provide
+ * @clk: clock source
+ * @rate: desired clock rate in Hz
+ *
+ * This helper function rounds the given rate to a value that hardware
+ * could actually accept, without taking into account current min/max
+ * requests. Useful for cases like OPP table buildup.
+ *
+ * Returns rounded clock rate in Hz, or negative errno.
+ */
+long clk_round_rate_unboundly(struct clk *clk, unsigned long rate);
+
 /**
  * clk_set_rate - set the clock rate for a clock source
  * @clk: clock source
@@ -857,6 +870,11 @@ static inline long clk_round_rate(struct clk *clk, unsigned long rate)
 	return 0;
 }
 
+static inline long clk_round_rate_unboundly(struct clk *clk, unsigned long rate)
+{
+	return 0;
+}
+
 static inline bool clk_has_parent(struct clk *clk, struct clk *parent)
 {
 	return true;
-- 
2.25.1

Re: [PATCH v1 2/5] clk: Introduce clk_round_rate_unboundly()

[Michał Mirosław]

On Tue, Mar 31, 2020 at 02:16:14AM +0300, Dmitry Osipenko wrote:
[...]
> +/**
> + * clk_round_rate_unboundly - unboundly round the given rate for a clk

Just grammar nits:

clk_round_rate_unbounded - round the given rate for a clk, ignoring users' min/max constraints

> + * @clk: the clk for which we are rounding a rate
> + * @rate: the rate which is to be rounded
> + *
> + * Takes in a rate as input and rounds it to a rate that the clk can use
> + * which is then returned.  The given rate isn't bounded by clk users min-max

s/bounded/limited/ ?

> + * rates, unlike in a case of clk_round_rate().  If clk doesn't support

"... unlike in clk_round_rate()."

> + * round_rate operation then the parent rate is returned.
> + */
[...]

Best Regards,
Michał Mirosław

Re: [PATCH v1 2/5] clk: Introduce clk_round_rate_unboundly()

[Dmitry Osipenko]

02.04.2020 03:33, Michał Mirosław пишет:
> On Tue, Mar 31, 2020 at 02:16:14AM +0300, Dmitry Osipenko wrote:
> [...]
>> +/**
>> + * clk_round_rate_unboundly - unboundly round the given rate for a clk
> 
> Just grammar nits:
> 
> clk_round_rate_unbounded - round the given rate for a clk, ignoring users' min/max constraints
> 
>> + * @clk: the clk for which we are rounding a rate
>> + * @rate: the rate which is to be rounded
>> + *
>> + * Takes in a rate as input and rounds it to a rate that the clk can use
>> + * which is then returned.  The given rate isn't bounded by clk users min-max
> 
> s/bounded/limited/ ?
> 
>> + * rates, unlike in a case of clk_round_rate().  If clk doesn't support
> 
> "... unlike in clk_round_rate()."
> 
>> + * round_rate operation then the parent rate is returned.
>> + */
> [...]

Hello Michał,

Thank you very much for the suggestion, I'll take it into account in the
next version.

Re: [PATCH v1 2/5] clk: Introduce clk_round_rate_unboundly()

[Stephen Boyd]

Quoting Dmitry Osipenko (2020-03-30 16:16:14)
> In same cases it may be desired to round clock's rate without taking into
> account current min/max requests made by the clock's users. One example is
> building up OPP table based on a possible clock rates.

Shouldn't the OPP table come from firmware/DT? I don't quite understand
why we're generating OPP tables on top of the rate rounding API.
clk_round_rate() is supposed to tell us what rate we'll get if we call
clk_set_rate() with the same arguments. An unboundly version of that
doesn't make sense. 

I wonder if perhaps the clk provider should be populating OPP tables in
this case? Or basically anything besides adding another clk consumer API
to solve this problem. Who is the caller? Something later in this
series?

Re: [PATCH v1 2/5] clk: Introduce clk_round_rate_unboundly()

[Dmitry Osipenko]

27.05.2020 08:55, Stephen Boyd пишет:
> Quoting Dmitry Osipenko (2020-03-30 16:16:14)
>> In same cases it may be desired to round clock's rate without taking into
>> account current min/max requests made by the clock's users. One example is
>> building up OPP table based on a possible clock rates.
> 
> Shouldn't the OPP table come from firmware/DT? I don't quite understand
> why we're generating OPP tables on top of the rate rounding API.
> clk_round_rate() is supposed to tell us what rate we'll get if we call
> clk_set_rate() with the same arguments. An unboundly version of that
> doesn't make sense. 

The OPP should come from the DT, but unfortunately DT and Tegra's
devfreq driver wasn't designed like that from the start, so it will take
some extra effort to re-do it properly now. I wanted to postpone that
effort a tad and get at least the basics upstreamed for the starter.

> I wonder if perhaps the clk provider should be populating OPP tables in
> this case? Or basically anything besides adding another clk consumer API
> to solve this problem. Who is the caller? Something later in this
> series?

I'll try to add a proper OPP table with freqs and voltages, will see how
it goes. We will need to do it sooner or later anyways. So perhaps it's
fine to drop the current approach with the clk_round_rate_unboundly()
and re-focus on a proper OPP implementation.

Thank you for getting back and replying to this topic :)

Re: [PATCH v1 2/5] clk: Introduce clk_round_rate_unboundly()

[Stephen Boyd]

Quoting Dmitry Osipenko (2020-05-27 10:57:01)
> 27.05.2020 08:55, Stephen Boyd \u043f\u0438\u0448\u0435\u0442:
> > Quoting Dmitry Osipenko (2020-03-30 16:16:14)
> >> In same cases it may be desired to round clock's rate without taking into
> >> account current min/max requests made by the clock's users. One example is
> >> building up OPP table based on a possible clock rates.
> > 
> > Shouldn't the OPP table come from firmware/DT? I don't quite understand
> > why we're generating OPP tables on top of the rate rounding API.
> > clk_round_rate() is supposed to tell us what rate we'll get if we call
> > clk_set_rate() with the same arguments. An unboundly version of that
> > doesn't make sense. 
> 
> The OPP should come from the DT, but unfortunately DT and Tegra's
> devfreq driver wasn't designed like that from the start, so it will take
> some extra effort to re-do it properly now. I wanted to postpone that
> effort a tad and get at least the basics upstreamed for the starter.
> 
> > I wonder if perhaps the clk provider should be populating OPP tables in
> > this case? Or basically anything besides adding another clk consumer API
> > to solve this problem. Who is the caller? Something later in this
> > series?
> 
> I'll try to add a proper OPP table with freqs and voltages, will see how
> it goes. We will need to do it sooner or later anyways. So perhaps it's
> fine to drop the current approach with the clk_round_rate_unboundly()
> and re-focus on a proper OPP implementation.
> 
> Thank you for getting back and replying to this topic :)

Alright, it sounds better to me if we can avoid a one off addition to
the clk API in favor of implementing a proper OPP table from the start.

Re: [PATCH v1 2/5] clk: Introduce clk_round_rate_unboundly()

[Stephen Boyd]

Quoting Dmitry Osipenko (2020-03-30 16:16:14)
> In same cases it may be desired to round clock's rate without taking into
> account current min/max requests made by the clock's users. One example is
> building up OPP table based on a possible clock rates.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/clk/clk.c   | 49 +++++++++++++++++++++++++++++++++++----------
>  include/linux/clk.h | 18 +++++++++++++++++

BTW, please run get_maintainers.pl patches. Russell maintains
include/linux/clk.h (the clk API). CCF implements it.

[PATCH v1 3/5] PM / devfreq: tegra20: Use clk_round_rate_unboundly()

[Dmitry Osipenko]

The clk_round_rate() doesn't work for us properly if clock rate is bounded
by a min/max rate that is requested by some other clk-user because we're
building devfreq's OPP table based on the rounding.

In particular this becomes a problem if display driver is probed earlier
than devfreq, and thus, display adds a memory bandwidth request using
interconnect API, which results in a minimum clock-rate being set for
the memory clk. In a result, the lowest devfreq OPP rate is getting
limited to the minimum rate imposed by the display driver.

Let's use new clk_round_rate_unboundly() that resolves the problem by
rounding clock rate without taking into account min/max limits imposed by
active clk users.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/devfreq/tegra20-devfreq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
index ff82bac9ee4e..1bb10ef11dfe 100644
--- a/drivers/devfreq/tegra20-devfreq.c
+++ b/drivers/devfreq/tegra20-devfreq.c
@@ -149,10 +149,10 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 
 	tegra->regs = mc->regs;
 
-	max_rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
+	max_rate = clk_round_rate_unboundly(tegra->emc_clock, ULONG_MAX);
 
 	for (rate = 0; rate <= max_rate; rate++) {
-		rate = clk_round_rate(tegra->emc_clock, rate);
+		rate = clk_round_rate_unboundly(tegra->emc_clock, rate);
 
 		err = dev_pm_opp_add(&pdev->dev, rate, 0);
 		if (err) {
-- 
2.25.1

Re: [PATCH v1 3/5] PM / devfreq: tegra20: Use clk_round_rate_unboundly()

[Chanwoo Choi]

Hi Dmitry,

On 3/31/20 8:16 AM, Dmitry Osipenko wrote:
> The clk_round_rate() doesn't work for us properly if clock rate is bounded
> by a min/max rate that is requested by some other clk-user because we're
> building devfreq's OPP table based on the rounding.
> 
> In particular this becomes a problem if display driver is probed earlier
> than devfreq, and thus, display adds a memory bandwidth request using
> interconnect API, which results in a minimum clock-rate being set for
> the memory clk. In a result, the lowest devfreq OPP rate is getting
> limited to the minimum rate imposed by the display driver.
> 
> Let's use new clk_round_rate_unboundly() that resolves the problem by
> rounding clock rate without taking into account min/max limits imposed by
> active clk users.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/devfreq/tegra20-devfreq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
> index ff82bac9ee4e..1bb10ef11dfe 100644
> --- a/drivers/devfreq/tegra20-devfreq.c
> +++ b/drivers/devfreq/tegra20-devfreq.c
> @@ -149,10 +149,10 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>  
>  	tegra->regs = mc->regs;
>  
> -	max_rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
> +	max_rate = clk_round_rate_unboundly(tegra->emc_clock, ULONG_MAX);
>  
>  	for (rate = 0; rate <= max_rate; rate++) {
> -		rate = clk_round_rate(tegra->emc_clock, rate);
> +		rate = clk_round_rate_unboundly(tegra->emc_clock, rate);
>  
>  		err = dev_pm_opp_add(&pdev->dev, rate, 0);
>  		if (err) {
> 

Firstly, patch1 have to be reviewed for this patch.
I have no any objection. It looks good to me.

If patch1 get the confirmation from clock maintainer,
feel free to add my acked tag:
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v1 3/5] PM / devfreq: tegra20: Use clk_round_rate_unboundly()

[Chanwoo Choi]

On 4/1/20 8:22 AM, Chanwoo Choi wrote:
> Hi Dmitry,
> 
> On 3/31/20 8:16 AM, Dmitry Osipenko wrote:
>> The clk_round_rate() doesn't work for us properly if clock rate is bounded
>> by a min/max rate that is requested by some other clk-user because we're
>> building devfreq's OPP table based on the rounding.
>>
>> In particular this becomes a problem if display driver is probed earlier
>> than devfreq, and thus, display adds a memory bandwidth request using
>> interconnect API, which results in a minimum clock-rate being set for
>> the memory clk. In a result, the lowest devfreq OPP rate is getting
>> limited to the minimum rate imposed by the display driver.
>>
>> Let's use new clk_round_rate_unboundly() that resolves the problem by
>> rounding clock rate without taking into account min/max limits imposed by
>> active clk users.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/devfreq/tegra20-devfreq.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
>> index ff82bac9ee4e..1bb10ef11dfe 100644
>> --- a/drivers/devfreq/tegra20-devfreq.c
>> +++ b/drivers/devfreq/tegra20-devfreq.c
>> @@ -149,10 +149,10 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>  
>>  	tegra->regs = mc->regs;
>>  
>> -	max_rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
>> +	max_rate = clk_round_rate_unboundly(tegra->emc_clock, ULONG_MAX);
>>  
>>  	for (rate = 0; rate <= max_rate; rate++) {
>> -		rate = clk_round_rate(tegra->emc_clock, rate);
>> +		rate = clk_round_rate_unboundly(tegra->emc_clock, rate);
>>  
>>  		err = dev_pm_opp_add(&pdev->dev, rate, 0);
>>  		if (err) {
>>
> 
> Firstly, patch1 have to be reviewed for this patch.
> I have no any objection. It looks good to me.
> 
> If patch1 get the confirmation from clock maintainer,
> feel free to add my acked tag:
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> 

Sorry. I mean the patch2 about clk_round_rate_unboundly() function.
instead of patch1. 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[PATCH v1 4/5] PM / devfreq: tegra30: Use clk_round_rate_unboundly()

[Dmitry Osipenko]

The clk_round_rate() doesn't work for us properly if clock rate is bounded
by a min/max rate that is requested by some other clk-user because we're
building devfreq's OPP table based on the rounding.

In particular this becomes a problem if display driver is probed earlier
than devfreq, and thus, display adds a memory bandwidth request using
interconnect API, which results in a minimum clock-rate being set for
the memory clk. In a result, the lowest devfreq OPP rate is getting
limited to the minimum rate imposed by the display driver.

Let's use new clk_round_rate_unboundly() that resolves the problem by
rounding clock rate without taking into account min/max limits imposed by
active clk users.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/devfreq/tegra30-devfreq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 28b2c7ca416e..34f6291e880c 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -834,7 +834,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 
 	reset_control_deassert(tegra->reset);
 
-	rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
+	rate = clk_round_rate_unboundly(tegra->emc_clock, ULONG_MAX);
 	if (rate < 0) {
 		dev_err(&pdev->dev, "Failed to round clock rate: %ld\n", rate);
 		return rate;
@@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	}
 
 	for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) {
-		rate = clk_round_rate(tegra->emc_clock, rate);
+		rate = clk_round_rate_unboundly(tegra->emc_clock, rate);
 
 		if (rate < 0) {
 			dev_err(&pdev->dev,
-- 
2.25.1

Re: [PATCH v1 4/5] PM / devfreq: tegra30: Use clk_round_rate_unboundly()

[Chanwoo Choi]

On 3/31/20 8:16 AM, Dmitry Osipenko wrote:
> The clk_round_rate() doesn't work for us properly if clock rate is bounded
> by a min/max rate that is requested by some other clk-user because we're
> building devfreq's OPP table based on the rounding.
> 
> In particular this becomes a problem if display driver is probed earlier
> than devfreq, and thus, display adds a memory bandwidth request using
> interconnect API, which results in a minimum clock-rate being set for
> the memory clk. In a result, the lowest devfreq OPP rate is getting
> limited to the minimum rate imposed by the display driver.
> 
> Let's use new clk_round_rate_unboundly() that resolves the problem by
> rounding clock rate without taking into account min/max limits imposed by
> active clk users.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/devfreq/tegra30-devfreq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
> index 28b2c7ca416e..34f6291e880c 100644
> --- a/drivers/devfreq/tegra30-devfreq.c
> +++ b/drivers/devfreq/tegra30-devfreq.c
> @@ -834,7 +834,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>  
>  	reset_control_deassert(tegra->reset);
>  
> -	rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
> +	rate = clk_round_rate_unboundly(tegra->emc_clock, ULONG_MAX);
>  	if (rate < 0) {
>  		dev_err(&pdev->dev, "Failed to round clock rate: %ld\n", rate);
>  		return rate;
> @@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>  	}
>  
>  	for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) {
> -		rate = clk_round_rate(tegra->emc_clock, rate);
> +		rate = clk_round_rate_unboundly(tegra->emc_clock, rate);
>  
>  		if (rate < 0) {
>  			dev_err(&pdev->dev,
> 

Firstly, patch2 have to be reviewed for this patch.
I have no any objection. It looks good to me.

If patch1 get the confirmation from clock maintainer,
feel free to add my acked tag:
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v1 4/5] PM / devfreq: tegra30: Use clk_round_rate_unboundly()

[Chanwoo Choi]

On 4/1/20 8:22 AM, Chanwoo Choi wrote:
> On 3/31/20 8:16 AM, Dmitry Osipenko wrote:
>> The clk_round_rate() doesn't work for us properly if clock rate is bounded
>> by a min/max rate that is requested by some other clk-user because we're
>> building devfreq's OPP table based on the rounding.
>>
>> In particular this becomes a problem if display driver is probed earlier
>> than devfreq, and thus, display adds a memory bandwidth request using
>> interconnect API, which results in a minimum clock-rate being set for
>> the memory clk. In a result, the lowest devfreq OPP rate is getting
>> limited to the minimum rate imposed by the display driver.
>>
>> Let's use new clk_round_rate_unboundly() that resolves the problem by
>> rounding clock rate without taking into account min/max limits imposed by
>> active clk users.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/devfreq/tegra30-devfreq.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
>> index 28b2c7ca416e..34f6291e880c 100644
>> --- a/drivers/devfreq/tegra30-devfreq.c
>> +++ b/drivers/devfreq/tegra30-devfreq.c
>> @@ -834,7 +834,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>  
>>  	reset_control_deassert(tegra->reset);
>>  
>> -	rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
>> +	rate = clk_round_rate_unboundly(tegra->emc_clock, ULONG_MAX);
>>  	if (rate < 0) {
>>  		dev_err(&pdev->dev, "Failed to round clock rate: %ld\n", rate);
>>  		return rate;
>> @@ -849,7 +849,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>  	}
>>  
>>  	for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) {
>> -		rate = clk_round_rate(tegra->emc_clock, rate);
>> +		rate = clk_round_rate_unboundly(tegra->emc_clock, rate);
>>  
>>  		if (rate < 0) {
>>  			dev_err(&pdev->dev,
>>
> 
> Firstly, patch2 have to be reviewed for this patch.
> I have no any objection. It looks good to me.
> 
> If patch1 get the confirmation from clock maintainer,

Sorry. I mean the patch2 about clk_round_rate_unboundly() function.
instead of patch1.

> feel free to add my acked tag:
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[PATCH v1 5/5] PM / devfreq: tegra30: Make CPUFreq notifier to take into account boosting

[Dmitry Osipenko]

MCCPU frequency boosting needs to be taken into account in order to avoid
scheduling of unnecessary devfreq updates.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/devfreq/tegra30-devfreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 34f6291e880c..3b57aac9894c 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -420,7 +420,7 @@ tegra_actmon_cpufreq_contribution(struct tegra_devfreq *tegra,
 
 	static_cpu_emc_freq = actmon_cpu_to_emc_rate(tegra, cpu_freq);
 
-	if (dev_freq >= static_cpu_emc_freq)
+	if (dev_freq + actmon_dev->boost_freq >= static_cpu_emc_freq)
 		return 0;
 
 	return static_cpu_emc_freq;
-- 
2.25.1

Re: [PATCH v1 5/5] PM / devfreq: tegra30: Make CPUFreq notifier to take into account boosting

[Chanwoo Choi]

Hi Dmitry,

On 3/31/20 8:16 AM, Dmitry Osipenko wrote:
> MCCPU frequency boosting needs to be taken into account in order to avoid
> scheduling of unnecessary devfreq updates.

"in order to avoid scheduling of unnecessary devfreq updates."
I don't understand the correct meaning of following description.
Could you explain it more detailed?

> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/devfreq/tegra30-devfreq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
> index 34f6291e880c..3b57aac9894c 100644
> --- a/drivers/devfreq/tegra30-devfreq.c
> +++ b/drivers/devfreq/tegra30-devfreq.c
> @@ -420,7 +420,7 @@ tegra_actmon_cpufreq_contribution(struct tegra_devfreq *tegra,
>  
>  	static_cpu_emc_freq = actmon_cpu_to_emc_rate(tegra, cpu_freq);
>  
> -	if (dev_freq >= static_cpu_emc_freq)
> +	if (dev_freq + actmon_dev->boost_freq >= static_cpu_emc_freq)
>  		return 0;
>  
>  	return static_cpu_emc_freq;
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v1 5/5] PM / devfreq: tegra30: Make CPUFreq notifier to take into account boosting

[Dmitry Osipenko]

01.04.2020 02:29, Chanwoo Choi пишет:
> Hi Dmitry,
> 
> On 3/31/20 8:16 AM, Dmitry Osipenko wrote:
>> MCCPU frequency boosting needs to be taken into account in order to avoid
>> scheduling of unnecessary devfreq updates.
> 
> "in order to avoid scheduling of unnecessary devfreq updates."
> I don't understand the correct meaning of following description.
> Could you explain it more detailed?

Hello Chanwoo,

I'll update the commit message with a more detailed description and make
this patch standalone in v2. Thank you very much for taking a look at
the patches.

BTW, please also take a look at this patch
https://patchwork.kernel.org/patch/11409037/