Re: [PATCH 2/2] drm/xe/tests: Fix subplatform selection

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 9/9/2026 9:13 PM, Gustavo Sousa wrote:
> Michal Wajdeczko <michal.wajdeczko@intel.com> writes:
> 
>> If a test does not specify subplatform details about the fake device
>> it wants to create, our helper may select a sentinel of the subplatform
>> descriptors list instead of the first available subplatform.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> ---
>> Cc: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  drivers/gpu/drm/xe/tests/xe_pci.c | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
>> index a82cab7a2b86..8ab4270cf5d8 100644
>> --- a/drivers/gpu/drm/xe/tests/xe_pci.c
>> +++ b/drivers/gpu/drm/xe/tests/xe_pci.c
>> @@ -364,13 +364,23 @@ int xe_pci_fake_device_init(struct xe_device *xe)
>>  	if (!ent->device)
>>  		return -ENODEV;
>>  
>> +	if (data->subplatform == XE_SUBPLATFORM_NONE) {
>> +		subplatform_desc = NULL;
>> +		goto done;
>> +	}
>> +
>> +	if (data->subplatform == XE_SUBPLATFORM_UNINITIALIZED) {
> 
> I guess here as well, subplatform_desc will be either NULL or the first
> one, but if we add a comment in patch #1, I think we don't need to
> repeat ourselves.

IMO code is not so complex that it would require a comment 

> 
> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

thanks!

> 
> On a side note, orthogonal to this patch: I wonder if it is okay to match
> no subplatform when the device descriptor defines a list of
> subplatforms.  The current behavior is that find_subplatform() returns
> NULL and doesn't complain.

it was complaining in the past, but that was removed by

commit 64c9ae213d2ab1cce824841518e9539f597ee91e

    drm/xe/kunit: Handle fake device creation for all platform/subplatform cases

to support special case of ADLP (but maybe ADLP descriptor should be fixed instead? 

> 
> --
> Gustavo Sousa
> 
>> +		subplatform_desc = desc->subplatforms;
>> +		goto done;
>> +	}
>> +
>>  	for (subplatform_desc = desc->subplatforms;
>>  	     subplatform_desc && subplatform_desc->subplatform;
>>  	     subplatform_desc++)
>>  		if (subplatform_desc->subplatform == data->subplatform)
>>  			break;
>>  
>> -	if (data->subplatform != XE_SUBPLATFORM_NONE && !subplatform_desc)
>> +	if (!subplatform_desc || !subplatform_desc->subplatform)
>>  		return -ENODEV;
>>  
>>  done:
>> -- 
>> 2.47.1