[PATCH i-g-t v1] drmtest: drop legacy path from filtered opens

[Zbigniew Kempczyński]

On Tue, Feb 13, 2024 at 06:57:22PM +0100, Kamil Konieczny wrote:
> Function __drm_open_driver_another() has two paths, first being
> for filtered devices, second for non-filtered ones. Separate
> them and do not fallback into legacy path when index for card
> exceeds number of filtered devices. Use local opened fds table
> only for legacy calls as this table can become stale after
> driver reload. In filtered one do not use opened fds guards as
> this is pointless, we already have info about card wanted by user
> and we do not need to check if we ancounter already opened one.
> Also allow opening the same device if user wants it, for example
> with repeating same card number in --device or IGT_DEVICE:
> 
> sudo IGT_DEVICE=pci:vendor=Intel,card=0\;pci:vendor=Intel,card=0 \
> 	build/tests/gem_basic --r multigpu-create-close
> 
> Cc: Arkadiusz Hiler <arek at hiler.eu>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Chris Wilson <chris.p.wilson at linux.intel.com>
> Cc: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> Cc: Petri Latvala <adrinael at adrinael.net>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> Signed-off-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> ---
>  lib/drmtest.c | 22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 52b5a2020..307c44ef6 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -481,13 +481,12 @@ static bool __get_card_for_nth_filter(int idx, struct igt_device_card *card)
>   *
>   * This function opens device in the following order:
>   *
> - * 1. when --device arguments are present:
> + * 1. when --device arguments are present or IGT_DEVICE env var is used:
>   *   * device scanning is executed,
>   *   * idx-th filter (starting with 0, filters are semicolon separated) is used
> - *   * if there is no idx-th filter, goto 2
> + *   * if there is no idx-th filter fail with -1
>   *   * first device maching the filter is selected
> - *   * if it's already opened (for indexes = 0..idx-1) we fail with -1
> - *   * otherwise open the device and return the fd
> + *   * open the device and return the fd
>   *
>   * 2. compatibility mode - open the first DRM device we can find that is not
>   *    already opened for indexes 0..idx-1, searching up to 16 device nodes
> @@ -528,10 +527,17 @@ int __drm_open_driver_another(int idx, int chipset)
>  {
>  	int fd = -1;
>  
> -	if (chipset != DRIVER_VGEM && igt_device_filter_count() > idx) {
> +	if (chipset != DRIVER_VGEM && igt_device_filter_count()) {
> +		int count = igt_device_filter_count();
>  		struct igt_device_card card;
>  		bool found;
>  
> +		if (idx < 0 || idx >= count) {
> +			igt_warn("Cannot open, index into filter out of bounds: %d, count=%d\n",
> +				 idx, count);
> +			return -1;
> +		}
> +
>  		found = __get_card_for_nth_filter(idx, &card);
>  
>  		if (!found) {
> @@ -542,19 +548,17 @@ int __drm_open_driver_another(int idx, int chipset)
>  		if (!found || !strlen(card.card))
>  			igt_warn("No card matches the filter! [%s]\n",
>  				 igt_device_filter_get(idx));
> -		else if (_is_already_opened(card.card, idx))
> -			igt_warn("card maching filter %d is already opened\n", idx);
>  		else
>  			fd = __open_driver_exact(card.card, chipset);

Great! You've relaxed this condition. It will make multigpu testing
on single gpu easier.

>From me:

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>

If noone will protest in few days then go ahead and merge.

--
Zbigniew

>  
>  	} else {
>  		/* no filter for device idx, let's open whatever is available */
>  		fd = __open_driver("/dev/dri/card", 0, chipset, idx);
> +		if (fd >= 0)
> +			_set_opened_fd(idx, fd);
>  	}
>  
>  	if (fd >= 0) {
> -		_set_opened_fd(idx, fd);
> -
>  		/* Cache xe_device struct. */
>  		if (is_xe_device(fd))
>  			xe_device_get(fd);
> -- 
> 2.42.0
>