2020-03-30 23:21:24

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v1 0/5] NVIDIA Tegra devfreq drivers improvements

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


2020-03-30 23:21:53

by Dmitry Osipenko

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

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 <[email protected]>
---
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

2020-03-30 23:22:20

by Dmitry Osipenko

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

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 <[email protected]>
---
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

2020-03-31 23:14:10

by Chanwoo Choi

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

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 <[email protected]>
> ---
> 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 <[email protected]>

--
Best Regards,
Chanwoo Choi
Samsung Electronics

2020-03-31 23:15:00

by Chanwoo Choi

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

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 <[email protected]>
> ---
> 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 <[email protected]>


--
Best Regards,
Chanwoo Choi
Samsung Electronics

2020-03-31 23:16:44

by Chanwoo Choi

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

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 <[email protected]>
>> ---
>> 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 <[email protected]>
>

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


--
Best Regards,
Chanwoo Choi
Samsung Electronics

2020-03-31 23:17:38

by Chanwoo Choi

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

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 <[email protected]>
>> ---
>> 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 <[email protected]>
>
>


--
Best Regards,
Chanwoo Choi
Samsung Electronics