Re: [RFC PATCH 0/2] thermal: Add generic devfreq cooling device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi Chanwoo,

Chanwoo Choi <cw00.choi@xxxxxxxxxxx> writes:

> Hi Punit,
>
> On 07/17/2015 07:53 PM, Punit Agrawal wrote:
>> Hi Chanwoo,
>> 
>> Chanwoo Choi <cw00.choi@xxxxxxxxxxx> writes:
>> 
>>> This patchset introduce the generic devfreq cooling device for generic thermal
>>> framework. The devfreq devices are used ad cooling device to reduce the
>>> overheating temperature. This patch is based on drivers/thermal/cpu_cooling.c.
>>> The devfreq cooling device can change the ragne of the frequency table of
>>> devfreq device according to cooling level in device tree file.
>>>
>> 
>> Have you had a look at the devfreq cooling patches from Javi[0][1]? How
>> is the current patchset different?
>
> I didn't see Javi's patchset before. Thanks for your information.
>
> I reviewed ths Javi's patchset. Both Javi's patchset and my patchset 
> has same concept except for applying the power allocator thermal governor
> as you below comment.
>
> But, there are some difference.
>
> First,
> I don't add new devfreq API (devfreq_set_max() / devfreq_set_min()).
> The my patchset used existing update_devfreq() to update the
> maximum frequency of devfreq device.
>

Ok.

> Second,
> In my patchset, the devfreq cooling device will be operated
> as existing cpu cooling device. If sensor measure the overheating
> temperature, devfreq cooling device will limit the maximum frequency
> of devfreq device.

Javi's patchset behaves exactly as you describe here.

> As below example, the devicetree file includes
> the overheating temperature information of each trip-point.
> - Javi's patchset used the static power value calculated by
> devfreq_cooling_gen_power_table() instead of temperature.
>

You've got this wrong.

The power table is used to model device power consumption which allows
the device to be used with the power_allocator governor. Refer to the
power_allocator documentation[2] to understand how it is used.

This doesn't change the behaviour when used with other governors.

[2] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/thermal/power_allocator.txt?id=refs/tags/v4.2-rc3

> Third,
> Javi's patchset used the same string of type when calling
> the thermal_of_cooling_device_register()
> - Javi's patchset used always the same "devfreq" string.
> - My patchset used the different "thermal-devfreq-%d" string
> according to each devfreq cooling device.
>

Except for this difference (which needs to be fixed), the current
patchset is a subset of the functionality proposed in [1].

With the merging of the power_allocator governor in v4.2, it makes sense
for the devfreq cooling device to also include support for it -
especially when the functionality is already under discussion on the
list.

As such, it would be great if you could provide feedback on that thread.

Thanks,
Punit

> In my patchset, devfreq cooling device uses the same method
> to determine the throttling situation as existing cpu cooling
> device. It is just my opinion.
>
>> 
>> At first glance, it seems that you are not implementing the extensions
>> that allow devfreq cooling devices to be used with power_allocator
>> thermal governor that got merged in v4.2-rc1.
>> 
>> Thanks,
>> Punit
>> 
>> [0] http://article.gmane.org/gmane.linux.power-management.general/61936
>> [1] http://article.gmane.org/gmane.linux.power-management.general/62417
>
> Thanks,
> Chanwoo Choi
>
>> 
>> 
>>> To verify the devfreq cooling device driver, I testd it with following platform:
>>>
>>> For example,
>>> - The Mali GPU of Exynos5433 SoC uses the devfreq framework to support the DVFS
>>> feature and Exynos5433 contains the G3D (GPU) thermal sensor. Following example
>>> explain the correlation between mali dt node and thermal sensor/zone.
>>> : thermal sensor : G3D sensor of Samsung Exynos5433 [1][2]
>>> : devfreq cooling device : Mali GPU [3]
>>>
>>> According to the temperature of g3d thermal sensor inclued in Exynos5433,
>>> devfreq cooling device can change the maximum frequency of Mali GPU.
>>>
>>> 1. In Exynos5433-based board dts file, Mali GPU dt node uses the devfreq
>>> framework to suppot the DVFS feature. Following dt node includes the
>>> both 'cooling-cells' and 'operating-points' which means the supported
>>> frequency entries:
>>>
>>> 	mali: mali@14AC0000 {
>>> 		compatible = "arm,mali-midgard";
>>> 		reg = <0x14AC0000 0x5000>;
>>> 		interrupts = <0 282 0>, <0 283 0>, <0 281 0>;
>>> 		interrupt-names = "JOB", "MMU", "GPU";
>>> 		clocks = <&cmu_g3d CLK_ACLK_G3D>;
>>> 		clock-names = "clk_mali";
>>> 		power-domains = <&pd_g3d>;
>>> 		status = "disabled";
>>>
>>> 		#cooling-cells = <2>;
>>>
>>> 		operating-points = <
>>> 			700000 1150000
>>> 			600000 1150000
>>> 			550000 1125000
>>> 			500000 1075000
>>> 			420000 1025000
>>> 			350000 1025000
>>> 			266000 1000000
>>> 			160000 1000000
>>> 		>;
>>> 	};
>>>
>>> 2. In exynos5433.dtsi, G3D thermal sensor measure the temperature of Mali GPU:
>>>
>>> 	tmu_g3d: tmu@10070000 {
>>> 		compatible = "samsung,exynos5433-tmu";
>>> 		reg = <0x10070000 0x200>;
>>> 		interrupts = <0 99 0>;
>>> 		clocks = <&cmu_peris CLK_PCLK_TMU1_APBIF>,
>>> 			 <&cmu_peris CLK_SCLK_TMU1>;
>>> 		clock-names = "tmu_apbif", "tmu_sclk";
>>> 		#include "exynos5433-tmu-sensor-conf.dtsi"
>>> 		status = "disabled";
>>> 	};
>>>
>>> 3. In exynos5433-tmu.dtsi, thermal-zones includes both trip points and
>>> cooling-maps of g3d thermal sensor. Following cooling-maps show the match
>>> between each trip point and each cooling device (devfreq device of mali):
>>>
>>> 	thermal-zones {
>>> 		/* ...... */
>>> 		g3d_thermal: g3d-thermal {
>>> 			thermal-sensors = <&tmu_g3d>;
>>> 			polling-delay-passive = <0>;
>>> 			polling-delay = <0>;
>>> 			trips {
>>> 				g3d_alert_0: g3d-alert-0 {
>>> 					temperature = <30000>;	/* millicelsius */
>>> 					hysteresis = <10000>;	/* millicelsius */
>>> 					type = "active";
>>> 				};
>>> 				g3d_alert_1: g3d-alert-1 {
>>> 					temperature = <40000>;	/* millicelsius */
>>> 					hysteresis = <10000>;	/* millicelsius */
>>> 					type = "active";
>>> 				};
>>>
>>> 				/* ...... */
>>> 			};
>>>
>>> 			cooling-maps {
>>> 				map0 {
>>> 					/* Set maximum frequency as 550MHz  */
>>> 					trip = <&g3d_alert_0>;
>>> 					cooling-device = <&mali 2 2>;
>>> 				};
>>> 				map1 {
>>> 					/* Set maximum frequency as 420MHz  */
>>> 					trip = <&g3d_alert_1>;
>>> 					cooling-device = <&mali 4 4>;
>>> 				};
>>>
>>> 				/* ...... */
>>> 			};
>>> 		};
>>>
>>> 		......
>>> 	};
>>>
>>> [1] https://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/commit/?h=v4.3-next/dt64-samsung&id=ac008f6b537703bb9a6fcc3882ca4af3331aa24f
>>> [2] https://git.kernel.org/cgit/linux/kernel/git/kgene/linux-samsung.git/commit/?h=v4.3-next/dt64-samsung&id=bcddc3a84e49ca1c646cf2081687a544a15f9218
>>> [3] malideveloper.arm.com/downloads/drivers/TX041/r5p0-06rel0/TX041-SW-99002-r5p0-06rel0.tgz
>>>
>>> Chanwoo Choi (2):
>>>   PM: devfreq: Add the prototype of update_devfreq() to export
>>>   thermal: devfreq_cooling: Add generic devfreq cooling device implementaion
>>>
>>>  .../devicetree/bindings/thermal/thermal.txt        |   8 +-
>>>  drivers/devfreq/devfreq.c                          |  22 +-
>>>  drivers/thermal/Kconfig                            |  11 +
>>>  drivers/thermal/Makefile                           |   3 +
>>>  drivers/thermal/devfreq-cooling.c                  | 309 +++++++++++++++++++++
>>>  include/linux/devfreq-cooling.h                    |  80 ++++++
>>>  include/linux/devfreq.h                            |   7 +
>>>  7 files changed, 425 insertions(+), 15 deletions(-)
>>>  create mode 100644 drivers/thermal/devfreq-cooling.c
>>>  create mode 100644 include/linux/devfreq-cooling.h
>> 
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux