On Thu, Sep 08, 2016 at 05:08:58PM +0800, Harry Pan wrote:
> @@ -177,6 +187,16 @@ static inline u64 rapl_scale(u64 v, int cfg)
> pr_warn("Invalid domain %d, failed to scale data\n", cfg);
> return v;
> }
> +
> + /*
> + * Some Atom series processors (BYT/BSW) use 2^ESU microjoules.
> + *
> + * TODO: this looks hacky, it's better to refactor scale-up mechanism
> + * to compromise the main stream processors and Atom ones.
> + */
> + if (is_baytrail)
> + return v << rapl_hw_unit[cfg - 1];
> +
Can't you simply set rapl_hw_unit[] such that 32 - rapl_hw_unit[] ends
up at the right number? Then you only get to much with values in
rapl_check_hw_unit without runtime overhead later.
> +static int rapl_check_hw_unit(enum rapl_quirk apply_quirk)
> {
> u64 msr_rapl_power_unit_bits;
> int i;
> @@ -634,10 +674,20 @@ static int rapl_check_hw_unit(bool apply_quirk)
> * "Intel Xeon Processor E5-1600 and E5-2600 v3 Product Families, V2
> * of 2. Datasheet, September 2014, Reference Number: 330784-001 "
> */
> - if (apply_quirk)
> + if (apply_quirk == RAPL_HSX_QUIRK)
> rapl_hw_unit[RAPL_IDX_RAM_NRG_STAT] = 16;
>
> /*
> + * Some Atom processors (BYT/BSW) have 2^ESU microjoules increment,
> + * refer to Software Developers' Manual, Vol. 3C, Order No. 325384,
> + * Table 35-8 of MSR_RAPL_POWER_UNIT
> + */
> + if (apply_quirk == RAPL_BYT_QUIRK)
> + is_baytrail = true;
> + else
> + is_baytrail = false;
it was already false...
/*
* comment explaining quirk goes here...
*/
if (apply_quirk = RAPL_BYT_QUIRK) {
for (i = 0; i < NR_RAPL_DOMAINS; i++)
rapl_hw_unit[i] = 32 - rapl_hw_unit[i];
}
and then you get to verify what to do with rapl_timer_ms.
> static const struct intel_rapl_init_fun snb_rapl_init __initconst = {
> - .apply_quirk = false,
> + .apply_quirk = 0,
Either leave it out (unmentioned members get initialized to 0) or add
RAPL_NO_QUIRK or so.
> .cntr_mask = RAPL_IDX_CLN,
> .attrs = rapl_events_cln_attr,
> };
>
