From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753417AbbGWBCK (ORCPT ); Wed, 22 Jul 2015 21:02:10 -0400 Received: from mailout2.samsung.com ([203.254.224.25]:55686 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753300AbbGWBCH (ORCPT ); Wed, 22 Jul 2015 21:02:07 -0400 X-AuditID: cbfee68d-f79106d00000728c-2a-55b03d0c5a9a Message-id: <55B03D0B.3080600@samsung.com> Date: Thu, 23 Jul 2015 10:02:03 +0900 From: Chanwoo Choi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 MIME-version: 1.0 To: Punit Agrawal Cc: edubezval@gmail.com, rui.zhang@intel.com, myungjoo.ham@samsung.com, kyungmin.park@samsung.com, ulf.hansson@linaro.org, khilman@linaro.org, robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, inki.dae@samsung.com, l.majewski@samsung.com, kgene.kim@samsung.com, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [RFC PATCH 0/2] thermal: Add generic devfreq cooling device References: <1437048173-23744-1-git-send-email-cw00.choi@samsung.com> <9hhh9p3caxm.fsf@e105922-lin.cambridge.arm.com> <55A8FA48.4030304@samsung.com> <9hhr3o2evo2.fsf@e105922-lin.cambridge.arm.com> In-reply-to: <9hhr3o2evo2.fsf@e105922-lin.cambridge.arm.com> Content-type: text/plain; charset=ISO-8859-1 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrCIsWRmVeSWpSXmKPExsWyRsSkWJfHdkOowd9dPBbzj5xjtZh/5Rqr xblXKxktJt2fwGLRu+Aqm8XXwysYLc42vWG3ePNwM6PF5V1z2Cw+9x5htFh6/SKTxe3GFWwW E6avZbF4/v40k0Xr3iPsFk8e9rFZHF8b7iDosWbeGkaPnbPusnusXP6FzWPxnpdMHptWdbJ5 3Lm2h82jb8sqRo/Pm+QCOKK4bFJSczLLUov07RK4Mi7e2sVY8NujYtn3BuYGxt+WXYycHBIC JhJn/x5mh7DFJC7cW88GYgsJLGWUePISrubtzxnMXYxcQPFFjBJnNi9mgih6wChx4awBiM0r oCVx5eNBsEEsAqoSnRsvsoLYbEDx/S9ugA0VFQiTWDn9CgtEvaDEj8n3wGwRAU2JfbdOM4Es YBboZZb42rYarFlYwENixdOLTBCbTzBKnOxeA7aZU8BG4t30z2DbmAV0JPa3TmODsOUlNq95 C3aqhMBaDonPD6YxQ5wkIPFt8iGgdRxACVmJTQeYIV6TlDi44gbLBEaxWUiOmoVk7CwkYxcw Mq9iFE0tSC4oTkovMtQrTswtLs1L10vOz93ECIz80/+e9e5gvH3A+hCjAAejEg/vhKPrQoVY E8uKK3MPMZoCXTGRWUo0OR+YXvJK4g2NzYwsTE1MjY3MLc2UxHkVpX4GCwmkJ5akZqemFqQW xReV5qQWH2Jk4uCUamB0WxDyjdd0w64vhg6MWWf/WV3XWqbMwF1w76l6Zqs265UX86QidjVm F7Ilvav/enM/V4/IztJv867rXzPstdbKyWFJWy/8YUHm0o0TLZPjJ3yR/2Te+VXBoF/seY// JNnCSYxa5568f/31eO/nL9t1dZYc2vC44JrsjINK4tqC0TnCFTO1pS2VWIozEg21mIuKEwFo P0Lr9wIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrPKsWRmVeSWpSXmKPExsVy+t9jAV0e2w2hBjvbWS3mHzkHJK5cY7U4 92olo8Wk+xNYLHoXXGWz+Hp4BaPF2aY37BZvHm5mtLi8aw6bxefeI4wWS69fZLK43biCzWLC 9LUsFs/fn2ayaN17hN3iycM+Novja8MdBD3WzFvD6LFz1l12j5XLv7B5LN7zkslj06pONo87 1/awefRtWcXo8XmTXABHVAOjTUZqYkpqkUJqXnJ+SmZeuq2Sd3C8c7ypmYGhrqGlhbmSQl5i bqqtkotPgK5bZg7QH0oKZYk5pUChgMTiYiV9O0wTQkPcdC1gGiN0fUOC4HqMDNBAwhrGjIu3 djEW/PaoWPa9gbmB8bdlFyMnh4SAicTbnzOYIWwxiQv31rN1MXJxCAksYpQ4s3kxE0hCSOAB o8SFswYgNq+AlsSVjwfZQWwWAVWJzo0XWUFsNqD4/hc32EBsUYEwiZXTr7BA1AtK/Jh8D8wW EdCU2HfrNBPIAmaBXmaJr22rwZqFBTwkVjy9yASx+QSjxMnuNWCbOQVsJN5N/wy2jVlAR2J/ 6zQ2CFteYvOat8wTGAVmIVkyC0nZLCRlCxiZVzGKphYkFxQnpeca6RUn5haX5qXrJefnbmIE J5Zn0jsYVzVYHGIU4GBU4uGdcHRdqBBrYllxZe4hRgkOZiUR3gqRDaFCvCmJlVWpRfnxRaU5 qcWHGE2BYTCRWUo0OR+Y9PJK4g2NTcyMLI3MDS2MjM2VxHlP5vuECgmkJ5akZqemFqQWwfQx cXBKNTBKrbZrfeKzjvvFZOd95YpWUc7LDVqkXqT9vlhxZKWH8u2LuvxPWesTE11uqjEXLLde /Yg3M2+ejutPs9vmSn5f9c/vXt3GefbZn1tLJmcc6FdV972d2XM3IkJM1Ktta6m2FvOqSsPq NVkqe7ZGC+lteqQrsmbRs7VViTqnyw93PdV7bnPg1E8lluKMREMt5qLiRABEp3SFQgMAAA== DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Punit, On 07/20/2015 11:43 PM, Punit Agrawal wrote: > Hi Chanwoo, > > Chanwoo Choi writes: > >> Hi Punit, >> >> On 07/17/2015 07:53 PM, Punit Agrawal wrote: >>> Hi Chanwoo, >>> >>> Chanwoo Choi 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@vger.kernel.org >> 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >