diff mbox series

[V2,1/2] thermal: ti-soc-thermal: Enable addition power management

Message ID 20200819125925.2119434-1-aford173@gmail.com
State New
Headers show
Series [V2,1/2] thermal: ti-soc-thermal: Enable addition power management | expand

Commit Message

Adam Ford Aug. 19, 2020, 12:59 p.m. UTC
The bandgap sensor can be idled when the processor is too, but it
isn't currently being done, so the power consumption of OMAP3
boards can elevated if the bangap sensor is enabled.

This patch attempts to use some additional power management
to idle the clock to the bandgap when not needed.

Signed-off-by: Adam Ford <aford173@gmail.com>
Reported-by: kernel test robot <lkp@intel.com>
---
V2: Fix issue where variable stating the suspend mode isn't being
    properly set and cleared.

Comments

Andreas Kemnade Aug. 19, 2020, 7:23 p.m. UTC | #1
On Wed, 19 Aug 2020 07:59:23 -0500
Adam Ford <aford173@gmail.com> wrote:

> The bandgap sensor can be idled when the processor is too, but it
> isn't currently being done, so the power consumption of OMAP3
> boards can elevated if the bangap sensor is enabled.
> 
> This patch attempts to use some additional power management
> to idle the clock to the bandgap when not needed.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
> V2: Fix issue where variable stating the suspend mode isn't being
>     properly set and cleared.
> 
I get
root@(none):/# cat /sys/class/thermal/thermal_zone0/type 
cpu_thermal
root@(none):/# cat /sys/class/thermal/thermal_zone0/temp 
50000
root@(none):/# cat /sys/class/thermal/thermal_zone1/     
available_policies  mode                subsystem/
integral_cutoff     offset              sustainable_power
k_d                 passive             temp
k_i                 policy              type
k_po                power/              uevent
k_pu                slope               
root@(none):/# cat /sys/class/thermal/thermal_zone1/type 
bq27000-battery
root@(none):/# cat /sys/kernel/debug/pm_debug/count 
usbhost_pwrdm (ON),OFF:3459,RET:635,INA:0,ON:4095,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
sgx_pwrdm (OFF),OFF:1,RET:0,INA:1,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
core_pwrdm (ON),OFF:86,RET:7,INA:0,ON:94,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0
per_pwrdm (ON),OFF:1518,RET:64,INA:0,ON:1583,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
dss_pwrdm (ON),OFF:3459,RET:635,INA:0,ON:4095,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
cam_pwrdm (OFF),OFF:1,RET:1,INA:0,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
neon_pwrdm (ON),OFF:2845,RET:1131,INA:119,ON:4096,RET-LOGIC-OFF:0
mpu_pwrdm (ON),OFF:2845,RET:1130,INA:119,ON:4095,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0
iva2_pwrdm (OFF),OFF:1,RET:1,INA:0,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RET-MEMBANK4-OFF:0
usbhost_clkdm->usbhost_pwrdm (1)
sgx_clkdm->sgx_pwrdm (0)
per_clkdm->per_pwrdm (13)
cam_clkdm->cam_pwrdm (0)
dss_clkdm->dss_pwrdm (1)
d2d_clkdm->core_pwrdm (0)
iva2_clkdm->iva2_pwrdm (0)
mpu_clkdm->mpu_pwrdm (0)
core_l4_clkdm->core_pwrdm (20)
core_l3_clkdm->core_pwrdm (1)
neon_clkdm->neon_pwrdm (0)
root@(none):/# 

So things still turn off.

Tested-by: Andreas Kemnade <andreas@kemnade.info> # GTA04

> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> index ab19ceff6e2a..9404631bea4d 100644
> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
> @@ -25,10 +25,18 @@
>  #include <linux/of_platform.h>
>  #include <linux/of_irq.h>
>  #include <linux/io.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pm.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>  
>  #include "ti-bandgap.h"
>  
>  static int ti_bandgap_force_single_read(struct ti_bandgap *bgp, int id);
> +static int bandgap_omap_cpu_notifier(struct notifier_block *nb,
> +				  unsigned long cmd, void *v);
>  
>  /***   Helper functions to access registers and their bitfields   ***/
>  
> @@ -1008,6 +1016,9 @@ int ti_bandgap_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	bgp->nb.notifier_call = bandgap_omap_cpu_notifier;
> +	cpu_pm_register_notifier(&bgp->nb);
> +
>  	return 0;
>  
>  remove_last_cooling:
> @@ -1041,7 +1052,9 @@ int ti_bandgap_remove(struct platform_device *pdev)
>  	struct ti_bandgap *bgp = platform_get_drvdata(pdev);
>  	int i;
>  
> -	/* First thing is to remove sensor interfaces */
> +	cpu_pm_unregister_notifier(&bgp->nb);
> +
> +	/* Remove sensor interfaces */
>  	for (i = 0; i < bgp->conf->sensor_count; i++) {
>  		if (bgp->conf->sensors[i].unregister_cooling)
>  			bgp->conf->sensors[i].unregister_cooling(bgp, i);
> @@ -1150,9 +1163,43 @@ static int ti_bandgap_suspend(struct device *dev)
>  	if (TI_BANDGAP_HAS(bgp, CLK_CTRL))
>  		clk_disable_unprepare(bgp->fclock);
>  
> +	bgp->is_suspended = true;
> +
>  	return err;
>  }
>  
> +static int bandgap_omap_cpu_notifier(struct notifier_block *nb,
> +				  unsigned long cmd, void *v)
> +{
> +	struct ti_bandgap *bgp;
> +
> +	bgp = container_of(nb, struct ti_bandgap, nb);
> +
> +	spin_lock(&bgp->lock);
> +	switch (cmd) {
> +	case CPU_CLUSTER_PM_ENTER:
> +		if (bgp->is_suspended)
> +			break;
> +		ti_bandgap_save_ctxt(bgp);
> +		ti_bandgap_power(bgp, false);
> +		if (TI_BANDGAP_HAS(bgp, CLK_CTRL))
> +			clk_disable(bgp->fclock);
> +		break;
> +	case CPU_CLUSTER_PM_ENTER_FAILED:
> +	case CPU_CLUSTER_PM_EXIT:
> +		if (bgp->is_suspended)
> +			break;
> +		if (TI_BANDGAP_HAS(bgp, CLK_CTRL))
> +			clk_enable(bgp->fclock);
> +		ti_bandgap_power(bgp, true);
> +		ti_bandgap_restore_ctxt(bgp);
> +		break;
> +	}
> +	spin_unlock(&bgp->lock);
> +
> +	return NOTIFY_OK;
> +}
> +
>  static int ti_bandgap_resume(struct device *dev)
>  {
>  	struct ti_bandgap *bgp = dev_get_drvdata(dev);
> @@ -1161,6 +1208,7 @@ static int ti_bandgap_resume(struct device *dev)
>  		clk_prepare_enable(bgp->fclock);
>  
>  	ti_bandgap_power(bgp, true);
> +	bgp->is_suspended = false;
>  
>  	return ti_bandgap_restore_ctxt(bgp);
>  }
> diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.h b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> index fce4657e9486..ed0ea4b17b25 100644
> --- a/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> +++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
> @@ -12,6 +12,10 @@
>  #include <linux/spinlock.h>
>  #include <linux/types.h>
>  #include <linux/err.h>
> +#include <linux/cpu_pm.h>
> +#include <linux/device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pm.h>
>  
>  struct gpio_desc;
>  
> @@ -203,6 +207,8 @@ struct ti_bandgap {
>  	int				irq;
>  	struct gpio_desc		*tshut_gpiod;
>  	u32				clk_rate;
> +	struct notifier_block		nb;
> +	unsigned int is_suspended:1;
>  };
>  
>  /**
Andreas Kemnade Sept. 10, 2020, 6:01 p.m. UTC | #2
On Wed, 19 Aug 2020 07:59:23 -0500
Adam Ford <aford173@gmail.com> wrote:

> The bandgap sensor can be idled when the processor is too, but it
> isn't currently being done, so the power consumption of OMAP3
> boards can elevated if the bangap sensor is enabled.
> 
> This patch attempts to use some additional power management
> to idle the clock to the bandgap when not needed.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
> V2: Fix issue where variable stating the suspend mode isn't being
>     properly set and cleared.
> 
hmm, it is not in linux-next. Can we expect that for v5.10?

Regards,
Andreas
Daniel Lezcano Sept. 10, 2020, 7:14 p.m. UTC | #3
On 10/09/2020 20:01, Andreas Kemnade wrote:
> On Wed, 19 Aug 2020 07:59:23 -0500
> Adam Ford <aford173@gmail.com> wrote:
> 
>> The bandgap sensor can be idled when the processor is too, but it
>> isn't currently being done, so the power consumption of OMAP3
>> boards can elevated if the bangap sensor is enabled.
>>
>> This patch attempts to use some additional power management
>> to idle the clock to the bandgap when not needed.
>>
>> Signed-off-by: Adam Ford <aford173@gmail.com>
>> Reported-by: kernel test robot <lkp@intel.com>
>> ---
>> V2: Fix issue where variable stating the suspend mode isn't being
>>     properly set and cleared.
>>
> hmm, it is not in linux-next. Can we expect that for v5.10?

The reason I did not pick this patch is because lkp reported an error on it.

https://marc.info/?l=linux-pm&m=159788472017308&w=2
Andreas Kemnade Sept. 10, 2020, 7:48 p.m. UTC | #4
On Thu, 10 Sep 2020 14:33:13 -0500
Adam Ford <aford173@gmail.com> wrote:

> On Thu, Sep 10, 2020 at 2:14 PM Daniel Lezcano <daniel.lezcano@linaro.org>
> wrote:
> 
> > On 10/09/2020 20:01, Andreas Kemnade wrote:  
> > > On Wed, 19 Aug 2020 07:59:23 -0500
> > > Adam Ford <aford173@gmail.com> wrote:
> > >  
> > >> The bandgap sensor can be idled when the processor is too, but it
> > >> isn't currently being done, so the power consumption of OMAP3
> > >> boards can elevated if the bangap sensor is enabled.
> > >>
> > >> This patch attempts to use some additional power management
> > >> to idle the clock to the bandgap when not needed.
> > >>
> > >> Signed-off-by: Adam Ford <aford173@gmail.com>
> > >> Reported-by: kernel test robot <lkp@intel.com>
> > >> ---
> > >> V2: Fix issue where variable stating the suspend mode isn't being
> > >>     properly set and cleared.
> > >>  
> > > hmm, it is not in linux-next. Can we expect that for v5.10?  
> >
> > The reason I did not pick this patch is because lkp reported an error on
> > it.
> >
> > https://marc.info/?l=linux-pm&m=159788472017308&w=2
> >
> >
> >  
> That error message shows it's trying to be built with 'sh' cross compiler,
> but should be build with an ARM.
> 
> I can run a manual test of the patch against a different branch if
> necessary, but I had built and tested it, so I know it worked at one time.
> 
hmm, what about compile-testing without CONFIG_PM_SLEEP?

The function definition is guarded by that.
So it is not a sh-specific problem.

Regards,
Andreas
diff mbox series

Patch

diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.c b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
index ab19ceff6e2a..9404631bea4d 100644
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.c
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.c
@@ -25,10 +25,18 @@ 
 #include <linux/of_platform.h>
 #include <linux/of_irq.h>
 #include <linux/io.h>
+#include <linux/cpu_pm.h>
+#include <linux/device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include "ti-bandgap.h"
 
 static int ti_bandgap_force_single_read(struct ti_bandgap *bgp, int id);
+static int bandgap_omap_cpu_notifier(struct notifier_block *nb,
+				  unsigned long cmd, void *v);
 
 /***   Helper functions to access registers and their bitfields   ***/
 
@@ -1008,6 +1016,9 @@  int ti_bandgap_probe(struct platform_device *pdev)
 		}
 	}
 
+	bgp->nb.notifier_call = bandgap_omap_cpu_notifier;
+	cpu_pm_register_notifier(&bgp->nb);
+
 	return 0;
 
 remove_last_cooling:
@@ -1041,7 +1052,9 @@  int ti_bandgap_remove(struct platform_device *pdev)
 	struct ti_bandgap *bgp = platform_get_drvdata(pdev);
 	int i;
 
-	/* First thing is to remove sensor interfaces */
+	cpu_pm_unregister_notifier(&bgp->nb);
+
+	/* Remove sensor interfaces */
 	for (i = 0; i < bgp->conf->sensor_count; i++) {
 		if (bgp->conf->sensors[i].unregister_cooling)
 			bgp->conf->sensors[i].unregister_cooling(bgp, i);
@@ -1150,9 +1163,43 @@  static int ti_bandgap_suspend(struct device *dev)
 	if (TI_BANDGAP_HAS(bgp, CLK_CTRL))
 		clk_disable_unprepare(bgp->fclock);
 
+	bgp->is_suspended = true;
+
 	return err;
 }
 
+static int bandgap_omap_cpu_notifier(struct notifier_block *nb,
+				  unsigned long cmd, void *v)
+{
+	struct ti_bandgap *bgp;
+
+	bgp = container_of(nb, struct ti_bandgap, nb);
+
+	spin_lock(&bgp->lock);
+	switch (cmd) {
+	case CPU_CLUSTER_PM_ENTER:
+		if (bgp->is_suspended)
+			break;
+		ti_bandgap_save_ctxt(bgp);
+		ti_bandgap_power(bgp, false);
+		if (TI_BANDGAP_HAS(bgp, CLK_CTRL))
+			clk_disable(bgp->fclock);
+		break;
+	case CPU_CLUSTER_PM_ENTER_FAILED:
+	case CPU_CLUSTER_PM_EXIT:
+		if (bgp->is_suspended)
+			break;
+		if (TI_BANDGAP_HAS(bgp, CLK_CTRL))
+			clk_enable(bgp->fclock);
+		ti_bandgap_power(bgp, true);
+		ti_bandgap_restore_ctxt(bgp);
+		break;
+	}
+	spin_unlock(&bgp->lock);
+
+	return NOTIFY_OK;
+}
+
 static int ti_bandgap_resume(struct device *dev)
 {
 	struct ti_bandgap *bgp = dev_get_drvdata(dev);
@@ -1161,6 +1208,7 @@  static int ti_bandgap_resume(struct device *dev)
 		clk_prepare_enable(bgp->fclock);
 
 	ti_bandgap_power(bgp, true);
+	bgp->is_suspended = false;
 
 	return ti_bandgap_restore_ctxt(bgp);
 }
diff --git a/drivers/thermal/ti-soc-thermal/ti-bandgap.h b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
index fce4657e9486..ed0ea4b17b25 100644
--- a/drivers/thermal/ti-soc-thermal/ti-bandgap.h
+++ b/drivers/thermal/ti-soc-thermal/ti-bandgap.h
@@ -12,6 +12,10 @@ 
 #include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/err.h>
+#include <linux/cpu_pm.h>
+#include <linux/device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm.h>
 
 struct gpio_desc;
 
@@ -203,6 +207,8 @@  struct ti_bandgap {
 	int				irq;
 	struct gpio_desc		*tshut_gpiod;
 	u32				clk_rate;
+	struct notifier_block		nb;
+	unsigned int is_suspended:1;
 };
 
 /**