All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@intel.com>
To: Jacob Pan <jacob.jun.pan@linux.intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux PM <linux-pm@vger.kernel.org>,
	Rafael Wysocki <rafael.j.wysocki@intel.com>,
	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
	the arch/x86 maintainers <x86@kernel.org>
Subject: Re: [PATCH 2/2] powercap/rapl: add support for denverton
Date: Tue, 31 May 2016 15:19:30 -0700	[thread overview]
Message-ID: <574E0DF2.2010408@intel.com> (raw)
In-Reply-To: <1464727290-5400-3-git-send-email-jacob.jun.pan@linux.intel.com>

On 05/31/2016 01:41 PM, Jacob Pan wrote:
> --- a/drivers/powercap/intel_rapl.c
> +++ b/drivers/powercap/intel_rapl.c
> @@ -1137,6 +1137,7 @@ static const struct x86_cpu_id rapl_ids[] __initconst = {
>  	RAPL_CPU(0x57, rapl_defaults_hsw_server),/* Knights Landing */
>  	RAPL_CPU(0x8E, rapl_defaults_core),/* Kabylake */
>  	RAPL_CPU(0x9E, rapl_defaults_core),/* Kabylake */
> +	RAPL_CPU(0x5F, rapl_defaults_core),/* Denverton micro server */
>  	{}
>  };

Not to derail this individual patch... but do we really want to continue
open-coding CPU model/family combos all over arch/x86?

For instance, arch/x86/events/intel/core.c has:

>         case 142: /* 14nm Kabylake Mobile */
>         case 158: /* 14nm Kabylake Desktop */
>         case 78: /* 14nm Skylake Mobile */
>         case 94: /* 14nm Skylake Desktop */
>         case 85: /* 14nm Skylake Server */

Which duplicates the two Kabylake family numbers from the RAPL_CPU()
context above (just in decimal instead of hex).

Should we just start sticking these things in a header like:

#define X86_INTEL_FAMILY_KABYLAKE1 	0x8E
#define X86_INTEL_FAMILY_KABYLAKE2	0x9E
#define X86_INTEL_FAMILY_DENVERTON 	0x5F

So we have this:

	RAPL_CPU(X86_INTEL_FAMILY_DENVERTON, rapl_defaults_core),

instead of having to explain our magic number in a comment.

  reply	other threads:[~2016-05-31 22:19 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-31 20:41 [PATCH 0/2] Powercap RAPL update for Denverton Jacob Pan
2016-05-31 20:41 ` [PATCH 1/2] powercap/rapl: handle missing msrs Jacob Pan
2016-06-13 18:53   ` jacob
2016-06-13 21:00     ` Rafael J. Wysocki
2016-06-16 14:02       ` Rafael J. Wysocki
2016-05-31 20:41 ` [PATCH 2/2] powercap/rapl: add support for denverton Jacob Pan
2016-05-31 22:19   ` Dave Hansen [this message]
2016-06-01  6:57     ` Thomas Gleixner
2016-06-01 15:08       ` Jacob Pan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=574E0DF2.2010408@intel.com \
    --to=dave.hansen@intel.com \
    --cc=jacob.jun.pan@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=srinivas.pandruvada@linux.intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.