[igt-dev] [PATCH i-g-t v3] lib/drmtest: Move open device to separate function

[Petri Latvala]

On Fri, Aug 31, 2018 at 01:00:49PM +0200, Katarzyna Dec wrote:
> While working on IGT code and during reviewes I've noticed that
> it could be nice to have function that is opening particular device.
> Let's move out conditions for opening device and rename __open_device
> to __search_and_open() function.
> 
> v2: Refactored open_device even more by getting device name once and
> returning fd for it. (Chris)
> v3: Added name_size to __get_drm_device_name.
> 
> Signed-off-by: Katarzyna Dec <katarzyna.dec at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---



This breaks opening virtio_gpu.


-- 
Petri Latvala






>  lib/drmtest.c | 108 ++++++++++++++++++++++----------------------------
>  1 file changed, 48 insertions(+), 60 deletions(-)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index fae6f86f..d2c7a332 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -77,12 +77,12 @@
>  
>  uint16_t __drm_device_id;
>  
> -static int __get_drm_device_name(int fd, char *name)
> +static int __get_drm_device_name(int fd, char *name, int name_size)
>  {
>  	drm_version_t version;
>  
>  	memset(&version, 0, sizeof(version));
> -	version.name_len = 4;
> +	version.name_len = name_size;
>  	version.name = name;
>  
>  	if (!drmIoctl(fd, DRM_IOCTL_VERSION, &version)){
> @@ -96,7 +96,7 @@ static bool __is_device(int fd, const char *expect)
>  {
>  	char name[5] = "";
>  
> -	if (__get_drm_device_name(fd, name))
> +	if (__get_drm_device_name(fd, name, 4))
>  		return false;
>  
>  	return strcmp(expect, name) == 0;
> @@ -107,26 +107,6 @@ bool is_i915_device(int fd)
>  	return __is_device(fd, "i915");
>  }
>  
> -static bool is_vc4_device(int fd)
> -{
> -	return __is_device(fd, "vc4");
> -}
> -
> -static bool is_vgem_device(int fd)
> -{
> -	return __is_device(fd, "vgem");
> -}
> -
> -static bool is_virtio_device(int fd)
> -{
> -	return __is_device(fd, "virt");
> -}
> -
> -static bool is_amd_device(int fd)
> -{
> -	return __is_device(fd, "amdg");
> -}
> -
>  static bool has_known_intel_chipset(int fd)
>  {
>  	struct drm_i915_getparam gp;
> @@ -218,38 +198,58 @@ static void modprobe_i915(const char *name)
>  	igt_i915_driver_load(NULL);
>  }
>  
> -static int __open_device(const char *base, int offset, unsigned int chipset)
> +static const struct module {
> +		unsigned int bit;
> +		const char *module;
> +		void (*modprobe)(const char *name);
> +	} modules[] = {
> +		{ DRIVER_AMDGPU, "amdgpu" },
> +		{ DRIVER_INTEL, "i915", modprobe_i915 },
> +		{ DRIVER_VC4, "vc4" },
> +		{ DRIVER_VGEM, "vgem" },
> +		{ DRIVER_VIRTIO, "virtio-gpu" },
> +		{}
> +	};
> +
> +static int open_device(const char *name, unsigned int chipset)
>  {
> -	for (int i = 0; i < 16; i++) {
> -		char name[80];
> -		int fd;
> +	int fd;
> +	char dev_name[16] = "";
> +	int chip = DRIVER_ANY;
>  
> -		sprintf(name, "%s%u", base, i + offset);
> -		fd = open(name, O_RDWR);
> -		if (fd == -1)
> -			continue;
> +	fd = open(name, O_RDWR);
> +	if (fd == -1)
> +		return -1;
>  
> -		if (chipset & DRIVER_INTEL && is_i915_device(fd) &&
> -		    has_known_intel_chipset(fd))
> -			return fd;
> +	if (__get_drm_device_name(fd, dev_name, (sizeof(dev_name) - 1)) == -1)
> +		return -1;
>  
> -		if (chipset & DRIVER_VC4 && is_vc4_device(fd))
> -			return fd;
> +	for (const struct module *m = modules; m->module; m++) {
> +		if (strcmp(m->module, dev_name) == 0) {
> +			chip = m->bit;
> +			break;
> +		}
> +	}
> +	if (chipset & chip)
> +		return fd;
>  
> -		if (chipset & DRIVER_VGEM && is_vgem_device(fd))
> -			return fd;
> +	close(fd);
>  
> -		if (chipset & DRIVER_VIRTIO && is_virtio_device(fd))
> -			return fd;
> +	return -1;
> +}
>  
> -		if (chipset & DRIVER_AMDGPU && is_amd_device(fd))
> -			return fd;
> +static int __search_and_open(const char *base, int offset, unsigned int chipset)
> +{
> +	for (int i = 0; i < 16; i++) {
> +		char name[80];
> +		int fd;
>  
> -		/* Only VGEM-specific tests should be run on VGEM */
> -		if (chipset == DRIVER_ANY && !is_vgem_device(fd))
> +		sprintf(name, "%s%u", base, i + offset);
> +		fd = open_device(name, chipset);
> +		if (fd == -1)
> +			continue;
> +		else
>  			return fd;
> -
> -		close(fd);
>  	}
>  
>  	return -1;
> @@ -258,21 +258,9 @@ static int __open_device(const char *base, int offset, unsigned int chipset)
>  static int __open_driver(const char *base, int offset, unsigned int chipset)
>  {
>  	static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
> -	static const struct module {
> -		unsigned int bit;
> -		const char *module;
> -		void (*modprobe)(const char *name);
> -	} modules[] = {
> -		{ DRIVER_AMDGPU, "amdgpu" },
> -		{ DRIVER_INTEL, "i915", modprobe_i915 },
> -		{ DRIVER_VC4, "vc4" },
> -		{ DRIVER_VGEM, "vgem" },
> -		{ DRIVER_VIRTIO, "virtio-gpu" },
> -		{}
> -	};
>  	int fd;
>  
> -	fd = __open_device(base, offset, chipset);
> +	fd = __search_and_open(base, offset, chipset);
>  	if (fd != -1)
>  		return fd;
>  
> @@ -287,7 +275,7 @@ static int __open_driver(const char *base, int offset, unsigned int chipset)
>  	}
>  	pthread_mutex_unlock(&mutex);
>  
> -	return __open_device(base, offset, chipset);
> +	return __search_and_open(base, offset, chipset);
>  }
>  
>  /**
> -- 
> 2.17.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev