Linux Power Management development
 help / color / mirror / Atom feed
* Re: [PATCH 0/3] coupled cpuidle state support
From: Shilimkar, Santosh @ 2011-12-22  8:35 UTC (permalink / raw)
  To: Colin Cross
  Cc: Kevin Hilman, Len Brown, linux-kernel, Amit Kucheria, linux-tegra,
	linux-pm, linux-omap, Arjan van de Ven, linux-arm-kernel
In-Reply-To: <CAMbhsRSPCo9MSGp57Fv0sRdYUFcdwx8jEz1-CfX8LdKf8X4MVg@mail.gmail.com>

On Thu, Dec 22, 2011 at 1:12 AM, Colin Cross <ccross@android.com> wrote:
> On Wed, Dec 21, 2011 at 11:36 AM, Arjan van de Ven
> <arjan@linux.intel.com> wrote:
>>>>
>>>> .. or it enters WFI, and a physical device sends it an interrupt,
>>>> at which point it exits.
>>>
>>> None of the cpus will return to the idle loop until all cpus have
>>> decremented the ready counter back to 0, so they can't wrap around
>>> again.
>>
>>
>> yikes, so you IPI all the cpus on the first exit.
>> that must burn power ;-(
>
> No, you're not understanding the point of this series.
>
> If your cpus can go in and out of idle independently, you don't use
> this code at all.  Each cpu can call WFI and come back out without
> talking to the other cpu.
>
Indeed. The SOCs, Arch's which does support low power
state independently and doesn't need any co-ordination between CPU's
will continue to work same way as before with this series.

> However, if you have two cpus that share some part of the SoC that can
> be turned off in idle, like the L2 cache controller or the system bus,
> the two cpus need to go to idle together, and they will both boot
> together when either one receives an interrupt (although one will
> likely immediately go back to a shallower state that doesn't require
> coordination with the other cpu).  There is no way around this, it's
> how the hardware works on some ARM platforms.

Apart from shared peripherals which Colin pointed out, OMAP also
brings in the security software state which is kind of executing in
parallel with linux. This state is lost in certain deeper low power
states and since the security software is affine to only master CPU
(because of OMAP security architecture), the co-ordination is
mandatory to achieve those low power states.

So this additional CPU co-ordination logic for such C-states
really helps to solve the issue in most generic way.

Regards
Santosh

^ permalink raw reply

* Re: [PATCH 0/3] coupled cpuidle state support
From: Colin Cross @ 2011-12-21 19:42 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Kevin Hilman, Len Brown, linux-kernel, Amit Kucheria, linux-tegra,
	linux-pm, linux-omap, linux-arm-kernel
In-Reply-To: <4EF2352C.3060102@linux.intel.com>

On Wed, Dec 21, 2011 at 11:36 AM, Arjan van de Ven
<arjan@linux.intel.com> wrote:
>>>
>>> .. or it enters WFI, and a physical device sends it an interrupt,
>>> at which point it exits.
>>
>> None of the cpus will return to the idle loop until all cpus have
>> decremented the ready counter back to 0, so they can't wrap around
>> again.
>
>
> yikes, so you IPI all the cpus on the first exit.
> that must burn power ;-(

No, you're not understanding the point of this series.

If your cpus can go in and out of idle independently, you don't use
this code at all.  Each cpu can call WFI and come back out without
talking to the other cpu.

However, if you have two cpus that share some part of the SoC that can
be turned off in idle, like the L2 cache controller or the system bus,
the two cpus need to go to idle together, and they will both boot
together when either one receives an interrupt (although one will
likely immediately go back to a shallower state that doesn't require
coordination with the other cpu).  There is no way around this, it's
how the hardware works on some ARM platforms.

^ permalink raw reply

* Re: [PATCH 0/3] coupled cpuidle state support
From: Arjan van de Ven @ 2011-12-21 19:36 UTC (permalink / raw)
  To: Colin Cross
  Cc: Kevin Hilman, Len Brown, linux-kernel, Amit Kucheria, linux-tegra,
	linux-pm, linux-omap, linux-arm-kernel
In-Reply-To: <CAMbhsRRnKoR3YPWrZzVynVe+dcuU5v0siYvmp2vbBcS1hquC7Q@mail.gmail.com>

>>
>> .. or it enters WFI, and a physical device sends it an interrupt,
>> at which point it exits.
> 
> None of the cpus will return to the idle loop until all cpus have
> decremented the ready counter back to 0, so they can't wrap around
> again.


yikes, so you IPI all the cpus on the first exit.
that must burn power ;-(

^ permalink raw reply

* Re: [PATCH 0/3] coupled cpuidle state support
From: Colin Cross @ 2011-12-21 19:05 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Kevin Hilman, Len Brown, linux-kernel, Amit Kucheria, linux-tegra,
	linux-pm, linux-omap, linux-arm-kernel
In-Reply-To: <4EF1CD49.9020800@linux.intel.com>

On Wed, Dec 21, 2011 at 4:12 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
> On 12/21/2011 10:55 AM, Colin Cross wrote:
>> On Wed, Dec 21, 2011 at 1:44 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
>>> On 12/21/2011 10:40 AM, Colin Cross wrote:
>>>
>>>>> this smells fundamentally racey to me; you can get an interrupt one
>>>>> cycle after you think you're done, but before the last guy enters WFI...
>>>>>
>>>>> how do you solve that issue ?
>>>>
>>>> All the cpus have interrupts off when they increment the counter, so
>>>> they cannot receive an interrupt.  If an interrupt is pending on one
>>>> of those cpus, it will be handled later when WFI aborts due to the
>>>> pending interrupt.
>>>
>>> ... but this leads to cases where you're aborting before other cpus are
>>> entering..... so your "last guy in" doesn't really work, since while cpu
>>> 0 thinks it's the last guy, cpu 1 is already on the way out/out
>>> already...  (heck it might already be going back to sleep if your idle
>>> code can run fast, like in the size of a cache miss)
>>
>> Once a cpu has incremented the counter, it has no way out unless either
>> 1: another cpu (that hasn't incremented the counter yet) receives an
>> interrupt, aborts idle, and clears its idle flag
>> or
>> 2: all cpus enter the ready counter, and call the cpuidle driver's
>> enter function.
>
> .. or it enters WFI, and a physical device sends it an interrupt,
> at which point it exits.

None of the cpus will return to the idle loop until all cpus have
decremented the ready counter back to 0, so they can't wrap around
again.

^ permalink raw reply

* Re: [PATCH 0/3] coupled cpuidle state support
From: Arjan van de Ven @ 2011-12-21 12:12 UTC (permalink / raw)
  To: Colin Cross
  Cc: Kevin Hilman, Len Brown, linux-kernel, Amit Kucheria, linux-tegra,
	linux-pm, linux-omap, linux-arm-kernel
In-Reply-To: <CAMbhsRSNTUvou9Gf6T4Ggq=jfxsWUcn7M2yNsKQz6aNrt6gLDg@mail.gmail.com>

On 12/21/2011 10:55 AM, Colin Cross wrote:
> On Wed, Dec 21, 2011 at 1:44 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
>> On 12/21/2011 10:40 AM, Colin Cross wrote:
>>
>>>> this smells fundamentally racey to me; you can get an interrupt one
>>>> cycle after you think you're done, but before the last guy enters WFI...
>>>>
>>>> how do you solve that issue ?
>>>
>>> All the cpus have interrupts off when they increment the counter, so
>>> they cannot receive an interrupt.  If an interrupt is pending on one
>>> of those cpus, it will be handled later when WFI aborts due to the
>>> pending interrupt.
>>
>> ... but this leads to cases where you're aborting before other cpus are
>> entering..... so your "last guy in" doesn't really work, since while cpu
>> 0 thinks it's the last guy, cpu 1 is already on the way out/out
>> already...  (heck it might already be going back to sleep if your idle
>> code can run fast, like in the size of a cache miss)
> 
> Once a cpu has incremented the counter, it has no way out unless either
> 1: another cpu (that hasn't incremented the counter yet) receives an
> interrupt, aborts idle, and clears its idle flag
> or
> 2: all cpus enter the ready counter, and call the cpuidle driver's
> enter function.

.. or it enters WFI, and a physical device sends it an interrupt,
at which point it exits.

^ permalink raw reply

* [RFC PATCH 3/3] ARM: exynos4: Add thermal sensor driver platform device support
From: Amit Daniel Kachhap @ 2011-12-21 11:59 UTC (permalink / raw)
  To: linux-pm, linux-samsung-soc
  Cc: linaro-dev, patches, linux-kernel, lm-sensors, linux-acpi
In-Reply-To: <1324468798-20423-1-git-send-email-amit.kachhap@linaro.org>

This patch adds necessary source definations needed for TMU driver and
the platform device support.

Signed-off-by: Amit Daniel Kachhap <amit.kachhap@linaro.org>
---
 arch/arm/mach-exynos/Kconfig              |   12 +++++
 arch/arm/mach-exynos/Makefile             |    1 +
 arch/arm/mach-exynos/clock.c              |    4 ++
 arch/arm/mach-exynos/dev-tmu.c            |   64 +++++++++++++++++++++++++++++
 arch/arm/mach-exynos/include/mach/irqs.h  |    2 +
 arch/arm/mach-exynos/include/mach/map.h   |    1 +
 arch/arm/mach-exynos/mach-origen.c        |    1 +
 arch/arm/plat-samsung/include/plat/devs.h |    1 +
 8 files changed, 86 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-exynos/dev-tmu.c

diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index 724ec0f..cfc6119 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -82,6 +82,17 @@ config EXYNOS4_DEV_DWMCI
 	help
 	  Compile in platform device definitions for DWMCI
 
+config EXYNOS4_DEV_TMU
+	bool "Exynos4 tmu device support"
+	default n
+	depends on ARCH_EXYNOS4
+	---help---
+	  Compile in platform device definitions for TMU. This macro also
+	  enables compilation hwmon base TMU driver and also allows compilation
+	  of the platform device files. The platform data in this case is trip
+	  temperature and some tmu h/w configurations related parameter.
+
+
 config EXYNOS4_SETUP_I2C1
 	bool
 	help
@@ -288,6 +299,7 @@ config MACH_ORIGEN
 	select SAMSUNG_DEV_BACKLIGHT
 	select SAMSUNG_DEV_PWM
 	select EXYNOS4_DEV_PD
+	select EXYNOS4_DEV_TMU
 	select EXYNOS4_SETUP_FIMD0
 	select EXYNOS4_SETUP_SDHCI
 	select EXYNOS4_SETUP_USB_PHY
diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index 59069a3..d2493e8 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_EXYNOS4_DEV_AHCI)		+= dev-ahci.o
 obj-$(CONFIG_EXYNOS4_DEV_PD)		+= dev-pd.o
 obj-$(CONFIG_EXYNOS4_DEV_SYSMMU)	+= dev-sysmmu.o
 obj-$(CONFIG_EXYNOS4_DEV_DWMCI)		+= dev-dwmci.o
+obj-$(CONFIG_EXYNOS4_DEV_TMU)           += dev-tmu.o
 
 obj-$(CONFIG_EXYNOS4_SETUP_FIMC)	+= setup-fimc.o
 obj-$(CONFIG_EXYNOS4_SETUP_FIMD0)	+= setup-fimd0.o
diff --git a/arch/arm/mach-exynos/clock.c b/arch/arm/mach-exynos/clock.c
index 2894f0a..edecc5e 100644
--- a/arch/arm/mach-exynos/clock.c
+++ b/arch/arm/mach-exynos/clock.c
@@ -567,6 +567,10 @@ static struct clk init_clocks_off[] = {
 		.enable		= exynos4_clk_ip_peril_ctrl,
 		.ctrlbit	= (1 << 15),
 	}, {
+		.name		= "tmu_apbif",
+		.enable		= exynos4_clk_ip_perir_ctrl,
+		.ctrlbit	= (1 << 17),
+	}, {
 		.name		= "keypad",
 		.enable		= exynos4_clk_ip_perir_ctrl,
 		.ctrlbit	= (1 << 16),
diff --git a/arch/arm/mach-exynos/dev-tmu.c b/arch/arm/mach-exynos/dev-tmu.c
new file mode 100644
index 0000000..2e98912
--- /dev/null
+++ b/arch/arm/mach-exynos/dev-tmu.c
@@ -0,0 +1,64 @@
+/* linux/arch/arm/mach-exynos4/dev-tmu.c
+ *
+ * Copyright 2011 by SAMSUNG
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/platform_data/exynos4_tmu.h>
+#include <asm/irq.h>
+
+#include <mach/irqs.h>
+#include <mach/map.h>
+#include <plat/devs.h>
+
+static struct resource exynos4_tmu_resource[] = {
+	[0] = {
+		.start	= EXYNOS4_PA_TMU,
+		.end	= EXYNOS4_PA_TMU + 0xFFFF - 1,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		.start	= IRQ_TMU_TRIG0,
+		.end	= IRQ_TMU_TRIG0,
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+static struct exynos4_tmu_platform_data default_tmu_data = {
+	.threshold = 80,
+	.trigger_levels[0] = 2,
+	.trigger_levels[1] = 5,
+	.trigger_levels[2] = 20,
+	.trigger_levels[3] = 30,
+	.trigger_level0_en = 1,
+	.trigger_level1_en = 1,
+	.trigger_level2_en = 1,
+	.trigger_level3_en = 1,
+	.gain = 15,
+	.reference_voltage = 7,
+	.cal_type = TYPE_ONE_POINT_TRIMMING,
+	.freq_tab[0] = {
+		.freq_clip_pctg[0] = 30,
+		},
+	.freq_tab[1] = {
+		.freq_clip_pctg[0] = 99,
+		},
+	.freq_tab_count = 2,
+};
+
+struct platform_device exynos4_device_tmu = {
+	.name		= "exynos4-tmu",
+	.id		= -1,
+	.num_resources	= ARRAY_SIZE(exynos4_tmu_resource),
+	.resource	= exynos4_tmu_resource,
+	.dev	= {
+		.platform_data	= &default_tmu_data,
+	},
+};
diff --git a/arch/arm/mach-exynos/include/mach/irqs.h b/arch/arm/mach-exynos/include/mach/irqs.h
index dfd4b7e..d66a24b 100644
--- a/arch/arm/mach-exynos/include/mach/irqs.h
+++ b/arch/arm/mach-exynos/include/mach/irqs.h
@@ -125,6 +125,8 @@
 #define COMBINER_GROUP(x)	((x) * MAX_IRQ_IN_COMBINER + IRQ_SPI(128))
 #define COMBINER_IRQ(x, y)	(COMBINER_GROUP(x) + y)
 
+#define IRQ_TMU_TRIG0		COMBINER_IRQ(2, 4)
+#define IRQ_TMU_TRIG1		COMBINER_IRQ(3, 4)
 #define IRQ_SYSMMU_MDMA0_0	COMBINER_IRQ(4, 0)
 #define IRQ_SYSMMU_SSS_0	COMBINER_IRQ(4, 1)
 #define IRQ_SYSMMU_FIMC0_0	COMBINER_IRQ(4, 2)
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index 058541d..0d79b88 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -66,6 +66,7 @@
 #define EXYNOS4_PA_COREPERI		0x10500000
 #define EXYNOS4_PA_TWD			0x10500600
 #define EXYNOS4_PA_L2CC			0x10502000
+#define EXYNOS4_PA_TMU			0x100C0000
 
 #define EXYNOS4_PA_MDMA			0x10810000
 #define EXYNOS4_PA_PDMA0		0x12680000
diff --git a/arch/arm/mach-exynos/mach-origen.c b/arch/arm/mach-exynos/mach-origen.c
index f80b563..699140a 100644
--- a/arch/arm/mach-exynos/mach-origen.c
+++ b/arch/arm/mach-exynos/mach-origen.c
@@ -615,6 +615,7 @@ static struct platform_device *origen_devices[] __initdata = {
 	&exynos4_device_pd[PD_MFC],
 	&origen_device_gpiokeys,
 	&origen_lcd_hv070wsa,
+	&exynos4_device_tmu,
 };
 
 /* LCD Backlight data */
diff --git a/arch/arm/plat-samsung/include/plat/devs.h b/arch/arm/plat-samsung/include/plat/devs.h
index ab633c9..74cbf0d 100644
--- a/arch/arm/plat-samsung/include/plat/devs.h
+++ b/arch/arm/plat-samsung/include/plat/devs.h
@@ -135,6 +135,7 @@ extern struct platform_device exynos4_device_pcm2;
 extern struct platform_device exynos4_device_pd[];
 extern struct platform_device exynos4_device_spdif;
 extern struct platform_device exynos4_device_sysmmu;
+extern struct platform_device exynos4_device_tmu;
 
 extern struct platform_device samsung_asoc_dma;
 extern struct platform_device samsung_asoc_idma;
-- 
1.7.1

^ permalink raw reply related

* [RFC PATCH 2/3] thermal: exynos4: Register the tmu sensor with the thermal interface layer
From: Amit Daniel Kachhap @ 2011-12-21 11:59 UTC (permalink / raw)
  To: linux-pm, linux-samsung-soc
  Cc: linaro-dev, patches, linux-kernel, lm-sensors, linux-acpi
In-Reply-To: <1324468798-20423-1-git-send-email-amit.kachhap@linaro.org>

Export and register information from the hwmon tmu sensor to the samsung
exynos kernel thermal framework where different cooling devices and thermal
zone are binded. The exported information is based according to the data
structure thermal_sensor_conf present in exynos_thermal.h. HWMON sysfs
functions are currently left although all of them are present in generic
linux thermal layer.
Also the platform data structure is modified to pass frequency cooling
in percentages for each thermal level.

Signed-off-by: Amit Daniel Kachhap <amit.kachhap@linaro.org>
---
 drivers/hwmon/exynos4_tmu.c               |   34 ++++++++++++++++++++++++++--
 include/linux/platform_data/exynos4_tmu.h |    7 ++++++
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/exynos4_tmu.c b/drivers/hwmon/exynos4_tmu.c
index f2359a0..6912a7f 100644
--- a/drivers/hwmon/exynos4_tmu.c
+++ b/drivers/hwmon/exynos4_tmu.c
@@ -37,6 +37,9 @@
 #include <linux/hwmon-sysfs.h>
 
 #include <linux/platform_data/exynos4_tmu.h>
+#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
+#include <linux/exynos_thermal.h>
+#endif
 
 #define EXYNOS4_TMU_REG_TRIMINFO	0x0
 #define EXYNOS4_TMU_REG_CONTROL		0x20
@@ -248,10 +251,13 @@ static void exynos4_tmu_work(struct work_struct *work)
 
 	kobject_uevent(&data->hwmon_dev->kobj, KOBJ_CHANGE);
 
-	enable_irq(data->irq);
 
 	clk_disable(data->clk);
 	mutex_unlock(&data->lock);
+#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
+	exynos4_report_trigger();
+#endif
+	enable_irq(data->irq);
 }
 
 static irqreturn_t exynos4_tmu_irq(int irq, void *id)
@@ -345,6 +351,14 @@ static const struct attribute_group exynos4_tmu_attr_group = {
 	.attrs = exynos4_tmu_attributes,
 };
 
+#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
+static struct thermal_sensor_conf exynos4_sensor_conf = {
+	.name			= "exynos4-therm",
+	.read_temperature	= (int (*)(void *))exynos4_tmu_read,
+};
+#endif
+/*CONFIG_SAMSUNG_THERMAL_INTERFACE*/
+
 static int __devinit exynos4_tmu_probe(struct platform_device *pdev)
 {
 	struct exynos4_tmu_data *data;
@@ -432,9 +446,20 @@ static int __devinit exynos4_tmu_probe(struct platform_device *pdev)
 	}
 
 	exynos4_tmu_control(pdev, true);
-
+#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
+	(&exynos4_sensor_conf)->private_data = data;
+	(&exynos4_sensor_conf)->sensor_data = pdata;
+	ret = exynos4_register_thermal(&exynos4_sensor_conf);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register thermal interface\n");
+		goto err_hwmon_device;
+	}
+#endif
 	return 0;
-
+#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
+err_hwmon_device:
+	hwmon_device_unregister(data->hwmon_dev);
+#endif
 err_create_group:
 	sysfs_remove_group(&pdev->dev.kobj, &exynos4_tmu_attr_group);
 err_clk:
@@ -458,6 +483,9 @@ static int __devexit exynos4_tmu_remove(struct platform_device *pdev)
 
 	exynos4_tmu_control(pdev, false);
 
+#ifdef CONFIG_SAMSUNG_THERMAL_INTERFACE
+	exynos4_unregister_thermal();
+#endif
 	hwmon_device_unregister(data->hwmon_dev);
 	sysfs_remove_group(&pdev->dev.kobj, &exynos4_tmu_attr_group);
 
diff --git a/include/linux/platform_data/exynos4_tmu.h b/include/linux/platform_data/exynos4_tmu.h
index 39e038c..642c508 100644
--- a/include/linux/platform_data/exynos4_tmu.h
+++ b/include/linux/platform_data/exynos4_tmu.h
@@ -21,6 +21,7 @@
 
 #ifndef _LINUX_EXYNOS4_TMU_H
 #define _LINUX_EXYNOS4_TMU_H
+#include <linux/cpu_cooling.h>
 
 enum calibration_type {
 	TYPE_ONE_POINT_TRIMMING,
@@ -64,6 +65,9 @@ enum calibration_type {
  *	in the positive-TC generator block
  *	0 <= reference_voltage <= 31
  * @cal_type: calibration type for temperature
+ * @freq_pctg_table: Table representing frequency reduction percentage.
+ * @freq_tab_count: Count of the above table as frequency reduction may
+ *	applicable to only some of the trigger levels.
  *
  * This structure is required for configuration of exynos4_tmu driver.
  */
@@ -79,5 +83,8 @@ struct exynos4_tmu_platform_data {
 	u8 reference_voltage;
 
 	enum calibration_type cal_type;
+
+	struct freq_pctg_table freq_tab[4];
+	unsigned int freq_tab_count;
 };
 #endif /* _LINUX_EXYNOS4_TMU_H */
-- 
1.7.1

^ permalink raw reply related

* [RFC PATCH 1/3] thermal: exynos: Add thermal interface support for linux thermal layer
From: Amit Daniel Kachhap @ 2011-12-21 11:59 UTC (permalink / raw)
  To: linux-pm, linux-samsung-soc
  Cc: linaro-dev, patches, linux-kernel, lm-sensors, linux-acpi
In-Reply-To: <1324468798-20423-1-git-send-email-amit.kachhap@linaro.org>

This codes uses the generic linux thermal layer and creates a bridge
between temperature sensors, linux thermal framework and cooling devices
for samsung exynos platform. This layer recieves or monitor the
temperature from the sensor and informs the generic thermal layer to take
the necessary cooling action.

Signed-off-by: Amit Daniel Kachhap <amit.kachhap@linaro.org>
---
 drivers/thermal/Kconfig          |    8 ++
 drivers/thermal/Makefile         |    1 +
 drivers/thermal/exynos_thermal.c |  255 ++++++++++++++++++++++++++++++++++++++
 include/linux/exynos_thermal.h   |   59 +++++++++
 4 files changed, 323 insertions(+), 0 deletions(-)
 create mode 100644 drivers/thermal/exynos_thermal.c
 create mode 100644 include/linux/exynos_thermal.h

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 298c1cd..4e8df56 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -29,3 +29,11 @@ config CPU_THERMAL
 	  This will be useful for platforms using the generic thermal interface
 	  and not the ACPI interface.
 	  If you want this support, you should say Y or M here.
+
+config SAMSUNG_THERMAL_INTERFACE
+	bool "Samsung Thermal interface support"
+	depends on THERMAL && CPU_THERMAL
+	help
+	  This is a samsung thermal interface which will be used as
+	  a link between sensors and cooling devices with linux thermal
+	  framework.
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 655cbc4..c67b6b2 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_THERMAL)		+= thermal_sys.o
 obj-$(CONFIG_CPU_THERMAL)	+= cpu_cooling.o
+obj-$(CONFIG_SAMSUNG_THERMAL_INTERFACE)	+= exynos_thermal.o
diff --git a/drivers/thermal/exynos_thermal.c b/drivers/thermal/exynos_thermal.c
new file mode 100644
index 0000000..7c916db
--- /dev/null
+++ b/drivers/thermal/exynos_thermal.c
@@ -0,0 +1,255 @@
+/* linux/drivers/thermal/exynos_thermal.c
+ *
+ * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/thermal.h>
+#include <linux/platform_device.h>
+#include <linux/cpufreq.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/cpu_cooling.h>
+#include <linux/platform_data/exynos4_tmu.h>
+#include <linux/exynos_thermal.h>
+
+struct exynos4_thermal_zone {
+	unsigned int idle_interval;
+	unsigned int active_interval;
+	struct thermal_zone_device *therm_dev;
+	struct thermal_cooling_device *cool_dev;
+	struct platform_device *exynos4_dev;
+	struct thermal_sensor_conf *sensor_conf;
+	struct exynos4_tmu_platform_data *sensor_data;
+};
+
+static struct exynos4_thermal_zone *th_zone;
+
+static int exynos4_get_mode(struct thermal_zone_device *thermal,
+			    enum thermal_device_mode *mode)
+{
+	if (th_zone->sensor_conf) {
+		pr_info("Temperature sensor not initialised\n");
+		*mode = THERMAL_DEVICE_DISABLED;
+	} else
+		*mode = THERMAL_DEVICE_ENABLED;
+	return 0;
+}
+
+static int exynos4_set_mode(struct thermal_zone_device *thermal,
+			    enum thermal_device_mode mode)
+{
+	if (!th_zone->therm_dev) {
+		pr_notice("thermal zone not registered\n");
+		return 0;
+	}
+	if (mode == THERMAL_DEVICE_ENABLED)
+		th_zone->therm_dev->polling_delay =
+				th_zone->active_interval*1000;
+	else
+		th_zone->therm_dev->polling_delay =
+				th_zone->idle_interval*1000;
+
+	thermal_zone_device_update(th_zone->therm_dev);
+	pr_info("thermal polling set for duration=%d sec\n",
+				th_zone->therm_dev->polling_delay/1000);
+	return 0;
+}
+
+/*This may be called from interrupt based temperature sensor*/
+void exynos4_report_trigger(void)
+{
+	unsigned int th_temp = th_zone->sensor_data->threshold;
+	unsigned int monitor_temp = th_temp +
+			th_zone->sensor_data->trigger_levels[1];
+
+	thermal_zone_device_update(th_zone->therm_dev);
+
+	if (th_zone->therm_dev->last_temperature > monitor_temp)
+		th_zone->therm_dev->polling_delay =
+					th_zone->active_interval*1000;
+	else
+		th_zone->therm_dev->polling_delay =
+					th_zone->idle_interval*1000;
+}
+
+static int exynos4_get_trip_type(struct thermal_zone_device *thermal, int trip,
+				 enum thermal_trip_type *type)
+{
+	if (trip == 0 || trip == 1)
+		*type = THERMAL_TRIP_STATE_ACTIVE;
+	else if (trip == 2)
+		*type = THERMAL_TRIP_CRITICAL;
+	else
+		return -EINVAL;
+
+	return 0;
+}
+
+static int exynos4_get_trip_temp(struct thermal_zone_device *thermal, int trip,
+				 unsigned long *temp)
+{
+	unsigned int th_temp = th_zone->sensor_data->threshold;
+
+	/*Monitor zone*/
+	if (trip == 0)
+		*temp = th_temp + th_zone->sensor_data->trigger_levels[1];
+	/*Warn zone*/
+	else if (trip == 1)
+		*temp = th_temp + th_zone->sensor_data->trigger_levels[2];
+	/*Panic zone*/
+	else if (trip == 2)
+		*temp = th_temp + th_zone->sensor_data->trigger_levels[3];
+	else
+		return -EINVAL;
+	/*convert the temperature into millicelsius*/
+	*temp = *temp * 1000;
+	return 0;
+}
+
+static int exynos4_get_crit_temp(struct thermal_zone_device *thermal,
+				 unsigned long *temp)
+{
+	unsigned int th_temp = th_zone->sensor_data->threshold;
+	/*Panic zone*/
+	*temp = th_temp + th_zone->sensor_data->trigger_levels[3];
+	/*convert the temperature into millicelsius*/
+	*temp = *temp * 1000;
+	return 0;
+}
+
+static int exynos4_bind(struct thermal_zone_device *thermal,
+			struct thermal_cooling_device *cdev)
+{
+	/* if the cooling device is the one from exynos4 bind it */
+	if (cdev != th_zone->cool_dev)
+		return 0;
+
+	if (thermal_zone_bind_cooling_device(thermal, 0, cdev)) {
+		pr_err("error binding cooling dev\n");
+		return -EINVAL;
+	}
+	if (thermal_zone_bind_cooling_device(thermal, 1, cdev)) {
+		pr_err("error binding cooling dev\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int exynos4_unbind(struct thermal_zone_device *thermal,
+			  struct thermal_cooling_device *cdev)
+{
+	if (cdev != th_zone->cool_dev)
+		return 0;
+	if (thermal_zone_unbind_cooling_device(thermal, 0, cdev)) {
+		pr_err("error unbinding cooling dev\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int exynos4_get_temp(struct thermal_zone_device *thermal,
+			       unsigned long *temp)
+{
+	void *data;
+
+	if (!th_zone->sensor_conf) {
+		pr_info("Temperature sensor not initialised\n");
+		return -EINVAL;
+	}
+	data = th_zone->sensor_conf->private_data;
+	*temp = th_zone->sensor_conf->read_temperature(data);
+	/*convert the temperature into millicelsius*/
+	*temp = *temp * 1000;
+	return 0;
+}
+
+/* bind callback functions to thermalzone */
+static struct thermal_zone_device_ops exynos4_dev_ops = {
+	.bind = exynos4_bind,
+	.unbind = exynos4_unbind,
+	.get_temp = exynos4_get_temp,
+	.get_mode = exynos4_get_mode,
+	.set_mode = exynos4_set_mode,
+	.get_trip_type = exynos4_get_trip_type,
+	.get_trip_temp = exynos4_get_trip_temp,
+	.get_crit_temp = exynos4_get_crit_temp,
+};
+
+int exynos4_register_thermal(struct thermal_sensor_conf *sensor_conf)
+{
+	int ret;
+
+	if (!sensor_conf) {
+		pr_err("Temperature sensor not initialised\n");
+		return -EINVAL;
+	}
+
+	th_zone = kzalloc(sizeof(struct exynos4_thermal_zone), GFP_KERNEL);
+	if (!th_zone) {
+		ret = -ENOMEM;
+		goto err_unregister;
+	}
+
+	th_zone->sensor_conf = sensor_conf;
+
+	th_zone->sensor_data = sensor_conf->sensor_data;
+	if (!th_zone->sensor_data) {
+		pr_err("Temperature sensor data not initialised\n");
+		ret = -EINVAL;
+		goto err_unregister;
+	}
+
+	th_zone->cool_dev = cpufreq_cooling_register(
+		(struct freq_pctg_table *)th_zone->sensor_data->freq_tab,
+		th_zone->sensor_data->freq_tab_count, cpumask_of(0));
+
+	if (IS_ERR(th_zone->cool_dev)) {
+		pr_err("Failed to register cpufreq cooling device\n");
+		ret = -EINVAL;
+		goto err_unregister;
+	}
+
+	th_zone->therm_dev = thermal_zone_device_register(sensor_conf->name,
+				3, NULL, &exynos4_dev_ops, 0, 0, 0, 1000);
+	if (IS_ERR(th_zone->therm_dev)) {
+		pr_err("Failed to register thermal zone device\n");
+		ret = -EINVAL;
+		goto err_unregister;
+	}
+
+	th_zone->active_interval = 1;
+	th_zone->idle_interval = 10;
+
+	exynos4_set_mode(th_zone->therm_dev, THERMAL_DEVICE_DISABLED);
+
+	pr_info("Exynos: Kernel Thermal management registered\n");
+
+	return 0;
+
+err_unregister:
+	exynos4_unregister_thermal();
+	return ret;
+}
+EXPORT_SYMBOL(exynos4_register_thermal);
+
+void exynos4_unregister_thermal(void)
+{
+	if (th_zone && th_zone->cool_dev)
+		cpufreq_cooling_unregister();
+
+	if (th_zone && th_zone->therm_dev)
+		thermal_zone_device_unregister(th_zone->therm_dev);
+
+	kfree(th_zone);
+
+	pr_info("Exynos: Kernel Thermal management unregistered\n");
+}
+EXPORT_SYMBOL(exynos4_unregister_thermal);
diff --git a/include/linux/exynos_thermal.h b/include/linux/exynos_thermal.h
new file mode 100644
index 0000000..de7195b
--- /dev/null
+++ b/include/linux/exynos_thermal.h
@@ -0,0 +1,59 @@
+/* linux/include/linux/exynos_thermal.h
+ *
+ * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#ifndef THERMAL_INTERFACE_H
+#define THERMAL_INTERFACE_H
+/* CPU Zone information */
+
+#define SENSOR_NAME_LEN	16
+
+#define PANIC_ZONE      4
+#define WARN_ZONE       3
+#define MONITOR_ZONE    2
+#define SAFE_ZONE       1
+#define NO_ACTION       0
+
+/**
+ * struct exynos4_tmu_platform_data
+ * @name: name of the temperature sensor
+ * @read_temperature: A function pointer to read temperature info
+ * @private_data: Temperature sensor private data
+ * @sensor_data: Sensor specific information like trigger temperature, level
+ */
+struct thermal_sensor_conf {
+	char	name[SENSOR_NAME_LEN];
+	int	(*read_temperature)(void *data);
+	void	*private_data;
+	void	*sensor_data;
+};
+
+/**
+ * exynos4_register_thermal: Register to the exynos thermal interface.
+ * @sensor_conf:   Structure containing temperature sensor information
+ *
+ * returns zero on success, else negative errno.
+ */
+int exynos4_register_thermal(struct thermal_sensor_conf *sensor_conf);
+
+/**
+ * exynos4_unregister_thermal: Un-register from the exynos thermal interface.
+ *
+ * return not applicable.
+ */
+void exynos4_unregister_thermal(void);
+
+/**
+ * exynos4_report_trigger: Report any trigger level crossed in the
+ *	temperature sensor. This may be useful to take any cooling action.
+ *
+ * return not applicable.
+ */
+extern void exynos4_report_trigger(void);
+#endif
-- 
1.7.1

^ permalink raw reply related

* [RFC PATCH 0/3] thermal: exynos: Add kernel thermal support for exynos platform
From: Amit Daniel Kachhap @ 2011-12-21 11:59 UTC (permalink / raw)
  To: linux-pm, linux-samsung-soc
  Cc: linaro-dev, patches, linux-kernel, lm-sensors, linux-acpi

All the patchset based on Kernel version 3.2-rc6 and uses the cpufreq
cooling registration api's implemented in earlier patchset 
http://www.spinics.net/lists/linux-pm/msg26500.html

The code added in this patchset adds a thermal interface layer for samsung
exynos platforms. This layer is registered from the hwmon based temperature
sensor and recieves/monitor the temperature from the sensor and informs the
generic thermal layer to take the necessary cooling action. Currently this
layer can be used to create only one thermal zone and hence only one
temperature sensor can register.

Some modifications are done in the temperature sensor driver to export the
information needed for the thermal interface to register with the core linux
thermal framework and with the cpu frequency based cooling devices.

A simple data/control flow diagrams to illustrate this,

Core Linux thermal <------->  Exynos thermal  <-------- Temperature Sensor
	  |                             |
	 \|/                            |
  Cpufreq cooling device <-----

Amit Daniel Kachhap (3):
  thermal: exynos: Add thermal interface support for linux thermal
    layer
  thermal: exynos4: Register the tmu sensor with the thermal interface
    layer
  ARM: exynos4: Add thermal sensor driver platform device support

 arch/arm/mach-exynos/Kconfig              |   12 ++
 arch/arm/mach-exynos/Makefile             |    1 +
 arch/arm/mach-exynos/clock.c              |    4 +
 arch/arm/mach-exynos/dev-tmu.c            |   64 +++++++
 arch/arm/mach-exynos/include/mach/irqs.h  |    2 +
 arch/arm/mach-exynos/include/mach/map.h   |    1 +
 arch/arm/mach-exynos/mach-origen.c        |    1 +
 arch/arm/plat-samsung/include/plat/devs.h |    1 +
 drivers/hwmon/exynos4_tmu.c               |   34 ++++-
 drivers/thermal/Kconfig                   |    8 +
 drivers/thermal/Makefile                  |    1 +
 drivers/thermal/exynos_thermal.c          |  255 +++++++++++++++++++++++++++++
 include/linux/exynos_thermal.h            |   59 +++++++
 include/linux/platform_data/exynos4_tmu.h |    7 +
 14 files changed, 447 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/mach-exynos/dev-tmu.c
 create mode 100644 drivers/thermal/exynos_thermal.c
 create mode 100644 include/linux/exynos_thermal.h

^ permalink raw reply

* Re: [PATCH 0/3] coupled cpuidle state support
From: Colin Cross @ 2011-12-21  9:55 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Kevin Hilman, Len Brown, linux-kernel, Amit Kucheria, linux-tegra,
	linux-pm, linux-omap, linux-arm-kernel
In-Reply-To: <4EF1AA8A.8060304@linux.intel.com>

On Wed, Dec 21, 2011 at 1:44 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
> On 12/21/2011 10:40 AM, Colin Cross wrote:
>
>>> this smells fundamentally racey to me; you can get an interrupt one
>>> cycle after you think you're done, but before the last guy enters WFI...
>>>
>>> how do you solve that issue ?
>>
>> All the cpus have interrupts off when they increment the counter, so
>> they cannot receive an interrupt.  If an interrupt is pending on one
>> of those cpus, it will be handled later when WFI aborts due to the
>> pending interrupt.
>
> ... but this leads to cases where you're aborting before other cpus are
> entering..... so your "last guy in" doesn't really work, since while cpu
> 0 thinks it's the last guy, cpu 1 is already on the way out/out
> already...  (heck it might already be going back to sleep if your idle
> code can run fast, like in the size of a cache miss)

Once a cpu has incremented the counter, it has no way out unless either
1: another cpu (that hasn't incremented the counter yet) receives an
interrupt, aborts idle, and clears its idle flag
or
2: all cpus enter the ready counter, and call the cpuidle driver's
enter function.

In your example, cpu 1 has incremented the counter, so it cannot be on
the way out unless cpu 0 aborts (in which case it will not increment
the counter, and the counter will never be equal to the number of
cpus), or unless cpu 0 turns off its interrupts and incrementes the
counter (in which case neither cpu can return until after the cpuidle
driver's enter function has been called on all cpus).

^ permalink raw reply

* Re: [PATCH 0/3] coupled cpuidle state support
From: Arjan van de Ven @ 2011-12-21  9:44 UTC (permalink / raw)
  To: Colin Cross
  Cc: Kevin Hilman, Len Brown, linux-kernel, Amit Kucheria, linux-tegra,
	linux-pm, linux-omap, linux-arm-kernel
In-Reply-To: <CAMbhsRRP7s9xsHqxhb=iMfHAJFcqAea-6CDORZS34RCPNo+UWw@mail.gmail.com>

On 12/21/2011 10:40 AM, Colin Cross wrote:

>> this smells fundamentally racey to me; you can get an interrupt one
>> cycle after you think you're done, but before the last guy enters WFI...
>>
>> how do you solve that issue ?
> 
> All the cpus have interrupts off when they increment the counter, so
> they cannot receive an interrupt.  If an interrupt is pending on one
> of those cpus, it will be handled later when WFI aborts due to the
> pending interrupt.

... but this leads to cases where you're aborting before other cpus are
entering..... so your "last guy in" doesn't really work, since while cpu
0 thinks it's the last guy, cpu 1 is already on the way out/out
already...  (heck it might already be going back to sleep if your idle
code can run fast, like in the size of a cache miss)

^ permalink raw reply

* Re: [PATCH 0/3] coupled cpuidle state support
From: Colin Cross @ 2011-12-21  9:40 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Kevin Hilman, Len Brown, linux-kernel, Amit Kucheria, linux-tegra,
	linux-pm, linux-omap, linux-arm-kernel
In-Reply-To: <4EF1A0B4.5080307@linux.intel.com>

On Wed, Dec 21, 2011 at 1:02 AM, Arjan van de Ven <arjan@linux.intel.com> wrote:
> On 12/21/2011 1:09 AM, Colin Cross wrote:
>> On some ARM SMP SoCs (OMAP4460, Tegra 2, and probably more), the
>> cpus cannot be independently powered down, either due to
>> sequencing restrictions (on Tegra 2, cpu 0 must be the last to
>> power down), or due to HW bugs (on OMAP4460, a cpu powering up
>> will corrupt the gic state unless the other cpu runs a work
>> around).  Each cpu has a power state that it can enter without
>> coordinating with the other cpu (usually Wait For Interrupt, or
>> WFI), and one or more "coupled" power states that affect blocks
>> shared between the cpus (L2 cache, interrupt controller, and
>> sometimes the whole SoC).  Entering a coupled power state must
>> be tightly controlled on both cpus.
>>
>> The easiest solution to implementing coupled cpu power states is
>> to hotplug all but one cpu whenever possible, usually using a
>> cpufreq governor that looks at cpu load to determine when to
>> enable the secondary cpus.  This causes problems, as hotplug is an
>> expensive operation, so the number of hotplug transitions must be
>> minimized, leading to very slow response to loads, often on the
>> order of seconds.
>>
>> This patch series implements an alternative solution, where each
>> cpu will wait in the WFI state until all cpus are ready to enter
>> a coupled state, at which point the coupled state function will
>> be called on all cpus at approximately the same time.
>>
>> Once all cpus are ready to enter idle, they are woken by an smp
>> cross call.  At this point, there is a chance that one of the
>> cpus will find work to do, and choose not to enter suspend.  A
>> final pass is needed to guarantee that all cpus will call the
>> power state enter function at the same time.  During this pass,
>> each cpu will increment the ready counter, and continue once the
>> ready counter matches the number of online coupled cpus.  If any
>> cpu exits idle, the other cpus will decrement their counter and
>> retry.
>
> this smells fundamentally racey to me; you can get an interrupt one
> cycle after you think you're done, but before the last guy enters WFI...
>
> how do you solve that issue ?

All the cpus have interrupts off when they increment the counter, so
they cannot receive an interrupt.  If an interrupt is pending on one
of those cpus, it will be handled later when WFI aborts due to the
pending interrupt.

^ permalink raw reply

* Re: [PATCH 0/3] coupled cpuidle state support
From: Arjan van de Ven @ 2011-12-21  9:02 UTC (permalink / raw)
  To: Colin Cross
  Cc: Kevin Hilman, Len Brown, linux-kernel, Amit Kucheria, linux-tegra,
	linux-pm, linux-omap, linux-arm-kernel
In-Reply-To: <1324426147-16735-1-git-send-email-ccross@android.com>

On 12/21/2011 1:09 AM, Colin Cross wrote:
> On some ARM SMP SoCs (OMAP4460, Tegra 2, and probably more), the
> cpus cannot be independently powered down, either due to
> sequencing restrictions (on Tegra 2, cpu 0 must be the last to
> power down), or due to HW bugs (on OMAP4460, a cpu powering up
> will corrupt the gic state unless the other cpu runs a work
> around).  Each cpu has a power state that it can enter without
> coordinating with the other cpu (usually Wait For Interrupt, or
> WFI), and one or more "coupled" power states that affect blocks
> shared between the cpus (L2 cache, interrupt controller, and
> sometimes the whole SoC).  Entering a coupled power state must
> be tightly controlled on both cpus.
> 
> The easiest solution to implementing coupled cpu power states is
> to hotplug all but one cpu whenever possible, usually using a
> cpufreq governor that looks at cpu load to determine when to
> enable the secondary cpus.  This causes problems, as hotplug is an
> expensive operation, so the number of hotplug transitions must be
> minimized, leading to very slow response to loads, often on the
> order of seconds.
> 
> This patch series implements an alternative solution, where each
> cpu will wait in the WFI state until all cpus are ready to enter
> a coupled state, at which point the coupled state function will
> be called on all cpus at approximately the same time.
> 
> Once all cpus are ready to enter idle, they are woken by an smp
> cross call.  At this point, there is a chance that one of the
> cpus will find work to do, and choose not to enter suspend.  A
> final pass is needed to guarantee that all cpus will call the
> power state enter function at the same time.  During this pass,
> each cpu will increment the ready counter, and continue once the
> ready counter matches the number of online coupled cpus.  If any
> cpu exits idle, the other cpus will decrement their counter and
> retry.

this smells fundamentally racey to me; you can get an interrupt one
cycle after you think you're done, but before the last guy enters WFI...

how do you solve that issue ?

^ permalink raw reply

* Re: [RFC PATCH 1/2] thermal: Add a new trip type to use cooling device instance number
From: Amit Kachhap @ 2011-12-21  5:11 UTC (permalink / raw)
  To: Vincent Guittot; +Cc: linux-acpi, linux-pm, linaro-dev, linux-kernel, patches
In-Reply-To: <CAK44p232PbvGRXKB7_3TYn0Q3iqgRJg6Qv81W_EwXCHL+79baA@mail.gmail.com>

 Hi Vincent,

Thanks for the review.
Well actually your are correct that current temperature and last
temperature can be used to increase or decrease the cpu frequency. But
this has to be done again in cooling devices so to make the cooling
devices generic and to avoid the temperature comparison again this new
trip type passes the cooling device instance id.
Also about your queries that this may add dependency between trip
index and cooling state. This is actually needed and this dependency
is created when the cooling device is binded with trip points(For
cpufreq type cooling device just the instance of cooling device is
associated with trip points). More over the existing PASSIVE cooling
trip type does the same thing and iterates across all the cooling
state.

 Thanks,
 Amit Daniel
>
> On 20 December 2011 18:07, Vincent Guittot <vincent.guittot@linaro.org> wrote:
>>
>> Hi Amit,
>>
>> I'm not sure that using the trip index for setting the state of a
>> cooling device is a generic solution because you are adding a
>> dependency between the trip index and the cooling device state that
>> might be difficult to handle. This dependency implies that a cooling
>> device like cpufreq_cooling_device must be registered in the 1st trips
>> of a thermal_zone which is not possible when we want to register 2
>> cpufreq_cooling_devices in the same thermal_zone.
>> You should only rely on the current and last temperatures to detect if
>> a trip_temp has been crossed and you should increase or decrease the
>> current state of the cooling device accordingly.
>>
>> something like below should work with cpufreq_cooling_device and will
>> not add any constraint on the trip index. The state of a cooling
>> device is increased/decreased once for each trip
>>
>> +               case THERMAL_TRIP_STATE_ACTIVE:
>> +                       list_for_each_entry(instance, &tz->cooling_devices,
>> +                                           node) {
>> +                               if (instance->trip != count)
>> +                                       continue;
>> +
>> +                               cdev = instance->cdev;
>> +
>> +                               if ((temp >= trip_temp)
>> +                                       && (trip_temp > tz->last_temperature)) {
>> +                                       cdev->ops->get_max_state(cdev,
>> +                                                       &max_state);
>> +                                       cdev->ops->get_cur_state(cdev,
>> +                                                       &current_state);
>> +                                       if (++current_state <= max_state)
>> +                                               cdev->ops->set_cur_state(cdev,
>> +                                                               current_state);
>> +                               }
>> +                               else if ((temp < trip_temp)
>> +                                       && (trip_temp <= tz->last_temperature)) {
>> +                                       cdev->ops->get_cur_state(cdev,
>> +                                                       &current_state);
>> +                                       if (current_state > 0)
>> +                                               cdev->ops->set_cur_state(cdev,
>> +                                                               --current_state);
>> +                       }
>> +                       break;
>>
>> Regards,
>> Vincent
>>
>> On 13 December 2011 16:13, Amit Daniel Kachhap <amit.kachhap@linaro.org> wrote:
>> > This patch adds a new trip type THERMAL_TRIP_STATE_ACTIVE. This
>> > trip behaves same as THERMAL_TRIP_ACTIVE but also passes the cooling
>> > device instance number. This helps the cooling device registered as
>> > different instances to perform appropriate cooling action decision in
>> > the set_cur_state call back function.
>> >
>> > Also since the trip temperature's are in ascending order so some logic
>> > is put in place to skip the un-necessary checks.
>> >
>> > Signed-off-by: Amit Daniel Kachhap <amit.kachhap@linaro.org>
>> > ---
>> >  Documentation/thermal/sysfs-api.txt |    4 ++--
>> >  drivers/thermal/thermal_sys.c       |   27 ++++++++++++++++++++++++++-
>> >  include/linux/thermal.h             |    1 +
>> >  3 files changed, 29 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
>> > index b61e46f..5c1d44e 100644
>> > --- a/Documentation/thermal/sysfs-api.txt
>> > +++ b/Documentation/thermal/sysfs-api.txt
>> > @@ -184,8 +184,8 @@ trip_point_[0-*]_temp
>> >
>> >  trip_point_[0-*]_type
>> >        Strings which indicate the type of the trip point.
>> > -       E.g. it can be one of critical, hot, passive, active[0-*] for ACPI
>> > -       thermal zone.
>> > +       E.g. it can be one of critical, hot, passive, active[0-1],
>> > +       state-active[0-*] for ACPI thermal zone.
>> >        RO, Optional
>> >
>> >  cdev[0-*]
>> > diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
>> > index dd9a574..72b1ab3 100644
>> > --- a/drivers/thermal/thermal_sys.c
>> > +++ b/drivers/thermal/thermal_sys.c
>> > @@ -192,6 +192,8 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr,
>> >                return sprintf(buf, "passive\n");
>> >        case THERMAL_TRIP_ACTIVE:
>> >                return sprintf(buf, "active\n");
>> > +       case THERMAL_TRIP_STATE_ACTIVE:
>> > +               return sprintf(buf, "state-active\n");
>> >        default:
>> >                return sprintf(buf, "unknown\n");
>> >        }
>> > @@ -1035,7 +1037,7 @@ EXPORT_SYMBOL(thermal_cooling_device_unregister);
>> >  void thermal_zone_device_update(struct thermal_zone_device *tz)
>> >  {
>> >        int count, ret = 0;
>> > -       long temp, trip_temp;
>> > +       long temp, trip_temp, max_state, last_trip_change = 0;
>> >        enum thermal_trip_type trip_type;
>> >        struct thermal_cooling_device_instance *instance;
>> >        struct thermal_cooling_device *cdev;
>> > @@ -1086,6 +1088,29 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
>> >                                        cdev->ops->set_cur_state(cdev, 0);
>> >                        }
>> >                        break;
>> > +               case THERMAL_TRIP_STATE_ACTIVE:
>> > +                       list_for_each_entry(instance, &tz->cooling_devices,
>> > +                                           node) {
>> > +                               if (instance->trip != count)
>> > +                                       continue;
>> > +
>> > +                               if (temp <= last_trip_change)
>> > +                                       continue;
>> > +
>> > +                               cdev = instance->cdev;
>> > +                               cdev->ops->get_max_state(cdev, &max_state);
>> > +
>> > +                               if ((temp >= trip_temp) &&
>> > +                                               ((count + 1) <= max_state))
>> > +                                       cdev->ops->set_cur_state(cdev,
>> > +                                                               count + 1);
>> > +                               else if ((temp < trip_temp) &&
>> > +                                                       (count <= max_state))
>> > +                                       cdev->ops->set_cur_state(cdev, count);
>> > +
>> > +                               last_trip_change = trip_temp;
>> > +                       }
>> > +                       break;
>> >                case THERMAL_TRIP_PASSIVE:
>> >                        if (temp >= trip_temp || tz->passive)
>> >                                thermal_zone_device_passive(tz, temp,
>> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>> > index 47b4a27..d7d0a27 100644
>> > --- a/include/linux/thermal.h
>> > +++ b/include/linux/thermal.h
>> > @@ -42,6 +42,7 @@ enum thermal_trip_type {
>> >        THERMAL_TRIP_PASSIVE,
>> >        THERMAL_TRIP_HOT,
>> >        THERMAL_TRIP_CRITICAL,
>> > +       THERMAL_TRIP_STATE_ACTIVE,
>> >  };
>> >
>> >  struct thermal_zone_device_ops {
>> > --
>> > 1.7.1
>> >
>> > _______________________________________________
>> > linux-pm mailing list
>> > linux-pm@lists.linux-foundation.org
>> > https://lists.linuxfoundation.org/mailman/listinfo/linux-pm
>
>

^ permalink raw reply

* Re: [RFC PATCH 1/2] thermal: Add a new trip type to use cooling device instance number
From: Amit Kachhap @ 2011-12-21  5:08 UTC (permalink / raw)
  To: Vincent Guittot; +Cc: linux-acpi, linux-pm, linaro-dev, linux-kernel, patches
In-Reply-To: <CAKfTPtCVtWfdUpW=Up6eWTsceBCfER5NcvjF1KRTNcqys2YArQ@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 8051 bytes --]

Hi Vincent,

Thanks for the review.
Well actually your are correct that current temperature and last
temperature can be used to increase or decrease the cufreq. But this has to
be done again in cooling devices so to make the cooling devices generic and
to avoid the temperature comparision again this new trip type passes the
cooling device instance id.
Also about your queries that this may add dependency between trip index and
cooling state. This is actually needed and this dependency is created when
the cooling device is binded with trip points(For cpufreq type cooling
device just the instance of cooling device is associated with trip points).
More over the existing PASSIVE cooling trip type does the same thing and
iterates across all the cooling state.

Thanks,
Amit Daniel

On 20 December 2011 18:07, Vincent Guittot <vincent.guittot@linaro.org>wrote:

> Hi Amit,
>
> I'm not sure that using the trip index for setting the state of a
> cooling device is a generic solution because you are adding a
> dependency between the trip index and the cooling device state that
> might be difficult to handle. This dependency implies that a cooling
> device like cpufreq_cooling_device must be registered in the 1st trips
> of a thermal_zone which is not possible when we want to register 2
> cpufreq_cooling_devices in the same thermal_zone.
> You should only rely on the current and last temperatures to detect if
> a trip_temp has been crossed and you should increase or decrease the
> current state of the cooling device accordingly.
>
> something like below should work with cpufreq_cooling_device and will
> not add any constraint on the trip index. The state of a cooling
> device is increased/decreased once for each trip
>
> +               case THERMAL_TRIP_STATE_ACTIVE:
> +                       list_for_each_entry(instance, &tz->cooling_devices,
> +                                           node) {
> +                               if (instance->trip != count)
> +                                       continue;
> +
> +                               cdev = instance->cdev;
> +
> +                               if ((temp >= trip_temp)
> +                                       && (trip_temp >
> tz->last_temperature)) {
> +                                       cdev->ops->get_max_state(cdev,
> +                                                       &max_state);
> +                                       cdev->ops->get_cur_state(cdev,
> +                                                       &current_state);
> +                                       if (++current_state <= max_state)
> +
> cdev->ops->set_cur_state(cdev,
> +
> current_state);
> +                               }
> +                               else if ((temp < trip_temp)
> +                                       && (trip_temp <=
> tz->last_temperature)) {
> +                                       cdev->ops->get_cur_state(cdev,
> +                                                       &current_state);
> +                                       if (current_state > 0)
> +
> cdev->ops->set_cur_state(cdev,
> +
> --current_state);
> +                       }
> +                       break;
>
> Regards,
> Vincent
>
> On 13 December 2011 16:13, Amit Daniel Kachhap <amit.kachhap@linaro.org>
> wrote:
> > This patch adds a new trip type THERMAL_TRIP_STATE_ACTIVE. This
> > trip behaves same as THERMAL_TRIP_ACTIVE but also passes the cooling
> > device instance number. This helps the cooling device registered as
> > different instances to perform appropriate cooling action decision in
> > the set_cur_state call back function.
> >
> > Also since the trip temperature's are in ascending order so some logic
> > is put in place to skip the un-necessary checks.
> >
> > Signed-off-by: Amit Daniel Kachhap <amit.kachhap@linaro.org>
> > ---
> >  Documentation/thermal/sysfs-api.txt |    4 ++--
> >  drivers/thermal/thermal_sys.c       |   27 ++++++++++++++++++++++++++-
> >  include/linux/thermal.h             |    1 +
> >  3 files changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/thermal/sysfs-api.txt
> b/Documentation/thermal/sysfs-api.txt
> > index b61e46f..5c1d44e 100644
> > --- a/Documentation/thermal/sysfs-api.txt
> > +++ b/Documentation/thermal/sysfs-api.txt
> > @@ -184,8 +184,8 @@ trip_point_[0-*]_temp
> >
> >  trip_point_[0-*]_type
> >        Strings which indicate the type of the trip point.
> > -       E.g. it can be one of critical, hot, passive, active[0-*] for
> ACPI
> > -       thermal zone.
> > +       E.g. it can be one of critical, hot, passive, active[0-1],
> > +       state-active[0-*] for ACPI thermal zone.
> >        RO, Optional
> >
> >  cdev[0-*]
> > diff --git a/drivers/thermal/thermal_sys.c
> b/drivers/thermal/thermal_sys.c
> > index dd9a574..72b1ab3 100644
> > --- a/drivers/thermal/thermal_sys.c
> > +++ b/drivers/thermal/thermal_sys.c
> > @@ -192,6 +192,8 @@ trip_point_type_show(struct device *dev, struct
> device_attribute *attr,
> >                return sprintf(buf, "passive\n");
> >        case THERMAL_TRIP_ACTIVE:
> >                return sprintf(buf, "active\n");
> > +       case THERMAL_TRIP_STATE_ACTIVE:
> > +               return sprintf(buf, "state-active\n");
> >        default:
> >                return sprintf(buf, "unknown\n");
> >        }
> > @@ -1035,7 +1037,7 @@ EXPORT_SYMBOL(thermal_cooling_device_unregister);
> >  void thermal_zone_device_update(struct thermal_zone_device *tz)
> >  {
> >        int count, ret = 0;
> > -       long temp, trip_temp;
> > +       long temp, trip_temp, max_state, last_trip_change = 0;
> >        enum thermal_trip_type trip_type;
> >        struct thermal_cooling_device_instance *instance;
> >        struct thermal_cooling_device *cdev;
> > @@ -1086,6 +1088,29 @@ void thermal_zone_device_update(struct
> thermal_zone_device *tz)
> >                                        cdev->ops->set_cur_state(cdev, 0);
> >                        }
> >                        break;
> > +               case THERMAL_TRIP_STATE_ACTIVE:
> > +                       list_for_each_entry(instance,
> &tz->cooling_devices,
> > +                                           node) {
> > +                               if (instance->trip != count)
> > +                                       continue;
> > +
> > +                               if (temp <= last_trip_change)
> > +                                       continue;
> > +
> > +                               cdev = instance->cdev;
> > +                               cdev->ops->get_max_state(cdev,
> &max_state);
> > +
> > +                               if ((temp >= trip_temp) &&
> > +                                               ((count + 1) <=
> max_state))
> > +                                       cdev->ops->set_cur_state(cdev,
> > +                                                               count +
> 1);
> > +                               else if ((temp < trip_temp) &&
> > +                                                       (count <=
> max_state))
> > +                                       cdev->ops->set_cur_state(cdev,
> count);
> > +
> > +                               last_trip_change = trip_temp;
> > +                       }
> > +                       break;
> >                case THERMAL_TRIP_PASSIVE:
> >                        if (temp >= trip_temp || tz->passive)
> >                                thermal_zone_device_passive(tz, temp,
> > diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> > index 47b4a27..d7d0a27 100644
> > --- a/include/linux/thermal.h
> > +++ b/include/linux/thermal.h
> > @@ -42,6 +42,7 @@ enum thermal_trip_type {
> >        THERMAL_TRIP_PASSIVE,
> >        THERMAL_TRIP_HOT,
> >        THERMAL_TRIP_CRITICAL,
> > +       THERMAL_TRIP_STATE_ACTIVE,
> >  };
> >
> >  struct thermal_zone_device_ops {
> > --
> > 1.7.1
> >
> > _______________________________________________
> > linux-pm mailing list
> > linux-pm@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/linux-pm
>

[-- Attachment #1.2: Type: text/html, Size: 9868 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply

* [PATCH 3/3] cpuidle: add support for states that affect multiple cpus
From: Colin Cross @ 2011-12-21  0:09 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-pm
  Cc: Kevin Hilman, Len Brown, Amit Kucheria, Colin Cross, linux-tegra,
	linux-omap, Arjan van de Ven
In-Reply-To: <1324426147-16735-1-git-send-email-ccross@android.com>

On some ARM SMP SoCs (OMAP4460, Tegra 2, and probably more), the
cpus cannot be independently powered down, either due to
sequencing restrictions (on Tegra 2, cpu 0 must be the last to
power down), or due to HW bugs (on OMAP4460, a cpu powering up
will corrupt the gic state unless the other cpu runs a work
around).  Each cpu has a power state that it can enter without
coordinating with the other cpu (usually Wait For Interrupt, or
WFI), and one or more "coupled" power states that affect blocks
shared between the cpus (L2 cache, interrupt controller, and
sometimes the whole SoC).  Entering a coupled power state must
be tightly controlled on both cpus.

The easiest solution to implementing coupled cpu power states is
to hotplug all but one cpu whenever possible, usually using a
cpufreq governor that looks at cpu load to determine when to
enable the secondary cpus.  This causes problems, as hotplug is an
expensive operation, so the number of hotplug transitions must be
minimized, leading to very slow response to loads, often on the
order of seconds.

This file implements an alternative solution, where each cpu will
wait in the WFI state until all cpus are ready to enter a coupled
state, at which point the coupled state function will be called
on all cpus at approximately the same time.

Once all cpus are ready to enter idle, they are woken by an smp
cross call.  At this point, there is a chance that one of the
cpus will find work to do, and choose not to enter suspend.  A
final pass is needed to guarantee that all cpus will call the
power state enter function at the same time.  During this pass,
each cpu will increment the ready counter, and continue once the
ready counter matches the number of online coupled cpus.  If any
cpu exits idle, the other cpus will decrement their counter and
retry.

To use coupled cpuidle states, a cpuidle driver must:

   Set struct cpuidle_device.coupled_cpus to the mask of all
   coupled cpus, usually the same as cpu_possible_mask if all cpus
   are part of the same cluster.  The coupled_cpus mask must be
   set in the struct cpuidle_device for each cpu.

   Set struct cpuidle_device.safe_state to a state that is not a
   coupled state.  This is usually WFI.

   Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each
   state that affects multiple cpus.

   Provide a struct cpuidle_state.enter function for each state
   that affects multiple cpus.  This function is guaranteed to be
   called on all cpus at approximately the same time.  The driver
   should ensure that the cpus all abort together if any cpu tries
   to abort once the function is called.

Signed-off-by: Colin Cross <ccross@android.com>
Cc: Len Brown <len.brown@intel.com>
Cc: Kevin Hilman <khilman@ti.com>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: Amit Kucheria <amit.kucheria@linaro.org>
Cc: Arjan van de Ven <arjan@linux.intel.com>
Cc: Trinabh Gupta <g.trinabh@gmail.com>
Cc: Deepthi Dharwar <deepthi@linux.vnet.ibm.com>
---
 drivers/cpuidle/Kconfig   |    3 +
 drivers/cpuidle/Makefile  |    1 +
 drivers/cpuidle/coupled.c |  413 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/cpuidle/cpuidle.c |   14 ++-
 drivers/cpuidle/cpuidle.h |   39 +++++
 include/linux/cpuidle.h   |    7 +
 6 files changed, 476 insertions(+), 1 deletions(-)
 create mode 100644 drivers/cpuidle/coupled.c

diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
index 7dbc4a8..7a72e55 100644
--- a/drivers/cpuidle/Kconfig
+++ b/drivers/cpuidle/Kconfig
@@ -18,3 +18,6 @@ config CPU_IDLE_GOV_MENU
 	bool
 	depends on CPU_IDLE && NO_HZ
 	default y
+
+config ARCH_NEEDS_CPU_IDLE_COUPLED
+	def_bool n
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index 5634f88..38c8f69 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-y += cpuidle.o driver.o governor.o sysfs.o governors/
+obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
new file mode 100644
index 0000000..3fb7d24
--- /dev/null
+++ b/drivers/cpuidle/coupled.c
@@ -0,0 +1,413 @@
+/*
+ * coupled.c - helper functions to enter the same idle state on multiple cpus
+ *
+ * Copyright (c) 2011 Google, Inc.
+ *
+ * Author: Colin Cross <ccross@android.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/cpu.h>
+#include <linux/cpuidle.h>
+#include <linux/mutex.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include "cpuidle.h"
+
+/*
+ * coupled cpuidle states
+ *
+ * On some ARM SMP SoCs (OMAP4460, Tegra 2, and probably more), the
+ * cpus cannot be independently powered down, either due to
+ * sequencing restrictions (on Tegra 2, cpu 0 must be the last to
+ * power down), or due to HW bugs (on OMAP4460, a cpu powering up
+ * will corrupt the gic state unless the other cpu runs a work
+ * around).  Each cpu has a power state that it can enter without
+ * coordinating with the other cpu (usually Wait For Interrupt, or
+ * WFI), and one or more "coupled" power states that affect blocks
+ * shared between the cpus (L2 cache, interrupt controller, and
+ * sometimes the whole SoC).  Entering a coupled power state must
+ * be tightly controlled on both cpus.
+ *
+ * The easiest solution to implementing coupled cpu power states is
+ * to hotplug all but one cpu whenever possible, usually using a
+ * cpufreq governor that looks at cpu load to determine when to
+ * enable the secondary cpus.  This causes problems, as hotplug is an
+ * expensive operation, so the number of hotplug transitions must be
+ * minimized, leading to very slow response to loads, often on the
+ * order of seconds.
+ *
+ * This file implements an alternative solution, where each cpu will
+ * wait in the WFI state until all cpus are ready to enter a coupled
+ * state, at which point the coupled state function will be called
+ * on all cpus at approximately the same time.
+ *
+ * Once all cpus are ready to enter idle, they are woken by an smp
+ * cross call.  At this point, there is a chance that one of the
+ * cpus will find work to do, and choose not to enter suspend.  A
+ * final pass is needed to guarantee that all cpus will call the
+ * power state enter function at the same time.  During this pass,
+ * each cpu will increment the ready counter, and continue once the
+ * ready counter matches the number of online coupled cpus.  If any
+ * cpu exits idle, the other cpus will decrement their counter and
+ * retry.
+ *
+ * To use coupled cpuidle states, a cpuidle driver must:
+ *
+ *    Set struct cpuidle_device.coupled_cpus to the mask of all
+ *    coupled cpus, usually the same as cpu_possible_mask if all cpus
+ *    are part of the same cluster.  The coupled_cpus mask must be
+ *    set in the struct cpuidle_device for each cpu.
+ *
+ *    Set struct cpuidle_device.safe_state to a state that is not a
+ *    coupled state.  This is usually WFI.
+ *
+ *    Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each
+ *    state that affects multiple cpus.
+ *
+ *    Provide a struct cpuidle_state.enter function for each state
+ *    that affects multiple cpus.  This function is guaranteed to be
+ *    called on all cpus at approximately the same time.  The driver
+ *    should ensure that the cpus all abort together if any cpu tries
+ *    to abort once the function is called.
+ *
+ */
+
+static DEFINE_MUTEX(cpuidle_coupled_lock);
+static DEFINE_PER_CPU(struct call_single_data, cpuidle_coupled_poke_cb);
+static cpumask_t cpuidle_coupled_poked_mask;
+
+/**
+ * cpuidle_state_is_coupled
+ *
+ * Returns true if the target state is coupled with cpus besides this one
+ */
+bool cpuidle_state_is_coupled(struct cpuidle_device *dev,
+	struct cpuidle_driver *drv, int state)
+{
+	return drv->states[state].flags & CPUIDLE_FLAG_COUPLED;
+}
+
+/**
+ * cpuidle_all_coupled_cpus_idle
+ *
+ * Returns true if all cpus coupled to this target state are idle
+ */
+static inline bool
+cpuidle_coupled_cpus_idle(struct cpuidle_coupled *coupled)
+{
+	int i;
+
+	assert_spin_locked(&coupled->lock);
+
+	smp_rmb();
+
+	for_each_cpu_mask(i, coupled->alive_coupled_cpus)
+		if (coupled->requested_state[i] < 0)
+			return false;
+
+	return true;
+}
+
+/**
+ * cpuidle_get_idle_state
+ *
+ * Returns the deepest idle state that all coupled cpus can enter
+ */
+static inline int cpuidle_coupled_get_state(struct cpuidle_device *dev,
+		struct cpuidle_coupled *coupled)
+{
+	int i;
+	int state = INT_MAX;
+
+	assert_spin_locked(&coupled->lock);
+
+	for_each_cpu_mask(i, coupled->alive_coupled_cpus)
+		if (coupled->requested_state[i] < state)
+			state = coupled->requested_state[i];
+
+	BUG_ON(state >= dev->state_count || state < 0);
+
+	return state;
+}
+
+static void cpuidle_coupled_poked(void *info)
+{
+	int cpu = (unsigned long)info;
+	cpumask_clear_cpu(cpu, &cpuidle_coupled_poked_mask);
+}
+
+static void cpuidle_coupled_poke(int cpu)
+{
+	struct call_single_data *csd = &per_cpu(cpuidle_coupled_poke_cb, cpu);
+	if (cpu_online(cpu))
+		if (!cpumask_test_and_set_cpu(cpu, &cpuidle_coupled_poked_mask))
+			__smp_call_function_single(cpu, csd, 0);
+}
+
+/**
+ * cpuidle_coupled_update_state
+ *
+ * Updates the requested idle state for the specified cpuidle device,
+ * poking all coupled cpus out of idle to let them see the new state.
+ */
+static void cpuidle_coupled_update_state(struct cpuidle_device *dev,
+		struct cpuidle_coupled *coupled, int next_state)
+{
+	int cpu;
+
+	assert_spin_locked(&coupled->lock);
+
+	coupled->requested_state[dev->cpu] = next_state;
+	smp_wmb();
+
+	if (next_state >= 0)
+		for_each_cpu_mask(cpu, coupled->alive_coupled_cpus)
+			if (cpu != dev->cpu)
+				cpuidle_coupled_poke(cpu);
+}
+
+/**
+ * cpuidle_enter_state_coupled
+ *
+ * Coordinate with coupled cpus to enter the target state.  This is a two
+ * stage process.  In the first stage, the cpus are operating independently,
+ * and may call into cpuidle_enter_state_coupled at completely different times.
+ * To save as much power as possible, the first cpus to call this function will
+ * go to an intermediate state (the cpuidle_device's safe state), and wait for
+ * all the other cpus to call this function.  Once all coupled cpus are idle,
+ * the second stage will start.  Each coupled cpu will spin until all cpus have
+ * guaranteed that they will call the target_state.
+ */
+int cpuidle_enter_state_coupled(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int next_state)
+{
+	int entered_state = -1;
+	struct cpuidle_coupled *coupled = dev->coupled;
+
+	spin_lock(&coupled->lock);
+	if (!cpumask_test_cpu(dev->cpu, &coupled->alive_coupled_cpus)) {
+		/*
+		 * somebody took us out of the online coupled cpus mask, we
+		 * must be on the way up or down
+		 */
+		spin_unlock(&coupled->lock);
+		return -1;
+	}
+
+	BUG_ON(coupled->ready_count);
+	cpuidle_coupled_update_state(dev, coupled, next_state);
+
+retry:
+	/*
+	 * Wait for all coupled cpus to be idle, using the deepest state
+	 * allowed for a single cpu.
+	 */
+	while (!need_resched() && !cpuidle_coupled_cpus_idle(coupled)) {
+		spin_unlock(&coupled->lock);
+
+		entered_state = cpuidle_enter_state(dev, drv,
+			dev->safe_state_index);
+
+		local_irq_enable();
+		cpu_relax();
+		local_irq_disable();
+
+		spin_lock(&coupled->lock);
+	}
+
+	/* give a chance to process any remaining pokes */
+	local_irq_enable();
+	cpu_relax();
+	local_irq_disable();
+
+	if (need_resched()) {
+		cpuidle_coupled_update_state(dev, coupled, -1);
+		goto out;
+	}
+
+	/*
+	 * All coupled cpus are probably idle.  There is a small chance that
+	 * one of the other cpus just became active.  Increment a counter when
+	 * ready, and spin until all coupled cpus have incremented the counter.
+	 * Once a cpu has incremented the counter, it cannot abort idle and must
+	 * spin until either the count has hit num_online_cpus(), or another
+	 * cpu leaves idle.
+	 */
+
+	coupled->ready_count++;
+
+	while (coupled->ready_count !=
+			cpumask_weight(&coupled->alive_coupled_cpus)) {
+		if (!cpuidle_coupled_cpus_idle(coupled)) {
+			coupled->ready_count--;
+			goto retry;
+		}
+
+		spin_unlock(&coupled->lock);
+		cpu_relax();
+		spin_lock(&coupled->lock);
+	}
+
+	/* all cpus have acked the coupled state */
+	next_state = cpuidle_coupled_get_state(dev, coupled);
+	spin_unlock(&coupled->lock);
+
+	entered_state = cpuidle_enter_state(dev, drv, next_state);
+
+	spin_lock(&coupled->lock);
+
+	cpuidle_coupled_update_state(dev, coupled, -1);
+	coupled->ready_count--;
+
+out:
+	local_irq_enable();
+
+	while (coupled->ready_count > 0) {
+		spin_unlock(&coupled->lock);
+		cpu_relax();
+		spin_lock(&coupled->lock);
+	}
+
+	spin_unlock(&coupled->lock);
+
+	return entered_state;
+}
+
+/**
+ * cpuidle_coupled_register_device
+ *
+ * Called from cpuidle_register_device to handle coupled idle init.  Finds the
+ * cpuidle_coupled struct for this set of coupled cpus, or creates one if none
+ * exists yet.
+ */
+int cpuidle_coupled_register_device(struct cpuidle_device *dev)
+{
+	int cpu;
+	struct cpuidle_device *other_dev;
+	struct call_single_data *csd;
+
+	if (cpumask_empty(&dev->coupled_cpus))
+		return 0;
+
+	for_each_cpu_mask(cpu, dev->coupled_cpus) {
+		other_dev = per_cpu(cpuidle_devices, cpu);
+		if (other_dev && other_dev->coupled) {
+			BUG_ON(!cpumask_equal(&dev->coupled_cpus,
+				&other_dev->coupled_cpus));
+			dev->coupled = other_dev->coupled;
+			goto have_coupled;
+		}
+	}
+
+	/* No existing coupled info found, create a new one */
+	dev->coupled = kzalloc(sizeof(struct cpuidle_coupled), GFP_KERNEL);
+	if (!dev->coupled)
+		return -ENOMEM;
+
+	spin_lock_init(&dev->coupled->lock);
+
+have_coupled:
+	spin_lock(&dev->coupled->lock);
+
+	dev->coupled->requested_state[dev->cpu] = -1;
+
+	if (cpu_online(dev->cpu))
+		cpumask_set_cpu(dev->cpu, &dev->coupled->alive_coupled_cpus);
+	dev->coupled->refcnt++;
+
+	csd = &per_cpu(cpuidle_coupled_poke_cb, dev->cpu);
+	csd->func = cpuidle_coupled_poked;
+	csd->info = (void *)(unsigned long)dev->cpu;
+
+	spin_unlock(&dev->coupled->lock);
+
+	return 0;
+}
+
+/**
+ * cpuidle_coupled_unregister_device
+ *
+ * Called from cpuidle_unregister_device to tear down coupled idle.  Removes the
+ * cpu from the coupled idle set, and frees the cpuidle_coupled_info struct if
+ * this was the last cpu in the set.
+ */
+void cpuidle_coupled_unregister_device(struct cpuidle_device *dev)
+{
+	if (cpumask_empty(&dev->coupled_cpus))
+		return;
+
+	cpumask_clear_cpu(dev->cpu, &dev->coupled->alive_coupled_cpus);
+	if (--dev->coupled->refcnt)
+		kfree(dev->coupled);
+
+	dev->coupled = NULL;
+}
+
+static void cpuidle_coupled_cpu_set_alive(int cpu, bool online)
+{
+	struct cpuidle_device *dev;
+
+	mutex_lock(&cpuidle_lock);
+
+	dev = per_cpu(cpuidle_devices, cpu);
+	if (!dev->coupled)
+		goto out;
+
+	spin_lock(&dev->coupled->lock);
+
+	if (online)
+		cpumask_set_cpu(dev->cpu, &dev->coupled->alive_coupled_cpus);
+	else
+		cpumask_clear_cpu(dev->cpu, &dev->coupled->alive_coupled_cpus);
+
+	spin_unlock(&dev->coupled->lock);
+
+out:
+	mutex_unlock(&cpuidle_lock);
+}
+
+/**
+ * cpuidle_coupled_cpu_notify
+ *
+ * Called when a cpu is brought on or offline using hotplug.  Updates the
+ * coupled cpu set appropriately
+ */
+static int cpuidle_coupled_cpu_notify(struct notifier_block *nb,
+		unsigned long action, void *hcpu)
+{
+	int cpu = (unsigned long)hcpu;
+
+	switch (action & ~CPU_TASKS_FROZEN) {
+	case CPU_DEAD:
+	case CPU_UP_CANCELED:
+		cpuidle_coupled_cpu_set_alive(cpu, false);
+		break;
+	case CPU_UP_PREPARE:
+		cpuidle_coupled_cpu_set_alive(cpu, true);
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block cpuidle_coupled_cpu_notifier = {
+	.notifier_call = cpuidle_coupled_cpu_notify,
+};
+
+static int __init cpuidle_coupled_init(void)
+{
+	return register_cpu_notifier(&cpuidle_coupled_cpu_notifier);
+}
+core_initcall(cpuidle_coupled_init);
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index ea00a16..e3d61b2 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -122,7 +122,10 @@ int cpuidle_idle_call(void)
 	trace_power_start(POWER_CSTATE, next_state, dev->cpu);
 	trace_cpu_idle(next_state, dev->cpu);
 
-	entered_state = cpuidle_enter_state(dev, drv, next_state);
+	if (cpuidle_state_is_coupled(dev, drv, next_state))
+		entered_state = cpuidle_enter_state_coupled(dev, drv, next_state);
+	else
+		entered_state = cpuidle_enter_state(dev, drv, next_state);
 
 	trace_power_end(dev->cpu);
 	trace_cpu_idle(PWR_EVENT_EXIT, dev->cpu);
@@ -322,9 +325,16 @@ static int __cpuidle_register_device(struct cpuidle_device *dev)
 	if (ret)
 		goto err_sysfs;
 
+	ret = cpuidle_coupled_register_device(dev);
+	if (ret)
+		goto err_coupled;
+
 	dev->registered = 1;
 	return 0;
 
+err_coupled:
+	cpuidle_remove_sysfs(sys_dev);
+	wait_for_completion(&dev->kobj_unregister);
 err_sysfs:
 	module_put(cpuidle_driver->owner);
 	list_del(&dev->device_list);
@@ -379,6 +389,8 @@ void cpuidle_unregister_device(struct cpuidle_device *dev)
 	wait_for_completion(&dev->kobj_unregister);
 	per_cpu(cpuidle_devices, dev->cpu) = NULL;
 
+	cpuidle_coupled_unregister_device(dev);
+
 	cpuidle_resume_and_unlock();
 
 	module_put(cpuidle_driver->owner);
diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
index dd2df8f..55a0c6f 100644
--- a/drivers/cpuidle/cpuidle.h
+++ b/drivers/cpuidle/cpuidle.h
@@ -32,4 +32,43 @@ extern void cpuidle_remove_state_sysfs(struct cpuidle_device *device);
 extern int cpuidle_add_sysfs(struct sys_device *sysdev);
 extern void cpuidle_remove_sysfs(struct sys_device *sysdev);
 
+/* coupled states */
+struct cpuidle_coupled {
+	spinlock_t lock;
+	int requested_state[NR_CPUS];
+	int ready_count;
+	cpumask_t alive_coupled_cpus;
+	int refcnt;
+};
+
+#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
+bool cpuidle_state_is_coupled(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int state);
+int cpuidle_enter_state_coupled(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int next_state);
+int cpuidle_coupled_register_device(struct cpuidle_device *dev);
+void cpuidle_coupled_unregister_device(struct cpuidle_device *dev);
+#else
+static inline bool cpuidle_state_is_coupled(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int state)
+{
+	return false;
+}
+
+static inline int cpuidle_enter_state_coupled(struct cpuidle_device *dev,
+		struct cpuidle_driver *drv, int next_state)
+{
+	return -1;
+}
+
+static inline int cpuidle_coupled_register_device(struct cpuidle_device *dev)
+{
+	return 0;
+}
+
+static inline void cpuidle_coupled_unregister_device(struct cpuidle_device *dev)
+{
+}
+#endif
+
 #endif /* __DRIVER_CPUIDLE_H */
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 7408af8..5438a09 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -53,6 +53,7 @@ struct cpuidle_state {
 
 /* Idle State Flags */
 #define CPUIDLE_FLAG_TIME_VALID	(0x01) /* is residency time measurable? */
+#define CPUIDLE_FLAG_COUPLED	(0x02) /* state applies to multiple cpus */
 
 #define CPUIDLE_DRIVER_FLAGS_MASK (0xFFFF0000)
 
@@ -97,6 +98,12 @@ struct cpuidle_device {
 	struct kobject		kobj;
 	struct completion	kobj_unregister;
 	void			*governor_data;
+
+#ifdef CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED
+	int			safe_state_index;
+	cpumask_t		coupled_cpus;
+	struct cpuidle_coupled	*coupled;
+#endif
 };
 
 DECLARE_PER_CPU(struct cpuidle_device *, cpuidle_devices);
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 2/3] cpuidle: fix error handling in __cpuidle_register_device
From: Colin Cross @ 2011-12-21  0:09 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-pm
  Cc: Kevin Hilman, Len Brown, Amit Kucheria, Colin Cross, linux-tegra,
	linux-omap, Arjan van de Ven
In-Reply-To: <1324426147-16735-1-git-send-email-ccross@android.com>

Fix the error handling in __cpuidle_register_device to include
the missing list_del.  Move it to a label, which will simplify
the error handling when coupled states are added.

Signed-off-by: Colin Cross <ccross@android.com>
---
 drivers/cpuidle/cpuidle.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 1486b3c..ea00a16 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -318,13 +318,18 @@ static int __cpuidle_register_device(struct cpuidle_device *dev)
 
 	per_cpu(cpuidle_devices, dev->cpu) = dev;
 	list_add(&dev->device_list, &cpuidle_detected_devices);
-	if ((ret = cpuidle_add_sysfs(sys_dev))) {
-		module_put(cpuidle_driver->owner);
-		return ret;
-	}
+	ret = cpuidle_add_sysfs(sys_dev);
+	if (ret)
+		goto err_sysfs;
 
 	dev->registered = 1;
 	return 0;
+
+err_sysfs:
+	module_put(cpuidle_driver->owner);
+	list_del(&dev->device_list);
+	per_cpu(cpuidle_devices, dev->cpu) = NULL;
+	return ret;
 }
 
 /**
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 1/3] cpuidle: refactor out cpuidle_enter_state
From: Colin Cross @ 2011-12-21  0:09 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-pm
  Cc: Kevin Hilman, Len Brown, Amit Kucheria, Colin Cross, linux-tegra,
	linux-omap, Arjan van de Ven
In-Reply-To: <1324426147-16735-1-git-send-email-ccross@android.com>

Split the code to enter a state and update the stats into a helper
function, cpuidle_enter_state, and export it.  This function will
be called by the coupled state code to handle entering the safe
state and the final coupled state.

Signed-off-by: Colin Cross <ccross@android.com>
---
 drivers/cpuidle/cpuidle.c |   43 +++++++++++++++++++++++++++++--------------
 drivers/cpuidle/cpuidle.h |    2 ++
 2 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index 06ce268..1486b3c 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -54,6 +54,34 @@ static void cpuidle_kick_cpus(void) {}
 static int __cpuidle_register_device(struct cpuidle_device *dev);
 
 /**
+ * cpuidle_enter_state
+ *
+ * enter the state and update stats
+ */
+int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv,
+		int next_state)
+{
+	int entered_state;
+	struct cpuidle_state *target_state;
+
+	target_state = &drv->states[next_state];
+
+	entered_state = target_state->enter(dev, drv, next_state);
+
+	if (entered_state >= 0) {
+		/* Update cpuidle counters */
+		/* This can be moved to within driver enter routine
+		 * but that results in multiple copies of same code.
+		 */
+		dev->states_usage[entered_state].time +=
+				(unsigned long long)dev->last_residency;
+		dev->states_usage[entered_state].usage++;
+	}
+
+	return entered_state;
+}
+
+/**
  * cpuidle_idle_call - the main idle loop
  *
  * NOTE: no locks or semaphores should be used here
@@ -63,7 +91,6 @@ int cpuidle_idle_call(void)
 {
 	struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
 	struct cpuidle_driver *drv = cpuidle_get_driver();
-	struct cpuidle_state *target_state;
 	int next_state, entered_state;
 
 	if (off)
@@ -92,26 +119,14 @@ int cpuidle_idle_call(void)
 		return 0;
 	}
 
-	target_state = &drv->states[next_state];
-
 	trace_power_start(POWER_CSTATE, next_state, dev->cpu);
 	trace_cpu_idle(next_state, dev->cpu);
 
-	entered_state = target_state->enter(dev, drv, next_state);
+	entered_state = cpuidle_enter_state(dev, drv, next_state);
 
 	trace_power_end(dev->cpu);
 	trace_cpu_idle(PWR_EVENT_EXIT, dev->cpu);
 
-	if (entered_state >= 0) {
-		/* Update cpuidle counters */
-		/* This can be moved to within driver enter routine
-		 * but that results in multiple copies of same code.
-		 */
-		dev->states_usage[entered_state].time +=
-				(unsigned long long)dev->last_residency;
-		dev->states_usage[entered_state].usage++;
-	}
-
 	/* give the governor an opportunity to reflect on the outcome */
 	if (cpuidle_curr_governor->reflect)
 		cpuidle_curr_governor->reflect(dev, entered_state);
diff --git a/drivers/cpuidle/cpuidle.h b/drivers/cpuidle/cpuidle.h
index 38c3fd8..dd2df8f 100644
--- a/drivers/cpuidle/cpuidle.h
+++ b/drivers/cpuidle/cpuidle.h
@@ -14,6 +14,8 @@ extern struct list_head cpuidle_detected_devices;
 extern struct mutex cpuidle_lock;
 extern spinlock_t cpuidle_driver_lock;
 extern int cpuidle_disabled(void);
+int cpuidle_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv,
+		int next_state);
 
 /* idle loop */
 extern void cpuidle_install_idle_handler(void);
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH 0/3] coupled cpuidle state support
From: Colin Cross @ 2011-12-21  0:09 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, linux-pm
  Cc: Kevin Hilman, Len Brown, Amit Kucheria, Colin Cross, linux-tegra,
	linux-omap, Arjan van de Ven

On some ARM SMP SoCs (OMAP4460, Tegra 2, and probably more), the
cpus cannot be independently powered down, either due to
sequencing restrictions (on Tegra 2, cpu 0 must be the last to
power down), or due to HW bugs (on OMAP4460, a cpu powering up
will corrupt the gic state unless the other cpu runs a work
around).  Each cpu has a power state that it can enter without
coordinating with the other cpu (usually Wait For Interrupt, or
WFI), and one or more "coupled" power states that affect blocks
shared between the cpus (L2 cache, interrupt controller, and
sometimes the whole SoC).  Entering a coupled power state must
be tightly controlled on both cpus.

The easiest solution to implementing coupled cpu power states is
to hotplug all but one cpu whenever possible, usually using a
cpufreq governor that looks at cpu load to determine when to
enable the secondary cpus.  This causes problems, as hotplug is an
expensive operation, so the number of hotplug transitions must be
minimized, leading to very slow response to loads, often on the
order of seconds.

This patch series implements an alternative solution, where each
cpu will wait in the WFI state until all cpus are ready to enter
a coupled state, at which point the coupled state function will
be called on all cpus at approximately the same time.

Once all cpus are ready to enter idle, they are woken by an smp
cross call.  At this point, there is a chance that one of the
cpus will find work to do, and choose not to enter suspend.  A
final pass is needed to guarantee that all cpus will call the
power state enter function at the same time.  During this pass,
each cpu will increment the ready counter, and continue once the
ready counter matches the number of online coupled cpus.  If any
cpu exits idle, the other cpus will decrement their counter and
retry.

To use coupled cpuidle states, a cpuidle driver must:

   Set struct cpuidle_device.coupled_cpus to the mask of all
   coupled cpus, usually the same as cpu_possible_mask if all cpus
   are part of the same cluster.  The coupled_cpus mask must be
   set in the struct cpuidle_device for each cpu.

   Set struct cpuidle_device.safe_state to a state that is not a
   coupled state.  This is usually WFI.

   Set CPUIDLE_FLAG_COUPLED in struct cpuidle_state.flags for each
   state that affects multiple cpus.

   Provide a struct cpuidle_state.enter function for each state
   that affects multiple cpus.  This function is guaranteed to be
   called on all cpus at approximately the same time.  The driver
   should ensure that the cpus all abort together if any cpu tries
   to abort once the function is called.

This series was functionally tested on v3.0, but has only been
compile-tested on v3.2 after the removal of per-cpu state fields.

This patch set has a few disadvantages over the hotplug governor,
but I think they are all fairly minor:
   * Worst-case interrupt latency can be increased.  If one cpu
     receives an interrupt while the other is spinning in the
     ready_count loop, the second cpu will be stuck with
     interrupts off until the first cpu finished processing
     its interrupt and exits idle.  This will increase the worst
     case interrupt latency by the worst-case interrupt processing
     time, but should be very rare.
   * Interrupts are processed while still inside pm_idle.
     Normally, interrupts are only processed at the very end of
     pm_idle, just before it returns to the idle loop.  Coupled
     states requires processing interrupts inside
     cpuidle_enter_state_coupled in order to distinguish between
     the smp_cross_call from another cpu that is now idle and an
     interrupt that should cause idle to exit.
     I don't see a way to fix this without either being able to
     read the next pending irq from the interrupt chip, or
     querying the irq core for which interrupts were processed.
   * Since interrupts are processed inside cpuidle, the next
     timer event could change.  The new timer event will be
     handled correctly, but the idle state decision made by
     the governor will be out of date, and will not be revisited.
     The governor select function could be called again every time,
     but this could lead to a lot of work being done by an idle
     cpu if the other cpu was mostly busy.
   * The spinlock that protects requested_state and ready_count is
     should probably be replaced with careful use of atomics and
     barriers.

None of the platforms I work with have an SMP idle implementation
upstream, so I can't easily show a patch that converts a platform
from hotplug governor to coupled cpuidle states.  Instead, I'll
give a quick example implementation assuming functions that handle
hotplug and single-cpu idle already exist.

static int mach_enter_idle_coupled(struct cpuidle_device *dev,
		struct cpuidle_driver *drv, int index)
{
	ktime_t enter, exit;
	s64 us;

	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);

	enter = ktime_get();

	cpu_pm_enter();

        if (dev->cpu == 0) {
	        for_each_online_cpu(i)
        	        while (i != dev->cpu && !mach_cpu_is_reset(i))
                	        cpu_relax();

		mach_cpu_idle();

		for_each_online_cpu(i)
			if (i != cpu)
				mach_cpu_online(i);
	} else {
		mach_cpu_offline();
	}

	cpu_pm_exit();

	exit = ktime_sub(ktime_get(), enter);
	us = ktime_to_us(exit);

	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);

	local_irq_enable();

	dev->last_residency = us;

	return index;
}

^ permalink raw reply

* [RFC]: [PATCH] Export cpuidle_(un)register_governor
From: Jan-Simon Möller @ 2011-12-20 16:15 UTC (permalink / raw)
  To: linux-pm


[-- Attachment #1.1: Type: text/plain, Size: 551 bytes --]

Hi all!

This patch exports cpuidle_register_governor and cpuidle_unregister_governor
with EXPORT_SYMBOL_GPL (as the other cpuidle_(un)register_* ).

Reasoning: allow runtime loading of a cpuidle governor (mainly for
development for now).

Usually we don't need this as we're using built-ins for the governors - but
during developments it is really handy to load and unload a cpuidle governor
(together with the cpuidle_sysfs_switch).

Comments ?

Best,
Jan-Simon
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

[-- Attachment #1.2: Type: text/html, Size: 655 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Export-cpuidle_register_governor-and-cpuidle_unregis.patch --]
[-- Type: text/x-patch; name="0001-Export-cpuidle_register_governor-and-cpuidle_unregis.patch", Size: 1165 bytes --]

From a21a054fa64f898576ee62fced30508e0affe243 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan-Simon=20M=C3=B6ller?= <jansimon.moeller@gmx.de>
Date: Mon, 12 Dec 2011 14:40:56 +0100
Subject: [PATCH] Export cpuidle_register_governor and
 cpuidle_unregister_governor with EXPORT_SYMBOL_GPL (as the
 other cpuidle_(un)register_* ).

Reasoning: allow runtime loading of a cpuidle governor (mainly for
development for now).
---
 drivers/cpuidle/governor.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/cpuidle/governor.c b/drivers/cpuidle/governor.c
index ea2f8e7..552f5a4 100644
--- a/drivers/cpuidle/governor.c
+++ b/drivers/cpuidle/governor.c
@@ -97,6 +97,8 @@ int cpuidle_register_governor(struct cpuidle_governor *gov)
 	return ret;
 }
 
+EXPORT_SYMBOL_GPL(cpuidle_register_governor);
+
 /**
  * cpuidle_replace_governor - find a replacement governor
  * @exclude_rating: the rating that will be skipped while looking for
@@ -139,3 +141,4 @@ void cpuidle_unregister_governor(struct cpuidle_governor *gov)
 	mutex_unlock(&cpuidle_lock);
 }
 
+EXPORT_SYMBOL_GPL(cpuidle_unregister_governor);
-- 
1.7.7

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related

* Re: [RFC PATCH 1/2] thermal: Add a new trip type to use cooling device instance number
From: Vincent Guittot @ 2011-12-20 12:37 UTC (permalink / raw)
  To: Amit Daniel Kachhap
  Cc: linux-acpi, linux-pm, linaro-dev, linux-kernel, patches
In-Reply-To: <1323789196-4942-2-git-send-email-amit.kachhap@linaro.org>

Hi Amit,

I'm not sure that using the trip index for setting the state of a
cooling device is a generic solution because you are adding a
dependency between the trip index and the cooling device state that
might be difficult to handle. This dependency implies that a cooling
device like cpufreq_cooling_device must be registered in the 1st trips
of a thermal_zone which is not possible when we want to register 2
cpufreq_cooling_devices in the same thermal_zone.
You should only rely on the current and last temperatures to detect if
a trip_temp has been crossed and you should increase or decrease the
current state of the cooling device accordingly.

something like below should work with cpufreq_cooling_device and will
not add any constraint on the trip index. The state of a cooling
device is increased/decreased once for each trip

+		case THERMAL_TRIP_STATE_ACTIVE:
+			list_for_each_entry(instance, &tz->cooling_devices,
+					    node) {
+				if (instance->trip != count)
+					continue;
+
+				cdev = instance->cdev;
+
+				if ((temp >= trip_temp)
+					&& (trip_temp > tz->last_temperature)) {
+					cdev->ops->get_max_state(cdev,
+							&max_state);
+					cdev->ops->get_cur_state(cdev,
+							&current_state);
+					if (++current_state <= max_state)
+						cdev->ops->set_cur_state(cdev,
+								current_state);
+				}
+				else if ((temp < trip_temp)
+					&& (trip_temp <= tz->last_temperature)) {
+					cdev->ops->get_cur_state(cdev,
+							&current_state);
+					if (current_state > 0)
+						cdev->ops->set_cur_state(cdev,
+								--current_state);
+			}
+			break;

Regards,
Vincent

On 13 December 2011 16:13, Amit Daniel Kachhap <amit.kachhap@linaro.org> wrote:
> This patch adds a new trip type THERMAL_TRIP_STATE_ACTIVE. This
> trip behaves same as THERMAL_TRIP_ACTIVE but also passes the cooling
> device instance number. This helps the cooling device registered as
> different instances to perform appropriate cooling action decision in
> the set_cur_state call back function.
>
> Also since the trip temperature's are in ascending order so some logic
> is put in place to skip the un-necessary checks.
>
> Signed-off-by: Amit Daniel Kachhap <amit.kachhap@linaro.org>
> ---
>  Documentation/thermal/sysfs-api.txt |    4 ++--
>  drivers/thermal/thermal_sys.c       |   27 ++++++++++++++++++++++++++-
>  include/linux/thermal.h             |    1 +
>  3 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/thermal/sysfs-api.txt b/Documentation/thermal/sysfs-api.txt
> index b61e46f..5c1d44e 100644
> --- a/Documentation/thermal/sysfs-api.txt
> +++ b/Documentation/thermal/sysfs-api.txt
> @@ -184,8 +184,8 @@ trip_point_[0-*]_temp
>
>  trip_point_[0-*]_type
>        Strings which indicate the type of the trip point.
> -       E.g. it can be one of critical, hot, passive, active[0-*] for ACPI
> -       thermal zone.
> +       E.g. it can be one of critical, hot, passive, active[0-1],
> +       state-active[0-*] for ACPI thermal zone.
>        RO, Optional
>
>  cdev[0-*]
> diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
> index dd9a574..72b1ab3 100644
> --- a/drivers/thermal/thermal_sys.c
> +++ b/drivers/thermal/thermal_sys.c
> @@ -192,6 +192,8 @@ trip_point_type_show(struct device *dev, struct device_attribute *attr,
>                return sprintf(buf, "passive\n");
>        case THERMAL_TRIP_ACTIVE:
>                return sprintf(buf, "active\n");
> +       case THERMAL_TRIP_STATE_ACTIVE:
> +               return sprintf(buf, "state-active\n");
>        default:
>                return sprintf(buf, "unknown\n");
>        }
> @@ -1035,7 +1037,7 @@ EXPORT_SYMBOL(thermal_cooling_device_unregister);
>  void thermal_zone_device_update(struct thermal_zone_device *tz)
>  {
>        int count, ret = 0;
> -       long temp, trip_temp;
> +       long temp, trip_temp, max_state, last_trip_change = 0;
>        enum thermal_trip_type trip_type;
>        struct thermal_cooling_device_instance *instance;
>        struct thermal_cooling_device *cdev;
> @@ -1086,6 +1088,29 @@ void thermal_zone_device_update(struct thermal_zone_device *tz)
>                                        cdev->ops->set_cur_state(cdev, 0);
>                        }
>                        break;
> +               case THERMAL_TRIP_STATE_ACTIVE:
> +                       list_for_each_entry(instance, &tz->cooling_devices,
> +                                           node) {
> +                               if (instance->trip != count)
> +                                       continue;
> +
> +                               if (temp <= last_trip_change)
> +                                       continue;
> +
> +                               cdev = instance->cdev;
> +                               cdev->ops->get_max_state(cdev, &max_state);
> +
> +                               if ((temp >= trip_temp) &&
> +                                               ((count + 1) <= max_state))
> +                                       cdev->ops->set_cur_state(cdev,
> +                                                               count + 1);
> +                               else if ((temp < trip_temp) &&
> +                                                       (count <= max_state))
> +                                       cdev->ops->set_cur_state(cdev, count);
> +
> +                               last_trip_change = trip_temp;
> +                       }
> +                       break;
>                case THERMAL_TRIP_PASSIVE:
>                        if (temp >= trip_temp || tz->passive)
>                                thermal_zone_device_passive(tz, temp,
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 47b4a27..d7d0a27 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -42,6 +42,7 @@ enum thermal_trip_type {
>        THERMAL_TRIP_PASSIVE,
>        THERMAL_TRIP_HOT,
>        THERMAL_TRIP_CRITICAL,
> +       THERMAL_TRIP_STATE_ACTIVE,
>  };
>
>  struct thermal_zone_device_ops {
> --
> 1.7.1
>
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/linux-pm

^ permalink raw reply

* Re: [PATCH 3/6] OMAP: PM: register to the per-device PM QoS framework
From: Paul Walmsley @ 2011-12-19 21:12 UTC (permalink / raw)
  To: jean.pihet
  Cc: Kevin Hilman, Linux PM mailing list, linux-omap, Jean Pihet,
	linux-arm
In-Reply-To: <1323874304-5001-4-git-send-email-j-pihet@ti.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 5481 bytes --]

Hi

On Wed, 14 Dec 2011, jean.pihet@newoldbits.com wrote:

> From: Jean Pihet <j-pihet@ti.com>
> 
> Implement the devices wake-up latency constraints using the global
> device PM QoS notification handler which applies the constraints to the
> underlying layer by calling the corresponding function at hwmod level.
> 
> Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using wake-up
> latency constraints on MPU, CORE and PER.
> 
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> Reviewed-by: Kevin Hilman <khilman@ti.com>

I've modified this patch to work with omap_devices that contain multiple 
hwmods, and to move the code to the mach-omap2/omap_device.c code, which 
seems to make more sense.  Since the hwmod set-constraint and 
remove-constraint functions are now split, this code also must determine 
which one to call.

- Paul

From: Jean Pihet <j-pihet@ti.com>
Date: Sun, 18 Dec 2011 16:42:07 -0700
Subject: [PATCH] ARM: OMAP: omap_device: register to the per-device PM QoS
 framework

Implement the devices wake-up latency constraints using the global
device PM QoS notification handler which applies the constraints to the
underlying layer by calling the corresponding function at hwmod level.

Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using wake-up
latency constraints on MPU, CORE and PER.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Reviewed-by: Kevin Hilman <khilman@ti.com>
[paul@pwsan.com: modified to work with omap_devices with large numbers of
 hwmods; moved code to mach-omap2/omap_device.c; added documentation; use
 notifier return codes]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/plat-omap/omap_device.c |   74 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 74 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index e8d9869..b2c18b7 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -2,6 +2,7 @@
  * omap_device implementation
  *
  * Copyright (C) 2009-2010 Nokia Corporation
+ * Copyright (C) 2011 Texas Instruments, Inc.
  * Paul Walmsley, Kevin Hilman
  *
  * Developed in collaboration with (alphabetical order): Benoit
@@ -88,6 +89,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
 #include <linux/notifier.h>
+#include <linux/pm_qos.h>
 
 #include <plat/omap_device.h>
 #include <plat/omap_hwmod.h>
@@ -409,6 +411,67 @@ static int _omap_device_notifier_call(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
+/**
+ * _omap_device_pm_qos_handler - interface to the per-device PM QoS framework
+ * @nb: pointer to omap_device_pm_qos_nb (not used)
+ * @new_value: new maximum wakeup latency constraint for @req->dev (in µs)
+ * @req: struct dev_pm_qos_request * passed by the Linux PM QoS code
+ *
+ * Called by the Linux core device PM QoS code to alter the maximum
+ * wakeup latency constraint on a device.  If the underlying device is
+ * an omap_device, then this code will pass the constraint on to the
+ * underlying hwmods.  Returns -EINVAL if this code can't handle the
+ * constraint for some reason, or passes along the return code from the
+ * hwmod wakeup latency constraint functions.
+ */
+static int _omap_device_pm_qos_handler(struct notifier_block *nb,
+				       unsigned long new_value,
+				       void *req)
+{
+	struct omap_device *od;
+	struct omap_hwmod *oh;
+	struct platform_device *pdev;
+	struct dev_pm_qos_request *dev_pm_qos_req = req;
+	int ret = NOTIFY_OK;
+	int r, i;
+
+	pr_debug("OMAP PM constraints: req@0x%p, new_value=%lu\n",
+		 req, new_value);
+
+	/* Look for the platform device for the constraint target device */
+	pdev = to_platform_device(dev_pm_qos_req->dev);
+
+	/* Try to catch non platform devices */
+	if (pdev->name == NULL) {
+		pr_err("%s: Error: platform device for device %s not valid\n",
+		       __func__, dev_name(dev_pm_qos_req->dev));
+		return NOTIFY_DONE;
+	}
+
+	/* Find the associated omap_device for dev */
+	od = to_omap_device(pdev);
+	if (od == NULL) {
+		pr_err("%s: Error: no omap_device for device %s\n",
+		       __func__, dev_name(dev_pm_qos_req->dev));
+		return NOTIFY_DONE;
+	}
+
+	pr_debug("OMAP PM constraints: req@0x%p, dev=0x%p, new_value=%lu\n",
+		 req, dev_pm_qos_req->dev, new_value);
+
+	for (i = 0; i < od->hwmods_cnt; i++) {
+		oh = od->hwmods[i];
+		if (new_value == PM_QOS_DEV_LAT_DEFAULT_VALUE)
+			r = omap_hwmod_remove_wakeuplat_constraint(oh, dev_pm_qos_req);
+		else
+			r = omap_hwmod_set_wakeuplat_constraint(oh, dev_pm_qos_req, new_value);
+
+		if (!r)
+			ret = NOTIFY_BAD;
+	}
+
+	return ret;
+}
 
 /* Public functions for use by core code */
 
@@ -1135,13 +1198,24 @@ struct device omap_device_parent = {
 	.parent         = &platform_bus,
 };
 
+static struct notifier_block omap_device_pm_qos_nb = {
+	.notifier_call	= _omap_device_pm_qos_handler,
+};
+
 static struct notifier_block platform_nb = {
 	.notifier_call = _omap_device_notifier_call,
 };
 
 static int __init omap_device_init(void)
 {
+	int ret;
+
 	bus_register_notifier(&platform_bus_type, &platform_nb);
+
+	ret = dev_pm_qos_add_global_notifier(&omap_device_pm_qos_nb);
+	if (!ret)
+		pr_err("omap_device: cannot add global notifier for dev PM QoS\n");
+
 	return device_register(&omap_device_parent);
 }
 core_initcall(omap_device_init);
-- 
1.7.7.3

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related

* Re: [PATCH 2/6] OMAP2+: omap_hwmod: manage the wake-up latency constraints
From: Paul Walmsley @ 2011-12-19 21:11 UTC (permalink / raw)
  To: jean.pihet
  Cc: Kevin Hilman, Linux PM mailing list, linux-omap, Jean Pihet,
	linux-arm
In-Reply-To: <1323874304-5001-3-git-send-email-j-pihet@ti.com>

Hi

On Wed, 14 Dec 2011, jean.pihet@newoldbits.com wrote:

> From: Jean Pihet <j-pihet@ti.com>
> 
> The OMAP PM code implements a handler for the per-device PM QoS framework.
> The handler queries the omap_hwmod layer in order to manage the power domains
> wake-up latency constraints. Hwmod retrieves the correct power domain
> and if it exists it calls the corresponding power domain function.
> 
> Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using wake-up
> latency constraints on MPU, CORE and PER.
> 
> Signed-off-by: Jean Pihet <j-pihet@ti.com>
> Reviewed-by: Kevin Hilman <khilman@ti.com>

I've modified this patch to split the omap_hwmod constraint functions into 
a set-constraint function and a remove-constraint function.  This removes 
the dependency on the use of a magic value to indicate constraint removal.  
It also removes the dependency on the Linux device PM layer's magic value, 
which is not really related to this code.

Since the underlying powerdomain code isn't ready yet for the reasons 
described in 

    http://marc.info/?l=linux-omap&m=132432863401849&w=2

those calls have been removed from this patch.  They'll be added back in a 
later series.


- Paul

From: Jean Pihet <j-pihet@ti.com>
Date: Sun, 18 Dec 2011 16:42:07 -0700
Subject: [PATCH] ARM: OMAP2+: hwmod: manage the wake-up latency constraints

The OMAP PM code implements a handler for the per-device PM QoS framework.
The handler queries the omap_hwmod layer in order to manage the power domains
wake-up latency constraints. Hwmod retrieves the correct power domain
and if it exists it calls the corresponding power domain function.

Tested on OMAP3 Beagleboard and OMAP4 Pandaboard in RET/OFF using wake-up
latency constraints on MPU, CORE and PER.

Signed-off-by: Jean Pihet <j-pihet@ti.com>
Reviewed-by: Kevin Hilman <khilman@ti.com>
[paul@pwsan.com: cleaned some documentation; split set/remove constraint
 functions; modified to return -EINVAL until underlying code is ready]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |   54 +++++++++++++++++++++++++-
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    5 ++
 2 files changed, 58 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 207a2ff..f28cad2 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -143,6 +143,7 @@
 #include "powerdomain.h"
 #include <plat/clock.h>
 #include <plat/omap_hwmod.h>
+#include <plat/omap_device.h>
 #include <plat/prcm.h>
 
 #include "cm2xxx_3xxx.h"
@@ -2616,10 +2617,61 @@ ohsps_unlock:
 }
 
 /**
+ * omap_hwmod_set_wakeuplat_constraint - set/release a wake-up latency
+ * constraint
+ * @oh: struct omap_hwmod* to which the target device belongs to.
+ * @cookie: identifier of the constraints list for @oh.
+ * @min_latency: the minimum allowed wake-up latency for @oh.
+ *
+ * Sets a wakeup latency contraint.  (To remove a wakeup latency
+ * constraint, call omap_hwmod_remove_wakeuplat_constraint()).
+ * Returns the return value from pwrdm_wakeuplat_remove_constraint(),
+ * or -EINVAL in case of invalid parameters.
+ */
+int omap_hwmod_set_wakeuplat_constraint(struct omap_hwmod *oh, void *cookie,
+					long min_latency)
+{
+	struct powerdomain *pwrdm = omap_hwmod_get_pwrdm(oh);
+
+	if (!pwrdm)
+		return -EINVAL;
+
+	/*
+	 * XXX Update to use pwrdm_wakeuplat_update_constraint() when
+	 * that code is ready
+	 */
+	return -EINVAL;
+}
+
+/**
+ * omap_hwmod_remove_wakeuplat_constraint - release a wake-up latency
+ * constraint
+ * @oh: struct omap_hwmod* to which the target device belongs to.
+ * @cookie: identifier of the constraints list for @oh.
+ *
+ * Removes a wakeup latency contraint.  Returns the return value from
+ * pwrdm_wakeuplat_update_constraint(), or -EINVAL in case of invalid
+ * parameters.
+ */
+int omap_hwmod_remove_wakeuplat_constraint(struct omap_hwmod *oh, void *cookie)
+{
+	struct powerdomain *pwrdm = omap_hwmod_get_pwrdm(oh);
+
+	if (!pwrdm)
+		return -EINVAL;
+
+	/*
+	 * XXX Update to use pwrdm_wakeuplat_remove_constraint() when
+	 * that code is ready
+	 */
+	return -EINVAL;
+}
+
+/**
  * omap_hwmod_get_context_loss_count - get lost context count
  * @oh: struct omap_hwmod *
  *
- * Query the powerdomain of of @oh to get the context loss
+ * Query the powerdomain of @oh to get the context loss
  * count for this device.
  *
  * Returns the context loss count of the powerdomain assocated with @oh
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 8b372ed..ff60532 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -600,6 +600,11 @@ int omap_hwmod_for_each_by_class(const char *classname,
 				 void *user);
 
 int omap_hwmod_set_postsetup_state(struct omap_hwmod *oh, u8 state);
+
+int omap_hwmod_set_wakeuplat_constraint(struct omap_hwmod *oh, void *cookie,
+					long min_latency);
+int omap_hwmod_remove_wakeuplat_constraint(struct omap_hwmod *oh, void *cookie);
+
 int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh);
 
 int omap_hwmod_no_setup_reset(struct omap_hwmod *oh);
-- 
1.7.7.3

^ permalink raw reply related

* Re: [PATCH v6 0/6] PM QoS: implement the OMAP low level constraints management code
From: Paul Walmsley @ 2011-12-19 21:02 UTC (permalink / raw)
  To: jean.pihet
  Cc: Kevin Hilman, Linux PM mailing list, linux-omap, Jean Pihet,
	linux-arm
In-Reply-To: <1323874304-5001-1-git-send-email-j-pihet@ti.com>

Hi Jean

I'm really sorry it's taken me so long to do detailed review of these 
patches for merging... anyway -

On Wed, 14 Dec 2011, jean.pihet@newoldbits.com wrote:

> From: Jean Pihet <j-pihet@ti.com>
> 
> . Implement the devices wake-up latency constraints using the global
>   device PM QoS notification handler which applies the constraints to the
>   underlying layer
> . Implement the low level code which controls the power domains next power
>   states, through the hwmod and pwrdm layers
> . Add cpuidle and power domains wake-up latency figures for OMAP3, cf. 
>   comments in the code and [1] for the details on where the numbers
>   are magically coming from
> . Implement the relation between the cpuidle and per-device PM QoS frameworks
>   in the OMAP3 specific idle callbacks.
>   The chosen C-state shall satisfy the following conditions:
>    . the 'valid' field is enabled,
>    . it satisfies the enable_off_mode flag,
>    . the next state for MPU and CORE power domains is not lower than the
>      state programmed by the per-device PM QoS.

I've been reviewing these closely.  It looks to me that are some issues 
that need to be resolved before all of them are mergeable.

One issue that I noticed in this series is that there are some locking 
issues in patch 1.  It looks to me that calls to 
_pwrdm_wakeuplat_update_pwrst() can race with each other, since it's 
called outside the lock?

Another issue is with the latency data, which we've discussed previously. 
Before we merge the latency data in mainline, we need to ensure that we've 
minimized the dependency of that data on other register settings, such as 
the AUTOEXTCLKMODE bits or the external high-frequency oscillator 
stabilization time.  Otherwise any board vendor that ships devices based 
on that data will need to redo it, which we'd like to avoid.  I'll comment 
more on this in replies to those data patches.

But there is a more fundamental issue that we need to deal with before we 
should merge the low-level powerdomain portion of your patches -- an issue 
that isn't directly related to your code.  We should first switch over to 
use functional powerstates throughout our PM code.  Right now, we have 
some code that tweaks powerdomain bits directly, and some code that calls 
omap_set_pwrdm_state().  We should get rid of the former, I think, so that 
there is one single code path that directly changes the low-level 
powerdomain bits.  Otherwise I think we run the risk of having one of our 
code paths lose track of what the powerdomain settings are -- and that way 
lies madness...

Anyway, I've got some experimental code to do this, but since it touches a 
lot of our existing PM code, I'd like to have Kevin's review, testing, and 
approval of it.  (And yours too, ideally ;-)  So what I'd like to do is to 
repost your series with some changes, and with the functional powerstate 
portion sorted out first.  Would you be able to help test those for 3.4?

In the meantime, it looks to me like two of your patches are good to go in 
the short term, with some tweaks - the omap_hwmod changes and the 
dev_pm_qos callback changes.  I'll repost those shortly and queue them up 
for whenever Tony wants to pull them.


- Paul

^ permalink raw reply

* Re: [PATCH 1/4] cpupower: Add cpupower-idle-info manpage
From: Thomas Renninger @ 2011-12-17  9:21 UTC (permalink / raw)
  To: Jean Pihet
  Cc: cpufreq, linux-acpi, linux, linux-pm, linux-omap,
	linux-arm-kernel
In-Reply-To: <CAORVsuVrm+dWGgs-taatnUMZsYvYg_tdx1=7ekWp0qM+25DorQ@mail.gmail.com>

On Friday 16 December 2011 17:18:37 Jean Pihet wrote:
> Hi Thomas,
> 
> On Fri, Dec 16, 2011 at 3:35 PM, Thomas Renninger <trenn@suse.de> wrote:
> > The last missing manpage for cpupower tools.
> Great!
> 
> > More info about other architecture's sleep state specialities would be great.
> I wonder if it is the right place for some arch specific information
> about the sleep states.
> 
> I propose to document this in a more generic doc file (e.g.
> Documentation/arm/OMAP/omap_pm) and link it from the manpage.
Duplicating info at different places increases needed maintenance.
Mabye above should be more about the kernel implementation details
and the manpage should more include the bits a typical user
wants to know?

> What do you think?
The POLL state is a bit confusing on X86 cpupower idle-info output.
Also on X86 the CPU may decide to use other states than the kernel
requested and the cpupower monitor tool should get used to see them.
This directly affects the output/usage of the tool and to understand it,
it should be documented in the manpage.

IMO it's not bad to have a somewhat bigger manpage here.
Especially on ARM, also the "ordinary" user wants to know
about how/whether the cpu power functionalities work as expected
without the need of looking at the kernel sources.

For now I could only try out cpupower on an ARM platform
with cpufreq support.
Would be great if someone could give cpupower idle-info
a try on such a machine.
Think of a typical user, not a kernel hacker and what he needs to
know to interpret and understand the output.
Maybe it's self-explanatory already, maybe the one or other arch
specific bit should get explained?

   Thomas

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox