Re: [PATCH 1/9] drm/xe: Add framework for info probing

[Gustavo Sousa <gustavo.sousa@intel.com>]

Matt Roper <matthew.d.roper@intel.com> writes:

> On Tue, Jun 09, 2026 at 05:17:33PM -0300, Gustavo Sousa wrote:
>> Functions xe_info_init_early() and xe_info_init() currently probe some
>> information from the hardware while doing initialization of info
>
> Is the mention of xe_info_init_early() here correct?  The general rule
> is supposed to be that *_early() functions are software-only setup
> (e.g., kzalloc'ing memory, initializing mutexes, etc.), whereas
> non-early functions are the ones that touch hardware in some manner
> (either for reading and/or writing).  There were a lot of mistakes in
> this area early on that we're still cleaning up, but I think
> xe_info_init_early() at least is following the rules as far as I can
> see?

Yeah, to be fair, xe_info_init_early() does not directly try to read
information from the hardware during its execution.  My argument is that
it "probes indirectly" when it gets the PCI device and revision ids from
the PCI subsystem; that becomes more evident by the fact that we need a
kunit static stub for init_devid().

--
Gustavo Sousa

>
>
> Matt
>
>> fields.  Besides mixing responsibilities, another issue from this
>> approach is that kunit tests need to implement static stubs for the
>> probing part.
>> 
>> Let's prepare the ground to ensuring that those functions stop probing
>> the information from the hardware by creating the necessary framework
>> for extracting the probing bits out of them.  Do that by creating a
>> new struct type called xe_probed_info and the functions responsible
>> for populating it.
>> 
>> In upcoming changes, we will gradually refactor the code so that all
>> info needed by xe_info_init_early() and xe_info_init() that is probed
>> from the hardware is passed to them via struct xe_probed_info.
>> 
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  drivers/gpu/drm/xe/tests/xe_pci.c | 16 +++++++++++++--
>>  drivers/gpu/drm/xe/xe_pci.c       | 41 +++++++++++++++++++++++++++++++++++----
>>  2 files changed, 51 insertions(+), 6 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
>> index 9240aff779da..51d032a9e01a 100644
>> --- a/drivers/gpu/drm/xe/tests/xe_pci.c
>> +++ b/drivers/gpu/drm/xe/tests/xe_pci.c
>> @@ -338,13 +338,21 @@ static void fake_xe_info_probe_tile_count(struct xe_device *xe)
>>  	/* Nothing to do, just use the statically defined value. */
>>  }
>>  
>> +static int fake_probe_info(struct xe_device *xe,
>> +			   struct xe_probed_info *probed_info)
>> +{
>> +	return 0;
>> +}
>> +
>>  int xe_pci_fake_device_init(struct xe_device *xe)
>>  {
>>  	struct kunit *test = kunit_get_current_test();
>>  	struct xe_pci_fake_data *data = test->priv;
>> +	struct xe_probed_info probed_info = {};
>>  	const struct pci_device_id *ent = pciidlist;
>>  	const struct xe_device_desc *desc;
>>  	const struct xe_subplatform_desc *subplatform_desc;
>> +	int err;
>>  
>>  	if (!data) {
>>  		desc = (const void *)ent->driver_data;
>> @@ -379,8 +387,12 @@ int xe_pci_fake_device_init(struct xe_device *xe)
>>  	kunit_activate_static_stub(test, xe_info_probe_tile_count,
>>  				   fake_xe_info_probe_tile_count);
>>  
>> -	xe_info_init_early(xe, desc, subplatform_desc);
>> -	xe_info_init(xe, desc);
>> +	err = fake_probe_info(xe, &probed_info);
>> +	if (err)
>> +		return err;
>> +
>> +	xe_info_init_early(xe, desc, subplatform_desc, &probed_info);
>> +	xe_info_init(xe, desc, &probed_info);
>>  
>>  	return 0;
>>  }
>> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>> index 78fc2e4dcfc6..7f1da6d25011 100644
>> --- a/drivers/gpu/drm/xe/xe_pci.c
>> +++ b/drivers/gpu/drm/xe/xe_pci.c
>> @@ -737,13 +737,27 @@ static void init_devid(struct xe_device *xe)
>>  	xe->info.revid = pdev->revision;
>>  }
>>  
>> +struct xe_probed_info {
>> +	/* Nothing for now. */
>> +};
>> +
>> +/*
>> + * Probe from the hardware the info required by xe_info_init_early().
>> + */
>> +static int xe_probe_info_early(struct xe_device *xe,
>> +			       struct xe_probed_info *probed_info)
>> +{
>> +	return 0;
>> +}
>> +
>>  /*
>>   * Initialize device info content that only depends on static driver_data
>>   * passed to the driver at probe time from PCI ID table.
>>   */
>>  static int xe_info_init_early(struct xe_device *xe,
>>  			      const struct xe_device_desc *desc,
>> -			      const struct xe_subplatform_desc *subplatform_desc)
>> +			      const struct xe_subplatform_desc *subplatform_desc,
>> +			      struct xe_probed_info *probed_info)
>>  {
>>  	int err;
>>  
>> @@ -910,6 +924,15 @@ static struct xe_gt *alloc_media_gt(struct xe_tile *tile,
>>  	return gt;
>>  }
>>  
>> +/*
>> + * Probe from the hardware the info required by xe_info_init().
>> + */
>> +static int xe_probe_info(struct xe_device *xe,
>> +			 struct xe_probed_info *probed_info)
>> +{
>> +	return 0;
>> +}
>> +
>>  /*
>>   * Initialize device info content that does require knowledge about
>>   * graphics / media IP version.
>> @@ -917,7 +940,8 @@ static struct xe_gt *alloc_media_gt(struct xe_tile *tile,
>>   * present in device info.
>>   */
>>  static int xe_info_init(struct xe_device *xe,
>> -			const struct xe_device_desc *desc)
>> +			const struct xe_device_desc *desc,
>> +			struct xe_probed_info *probed_info)
>>  {
>>  	u32 graphics_gmdid_revid = 0, media_gmdid_revid = 0;
>>  	const struct xe_ip *graphics_ip;
>> @@ -1073,6 +1097,7 @@ static void xe_pci_remove(struct pci_dev *pdev)
>>   */
>>  static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  {
>> +	struct xe_probed_info probed_info = {};
>>  	const struct xe_device_desc *desc = (const void *)ent->driver_data;
>>  	const struct xe_subplatform_desc *subplatform_desc;
>>  	struct xe_device *xe;
>> @@ -1117,7 +1142,11 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  
>>  	pci_set_master(pdev);
>>  
>> -	err = xe_info_init_early(xe, desc, subplatform_desc);
>> +	err = xe_probe_info_early(xe, &probed_info);
>> +	if (err)
>> +		return err;
>> +
>> +	err = xe_info_init_early(xe, desc, subplatform_desc, &probed_info);
>>  	if (err)
>>  		return err;
>>  
>> @@ -1136,7 +1165,11 @@ static int xe_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>  	if (err)
>>  		return err;
>>  
>> -	err = xe_info_init(xe, desc);
>> +	err = xe_probe_info(xe, &probed_info);
>> +	if (err)
>> +		return err;
>> +
>> +	err = xe_info_init(xe, desc, &probed_info);
>>  	if (err)
>>  		return err;
>>  
>> 
>> -- 
>> 2.53.0
>> 
>
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation