| Message ID | 1516721671-16360-5-git-send-email-daniel.lezcano@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | CPU cooling device new strategies | expand |
On Wed, Jan 24, 2018 at 05:59:09PM +0100, Daniel Lezcano wrote: > On 24/01/2018 17:34, Daniel Thompson wrote: > > On Tue, Jan 23, 2018 at 04:34:27PM +0100, Daniel Lezcano wrote: > >> The next changes will add new way to cool down a CPU. In order to > >> sanitize and make the overall cpu cooling code consistent and robust > >> we must prevent the cpu cooling devices to co-exists with the same > >> purpose at the same time in the kernel. > >> > >> Make the CPU cooling device a choice in the Kconfig, so only one CPU > >> cooling strategy can be chosen. > > > > I puzzled by the role of Kconfig here. > > > > IIUC a distro kernel will always be forced to select the combo strategy > > otherwise it will never be able to cool systems that don't have cpufreq > > (I hope the combo strategy treats such system as a special case with > > only one OPP). > > Actually it does not make sense to select the combo if there is no > cpufreq support. The cpuidle cooling device must be used instead. Well, I said before what I hoped. This is what I feared! ;-) > > This raises the question what the other options (cpufreq-only > > idle-injection-only) are for? Are they just for petrol heads who want to > > save a few bytes of code or is idle-injection undesirable for some > > users. > > The combo cooling device must be used on a system with a proper support > for cpuidle and cpufreq and with the power numbers specified in the DT. > > By proper support of cpuidle, I mean a cluster power down idle state, > fast enough. This idle state allows to drop the dynamic power *and* the > static leakage (the latter to prevent a thermal runaway). > > If the system does not have power numbers, no (or bad) cpuidle, the > combo cooling device must not be used. If there is no cpufreq support, > the cpuidle cooling must be used and if there is no proper support for > both, the CPU cooling can't be used. In this case, you have to put a fan > on your board or reduce the frequency where the system stays in its > thermal envelope. How can we know (in the general case) what is going to be in the DT at compile time? > > If the later, how can a distro kernel mitigate the undesired effects > > whilst still selecting the combo strategy. > > I'm not sure to understand the question. Distros always use the make > allmodconfig, so that chooses the cpufreq CPU cooling device which was > the case before without this change. So there's no regression. That's nice but doesn't that mean distros cannot exploit the new features. > However, we are talking about distros here but the CPU cooling mechanism > is for mobile and in this case the kernel (usually Android based) comes > with a specific configuration file and this is where the SoC vendor has > to choose the right strategy. I agree its hard to conceive of a modern Android device that doesn't implement both the needed features but the performance figures in the covering letter come from Hikey (and they look pretty good) and Hikey is supported by a good number of regular Linux distros now. Using Kconfig to force what should be runtime decisions to happen at compile time means that Hikey becomes an example of a platform that is unable to run at max performance on the distros that have added support for it. Daniel. > >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> > >> --- > >> drivers/thermal/Kconfig | 20 +++++++++++++++++--- > >> 1 file changed, 17 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig > >> index 315ae29..925e73b 100644 > >> --- a/drivers/thermal/Kconfig > >> +++ b/drivers/thermal/Kconfig > >> @@ -142,17 +142,31 @@ config THERMAL_GOV_POWER_ALLOCATOR > >> allocating and limiting power to devices. > >> > >> config CPU_THERMAL > >> - bool "generic cpu cooling support" > >> - depends on CPU_FREQ > >> + bool "Generic cpu cooling support" > >> depends on THERMAL_OF > >> help > >> + Enable the CPU cooling features. If the system has no active > >> + cooling device available, this option allows to use the CPU > >> + as a cooling device. > >> + > >> +choice > >> + prompt "CPU cooling strategies" > >> + depends on CPU_THERMAL > >> + default CPU_FREQ_THERMAL > >> + help > >> + Select the CPU cooling strategy. > >> + > >> +config CPU_FREQ_THERMAL > >> + bool "CPU frequency cooling strategy" > >> + depends on CPU_FREQ > >> + help > >> This implements the generic cpu cooling mechanism through frequency > >> reduction. An ACPI version of this already exists > >> (drivers/acpi/processor_thermal.c). > >> 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 here. > > > > ... this may not be great advice... if you think you want this support > > then you *probably* actually want the comboappears to be terrible advice. > > > > This cooling strategies should only be selected by petrolheads making a > > device specific *and* are obsessive about code size. > > > > The > >> +endchoice > >> > >> config CLOCK_THERMAL > >> bool "Generic clock cooling support" > >> -- > >> 2.7.4 > >> > > > -- > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | > <http://twitter.com/#!/linaroorg> Twitter | > <http://www.linaro.org/linaro-blog/> Blog >
On 25/01/2018 11:57, Daniel Thompson wrote: > On Wed, Jan 24, 2018 at 05:59:09PM +0100, Daniel Lezcano wrote: >> On 24/01/2018 17:34, Daniel Thompson wrote: >>> On Tue, Jan 23, 2018 at 04:34:27PM +0100, Daniel Lezcano wrote: >>>> The next changes will add new way to cool down a CPU. In order to >>>> sanitize and make the overall cpu cooling code consistent and robust >>>> we must prevent the cpu cooling devices to co-exists with the same >>>> purpose at the same time in the kernel. >>>> >>>> Make the CPU cooling device a choice in the Kconfig, so only one CPU >>>> cooling strategy can be chosen. >>> >>> I puzzled by the role of Kconfig here. >>> >>> IIUC a distro kernel will always be forced to select the combo strategy >>> otherwise it will never be able to cool systems that don't have cpufreq >>> (I hope the combo strategy treats such system as a special case with >>> only one OPP). >> >> Actually it does not make sense to select the combo if there is no >> cpufreq support. The cpuidle cooling device must be used instead. > > Well, I said before what I hoped. This is what I feared! ;-) > > >>> This raises the question what the other options (cpufreq-only >>> idle-injection-only) are for? Are they just for petrol heads who want to >>> save a few bytes of code or is idle-injection undesirable for some >>> users. >> >> The combo cooling device must be used on a system with a proper support >> for cpuidle and cpufreq and with the power numbers specified in the DT. >> >> By proper support of cpuidle, I mean a cluster power down idle state, >> fast enough. This idle state allows to drop the dynamic power *and* the >> static leakage (the latter to prevent a thermal runaway). >> >> If the system does not have power numbers, no (or bad) cpuidle, the >> combo cooling device must not be used. If there is no cpufreq support, >> the cpuidle cooling must be used and if there is no proper support for >> both, the CPU cooling can't be used. In this case, you have to put a fan >> on your board or reduce the frequency where the system stays in its >> thermal envelope. > > How can we know (in the general case) what is going to be in the DT at > compile time? > > >>> If the later, how can a distro kernel mitigate the undesired effects >>> whilst still selecting the combo strategy. >> >> I'm not sure to understand the question. Distros always use the make >> allmodconfig, so that chooses the cpufreq CPU cooling device which was >> the case before without this change. > > So there's no regression. That's nice but doesn't that mean distros > cannot exploit the new features. > > >> However, we are talking about distros here but the CPU cooling mechanism >> is for mobile and in this case the kernel (usually Android based) comes >> with a specific configuration file and this is where the SoC vendor has >> to choose the right strategy. > > I agree its hard to conceive of a modern Android device that doesn't implement > both the needed features but the performance figures in the covering > letter come from Hikey (and they look pretty good) and Hikey is > supported by a good number of regular Linux distros now. > > Using Kconfig to force what should be runtime decisions to happen at > compile time means that Hikey becomes an example of a platform that > is unable to run at max performance on the distros that have added > support for it. I disagree. The ARM64's defconfig is not distro ready. We have always to change the default option and fix things in the Kconfig to make the hikey to work (eg. the missing hi655x clock config), without speaking about the hikey960 which is not yet ready for full support. Hence, tweaking the Kconfig to choose the better strategy is not necessarily a problem for this first iteration of code. Note I'm not against changing the code to make it runtime selectable but that will need a major rework of the current CPU cooling code especially handling the change while the thermal framework is doing the mitigation (and probably also changes of the thermal framework). I prefer to keep simple self-encapsulated feature code and make it evolve to something better instead of sending a blowing patch series taking into account all possible combinations. Choosing the strategy at compile time may be look restrictive but we can live with that without problem and iteratively do the change until the choice becomes the default strategy selection option. <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
On 26/01/2018 13:16, Daniel Thompson wrote: > On Thu, Jan 25, 2018 at 02:36:12PM +0100, Daniel Lezcano wrote: >> On 25/01/2018 11:57, Daniel Thompson wrote: >>> On Wed, Jan 24, 2018 at 05:59:09PM +0100, Daniel Lezcano wrote: >>>> On 24/01/2018 17:34, Daniel Thompson wrote: >>>> However, we are talking about distros here but the CPU cooling mechanism >>>> is for mobile and in this case the kernel (usually Android based) comes >>>> with a specific configuration file and this is where the SoC vendor has >>>> to choose the right strategy. >>> >>> I agree its hard to conceive of a modern Android device that doesn't implement >>> both the needed features but the performance figures in the covering >>> letter come from Hikey (and they look pretty good) and Hikey is >>> supported by a good number of regular Linux distros now. >>> >>> Using Kconfig to force what should be runtime decisions to happen at >>> compile time means that Hikey becomes an example of a platform that >>> is unable to run at max performance on the distros that have added >>> support for it. >> >> I disagree. The ARM64's defconfig is not distro ready. We have always to >> change the default option and fix things in the Kconfig to make the >> hikey to work (eg. the missing hi655x clock config), without speaking >> about the hikey960 which is not yet ready for full support. >> >> Hence, tweaking the Kconfig to choose the better strategy is not >> necessarily a problem for this first iteration of code. > > The problem is not about tweaking config for a distro. No distro I know > defconfig kernels. Tweaking is normal and expected. > > The problem is that a distro cannot generate a config that performs > optimally on all devices that it supports because enabling one form of > CPU cooling prevents other forms from being selected. There is no > correct value for us to select if we don't know in advance what board > the kernel image will be loaded on. > > Given the massive work that has gone on (especially in ARM ecosystem) > to allow one kernel binary to support many boards at once it surprises > me to see new features proposed that cannot be exploited without > recompiling the kernel from platform to platform. > > >> Note I'm not against changing the code to make it runtime selectable but >> that will need a major rework of the current CPU cooling code especially >> handling the change while the thermal framework is doing the mitigation >> (and probably also changes of the thermal framework). > > Perhaps I should have distinguished more between runtime-meaning-boot-time > and runtime-meaning-full-system-operation. To be clear none of my > comments are about being able to enable/disable idle injection on a > fully running system. It is the impact on multi-platform binaries that > attracted my attention. > > >> I prefer to keep simple self-encapsulated feature code and make it >> evolve to something better instead of sending a blowing patch series >> taking into account all possible combinations. Choosing the strategy at >> compile time may be look restrictive but we can live with that without >> problem and iteratively do the change until the choice becomes the >> default strategy selection option. > > You won't find me arguing against iterative improvement. However I > did observe that the idle injection code consists almost exclusively > of new lines of code. Thus I don't understand why this new code > interferes with cpufreq cooling to the point that the existing cooling > options must compiled out of the kernel before we can exploit it. > > I can see the combo code does have tentacles in more places but even so > I have the same question. What prevents the existing cooling strategy > from being compiled at the same time. > > You appear to be saying that there's not yet enough infrastructure to > decide which type of cooler to instantiate[1] so its OK to hack around > the decision by forcing it to be made it at compile time. Even if we > want to force a decision by some means, is compile time really the best > point to do that? For the moment, yes. We are backward compatible, there is no change with the current feature. I don't see really the point of being so afraid with this compilation option, it is not the first time we have compile time code which migrates to runtime code. The important thing is we don't break the existing. Having the runtime selection is the objective after improving the CPU cooling devices. But this is far from being trivial and will need a rework of the cooling device / framework (and the changes grouped in a separate patch series). > [1] Is that because the DT isn't rich enough for us to figure out > the trade off between using real OPPs and virtual idle-injected > ones nor whether cpuidle entry/exit is fast enough for cooling > to be effective? Yes, it is one aspect. -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index 315ae29..925e73b 100644 --- a/drivers/thermal/Kconfig +++ b/drivers/thermal/Kconfig @@ -142,17 +142,31 @@ config THERMAL_GOV_POWER_ALLOCATOR allocating and limiting power to devices. config CPU_THERMAL - bool "generic cpu cooling support" - depends on CPU_FREQ + bool "Generic cpu cooling support" depends on THERMAL_OF help + Enable the CPU cooling features. If the system has no active + cooling device available, this option allows to use the CPU + as a cooling device. + +choice + prompt "CPU cooling strategies" + depends on CPU_THERMAL + default CPU_FREQ_THERMAL + help + Select the CPU cooling strategy. + +config CPU_FREQ_THERMAL + bool "CPU frequency cooling strategy" + depends on CPU_FREQ + help This implements the generic cpu cooling mechanism through frequency reduction. An ACPI version of this already exists (drivers/acpi/processor_thermal.c). 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 here. +endchoice config CLOCK_THERMAL bool "Generic clock cooling support"
The next changes will add new way to cool down a CPU. In order to sanitize and make the overall cpu cooling code consistent and robust we must prevent the cpu cooling devices to co-exists with the same purpose at the same time in the kernel. Make the CPU cooling device a choice in the Kconfig, so only one CPU cooling strategy can be chosen. Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> --- drivers/thermal/Kconfig | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) -- 2.7.4