[PATCH V2 RESEND 0/4] PM / devfreq: a few small fixes and improvements

STALE1902d

Revision v2 of 4 in this series.

10 messages, 3 authors, 2021-05-19 · open the first message on its own page

[PATCH V2 RESEND 0/4] PM / devfreq: a few small fixes and improvements

From: Dong Aisheng <aisheng.dong@nxp.com>
Date: 2021-03-23 07:35:00

A few small fixes and improvements

ChangeLog:
v2 Resend:
 * rebase to devfreq-next
 * drop original patch 1 & 5.
   Patch 5 will be re-sent later when dependent patches merged.
v1->v2:
 * squash a few patches
 * rebase to devfreq-testing


Dong Aisheng (4):
  PM / devfreq: Use more accurate returned new_freq as resume_freq
  PM / devfreq: Remove the invalid description for get_target_freq
  PM / devfreq: bail out early if no freq changes in devfreq_set_target
  PM / devfreq: imx8m-ddrc: remove imx8m_ddrc_get_dev_status

 Documentation/ABI/testing/sysfs-class-devfreq |  5 +----
 drivers/devfreq/devfreq.c                     | 11 +++++++----
 drivers/devfreq/governor.h                    |  2 --
 drivers/devfreq/imx8m-ddrc.c                  | 14 --------------
 4 files changed, 8 insertions(+), 24 deletions(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 RESEND 2/4] PM / devfreq: Remove the invalid description for get_target_freq

From: Dong Aisheng <aisheng.dong@nxp.com>
Date: 2021-03-23 07:35:00

First of all, no_central_polling was removed since
commit 7e6fdd4bad03 ("PM / devfreq: Core updates to support devices
which can idle")
Secondly, get_target_freq() is not only called only with update_devfreq()
notified by OPP now, but also min/max freq qos notifier.

So remove this invalid description now to avoid confusing.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 Documentation/ABI/testing/sysfs-class-devfreq | 5 +----
 drivers/devfreq/governor.h                    | 2 --
 2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq
index 386bc230a33d..5e6b74f30406 100644
--- a/Documentation/ABI/testing/sysfs-class-devfreq
+++ b/Documentation/ABI/testing/sysfs-class-devfreq
@@ -97,10 +97,7 @@ Description:
 		object. The values are represented in ms. If the value is
 		less than 1 jiffy, it is considered to be 0, which means
 		no polling. This value is meaningless if the governor is
-		not polling; thus. If the governor is not using
-		devfreq-provided central polling
-		(/sys/class/devfreq/.../central_polling is 0), this value
-		may be useless.
+		not polling.
 
 		A list of governors that support the node:
 		- simple_ondmenad
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index 244634465170..2d69a0ce6291 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -57,8 +57,6 @@
  *			Basically, get_target_freq will run
  *			devfreq_dev_profile.get_dev_status() to get the
  *			status of the device (load = busy_time / total_time).
- *			If no_central_polling is set, this callback is called
- *			only with update_devfreq() notified by OPP.
  * @event_handler:      Callback for devfreq core framework to notify events
  *                      to governors. Events include per device governor
  *                      init and exit, opp changes out of devfreq, suspend
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 RESEND 2/4] PM / devfreq: Remove the invalid description for get_target_freq

From: Chanwoo Choi <hidden>
Date: 2021-03-23 14:57:09

On 21. 3. 23. 오후 4:20, Dong Aisheng wrote:
quoted hunk
First of all, no_central_polling was removed since
commit 7e6fdd4bad03 ("PM / devfreq: Core updates to support devices
which can idle")
Secondly, get_target_freq() is not only called only with update_devfreq()
notified by OPP now, but also min/max freq qos notifier.

So remove this invalid description now to avoid confusing.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
  Documentation/ABI/testing/sysfs-class-devfreq | 5 +----
  drivers/devfreq/governor.h                    | 2 --
  2 files changed, 1 insertion(+), 6 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq
index 386bc230a33d..5e6b74f30406 100644
--- a/Documentation/ABI/testing/sysfs-class-devfreq
+++ b/Documentation/ABI/testing/sysfs-class-devfreq
@@ -97,10 +97,7 @@ Description:
  		object. The values are represented in ms. If the value is
  		less than 1 jiffy, it is considered to be 0, which means
  		no polling. This value is meaningless if the governor is
-		not polling; thus. If the governor is not using
-		devfreq-provided central polling
-		(/sys/class/devfreq/.../central_polling is 0), this value
-		may be useless.
+		not polling.
  
  		A list of governors that support the node:
  		- simple_ondmenad
diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h
index 244634465170..2d69a0ce6291 100644
--- a/drivers/devfreq/governor.h
+++ b/drivers/devfreq/governor.h
@@ -57,8 +57,6 @@
   *			Basically, get_target_freq will run
   *			devfreq_dev_profile.get_dev_status() to get the
   *			status of the device (load = busy_time / total_time).
- *			If no_central_polling is set, this callback is called
- *			only with update_devfreq() notified by OPP.
   * @event_handler:      Callback for devfreq core framework to notify events
   *                      to governors. Events include per device governor
   *                      init and exit, opp changes out of devfreq, suspend
Applied it. Thanks.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 RESEND 1/4] PM / devfreq: Use more accurate returned new_freq as resume_freq

From: Dong Aisheng <aisheng.dong@nxp.com>
Date: 2021-03-23 07:35:01

Use the more accurate returned new_freq as resume_freq.
It's the same as how devfreq->previous_freq was updated.

Fixes: 83f8ca45afbf0 ("PM / devfreq: add support for suspend/resume of a
devfreq device")
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/devfreq/devfreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b6d3e7db0b09..85927bd7ee76 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -388,7 +388,7 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
 	devfreq->previous_freq = new_freq;
 
 	if (devfreq->suspend_freq)
-		devfreq->resume_freq = cur_freq;
+		devfreq->resume_freq = new_freq;
 
 	return err;
 }
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 RESEND 1/4] PM / devfreq: Use more accurate returned new_freq as resume_freq

From: Chanwoo Choi <hidden>
Date: 2021-03-23 14:56:31

On 21. 3. 23. 오후 4:20, Dong Aisheng wrote:
quoted hunk
Use the more accurate returned new_freq as resume_freq.
It's the same as how devfreq->previous_freq was updated.

Fixes: 83f8ca45afbf0 ("PM / devfreq: add support for suspend/resume of a
devfreq device")
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
  drivers/devfreq/devfreq.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b6d3e7db0b09..85927bd7ee76 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -388,7 +388,7 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
  	devfreq->previous_freq = new_freq;
  
  	if (devfreq->suspend_freq)
-		devfreq->resume_freq = cur_freq;
+		devfreq->resume_freq = new_freq;
  
  	return err;
  }
Applied it.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 RESEND 3/4] PM / devfreq: bail out early if no freq changes in devfreq_set_target

From: Dong Aisheng <aisheng.dong@nxp.com>
Date: 2021-03-23 07:35:01

It's unnecessary to set the same freq again and run notifier calls.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/devfreq/devfreq.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 85927bd7ee76..a6ee91dd17bd 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -352,13 +352,16 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
 {
 	struct devfreq_freqs freqs;
 	unsigned long cur_freq;
-	int err = 0;
+	int err;
 
 	if (devfreq->profile->get_cur_freq)
 		devfreq->profile->get_cur_freq(devfreq->dev.parent, &cur_freq);
 	else
 		cur_freq = devfreq->previous_freq;
 
+	if (new_freq == cur_freq)
+		return 0;
+
 	freqs.old = cur_freq;
 	freqs.new = new_freq;
 	devfreq_notify_transition(devfreq, &freqs, DEVFREQ_PRECHANGE);
@@ -375,7 +378,7 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
 	 * and DEVFREQ_POSTCHANGE because for showing the correct frequency
 	 * change order of between devfreq device and passive devfreq device.
 	 */
-	if (trace_devfreq_frequency_enabled() && new_freq != cur_freq)
+	if (trace_devfreq_frequency_enabled())
 		trace_devfreq_frequency(devfreq, new_freq, cur_freq);
 
 	freqs.new = new_freq;
@@ -390,7 +393,7 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
 	if (devfreq->suspend_freq)
 		devfreq->resume_freq = new_freq;
 
-	return err;
+	return 0;
 }
 
 /**
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 RESEND 3/4] PM / devfreq: bail out early if no freq changes in devfreq_set_target

From: Chanwoo Choi <hidden>
Date: 2021-03-23 15:02:55

On 21. 3. 23. 오후 4:20, Dong Aisheng wrote:
quoted hunk
It's unnecessary to set the same freq again and run notifier calls.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
  drivers/devfreq/devfreq.c | 9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 85927bd7ee76..a6ee91dd17bd 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -352,13 +352,16 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
  {
  	struct devfreq_freqs freqs;
  	unsigned long cur_freq;
-	int err = 0;
+	int err;
  
  	if (devfreq->profile->get_cur_freq)
  		devfreq->profile->get_cur_freq(devfreq->dev.parent, &cur_freq);
  	else
  		cur_freq = devfreq->previous_freq;
  
+	if (new_freq == cur_freq)
+		return 0;
cur_freq is one of the OPP frequencies. But, new_freq is calculated from
governor algorithm. It means that new_freq is not one of the
frequencies. Actually, it is not efficient.

After devfreq->profile->target() which almost uses
devfreq_recommended_opp(), new_freq is one of OPP frequencies.
quoted hunk
+
  	freqs.old = cur_freq;
  	freqs.new = new_freq;
  	devfreq_notify_transition(devfreq, &freqs, DEVFREQ_PRECHANGE);
@@ -375,7 +378,7 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
  	 * and DEVFREQ_POSTCHANGE because for showing the correct frequency
  	 * change order of between devfreq device and passive devfreq device.
  	 */
-	if (trace_devfreq_frequency_enabled() && new_freq != cur_freq)
+	if (trace_devfreq_frequency_enabled())
  		trace_devfreq_frequency(devfreq, new_freq, cur_freq);
  
  	freqs.new = new_freq;
@@ -390,7 +393,7 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
  	if (devfreq->suspend_freq)
  		devfreq->resume_freq = new_freq;
  
-	return err;
+	return 0;
  }
  
  /**

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 RESEND 3/4] PM / devfreq: bail out early if no freq changes in devfreq_set_target

From: Dong Aisheng <hidden>
Date: 2021-05-19 06:52:13

On Tue, Mar 23, 2021 at 11:01 PM Chanwoo Choi [off-list ref] wrote:
On 21. 3. 23. 오후 4:20, Dong Aisheng wrote:
quoted
It's unnecessary to set the same freq again and run notifier calls.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
  drivers/devfreq/devfreq.c | 9 ++++++---
  1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 85927bd7ee76..a6ee91dd17bd 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -352,13 +352,16 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
  {
      struct devfreq_freqs freqs;
      unsigned long cur_freq;
-     int err = 0;
+     int err;

      if (devfreq->profile->get_cur_freq)
              devfreq->profile->get_cur_freq(devfreq->dev.parent, &cur_freq);
      else
              cur_freq = devfreq->previous_freq;

+     if (new_freq == cur_freq)
+             return 0;
cur_freq is one of the OPP frequencies. But, new_freq is calculated from
governor algorithm. It means that new_freq is not one of the
frequencies. Actually, it is not efficient.

After devfreq->profile->target() which almost uses
devfreq_recommended_opp(), new_freq is one of OPP frequencies.
Yes, but i feel at least when the desired new_freq is equal to cur_freq
which is the last successfully set rate,  it is sufficient to bail out early as
it's meaningless to re-set the same rate as the last one and notify the
an unchanged rate transition in HW.
Does that make sense?

Regards
Aisheng
quoted
+
      freqs.old = cur_freq;
      freqs.new = new_freq;
      devfreq_notify_transition(devfreq, &freqs, DEVFREQ_PRECHANGE);
@@ -375,7 +378,7 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
       * and DEVFREQ_POSTCHANGE because for showing the correct frequency
       * change order of between devfreq device and passive devfreq device.
       */
-     if (trace_devfreq_frequency_enabled() && new_freq != cur_freq)
+     if (trace_devfreq_frequency_enabled())
              trace_devfreq_frequency(devfreq, new_freq, cur_freq);

      freqs.new = new_freq;
