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.
next prev parent 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.