Re: [PATCH v10 1/2] lib/xe/xe_query: Get runtime xe device graphics version from GMD_ID

["Wang, X" <x.wang@intel.com>]

Hi Zbigniew,
On 1/15/2026 00:00, Zbigniew Kempczyński wrote:
> On Mon, Jan 12, 2026 at 07:17:31PM +0000, Xin Wang wrote:
>> This allows IGT to query the exact IP version for xe platforms.
>>
>> Key changes:
>> - Add xe_device_ipver field to xe_device structure
>> - set the graphics versions based on the GMD_ID
>> - Cache device ipver in global map indexed by devid for efficient lookup
>> - Implement xe_ipver_cache_lookup() to retrieve cached ipver by devid
>> - Clean up cached device ipver when xe_device is released
>>
>> V2:
>> - add new struct xe_device_ipver to hold the ipver info
>> - separate cache map to eliminate collision (Roper, Matthew D)
>> - changed function name to xe_ipver_cache_lookup() to avoid
>>    confusion (Roper, Matthew D)
>>
>> V3:
>> - optimize the coding style. (Summers, Stuart)
>>
>> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> Signed-off-by: Xin Wang <x.wang@intel.com>
>> Reviewed-by: Ravi Kumar V <ravi.kumar.vodapalli@intel.com>
>> ---
>>   lib/intel_chipset.h |  6 +++++
>>   lib/xe/xe_query.c   | 54 ++++++++++++++++++++++++++++++++++++++++++++-
>>   lib/xe/xe_query.h   |  4 ++++
>>   3 files changed, 63 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h
>> index cc2225110..424811f7c 100644
>> --- a/lib/intel_chipset.h
>> +++ b/lib/intel_chipset.h
>> @@ -100,6 +100,12 @@ struct intel_device_info {
>>   	const char *codename;
>>   };
>>   
>> +struct xe_device_ipver {
>> +	uint32_t devid;
>> +	uint16_t graphics_ver;
>> +	uint16_t graphics_rel;
>> +};
>> +
>>   const struct intel_device_info *intel_get_device_info(uint16_t devid) __attribute__((pure));
>>   
>>   const struct intel_cmds_info *intel_get_cmds_info(uint16_t devid) __attribute__((pure));
>> diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
>> index 981d76948..823a29f2d 100644
>> --- a/lib/xe/xe_query.c
>> +++ b/lib/xe/xe_query.c
>> @@ -210,6 +210,22 @@ static struct xe_device_cache {
>>   	struct igt_map *map;
>>   } cache;
>>   
>> +static struct xe_ipver_cache {
>> +	pthread_mutex_t mutex;
>> +	struct igt_map *map;
>> +} xe_ipver;
>> +
>> +struct xe_device_ipver *xe_ipver_cache_lookup(uint32_t devid)
>> +{
>> +	struct xe_device_ipver *ipver;
>> +
>> +	pthread_mutex_lock(&xe_ipver.mutex);
>> +	ipver = igt_map_search(xe_ipver.map, &devid);
>> +	pthread_mutex_unlock(&xe_ipver.mutex);
>> +
>> +	return ipver;
>> +}
>> +
> Please forgive me for sharing my thoughts only now.
>
> I wondered about this a bit and according to current code shape
> I don't like this design - especially ip_ver cache in xe_query.
> I mean I don't like to use external cache which might or might not
> be filled at time of calling intel_get_device_info().
>
> At the moment I see following options:
>
> 1. Provide update function intel_device_info_update(devid, ver, rel)
>     allowing to update rel field during xe_device_get(). This still
>     keeps intel_device_info.c independent but rel field might be
>     incorrect. BTW I would set it in static definition to -1 (max
>     unsigned) to catch it is not set/updated properly during
>     intel_get_device_info().
>
> 2. Add new function intel_get_device_info_by_fd() which will support
>     passing fd instead devid and migrate code to use it. Most of our code
>     uses fd to acquire device id before calling intel_get_device_info()
>     [exception is igt_device_scan which uses pci devid directly]. This
>     will break intel_device_info.c independency because we start calling
>     driver functions, but maybe it is time to do this (GMD_ID).
>
>     Digression: It seems when INTEL_DEVID_OVERRIDE won't work properly
>     but I wouldn't care about it for now.
>
> 3. Mix 1 and 2 but build hash map from intel_device_match[] in the
>     constructor and update it during xe_device_get(). Pros of this
>     is we could stop using 'static __thread' for caching last info.
>
> --
> Zbigniew
Thanks for the review. In the first version of the patch
(https://patchwork.freedesktop.org/series/155527/#rev2), I
introduced three new APIs specifically for Xe devices
(gen >= 20). However, the Xe.CI Full runs reported many
failures, and it became clear that a proper fix would
require changes across more components than I initially
expected.

For this series, my goal is to provide the smallest and
least-intrusive set of changes needed to resolve the
immediate issue, without triggering a broader refactor of
the device-info infrastructure.

I would like to try option 3 to address the current
solution.

>>   static struct xe_device *find_in_cache_unlocked(int fd)
>>   {
>> ...cut...
>>