Re: [PATCH i-g-t 10/10] lib: switch i915_pciids_local.h to xe driver style PCI ID macros

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 22 May 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Wed, May 22, 2024 at 01:35:23PM +0300, Jani Nikula wrote:
>> Follow-up the kernel i915_pciids.h switching to xe driver style PCI ID
>> macros, and do the same for i915_pciids_local.h. This is a clear
>> improvement in the perf code, for example.
>
> I'm confused now on why do we need this i915_pciids_local.h o.O
> but anyway, the change is good and right:

We'd need to split the MTL macros up kernel side to match IGT perf.c
needs, even if we don't need that kernel side. But might as well do it
for Single Point of Truth.

Ditto for PVC, we should use XE_PVC_IDS from xe_pciids.h, but there's a
diff between those and what's in i915_pciids_local.h. Needs some
analysis why.

Anyway, all this can come as follow-up.

Thanks for the review,
Jani.


>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  lib/i915/perf.c         | 12 +++--------
>>  lib/i915_pciids_local.h | 44 ++++++++++++++++++++---------------------
>>  lib/intel_device_info.c |  2 +-
>>  3 files changed, 26 insertions(+), 32 deletions(-)
>> 
>> diff --git a/lib/i915/perf.c b/lib/i915/perf.c
>> index 4b00ba5de9d4..ee950b3c03e4 100644
>> --- a/lib/i915/perf.c
>> +++ b/lib/i915/perf.c
>> @@ -204,13 +204,10 @@ is_acm_gt3(const struct intel_perf_devinfo *devinfo)
>>  static bool
>>  is_mtl_gt2(const struct intel_perf_devinfo *devinfo)
>>  {
>> -#undef INTEL_VGA_DEVICE
>> -#define INTEL_VGA_DEVICE(_id, _info) _id
>>  	static const uint32_t devids[] = {
>> -		INTEL_MTL_M_IDS(NULL),
>> -		INTEL_MTL_P_GT2_IDS(NULL),
>> +		INTEL_MTL_M_IDS(ID),
>> +		INTEL_MTL_P_GT2_IDS(ID),
>>  	};
>> -#undef INTEL_VGA_DEVICE
>>  	for (uint32_t i = 0; i < ARRAY_SIZE(devids); i++) {
>>  		if (devids[i] == devinfo->devid)
>>  			return true;
>> @@ -222,12 +219,9 @@ is_mtl_gt2(const struct intel_perf_devinfo *devinfo)
>>  static bool
>>  is_mtl_gt3(const struct intel_perf_devinfo *devinfo)
>>  {
>> -#undef INTEL_VGA_DEVICE
>> -#define INTEL_VGA_DEVICE(_id, _info) _id
>>  	static const uint32_t devids[] = {
>> -		INTEL_MTL_P_GT3_IDS(NULL),
>> +		INTEL_MTL_P_GT3_IDS(ID),
>>  	};
>> -#undef INTEL_VGA_DEVICE
>>  	for (uint32_t i = 0; i < ARRAY_SIZE(devids); i++) {
>>  		if (devids[i] == devinfo->devid)
>>  			return true;
>> diff --git a/lib/i915_pciids_local.h b/lib/i915_pciids_local.h
>> index 0043b0cd9b34..92879704aa8e 100644
>> --- a/lib/i915_pciids_local.h
>> +++ b/lib/i915_pciids_local.h
>> @@ -9,41 +9,41 @@
>>  
>>  /* MTL perf */
>>  #ifndef INTEL_MTL_M_IDS
>> -#define INTEL_MTL_M_IDS(info) \
>> -	INTEL_VGA_DEVICE(0x7D60, info), \
>> -	INTEL_VGA_DEVICE(0x7D67, info)
>> +#define INTEL_MTL_M_IDS(MACRO__, ...) \
>> +	MACRO__(0x7D60, ## __VA_ARGS__), \
>> +	MACRO__(0x7D67, ## __VA_ARGS__)
>>  #endif
>>  
>>  #ifndef INTEL_MTL_P_GT2_IDS
>> -#define INTEL_MTL_P_GT2_IDS(info) \
>> -	INTEL_VGA_DEVICE(0x7D45, info)
>> +#define INTEL_MTL_P_GT2_IDS(MACRO__, ...) \
>> +	MACRO__(0x7D45, ## __VA_ARGS__)
>>  #endif
>>  
>>  #ifndef INTEL_MTL_P_GT3_IDS
>> -#define INTEL_MTL_P_GT3_IDS(info) \
>> -	INTEL_VGA_DEVICE(0x7D55, info), \
>> -	INTEL_VGA_DEVICE(0x7DD5, info)
>> +#define INTEL_MTL_P_GT3_IDS(MACRO__, ...) \
>> +	MACRO__(0x7D55, ## __VA_ARGS__), \
>> +	MACRO__(0x7DD5, ## __VA_ARGS__)
>>  #endif
>>  
>>  #ifndef INTEL_MTL_P_IDS
>> -#define INTEL_MTL_P_IDS(info) \
>> -	INTEL_MTL_P_GT2_IDS(info), \
>> -	INTEL_MTL_P_GT3_IDS(info)
>> +#define INTEL_MTL_P_IDS(MACRO__, ...) \
>> +	INTEL_MTL_P_GT2_IDS(MACRO__, ## __VA_ARGS__), \
>> +	INTEL_MTL_P_GT3_IDS(MACRO__, ## __VA_ARGS__)
>>  #endif
>>  
>>  /* PVC */
>>  #ifndef INTEL_PVC_IDS
>> -#define INTEL_PVC_IDS(info) \
>> -	INTEL_VGA_DEVICE(0x0BD0, info), \
>> -	INTEL_VGA_DEVICE(0x0BD1, info), \
>> -	INTEL_VGA_DEVICE(0x0BD2, info), \
>> -	INTEL_VGA_DEVICE(0x0BD5, info), \
>> -	INTEL_VGA_DEVICE(0x0BD6, info), \
>> -	INTEL_VGA_DEVICE(0x0BD7, info), \
>> -	INTEL_VGA_DEVICE(0x0BD8, info), \
>> -	INTEL_VGA_DEVICE(0x0BD9, info), \
>> -	INTEL_VGA_DEVICE(0x0BDA, info), \
>> -	INTEL_VGA_DEVICE(0x0BDB, info)
>> +#define INTEL_PVC_IDS(MACRO__, ...) \
>> +	MACRO__(0x0BD0, ## __VA_ARGS__), \
>> +	MACRO__(0x0BD1, ## __VA_ARGS__), \
>> +	MACRO__(0x0BD2, ## __VA_ARGS__), \
>> +	MACRO__(0x0BD5, ## __VA_ARGS__), \
>> +	MACRO__(0x0BD6, ## __VA_ARGS__), \
>> +	MACRO__(0x0BD7, ## __VA_ARGS__), \
>> +	MACRO__(0x0BD8, ## __VA_ARGS__), \
>> +	MACRO__(0x0BD9, ## __VA_ARGS__), \
>> +	MACRO__(0x0BDA, ## __VA_ARGS__), \
>> +	MACRO__(0x0BDB, ## __VA_ARGS__)
>>  #endif
>>  
>>  #endif /* _I915_PCIIDS_LOCAL_H */
>> diff --git a/lib/intel_device_info.c b/lib/intel_device_info.c
>> index 30aca2abd7be..e80ea54707de 100644
>> --- a/lib/intel_device_info.c
>> +++ b/lib/intel_device_info.c
>> @@ -617,7 +617,7 @@ static const struct pci_id_match intel_device_match[] = {
>>  
>>  	INTEL_MTL_IDS(INTEL_VGA_DEVICE, &intel_meteorlake_info),
>>  
>> -	INTEL_PVC_IDS(&intel_pontevecchio_info),
>> +	INTEL_PVC_IDS(INTEL_VGA_DEVICE, &intel_pontevecchio_info),
>>  
>>  	XE_LNL_IDS(INTEL_VGA_DEVICE, &intel_lunarlake_info),
>>  
>> -- 
>> 2.39.2
>> 

-- 
Jani Nikula, Intel