| Message ID | 1374770011-22171-6-git-send-email-l.majewski@samsung.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On 25 July 2013 22:03, Lukasz Majewski <l.majewski@samsung.com> wrote: > For safety reasons new flag - CONFIG_CPU_FREQ_BOOST_SW has been added. > Only after selecting "EXYNOS Frequency Overclocking - Software" Kconfig You shouldn't mention Exynos here and must do exynos stuff at the end in a separate patch. This one must be generic. > option the software managed boost is enabled. It also selects thermal > subsystem to be compiled in. Thermal is necessary for disabling boost > and cooling down the device when overheating detected. > > Boost _MUST_NOT_ work without thermal subsystem with properly defined > overheating temperatures. > > This option doesn't affect x86's ACPI hardware managed boost support > (i.e. Intel, AMD). In this situation boost management is embedded at > hardware. > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com> > > --- > Changes for v6: > - CPU_FREQ_BOOST_SW [1] is now defined as "invisible" bool option. > - Platform dependent ARM_EXYNOS_CPU_FREQ_BOOST_SW config option has been > added. It depends on ARM_EXYNOS_CPUFREQ options and selects > EXYNOS_THERMAL with the main boost config [1]. > > Changes for v5: > - New patch > > drivers/cpufreq/Kconfig | 3 +++ > drivers/cpufreq/Kconfig.arm | 16 ++++++++++++++++ > 2 files changed, 19 insertions(+) > > diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig > index 534fcb8..3f058a3 100644 > --- a/drivers/cpufreq/Kconfig > +++ b/drivers/cpufreq/Kconfig > @@ -23,6 +23,9 @@ config CPU_FREQ_TABLE > config CPU_FREQ_GOV_COMMON > bool > > +config CPU_FREQ_BOOST_SW > + bool Invisible is fine but this must be disabled by default and must depend on thermal, rather than moving dependency on platform's config. -- 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
On Fri, 26 Jul 2013 15:54:56 +0530 Viresh Kumar wrote, > On 25 July 2013 22:03, Lukasz Majewski <l.majewski@samsung.com> wrote: > > For safety reasons new flag - CONFIG_CPU_FREQ_BOOST_SW has been > > added. Only after selecting "EXYNOS Frequency Overclocking - > > Software" Kconfig > > You shouldn't mention Exynos here and must do exynos stuff at the end > in a separate patch. This one must be generic. Please see below comments. > > > option the software managed boost is enabled. It also selects > > thermal subsystem to be compiled in. Thermal is necessary for > > disabling boost and cooling down the device when overheating > > detected. > > > > Boost _MUST_NOT_ work without thermal subsystem with properly > > defined overheating temperatures. > > > > This option doesn't affect x86's ACPI hardware managed boost support > > (i.e. Intel, AMD). In this situation boost management is embedded at > > hardware. > > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > > Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com> > > > > --- > > Changes for v6: > > - CPU_FREQ_BOOST_SW [1] is now defined as "invisible" bool option. > > - Platform dependent ARM_EXYNOS_CPU_FREQ_BOOST_SW config option has > > been added. It depends on ARM_EXYNOS_CPUFREQ options and selects > > EXYNOS_THERMAL with the main boost config [1]. > > > > Changes for v5: > > - New patch > > > > drivers/cpufreq/Kconfig | 3 +++ > > drivers/cpufreq/Kconfig.arm | 16 ++++++++++++++++ > > 2 files changed, 19 insertions(+) > > > > diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig > > index 534fcb8..3f058a3 100644 > > --- a/drivers/cpufreq/Kconfig > > +++ b/drivers/cpufreq/Kconfig > > @@ -23,6 +23,9 @@ config CPU_FREQ_TABLE > > config CPU_FREQ_GOV_COMMON > > bool > > > > +config CPU_FREQ_BOOST_SW > > + bool > > Invisible is fine but this must be disabled by default and must > depend on thermal, rather than moving dependency on platform's > config. The CPU_FREQ_BOOST_SW [1] is a generic flag (invisible). I will add "default n" to it. It shall be used at all BOOST dependent #ifdefs (generic + platform). Also I've introduced the ARM_EXYNOS_CPU_FREQ_BOOST_SW [2] config option. The CPU_FREQ_BOOST_SW option is selected from it. Moreover the [2] depends on ARM_EXYNOS_CPUFREQ (which prevents from accidental enable/disable). And it selects automatically the EXYNOS_THERMAL, which is much better than depending only on THERMAL [3] (the generic framework). Depending only on [3], results at situation where SW BOOST can be enabled at x86 or ARM target with only generic THERMAL support (which doesn't protect from overheating). So the proposed solution: 1. Defines global flag [1] 2. This flag is selected only when dependencies for more HW close flag [2] are meet. 3. It is more natural to have Kconfig BOOST enable/disable checkbox at platform cpufreq driver (with description closer to HW). 4. Other ARCHs can easily define similar to [2] flag and by it select [1]. For me the approach proposed in this patch is more natural and provides high protection level from accidental SW boost enable.
On 26 July 2013 16:51, Lukasz Majewski <l.majewski@samsung.com> wrote: > On Fri, 26 Jul 2013 15:54:56 +0530 Viresh Kumar wrote, >> On 25 July 2013 22:03, Lukasz Majewski <l.majewski@samsung.com> wrote: >> > +config CPU_FREQ_BOOST_SW >> > + bool >> >> Invisible is fine but this must be disabled by default and must >> depend on thermal, rather than moving dependency on platform's >> config. > > The CPU_FREQ_BOOST_SW [1] is a generic flag (invisible). > > I will add "default n" to it. Leave it.. We don't need it now.. that's how these kind of config options are defined as they are disabled by default. > Depending only on [3], results at situation where SW BOOST can be > enabled at x86 or ARM target with only generic THERMAL support (which > doesn't protect from overheating). I had a similar concern.. Currently also we aren't stopping anybody to enable boost. By selecting thermal from CPU_FREQ_BOOST_SW, atleast we are communicating this very well to developers that they need something else as well. And currently we only have thermal as a source for telling when to block boost but it can be something else too.. I never said, don't use EXYNOS_THERMAL, its good to have a dependency on it in the Exynos specific config for boost, but I wanted normal sw boost also to depend on thermal.. -- 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
On Mon, 29 Jul 2013 12:28:02 +0530 Viresh Kumar viresh.kumar@linaro.org wrote, > On 26 July 2013 16:51, Lukasz Majewski <l.majewski@samsung.com> wrote: > > On Fri, 26 Jul 2013 15:54:56 +0530 Viresh Kumar wrote, > >> On 25 July 2013 22:03, Lukasz Majewski <l.majewski@samsung.com> > >> wrote: > > >> > +config CPU_FREQ_BOOST_SW > >> > + bool > >> > >> Invisible is fine but this must be disabled by default and must > >> depend on thermal, rather than moving dependency on platform's > >> config. > > > > The CPU_FREQ_BOOST_SW [1] is a generic flag (invisible). > > > > I will add "default n" to it. > > Leave it.. We don't need it now.. that's how these kind of config > options are defined as they are disabled by default. Ok. Please see below proposition. > > > Depending only on [3], results at situation where SW BOOST can be > > enabled at x86 or ARM target with only generic THERMAL support > > (which doesn't protect from overheating). > > I had a similar concern.. Currently also we aren't stopping anybody to > enable boost. By selecting thermal from CPU_FREQ_BOOST_SW, atleast > we are communicating this very well to developers that they need > something else as well. And currently we only have thermal as a source > for telling when to block boost but it can be something else too.. > > I never said, don't use EXYNOS_THERMAL, its good to have a > dependency on it in the Exynos specific config for boost, but I wanted > normal sw boost also to depend on thermal.. 1. at ./drivers/cpufreq/Kconfig: +config CPU_FREQ_BOOST_SW + bool + depends on THERMAL 2. at ./drivers/cpufreq/Kconfig.arm: +config ARM_EXYNOS_CPU_FREQ_BOOST_SW + bool "EXYNOS Frequency Overclocking - Software" + depends on ARM_EXYNOS_CPUFREQ + select CPU_FREQ_BOOST_SW + select EXYNOS_THERMAL + default n + help + This driver supports software managed overclocking (BOOST). + It allows usage of special frequencies for Samsung Exynos + processors if thermal conditions are appropriate. + + It reguires, for safe operation, thermal framework with properly + defined trip points. + + If in doubt, say N. + Shall I split this patch to two (1. and 2.) or leave it as a single one?
On 12 August 2013 15:56, Lukasz Majewski <l.majewski@samsung.com> wrote: > 1. at ./drivers/cpufreq/Kconfig: > > +config CPU_FREQ_BOOST_SW > + bool > + depends on THERMAL > > 2. at ./drivers/cpufreq/Kconfig.arm: > > +config ARM_EXYNOS_CPU_FREQ_BOOST_SW > + bool "EXYNOS Frequency Overclocking - Software" > + depends on ARM_EXYNOS_CPUFREQ > + select CPU_FREQ_BOOST_SW > + select EXYNOS_THERMAL > + default n > + help > + This driver supports software managed overclocking (BOOST). > + It allows usage of special frequencies for Samsung Exynos > + processors if thermal conditions are appropriate. > + > + It reguires, for safe operation, thermal framework with > properly > + defined trip points. > + > + If in doubt, say N. > + > > Shall I split this patch to two (1. and 2.) or leave it as a single one? Looks fine.. Keep it in a single patch. -- 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
On Mon, 12 Aug 2013 15:58:12 +0530 Viresh Kumar viresh.kumar@linaro.org wrote, > On 12 August 2013 15:56, Lukasz Majewski <l.majewski@samsung.com> > wrote: > > 1. at ./drivers/cpufreq/Kconfig: > > > > +config CPU_FREQ_BOOST_SW > > + bool > > + depends on THERMAL > > > > 2. at ./drivers/cpufreq/Kconfig.arm: > > > > +config ARM_EXYNOS_CPU_FREQ_BOOST_SW > > + bool "EXYNOS Frequency Overclocking - Software" > > + depends on ARM_EXYNOS_CPUFREQ > > + select CPU_FREQ_BOOST_SW > > + select EXYNOS_THERMAL > > + default n ^^^^^^^^^^^^^^^^ I will also remove this line. > > + help > > + This driver supports software managed overclocking > > (BOOST). > > + It allows usage of special frequencies for Samsung Exynos > > + processors if thermal conditions are appropriate. > > + > > + It reguires, for safe operation, thermal framework with > > properly > > + defined trip points. > > + > > + If in doubt, say N. > > + > > > > Shall I split this patch to two (1. and 2.) or leave it as a single > > one? > > Looks fine.. Keep it in a single patch. Ok.
diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig index 534fcb8..3f058a3 100644 --- a/drivers/cpufreq/Kconfig +++ b/drivers/cpufreq/Kconfig @@ -23,6 +23,9 @@ config CPU_FREQ_TABLE config CPU_FREQ_GOV_COMMON bool +config CPU_FREQ_BOOST_SW + bool + config CPU_FREQ_STAT tristate "CPU frequency translation statistics" select CPU_FREQ_TABLE diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index de4d5d9..c8abb93 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -55,6 +55,22 @@ config ARM_EXYNOS5440_CPUFREQ different than previous exynos controllers so not using the common exynos framework. +config ARM_EXYNOS_CPU_FREQ_BOOST_SW + bool "EXYNOS Frequency Overclocking - Software" + depends on ARM_EXYNOS_CPUFREQ + select CPU_FREQ_BOOST_SW + select EXYNOS_THERMAL + default n + help + This driver supports software managed overclocking (BOOST). + It allows usage of special frequencies for Samsung Exynos + processors if thermal conditions are appropriate. + + It reguires, for safe operation, thermal framework with properly + defined trip points. + + If in doubt, say N. + config ARM_HIGHBANK_CPUFREQ tristate "Calxeda Highbank-based" depends on ARCH_HIGHBANK