Add Meteor Lake SoC to the list of processor models for which
Power Limit4 is supported by the Intel RAPL driver.
Signed-off-by: Sumeet Pawnikar <[email protected]>
---
drivers/powercap/intel_rapl_msr.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/powercap/intel_rapl_msr.c b/drivers/powercap/intel_rapl_msr.c
index bc6adda58883..a27673706c3d 100644
--- a/drivers/powercap/intel_rapl_msr.c
+++ b/drivers/powercap/intel_rapl_msr.c
@@ -143,6 +143,8 @@ static const struct x86_cpu_id pl4_support_ids[] = {
{ X86_VENDOR_INTEL, 6, INTEL_FAM6_ALDERLAKE_N, X86_FEATURE_ANY },
{ X86_VENDOR_INTEL, 6, INTEL_FAM6_RAPTORLAKE, X86_FEATURE_ANY },
{ X86_VENDOR_INTEL, 6, INTEL_FAM6_RAPTORLAKE_P, X86_FEATURE_ANY },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_METEORLAKE, X86_FEATURE_ANY },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_METEORLAKE_L, X86_FEATURE_ANY },
{}
};
--
2.17.1
On Wed, Feb 15, 2023 at 1:46 PM Sumeet Pawnikar
<[email protected]> wrote:
>
> Add Meteor Lake SoC to the list of processor models for which
> Power Limit4 is supported by the Intel RAPL driver.
>
> Signed-off-by: Sumeet Pawnikar <[email protected]>
> ---
> drivers/powercap/intel_rapl_msr.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/powercap/intel_rapl_msr.c b/drivers/powercap/intel_rapl_msr.c
> index bc6adda58883..a27673706c3d 100644
> --- a/drivers/powercap/intel_rapl_msr.c
> +++ b/drivers/powercap/intel_rapl_msr.c
> @@ -143,6 +143,8 @@ static const struct x86_cpu_id pl4_support_ids[] = {
> { X86_VENDOR_INTEL, 6, INTEL_FAM6_ALDERLAKE_N, X86_FEATURE_ANY },
> { X86_VENDOR_INTEL, 6, INTEL_FAM6_RAPTORLAKE, X86_FEATURE_ANY },
> { X86_VENDOR_INTEL, 6, INTEL_FAM6_RAPTORLAKE_P, X86_FEATURE_ANY },
> + { X86_VENDOR_INTEL, 6, INTEL_FAM6_METEORLAKE, X86_FEATURE_ANY },
> + { X86_VENDOR_INTEL, 6, INTEL_FAM6_METEORLAKE_L, X86_FEATURE_ANY },
> {}
> };
>
> --
Applied as 6.3-rc material, thanks!
On 2/15/23 04:32, Sumeet Pawnikar wrote:
> diff --git a/drivers/powercap/intel_rapl_msr.c b/drivers/powercap/intel_rapl_msr.c
> index bc6adda58883..a27673706c3d 100644
> --- a/drivers/powercap/intel_rapl_msr.c
> +++ b/drivers/powercap/intel_rapl_msr.c
> @@ -143,6 +143,8 @@ static const struct x86_cpu_id pl4_support_ids[] = {
> { X86_VENDOR_INTEL, 6, INTEL_FAM6_ALDERLAKE_N, X86_FEATURE_ANY },
> { X86_VENDOR_INTEL, 6, INTEL_FAM6_RAPTORLAKE, X86_FEATURE_ANY },
> { X86_VENDOR_INTEL, 6, INTEL_FAM6_RAPTORLAKE_P, X86_FEATURE_ANY },
> + { X86_VENDOR_INTEL, 6, INTEL_FAM6_METEORLAKE, X86_FEATURE_ANY },
> + { X86_VENDOR_INTEL, 6, INTEL_FAM6_METEORLAKE_L, X86_FEATURE_ANY },
> {}
> };
Sumeet, could you _please_ go take a close look at 'struct x86_cpu_id'?
> struct x86_cpu_id {
> __u16 vendor;
> __u16 family;
> __u16 model;
> __u16 steppings;
> __u16 feature; /* bit index */
> kernel_ulong_t driver_data;
> };
You might also want to very carefully count the fields in the structure.
Which field is being initialized to X86_FEATURE_ANY? Is it:
a. ->feature
b. ->steppings
c. ->model
How could this _possibly_ work, you ask yourself? Well, you lucked out:
#define X86_FAMILY_ANY 0
#define X86_MODEL_ANY 0
#define X86_STEPPING_ANY 0
#define X86_FEATURE_ANY 0
so, you actually accidentally *explicitly* specified a 0 for ->steppings
*AND* accidentally *implicitly* specified a 0 for ->feature.
... and you did this in at least five separate commits over four years.
Why does this matter? Because some hapless maintainer might take your
code, copy it, and then s/X86_FEATURE_ANY/X86_FEATURE_FOO/ and then
scratch their head for an hour as to why it doesn't work.
Could you please fix this up? As penance, you could even fix the _ANY
defines so that people can't do this accidentally any longer.
On Thu, May 11, 2023 at 07:55:08AM -0700, Dave Hansen wrote:
> Could you please fix this up? As penance, you could even fix the _ANY
> defines so that people can't do this accidentally any longer.
See the X86_MATCH_INTEL_FAM6_MODEL() for how to do this right.
-Tony
> -----Original Message-----
> From: Luck, Tony <[email protected]>
> Sent: Thursday, May 11, 2023 10:15 PM
> To: Hansen, Dave <[email protected]>
> Cc: Pawnikar, Sumeet R <[email protected]>; [email protected];
> [email protected]; Zhang, Rui <[email protected]>;
> [email protected]; [email protected]
> Subject: Re: [PATCH] powercap: RAPL: Add Power Limit4 support for Meteor
> Lake SoC
>
> On Thu, May 11, 2023 at 07:55:08AM -0700, Dave Hansen wrote:
> > Could you please fix this up? As penance, you could even fix the _ANY
> > defines so that people can't do this accidentally any longer.
>
> See the X86_MATCH_INTEL_FAM6_MODEL() for how to do this right.
>
Thanks Dave and Tony for this information.
Let me check and submit the fix for this.
Regards,
Sumeet.
> -Tony