@@ -390,7 +393,7 @@ static int devfreq_set_target(struct devfreq *devfreq, unsigned long new_freq,
      if (devfreq->suspend_freq)
              devfreq->resume_freq = new_freq;

-     return err;
+     return 0;
  }

  /**

--
Best Regards,
Samsung Electronics
Chanwoo Choi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2 RESEND 4/4] PM / devfreq: imx8m-ddrc: remove imx8m_ddrc_get_dev_status

From: Dong Aisheng <aisheng.dong@nxp.com>
Date: 2021-03-23 07:35:01

Current driver actually does not support simple ondemand governor
as it's unable to provide device load information. So removing
the unnecessary callback to avoid confusing.
Right now the driver is using userspace governor by default.

polling_ms was also dropped as it's not needed for non-ondemand
governor.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/devfreq/imx8m-ddrc.c | 14 --------------
 1 file changed, 14 deletions(-)
diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
index bc82d3653bff..ecb9375aa877 100644
--- a/drivers/devfreq/imx8m-ddrc.c
+++ b/drivers/devfreq/imx8m-ddrc.c
@@ -280,18 +280,6 @@ static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned long *freq)
 	return 0;
 }
 
-static int imx8m_ddrc_get_dev_status(struct device *dev,
-				     struct devfreq_dev_status *stat)
-{
-	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
-
-	stat->busy_time = 0;
-	stat->total_time = 0;
-	stat->current_frequency = clk_get_rate(priv->dram_core);
-
-	return 0;
-}
-
 static int imx8m_ddrc_init_freq_info(struct device *dev)
 {
 	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
@@ -429,9 +417,7 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err;
 
-	priv->profile.polling_ms = 1000;
 	priv->profile.target = imx8m_ddrc_target;
-	priv->profile.get_dev_status = imx8m_ddrc_get_dev_status;
 	priv->profile.exit = imx8m_ddrc_exit;
 	priv->profile.get_cur_freq = imx8m_ddrc_get_cur_freq;
 	priv->profile.initial_freq = clk_get_rate(priv->dram_core);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2 RESEND 4/4] PM / devfreq: imx8m-ddrc: remove imx8m_ddrc_get_dev_status

From: Chanwoo Choi <hidden>
Date: 2021-03-23 14:57:37

On 21. 3. 23. 오후 4:20, Dong Aisheng wrote:
quoted hunk
Current driver actually does not support simple ondemand governor
as it's unable to provide device load information. So removing
the unnecessary callback to avoid confusing.
Right now the driver is using userspace governor by default.

polling_ms was also dropped as it's not needed for non-ondemand
governor.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
  drivers/devfreq/imx8m-ddrc.c | 14 --------------
  1 file changed, 14 deletions(-)
diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c
index bc82d3653bff..ecb9375aa877 100644
--- a/drivers/devfreq/imx8m-ddrc.c
+++ b/drivers/devfreq/imx8m-ddrc.c
@@ -280,18 +280,6 @@ static int imx8m_ddrc_get_cur_freq(struct device *dev, unsigned long *freq)
  	return 0;
  }
  
-static int imx8m_ddrc_get_dev_status(struct device *dev,
-				     struct devfreq_dev_status *stat)
-{
-	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
-
-	stat->busy_time = 0;
-	stat->total_time = 0;
-	stat->current_frequency = clk_get_rate(priv->dram_core);
-
-	return 0;
-}
-
  static int imx8m_ddrc_init_freq_info(struct device *dev)
  {
  	struct imx8m_ddrc *priv = dev_get_drvdata(dev);
@@ -429,9 +417,7 @@ static int imx8m_ddrc_probe(struct platform_device *pdev)
  	if (ret < 0)
  		goto err;
  
-	priv->profile.polling_ms = 1000;
  	priv->profile.target = imx8m_ddrc_target;
-	priv->profile.get_dev_status = imx8m_ddrc_get_dev_status;
  	priv->profile.exit = imx8m_ddrc_exit;
  	priv->profile.get_cur_freq = imx8m_ddrc_get_cur_freq;
  	priv->profile.initial_freq = clk_get_rate(priv->dram_core);
Applied it. Thanks.

-- 
Best Regards,
Samsung Electronics
Chanwoo Choi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help