Messages in this thread | | | Subject | Re: [PATCH] powercap / RAPL : remove dependency on iosf_mbi | From | Pengyu Ma <> | Date | Thu, 24 Sep 2015 18:03:32 +0800 |
| |
On 09/23/2015 01:01 AM, Jacob Pan wrote: > On Tue, 22 Sep 2015 11:11:36 +0800 > Pengyu Ma <pengyu.ma@windriver.com> wrote: > >> >> On 09/22/2015 05:36 AM, Jacob Pan wrote: >>> On Mon, 21 Sep 2015 11:48:14 +0800 >>> Pengyu Ma <pengyu.ma@windriver.com> wrote: >>> >>>> On 09/18/2015 11:43 PM, Jacob Pan wrote: >>>>> On Fri, 18 Sep 2015 02:09:55 +0200 >>>>> "Rafael J. Wysocki" <rjw@rjwysocki.net> wrote: >>>>> >>>>>> On Thursday, September 17, 2015 03:31:41 PM Pengyu Ma wrote: >>>>>>> iosf_mbi is supported on Quark, Braswell, Baytrail and some Atom >>>>>>> SoC, but RAPL is not limited to these SoC, it supports almost >>>>>>> Intel CPUs. Remove this dependece to make RAPL support more >>>>>>> Intel CPUs. >>>>>>> >>>>>>> Please select IOSF_MBI on Atom SoCs. >>>>>>> >>>>> Unlike Quark, I don't think we want to or do differentiate Atom >>>>> from other x86 at compile time. IOSF driver can be compiled as a >>>>> module also, therefore RAPL driver needs this explicit dependency >>>>> at compile time. >>>> As commit had exported iosf_mbi to let user use it. >>>> >>>> commit aa8e4f22ab7773352ba3895597189b8097f2c307 >>>> Author: David E. Box <david.e.box@linux.intel.com> >>>> Date: Wed Aug 27 14:40:39 2014 -0700 >>>> >>>> x86/iosf: Add Kconfig prompt for IOSF_MBI selection >>>> >>>> >>>> While selecting IOSF_MBI is preferred, it does mean carrying extra >>>> code on non-SoC architectures. >>>> >>>> We can NOT force user to build in iosf_mbi if they want use RAPL on >>>> haswell/broadwell/skylake. >>>> And RAPL can be compiled and worked well on >>>> haswell/broadwell/skylake without IOSF_MBI. >>>> RAPL is really NOT depended on IOSF_MBI. >>>> >>> True for haswell/broadwell/skylake platforms. But if we want binary >>> compatibility for Atom and Core, I can' see how simply removing the >>> dependency would work, unless we have runtime detection of IOSF. >> If you want use iosf_mbi on atom, please select it on generic x86 >> config. But not force it depend on another feature that not related >> on it with other boards. >> I don't care how iosf_mbi is added to kernel config, but why should I >> be forced to add it if I want use RAPL? >> It doesn't make any sense. >> > I understand your concern about wasting code. But let's look at all the > cases of config options here. (without Kconfig dependency as you > suggested) > > RAPL\IOSF Y M N > ___________________________________________________ > Y OK DC* Warn on Atom** > M OK OK Warn on Atom > N OK OK OK > ___________________________________________________ > > Notes: > * DC: don't compile > ** Warn on Atom is runtime if I add the following code to RAPL driver, > but this case is ok. > > --- a/drivers/powercap/intel_rapl.c > +++ b/drivers/powercap/intel_rapl.c > @@ -982,6 +982,11 @@ static void set_floor_freq_atom(struct rapl_domain > *rd, bool enable) static u32 power_ctrl_orig_val; > u32 mdata; > > + if (!iosf_mbi_available()) { > + pr_warn("No IOSF MBI access to set floor frequency\n"); > + return; > + } > + > > So the problematic case is when RAPL=Y IOSF=M > Since real IOSF functions are available when > #if IS_ENABLED(CONFIG_IOSF_MBI) > There will be no dummy functions for RAPL to reference in this case. iosf_mbi_write/read will warn itself. > Since IOSF is a driver, making it a module is a reasonable requirement. > As I mentioned before, I don't think we want to have a CONFIG_ATOM > option for X86. Actually there is a CONFIG_MATOM already in Kconfig.cpu
Pengyu > > +David, HPA > > Jacob > >> Pengyu >> >>>> Pengyu >>>>>>> Signed-off-by: Pengyu Ma <pengyu.ma@windriver.com> >>>>>> Jacob? >>>>>> >>>>>>> --- >>>>>>> drivers/powercap/Kconfig | 2 +- >>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/drivers/powercap/Kconfig b/drivers/powercap/Kconfig >>>>>>> index 85727ef..a7c81b5 100644 >>>>>>> --- a/drivers/powercap/Kconfig >>>>>>> +++ b/drivers/powercap/Kconfig >>>>>>> @@ -17,7 +17,7 @@ if POWERCAP >>>>>>> # Client driver configurations go here. >>>>>>> config INTEL_RAPL >>>>>>> tristate "Intel RAPL Support" >>>>>>> - depends on X86 && IOSF_MBI >>>>>>> + depends on X86 >>>>>>> default n >>>>>>> ---help--- >>>>>>> This enables support for the Intel Running Average >>>>>>> Power Limit (RAPL) >>>>>>> >>>>> [Jacob Pan] >>> [Jacob Pan] > [Jacob Pan]
| |