(Adding hpa to Cc) From: Thomas Renninger <trenn@xxxxxxx> Date: Tue, Mar 23, 2010 at 12:17:16PM +0100 > > +/* core performance boost */ > > +static bool cpb_capable, cpb_disabled; > Whatabout using a cpufeature (arch/x86/include/asm/cpufeature.h) > instead of cpb_capable. Then people could see this feature in > /proc/cpuinfo and other code parts could check for it easily if > needed later. I don't have a problem with that per se. It's just that /proc/cpuinfo is a widely used interface and, AFAIR, changing it is not taken that lightly. Peter, what do you think? > It could already be set in arch/x86/kernel/cpu/amd.c and > powernow-k8 could use cpu_has(cpu, X86_FEATURE_CPB); I'd still like to cache the cpb_capable value locally instead of getting x86_cpuinfo percpu var and querying it. Especially if this happens often and not only at driver init. > Instead of cpb_disabled, I'd use cpb_enabled. Checking for > !cpb_disabled whether it's enabled, is ugly to read. Fair enough, cpb_disabled reflects the bit semantics in the MSR but why not, don't matter which to me. [..] > > + _cpb_toggle_msrs(t); > > + printk(KERN_INFO PFX "Core Boosting enabled.\n"); > Always printk on every toggle? > That should not happen often and a user might want to get noticed if > an app does this behind his back -> should be fine w/ or w/o, just not > sure whether it's intended. Well, actually, this should be on by default and the user or an app shouldn't be fidgeting with it all the time. It's there only for benchmarking purposes so that you can disable it when you really have to. But I guess it actually is going to get used if its there so maybe we should have to rethink our approach. Hmm... -- Regards/Gruss, Boris. -- Advanced Micro Devices, Inc. Operating Systems Research Center -- To unsubscribe from this list: send the line "unsubscribe cpufreq" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
