diff mbox series

[i-g-t,v2] Add support for forcing a specific driver

Message ID 20180822102038.24308-1-petri.latvala@intel.com (mailing list archive)
State New, archived
Headers show
Series [i-g-t,v2] Add support for forcing a specific driver | expand

Commit Message

Petri Latvala Aug. 22, 2018, 10:20 a.m. UTC
This commit adds a new option for forcing the use of a specific driver
indicated via an environment variable.

v2 (Petri):
 - Use an environment variable instead of command line
 - Refactor the loop in __open_device
 - Don't try to load kernel modules

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: gustavo@padovan.org
---

Did the changes I suggested myself to see what else is lurking
there. I'm still unsure whether using IGT_FORCE_DRIVER=something
should still accept opening e.g. an i915 device with DRIVER_INTEL,
currently this does. Both ways have their merits.

I was able to test vkms with this patch, revealing some amount of
intelisms still lurking.


 lib/drmtest.c  | 35 +++++++++++++++++++++++++++++++++++
 lib/drmtest.h  |  2 ++
 lib/igt_core.c |  5 +++++
 3 files changed, 42 insertions(+)

Comments

Daniel Vetter Dec. 17, 2018, 3:47 p.m. UTC | #1
On Wed, Aug 22, 2018 at 01:20:38PM +0300, Petri Latvala wrote:
> This commit adds a new option for forcing the use of a specific driver
> indicated via an environment variable.
> 
> v2 (Petri):
>  - Use an environment variable instead of command line
>  - Refactor the loop in __open_device
>  - Don't try to load kernel modules
> 
> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> Signed-off-by: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: gustavo@padovan.org
> ---
> 
> Did the changes I suggested myself to see what else is lurking
> there. I'm still unsure whether using IGT_FORCE_DRIVER=something
> should still accept opening e.g. an i915 device with DRIVER_INTEL,
> currently this does. Both ways have their merits.
> 
> I was able to test vkms with this patch, revealing some amount of
> intelisms still lurking.
> 
> 
>  lib/drmtest.c  | 35 +++++++++++++++++++++++++++++++++++
>  lib/drmtest.h  |  2 ++
>  lib/igt_core.c |  5 +++++
>  3 files changed, 42 insertions(+)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index fae6f86f..e8308b0d 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -146,6 +146,31 @@ static bool has_known_intel_chipset(int fd)
>  	return true;
>  }
>  
> +static char _forced_driver[5] = "";
> +
> +/**
> + * __set_forced_driver:
> + * @name: name of driver to forcibly use
> + *
> + * Set the name of a driver to use when calling #drm_open_driver with
> + * the #DRIVER_ANY flag.
> + */
> +void __set_forced_driver(const char *name)
> +{
> +	if (!name)
> +		igt_warn("No driver specified, keep default behaviour");
> +
> +	strncpy(_forced_driver, name, 4);
> +}
> +
> +static const char *forced_driver(void)
> +{
> +	if (_forced_driver[0])
> +		return _forced_driver;
> +
> +	return NULL;
> +}
> +
>  #define LOCAL_I915_EXEC_VEBOX	(4 << 0)
>  /**
>   * gem_quiescent_gpu:
> @@ -229,6 +254,16 @@ static int __open_device(const char *base, int offset, unsigned int chipset)
>  		if (fd == -1)
>  			continue;
>  
> +		// Force options

I'd drop that comment. With that

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I think what this tries to solve here, and what Chris tries to solve in
his other reply on

https://patchwork.freedesktop.org/patch/247451/

are different issues. We want something to run generic DRIVER_ANY tests on
a specific driver. That's somewhat orthogonal to selecting a specific
device imo, and I think we need both. We definitely don't want to move to
DRIVER_X from DRIVER_ANY for the generic kms tests. Maybe we should rename
DRIVER_ANY to DRIVER_KMS ... (and double-check it supports modesetting).
-Daniel

> +		if (chipset == DRIVER_ANY && forced_driver()) {
> +			if (__is_device(fd, forced_driver())) {
> +				igt_debug("Force option used: Using driver %s\n", forced_driver());
> +				return fd;
> +			}
> +
> +			continue;
> +		}
> +
>  		if (chipset & DRIVER_INTEL && is_i915_device(fd) &&
>  		    has_known_intel_chipset(fd))
>  			return fd;
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 949865ee..62f53ec3 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -51,6 +51,8 @@
>   */
>  #define DRIVER_ANY 	~(DRIVER_VGEM)
>  
> +void __set_forced_driver(const char *name);
> +
>  /**
>   * ARRAY_SIZE:
>   * @arr: static array
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index c52c0818..c17e53c5 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -646,6 +646,11 @@ static void common_init_env(void)
>  	igt_frame_dump_path = getenv("IGT_FRAME_DUMP_PATH");
>  
>  	stderr_needs_sentinel = getenv("IGT_SENTINEL_ON_STDERR") != NULL;
> +
> +	env = getenv("IGT_FORCE_DRIVER");
> +	if (env) {
> +		__set_forced_driver(env);
> +	}
>  }
>  
>  static int common_init(int *argc, char **argv,
> -- 
> 2.14.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
diff mbox series

Patch

diff --git a/lib/drmtest.c b/lib/drmtest.c
index fae6f86f..e8308b0d 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -146,6 +146,31 @@  static bool has_known_intel_chipset(int fd)
 	return true;
 }
 
+static char _forced_driver[5] = "";
+
+/**
+ * __set_forced_driver:
+ * @name: name of driver to forcibly use
+ *
+ * Set the name of a driver to use when calling #drm_open_driver with
+ * the #DRIVER_ANY flag.
+ */
+void __set_forced_driver(const char *name)
+{
+	if (!name)
+		igt_warn("No driver specified, keep default behaviour");
+
+	strncpy(_forced_driver, name, 4);
+}
+
+static const char *forced_driver(void)
+{
+	if (_forced_driver[0])
+		return _forced_driver;
+
+	return NULL;
+}
+
 #define LOCAL_I915_EXEC_VEBOX	(4 << 0)
 /**
  * gem_quiescent_gpu:
@@ -229,6 +254,16 @@  static int __open_device(const char *base, int offset, unsigned int chipset)
 		if (fd == -1)
 			continue;
 
+		// Force options
+		if (chipset == DRIVER_ANY && forced_driver()) {
+			if (__is_device(fd, forced_driver())) {
+				igt_debug("Force option used: Using driver %s\n", forced_driver());
+				return fd;
+			}
+
+			continue;
+		}
+
 		if (chipset & DRIVER_INTEL && is_i915_device(fd) &&
 		    has_known_intel_chipset(fd))
 			return fd;
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 949865ee..62f53ec3 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -51,6 +51,8 @@ 
  */
 #define DRIVER_ANY 	~(DRIVER_VGEM)
 
+void __set_forced_driver(const char *name);
+
 /**
  * ARRAY_SIZE:
  * @arr: static array
diff --git a/lib/igt_core.c b/lib/igt_core.c
index c52c0818..c17e53c5 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -646,6 +646,11 @@  static void common_init_env(void)
 	igt_frame_dump_path = getenv("IGT_FRAME_DUMP_PATH");
 
 	stderr_needs_sentinel = getenv("IGT_SENTINEL_ON_STDERR") != NULL;
+
+	env = getenv("IGT_FORCE_DRIVER");
+	if (env) {
+		__set_forced_driver(env);
+	}
 }
 
 static int common_init(int *argc, char **argv,