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

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

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.

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

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.

--
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