Re: [PATCH v3 0/2] thermal: mediatek: Add cpu power cooling model
On Fri, 2015-11-06 at 08:50 +0530, Viresh Kumar wrote: > On 05-11-15, 19:09, dawei chien wrote: > > Thank you for your kindly explaining, now I could understand what I > > miss, I will send device tree binding on next version such like > > following description. > > > > --- a/Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt > > +++ b/Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt > > @@ -10,6 +10,17 @@ Required properties: > > Please refer to > > Documentation/devicetree/bindings/clk/clock-bindings.txt for > > generic clock consumer properties. > > - proc-supply: Regulator for Vproc of CPU cluster. > > +- dynamic-power-coefficient: > > + Usage: optional > > + Value type: > > + Definition: A u32 value that represents an indicative > > + running time dynamic power coefficient in > > + fundamental units of mW/MHz/uVolt^2. > > + The dynamic energy consumption of the CPU > > + is proportional to the square of the > > + Voltage (V) and the clock frequency (f). > > + Pdyn = dynamic-power-coefficient * V^2 * f > > + where voltage is in uV, frequency is in MHz. > > Please check with Punit if he is planning to add the same. Punit just sent the patch for this binding[1] yesterday, so I will re-send next version once his patch has been reviewed. > > Thank you for your kindly explaining, Now I know I should develop and > > test on mainline branch since this is where I try to add code. > > > > However, please understanding currently mt8173_cpufreq.c is not ready > > for OPPV2 in mainline as far, that's the reason why currently I can't > > reuse OPPV2 and extend for static power table. My propose is for adding > > CPU cooling device for our own product. > > Firstly, we don't care. You are pushing something to mainline, you > have to get it tested someway on mainline. > > Secondly, there are *almost* no changes required to the mtk cpufreq > driver for OPPV2. Just update your DT in a similar way it is done for > one of the exynos platforms and it should just work fine. > In our platform, thermal throttling is good enough with dynamic power only, so my plan is to send dynamic power model first in next version. Regarding static power model, we will continue discussing with ARM to find a better solution. Thanks. [1] https://lkml.org/lkml/2015/11/9/542 BR, Dawei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 0/2] thermal: mediatek: Add cpu power cooling model
On 05-11-15, 19:09, dawei chien wrote: > Thank you for your kindly explaining, now I could understand what I > miss, I will send device tree binding on next version such like > following description. > > --- a/Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt > +++ b/Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt > @@ -10,6 +10,17 @@ Required properties: > Please refer to > Documentation/devicetree/bindings/clk/clock-bindings.txt for > generic clock consumer properties. > - proc-supply: Regulator for Vproc of CPU cluster. > +- dynamic-power-coefficient: > + Usage: optional > + Value type: > + Definition: A u32 value that represents an indicative > + running time dynamic power coefficient in > + fundamental units of mW/MHz/uVolt^2. > + The dynamic energy consumption of the CPU > + is proportional to the square of the > + Voltage (V) and the clock frequency (f). > + Pdyn = dynamic-power-coefficient * V^2 * f > + where voltage is in uV, frequency is in MHz. Please check with Punit if he is planning to add the same.. > Thank you for your kindly explaining, Now I know I should develop and > test on mainline branch since this is where I try to add code. > > However, please understanding currently mt8173_cpufreq.c is not ready > for OPPV2 in mainline as far, that's the reason why currently I can't > reuse OPPV2 and extend for static power table. My propose is for adding > CPU cooling device for our own product. Firstly, we don't care. You are pushing something to mainline, you have to get it tested someway on mainline. Secondly, there are *almost* no changes required to the mtk cpufreq driver for OPPV2. Just update your DT in a similar way it is done for one of the exynos platforms and it should just work fine. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 0/2] thermal: mediatek: Add cpu power cooling model
On Mon, 2015-11-02 at 17:40 +0530, Viresh Kumar wrote: > On 02-11-15, 18:46, dawei chien wrote: > > On Wed, 2015-10-28 at 21:14 +0530, Viresh Kumar wrote: > > > Sorry for being extremely late in reviewing this stuff. You are > > > already on v3 and I haven't reviewed it once. Mostly due to bad timing > > > of my holidays and other work pressure. > > > > You're welcome, truly thank you for your kindly reviewing > > Thanks for understanding. > > > > Now, there are few things that I feel are not properly addressed here, > > > and I may be wrong: > > > - Where are the bindings for static-power-points and > > > dynamic-power-coefficient. Sorry I failed to see them in this or > > > other series you mentioned. > > > > Please refer to following document (2-1,2-2) for dynamic-power & > > static-power in detail. Besides, do I need to add another document for > > our own MT8173 IC. > > http://lxr.free-electrons.com/source/Documentation/thermal/cpu-cooling-api.txt > > That's about the power-API, but I am talking about the Device Tree > bindings here. So, when you add any new DT bindings (Or a new property > in device tree blobs), you need to add its documentation in > Documentation/devicetree/bindings/... and get it approved by DT > maintainers as well. You perhaps missed that completely, otherwise you > would have been told really early that the new bindings aren't going > to help. Thank you for your kindly explaining, now I could understand what I miss, I will send device tree binding on next version such like following description. --- a/Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt +++ b/Documentation/devicetree/bindings/clock/mt8173-cpu-dvfs.txt @@ -10,6 +10,17 @@ Required properties: Please refer to Documentation/devicetree/bindings/clk/clock-bindings.txt for generic clock consumer properties. - proc-supply: Regulator for Vproc of CPU cluster. +- dynamic-power-coefficient: + Usage: optional + Value type: + Definition: A u32 value that represents an indicative + running time dynamic power coefficient in + fundamental units of mW/MHz/uVolt^2. + The dynamic energy consumption of the CPU + is proportional to the square of the + Voltage (V) and the clock frequency (f). + Pdyn = dynamic-power-coefficient * V^2 * f + where voltage is in uV, frequency is in MHz. > > > - Even then, why should we be adding another table into DT for > > > voltage/power ? And not reuse and extend the opp-v2 stuff which is > > > already mainlined now. > > > > We could reuse opp-v2 for static power points after OPPV2 back port to > > our currently branch. > > Your current branch doesn't matter to us. All that matters here is > mainline, that's where you are adding code to. And you must test your > stuff on the latest upstream branch only, not on some old kernel > release. You can include other dependency patches though, that are > required to make it work and mention them in cover-letter. Thank you for your kindly explaining, Now I know I should develop and test on mainline branch since this is where I try to add code. However, please understanding currently mt8173_cpufreq.c is not ready for OPPV2 in mainline as far, that's the reason why currently I can't reuse OPPV2 and extend for static power table. My propose is for adding CPU cooling device for our own product. Actually, I would like to remove static power table on next patch since static power is optional and dynamic power is much more than static power. > > However, as far as I know, there is no "power" in opp.c (suck like > > s/suck/such ? > > > opp-hz) as far, so I need to add something in opp.c for my purpose, suck > > like add power in _opp_add_static_v2, and add something for return > > "power", right? I may be wrong, please kindly give me your suggestion, > > thank you. > > You first need to propose a change in DT bindings for OPPs: > Documentation/devicetree/bindings/opp/opp.txt > > And then we can change the code properly. > > > Actually, I am considering to remove the part of static power point > > since it is optional for Power Model. Could you agree with this? > > If its not important for your platform, then I don't have any issues > with that.. > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 0/2] thermal: mediatek: Add cpu power cooling model
On Mon, Nov 02, 2015 at 03:53:33PM +, Punit Agrawal wrote: > Viresh Kumar writes: > > > On 22-10-15, 20:02, Dawei Chien wrote: > >> Use Intelligent Power Allocation (IPA) technical to add static/dynamic > >> power model for binding CPU thermal zone. > >> The power allocator governor allocates power budget to control CPU > >> temperature. > >> > >> Power Allocator governor is able to keep SOC temperature within a > >> defined temperature range to avoid SOC overheat and keep it's > >> performance. mt8173-cpufreq.c need to register its' own power model > >> with power allocator thermal governor, so that power allocator > >> governor can allocates suitable power budget to control CPU > >> temperature. > >> > >> PATCH1 is base on > >> https://patchwork.kernel.org/patch/7034601/ > >> > >> PATCH2 is base on Sascha's thermal driver V9 > >> https://patchwork.kernel.org/patch/7249821/ > >> https://patchwork.kernel.org/patch/7249861/ > >> https://patchwork.kernel.org/patch/7249891/ > >> > >> Change since V1: > >> include mt8171.h and sort header file for mt8173.dtsi > >> > >> Change since V2: > >> Move dynamic/static power model in device tree > >> > >> Dawei.Chien (2): > >> thermal: mediatek: Add cpu power cooling model. > >> arm64: dts: mt8173: Add thermal zone node for mt8173. > > > > Sorry for being extremely late in reviewing this stuff. You are > > already on v3 and I haven't reviewed it once. Mostly due to bad timing > > of my holidays and other work pressure. > > > > Now, there are few things that I feel are not properly addressed here, > > and I may be wrong: > > - Where are the bindings for static-power-points and > > dynamic-power-coefficient. Sorry I failed to see them in this or > > other series you mentioned. > > For dynamic power, I had posted some patches[0][1][2] introducing the > binding as well as updating cooling device registration via cpufreq > driver. Now that the SCPI hwmon driver is merged, I should re-send the > remaining patches. > > [0] http://lkml.iu.edu/hypermail/linux/kernel/1508.0/01020.html Are you sure this binding is applicable only to ARM cpus? > [1] http://lkml.iu.edu/hypermail/linux/kernel/1508.0/01022.html > [3] http://lkml.iu.edu/hypermail/linux/kernel/1508.0/01031.html > > > - Even then, why should we be adding another table into DT for > > voltage/power ? And not reuse and extend the opp-v2 stuff which is > > already mainlined now. > > - There are few issues with the code as well, but I want to see where > > the bindings should go first. And then only discuss the code > > further. > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majord...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 0/2] thermal: mediatek: Add cpu power cooling model
On 02-11-15, 15:53, Punit Agrawal wrote: > For dynamic power, I had posted some patches[0][1][2] introducing the > binding as well as updating cooling device registration via cpufreq > driver. Now that the SCPI hwmon driver is merged, I should re-send the > remaining patches. > > [0] http://lkml.iu.edu/hypermail/linux/kernel/1508.0/01020.html > [1] http://lkml.iu.edu/hypermail/linux/kernel/1508.0/01022.html > [3] http://lkml.iu.edu/hypermail/linux/kernel/1508.0/01031.html Sure. Just that whatever can be merged with opp-v2 should be merged. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 0/2] thermal: mediatek: Add cpu power cooling model
Viresh Kumar writes: > On 22-10-15, 20:02, Dawei Chien wrote: >> Use Intelligent Power Allocation (IPA) technical to add static/dynamic power >> model for binding CPU thermal zone. >> The power allocator governor allocates power budget to control CPU >> temperature. >> >> Power Allocator governor is able to keep SOC temperature within a >> defined temperature range to avoid SOC overheat and keep it's >> performance. mt8173-cpufreq.c need to register its' own power model >> with power allocator thermal governor, so that power allocator >> governor can allocates suitable power budget to control CPU >> temperature. >> >> PATCH1 is base on >> https://patchwork.kernel.org/patch/7034601/ >> >> PATCH2 is base on Sascha's thermal driver V9 >> https://patchwork.kernel.org/patch/7249821/ >> https://patchwork.kernel.org/patch/7249861/ >> https://patchwork.kernel.org/patch/7249891/ >> >> Change since V1: >> include mt8171.h and sort header file for mt8173.dtsi >> >> Change since V2: >> Move dynamic/static power model in device tree >> >> Dawei.Chien (2): >> thermal: mediatek: Add cpu power cooling model. >> arm64: dts: mt8173: Add thermal zone node for mt8173. > > Sorry for being extremely late in reviewing this stuff. You are > already on v3 and I haven't reviewed it once. Mostly due to bad timing > of my holidays and other work pressure. > > Now, there are few things that I feel are not properly addressed here, > and I may be wrong: > - Where are the bindings for static-power-points and > dynamic-power-coefficient. Sorry I failed to see them in this or > other series you mentioned. For dynamic power, I had posted some patches[0][1][2] introducing the binding as well as updating cooling device registration via cpufreq driver. Now that the SCPI hwmon driver is merged, I should re-send the remaining patches. [0] http://lkml.iu.edu/hypermail/linux/kernel/1508.0/01020.html [1] http://lkml.iu.edu/hypermail/linux/kernel/1508.0/01022.html [3] http://lkml.iu.edu/hypermail/linux/kernel/1508.0/01031.html > - Even then, why should we be adding another table into DT for > voltage/power ? And not reuse and extend the opp-v2 stuff which is > already mainlined now. > - There are few issues with the code as well, but I want to see where > the bindings should go first. And then only discuss the code > further. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 0/2] thermal: mediatek: Add cpu power cooling model
On 02-11-15, 18:46, dawei chien wrote: > On Wed, 2015-10-28 at 21:14 +0530, Viresh Kumar wrote: > > Sorry for being extremely late in reviewing this stuff. You are > > already on v3 and I haven't reviewed it once. Mostly due to bad timing > > of my holidays and other work pressure. > > You're welcome, truly thank you for your kindly reviewing Thanks for understanding. > > Now, there are few things that I feel are not properly addressed here, > > and I may be wrong: > > - Where are the bindings for static-power-points and > > dynamic-power-coefficient. Sorry I failed to see them in this or > > other series you mentioned. > > Please refer to following document (2-1,2-2) for dynamic-power & > static-power in detail. Besides, do I need to add another document for > our own MT8173 IC. > http://lxr.free-electrons.com/source/Documentation/thermal/cpu-cooling-api.txt That's about the power-API, but I am talking about the Device Tree bindings here. So, when you add any new DT bindings (Or a new property in device tree blobs), you need to add its documentation in Documentation/devicetree/bindings/... and get it approved by DT maintainers as well. You perhaps missed that completely, otherwise you would have been told really early that the new bindings aren't going to help. > > - Even then, why should we be adding another table into DT for > > voltage/power ? And not reuse and extend the opp-v2 stuff which is > > already mainlined now. > > We could reuse opp-v2 for static power points after OPPV2 back port to > our currently branch. Your current branch doesn't matter to us. All that matters here is mainline, that's where you are adding code to. And you must test your stuff on the latest upstream branch only, not on some old kernel release. You can include other dependency patches though, that are required to make it work and mention them in cover-letter. > However, as far as I know, there is no "power" in opp.c (suck like s/suck/such ? > opp-hz) as far, so I need to add something in opp.c for my purpose, suck > like add power in _opp_add_static_v2, and add something for return > "power", right? I may be wrong, please kindly give me your suggestion, > thank you. You first need to propose a change in DT bindings for OPPs: Documentation/devicetree/bindings/opp/opp.txt And then we can change the code properly. > Actually, I am considering to remove the part of static power point > since it is optional for Power Model. Could you agree with this? If its not important for your platform, then I don't have any issues with that.. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 0/2] thermal: mediatek: Add cpu power cooling model
Hi Viresh, On Wed, 2015-10-28 at 21:14 +0530, Viresh Kumar wrote: > On 22-10-15, 20:02, Dawei Chien wrote: > > Use Intelligent Power Allocation (IPA) technical to add static/dynamic > > power model for binding CPU thermal zone. > > The power allocator governor allocates power budget to control CPU > > temperature. > > > > Power Allocator governor is able to keep SOC temperature within a defined > > temperature range to avoid SOC overheat and keep it's performance. > > mt8173-cpufreq.c need to register its' own power model with power allocator > > thermal governor, so that power allocator governor can allocates suitable > > power budget to control CPU temperature. > > > > PATCH1 is base on > > https://patchwork.kernel.org/patch/7034601/ > > > > PATCH2 is base on Sascha's thermal driver V9 > > https://patchwork.kernel.org/patch/7249821/ > > https://patchwork.kernel.org/patch/7249861/ > > https://patchwork.kernel.org/patch/7249891/ > > > > Change since V1: > > include mt8171.h and sort header file for mt8173.dtsi > > > > Change since V2: > > Move dynamic/static power model in device tree > > > > Dawei.Chien (2): > > thermal: mediatek: Add cpu power cooling model. > > arm64: dts: mt8173: Add thermal zone node for mt8173. > > Sorry for being extremely late in reviewing this stuff. You are > already on v3 and I haven't reviewed it once. Mostly due to bad timing > of my holidays and other work pressure. You're welcome, truly thank you for your kindly reviewing > Now, there are few things that I feel are not properly addressed here, > and I may be wrong: > - Where are the bindings for static-power-points and > dynamic-power-coefficient. Sorry I failed to see them in this or > other series you mentioned. Please refer to following document (2-1,2-2) for dynamic-power & static-power in detail. Besides, do I need to add another document for our own MT8173 IC. http://lxr.free-electrons.com/source/Documentation/thermal/cpu-cooling-api.txt > - Even then, why should we be adding another table into DT for > voltage/power ? And not reuse and extend the opp-v2 stuff which is > already mainlined now. We could reuse opp-v2 for static power points after OPPV2 back port to our currently branch. However, as far as I know, there is no "power" in opp.c (suck like opp-hz) as far, so I need to add something in opp.c for my purpose, suck like add power in _opp_add_static_v2, and add something for return "power", right? I may be wrong, please kindly give me your suggestion, thank you. Actually, I am considering to remove the part of static power point since it is optional for Power Model. Could you agree with this? > - There are few issues with the code as well, but I want to see where > the bindings should go first. And then only discuss the code > further. > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 0/2] thermal: mediatek: Add cpu power cooling model
On 22-10-15, 20:02, Dawei Chien wrote: > Use Intelligent Power Allocation (IPA) technical to add static/dynamic power > model for binding CPU thermal zone. > The power allocator governor allocates power budget to control CPU > temperature. > > Power Allocator governor is able to keep SOC temperature within a defined > temperature range to avoid SOC overheat and keep it's performance. > mt8173-cpufreq.c need to register its' own power model with power allocator > thermal governor, so that power allocator governor can allocates suitable > power budget to control CPU temperature. > > PATCH1 is base on > https://patchwork.kernel.org/patch/7034601/ > > PATCH2 is base on Sascha's thermal driver V9 > https://patchwork.kernel.org/patch/7249821/ > https://patchwork.kernel.org/patch/7249861/ > https://patchwork.kernel.org/patch/7249891/ > > Change since V1: > include mt8171.h and sort header file for mt8173.dtsi > > Change since V2: > Move dynamic/static power model in device tree > > Dawei.Chien (2): > thermal: mediatek: Add cpu power cooling model. > arm64: dts: mt8173: Add thermal zone node for mt8173. Sorry for being extremely late in reviewing this stuff. You are already on v3 and I haven't reviewed it once. Mostly due to bad timing of my holidays and other work pressure. Now, there are few things that I feel are not properly addressed here, and I may be wrong: - Where are the bindings for static-power-points and dynamic-power-coefficient. Sorry I failed to see them in this or other series you mentioned. - Even then, why should we be adding another table into DT for voltage/power ? And not reuse and extend the opp-v2 stuff which is already mainlined now. - There are few issues with the code as well, but I want to see where the bindings should go first. And then only discuss the code further. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 0/2] thermal: mediatek: Add cpu power cooling model
Use Intelligent Power Allocation (IPA) technical to add static/dynamic power model for binding CPU thermal zone. The power allocator governor allocates power budget to control CPU temperature. Power Allocator governor is able to keep SOC temperature within a defined temperature range to avoid SOC overheat and keep it's performance. mt8173-cpufreq.c need to register its' own power model with power allocator thermal governor, so that power allocator governor can allocates suitable power budget to control CPU temperature. PATCH1 is base on https://patchwork.kernel.org/patch/7034601/ PATCH2 is base on Sascha's thermal driver V9 https://patchwork.kernel.org/patch/7249821/ https://patchwork.kernel.org/patch/7249861/ https://patchwork.kernel.org/patch/7249891/ Change since V1: include mt8171.h and sort header file for mt8173.dtsi Change since V2: Move dynamic/static power model in device tree Dawei.Chien (2): thermal: mediatek: Add cpu power cooling model. arm64: dts: mt8173: Add thermal zone node for mt8173. arch/arm64/boot/dts/mediatek/mt8173.dtsi | 44 ++ drivers/cpufreq/mt8173-cpufreq.c | 97 ++ 2 files changed, 130 insertions(+), 11 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
