Hi Punit, On 07/20/2015 11:43 PM, Punit Agrawal wrote: > 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 OK. I'll check it. > >> 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. First of all, I'm not opposite to apply power_allocator to cooling device (CPU, Devfreq). I think that thermal framework may need the basic patch-set for devfreq cooling device as cpu cooling device without power allocator. And then the additional patch-set(e.g., to apply power_allocator) will be implemented on basic patchset. The your proposed patch-set[1] include the two features with power_allocator. Lastly, I want to simplify the devfreq cooling device driver without unneed/additional functions as I explained on previous reply. Thanks, Chanwoo Choi > > 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 linux-kernel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- 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
