public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module
@ 2018-09-05 20:38 Rodrigo Siqueira
  2018-09-05 22:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Rodrigo Siqueira @ 2018-09-05 20:38 UTC (permalink / raw)
  To: Petri Latvala, Arkadiusz Hiler; +Cc: igt-dev, gustavo, intel-gfx

This commit adds a new option for forcing the use of a specific driver
indicated via an environment variable.

Changes since V1:
 Petri:
 - Use an environment variable instead of command line
 - Refactor the loop in __search_and_open to accept forced module
 - Don't try to load kernel modules

Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
---
 lib/drmtest.c  | 44 ++++++++++++++++++++++++++++++++++++++++++--
 lib/drmtest.h  |  2 ++
 lib/igt_core.c |  5 +++++
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index bfa2e0f0..6e35d1be 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -123,6 +123,36 @@ 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 (!strcmp(name, "")) {
+		igt_warn("IGT_FORCE_DRIVER flag specified without a value,"
+			 "ignoring force option\n");
+		return;
+	}
+
+	igt_info("Attempt to force module %s\n", name);
+
+	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:
@@ -250,8 +280,18 @@ static int __search_and_open(const char *base, int offset, unsigned int chipset)
 
 		sprintf(name, "%s%u", base, i + offset);
 		fd = open_device(name, chipset);
-		if (fd != -1)
-			return fd;
+		if (fd == -1)
+			continue;
+
+		// Force module
+		if (chipset == DRIVER_ANY && forced_driver()) {
+			if (__is_device(fd, forced_driver()))
+				return fd;
+			close(fd);
+			continue;
+		}
+
+		return fd;
 	}
 
 	return -1;
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 23bb858f..8e65b5e3 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -647,6 +647,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.18.0


-- 
Rodrigo Siqueira
http://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [igt-dev] ✗ Fi.CI.BAT: failure for Add support for forcing specific module
  2018-09-05 20:38 [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module Rodrigo Siqueira
@ 2018-09-05 22:27 ` Patchwork
  2018-09-05 22:49 ` [igt-dev] [PATCH i-g-t v2] " Chris Wilson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-09-05 22:27 UTC (permalink / raw)
  To: Rodrigo Siqueira; +Cc: igt-dev

== Series Details ==

Series: Add support for forcing specific module
URL   : https://patchwork.freedesktop.org/series/49233/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4772 -> IGTPW_1795 =

== Summary - FAILURE ==

  Serious unknown changes coming with IGTPW_1795 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1795, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49233/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1795:

  === IGT changes ===

    ==== Possible regressions ====

    igt@gem_exec_suspend@basic-s3:
      fi-snb-2600:        PASS -> INCOMPLETE
      fi-icl-u:           PASS -> INCOMPLETE

    
== Known issues ==

  Here are the changes found in IGTPW_1795 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     PASS -> FAIL (fdo#103167)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#105128, fdo#107139) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
      fi-ilk-650:         DMESG-WARN (fdo#106387) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#103191, fdo#107362) -> PASS
      fi-blb-e6850:       INCOMPLETE (fdo#107718) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718


== Participating hosts (54 -> 49) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4629 -> IGTPW_1795

  CI_DRM_4772: 1351ee8f3aacdb8f4a71cd17a7035556065c59a9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1795: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1795/
  IGT_4629: c3b6d69aa3dd2d1a6c1f2e787670a0aef78f2ea5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1795/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module
  2018-09-05 20:38 [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module Rodrigo Siqueira
  2018-09-05 22:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
@ 2018-09-05 22:49 ` Chris Wilson
  2018-09-05 22:58 ` [igt-dev] ✗ Fi.CI.BAT: failure for Add support for forcing specific module (rev2) Patchwork
  2018-12-17 15:49 ` [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module Daniel Vetter
  3 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2018-09-05 22:49 UTC (permalink / raw)
  To: Arkadiusz Hiler, Petri Latvala, Rodrigo Siqueira
  Cc: igt-dev, gustavo, intel-gfx

Quoting Rodrigo Siqueira (2018-09-05 21:38:27)
> This commit adds a new option for forcing the use of a specific driver
> indicated via an environment variable.
> 
> Changes since V1:
>  Petri:
>  - Use an environment variable instead of command line
>  - Refactor the loop in __search_and_open to accept forced module
>  - Don't try to load kernel modules

I am still not convinced this is a good solution to the problem of
running tests against all applicable devices, along with generic
filtering of that set (both from the test profile and user config).

Short term wise I'd rather see DRIVER_ANY translated into a known
DRIVER_X selector (so that we have a complete list of drivers for later
exploitation), say

diff --git a/lib/drmtest.c b/lib/drmtest.c
index bfa2e0f..5c96c1d 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -257,11 +257,16 @@ static int __search_and_open(const char *base, int offset, unsigned int chipset)
        return -1;
 }
 
+static unsigned int driver_any_chipset = DRIVER_ANY; // give me a better name
+
 static int __open_driver(const char *base, int offset, unsigned int chipset)
 {
        static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
        int fd;
 
+       if (chipset == DRIVER_ANY)
+               chipset = driver_any_chipset;
+
        fd = __search_and_open(base, offset, chipset);
        if (fd != -1)
                return fd;
@@ -428,3 +433,21 @@ void igt_require_intel(int fd)
 {
        igt_require(is_i915_device(fd) && has_known_intel_chipset(fd));
 }
+
+bool set_driver_any(const char *name)
+{
+       for (int start = 0, end = ARRAY_SIZE(modules) - 1; start < end; ) { // repetitive much?
+               int mid = start + (end - start) / 2;
+               int ret = strcmp(modules[mid].module, name);
+               if (ret < 0) {
+                       start = mid + 1;
+               } else if (ret > 0) {
+                       end = mid;
+               } else {
+                       driver_any_chipset = modules[mid].bit;
+                       return true;
+               }
+       }
+
+       return false;
+}
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [igt-dev] ✗ Fi.CI.BAT: failure for Add support for forcing specific module (rev2)
  2018-09-05 20:38 [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module Rodrigo Siqueira
  2018-09-05 22:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
  2018-09-05 22:49 ` [igt-dev] [PATCH i-g-t v2] " Chris Wilson
@ 2018-09-05 22:58 ` Patchwork
  2018-12-17 15:49 ` [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module Daniel Vetter
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-09-05 22:58 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: Add support for forcing specific module (rev2)
URL   : https://patchwork.freedesktop.org/series/49233/
State : failure

== Summary ==

Applying: Add support for forcing specific module
Using index info to reconstruct a base tree...
Patch failed at 0001 Add support for forcing specific module
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module
  2018-09-05 20:38 [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module Rodrigo Siqueira
                   ` (2 preceding siblings ...)
  2018-09-05 22:58 ` [igt-dev] ✗ Fi.CI.BAT: failure for Add support for forcing specific module (rev2) Patchwork
@ 2018-12-17 15:49 ` Daniel Vetter
  2018-12-17 16:45   ` Rodrigo Siqueira
  2018-12-17 17:22   ` Lucas De Marchi
  3 siblings, 2 replies; 9+ messages in thread
From: Daniel Vetter @ 2018-12-17 15:49 UTC (permalink / raw)
  To: Rodrigo Siqueira; +Cc: igt-dev, gustavo, intel-gfx, Petri Latvala

On Wed, Sep 05, 2018 at 05:38:27PM -0300, Rodrigo Siqueira wrote:
> This commit adds a new option for forcing the use of a specific driver
> indicated via an environment variable.
> 
> Changes since V1:
>  Petri:
>  - Use an environment variable instead of command line
>  - Refactor the loop in __search_and_open to accept forced module
>  - Don't try to load kernel modules
> 
> Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>

Note: You can't drop the s-o-b line if your patch contains work by other
people, like from Petri here. Proper way to resend a patch by someone else
is to just add a subject prefix of "PATCH RESEND" and otherwise keep
everything unchanged (including author and everything).

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

Cheers, Daniel

> ---
>  lib/drmtest.c  | 44 ++++++++++++++++++++++++++++++++++++++++++--
>  lib/drmtest.h  |  2 ++
>  lib/igt_core.c |  5 +++++
>  3 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index bfa2e0f0..6e35d1be 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -123,6 +123,36 @@ 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 (!strcmp(name, "")) {
> +		igt_warn("IGT_FORCE_DRIVER flag specified without a value,"
> +			 "ignoring force option\n");
> +		return;
> +	}
> +
> +	igt_info("Attempt to force module %s\n", name);
> +
> +	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:
> @@ -250,8 +280,18 @@ static int __search_and_open(const char *base, int offset, unsigned int chipset)
>  
>  		sprintf(name, "%s%u", base, i + offset);
>  		fd = open_device(name, chipset);
> -		if (fd != -1)
> -			return fd;
> +		if (fd == -1)
> +			continue;
> +
> +		// Force module
> +		if (chipset == DRIVER_ANY && forced_driver()) {
> +			if (__is_device(fd, forced_driver()))
> +				return fd;
> +			close(fd);
> +			continue;
> +		}
> +
> +		return fd;
>  	}
>  
>  	return -1;
> 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 23bb858f..8e65b5e3 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -647,6 +647,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.18.0
> 
> 
> -- 
> Rodrigo Siqueira
> http://siqueira.tech
> Graduate Student
> Department of Computer Science
> University of São Paulo
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module
  2018-12-17 15:49 ` [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module Daniel Vetter
@ 2018-12-17 16:45   ` Rodrigo Siqueira
  2018-12-17 17:22   ` Lucas De Marchi
  1 sibling, 0 replies; 9+ messages in thread
From: Rodrigo Siqueira @ 2018-12-17 16:45 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: igt-dev, gustavo, intel-gfx, Petri Latvala


[-- Attachment #1.1: Type: text/plain, Size: 4141 bytes --]

On 12/17, Daniel Vetter wrote:
> On Wed, Sep 05, 2018 at 05:38:27PM -0300, Rodrigo Siqueira wrote:
> > This commit adds a new option for forcing the use of a specific driver
> > indicated via an environment variable.
> > 
> > Changes since V1:
> >  Petri:
> >  - Use an environment variable instead of command line
> >  - Refactor the loop in __search_and_open to accept forced module
> >  - Don't try to load kernel modules
> > 
> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> 
> Note: You can't drop the s-o-b line if your patch contains work by other
> people, like from Petri here. Proper way to resend a patch by someone else
> is to just add a subject prefix of "PATCH RESEND" and otherwise keep
> everything unchanged (including author and everything).
> 
> https://patchwork.freedesktop.org/patch/245532/

Hi,

Thanks for your feedback.
Next time I will take care for not make this mistake again.

Best Regards
Rodrigo Siqueira
 
> Cheers, Daniel
> 
> > ---
> >  lib/drmtest.c  | 44 ++++++++++++++++++++++++++++++++++++++++++--
> >  lib/drmtest.h  |  2 ++
> >  lib/igt_core.c |  5 +++++
> >  3 files changed, 49 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index bfa2e0f0..6e35d1be 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -123,6 +123,36 @@ 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 (!strcmp(name, "")) {
> > +		igt_warn("IGT_FORCE_DRIVER flag specified without a value,"
> > +			 "ignoring force option\n");
> > +		return;
> > +	}
> > +
> > +	igt_info("Attempt to force module %s\n", name);
> > +
> > +	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:
> > @@ -250,8 +280,18 @@ static int __search_and_open(const char *base, int offset, unsigned int chipset)
> >  
> >  		sprintf(name, "%s%u", base, i + offset);
> >  		fd = open_device(name, chipset);
> > -		if (fd != -1)
> > -			return fd;
> > +		if (fd == -1)
> > +			continue;
> > +
> > +		// Force module
> > +		if (chipset == DRIVER_ANY && forced_driver()) {
> > +			if (__is_device(fd, forced_driver()))
> > +				return fd;
> > +			close(fd);
> > +			continue;
> > +		}
> > +
> > +		return fd;
> >  	}
> >  
> >  	return -1;
> > 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 23bb858f..8e65b5e3 100644
> > --- a/lib/igt_core.c
> > +++ b/lib/igt_core.c
> > @@ -647,6 +647,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.18.0
> > 
> > 
> > -- 
> > Rodrigo Siqueira
> > http://siqueira.tech
> > Graduate Student
> > Department of Computer Science
> > University of São Paulo
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module
  2018-12-17 15:49 ` [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module Daniel Vetter
  2018-12-17 16:45   ` Rodrigo Siqueira
@ 2018-12-17 17:22   ` Lucas De Marchi
  2018-12-18 12:08     ` Petri Latvala
  1 sibling, 1 reply; 9+ messages in thread
From: Lucas De Marchi @ 2018-12-17 17:22 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Petri Latvala, Rodrigo Siqueira, Gustavo F. Padovan, intel-gfx,
	igt-dev

On Mon, Dec 17, 2018 at 7:49 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Wed, Sep 05, 2018 at 05:38:27PM -0300, Rodrigo Siqueira wrote:
> > This commit adds a new option for forcing the use of a specific driver
> > indicated via an environment variable.
> >
> > Changes since V1:
> >  Petri:
> >  - Use an environment variable instead of command line
> >  - Refactor the loop in __search_and_open to accept forced module
> >  - Don't try to load kernel modules
> >
> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
>
> Note: You can't drop the s-o-b line if your patch contains work by other
> people, like from Petri here. Proper way to resend a patch by someone else
> is to just add a subject prefix of "PATCH RESEND" and otherwise keep
> everything unchanged (including author and everything).
>
> https://patchwork.freedesktop.org/patch/245532/

Last time I was told I have to _add_ my s-o-b nonetheless, even if
just re-sending the patch.
I don't think I should, but in the end I had to change the series, add
and change patches,
so it didn't matter.

Maybe we need some clarification on this?

https://lists.freedesktop.org/archives/intel-gfx/2018-November/183291.html

Lucas De Marchi


>
> Cheers, Daniel
>
> > ---
> >  lib/drmtest.c  | 44 ++++++++++++++++++++++++++++++++++++++++++--
> >  lib/drmtest.h  |  2 ++
> >  lib/igt_core.c |  5 +++++
> >  3 files changed, 49 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index bfa2e0f0..6e35d1be 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -123,6 +123,36 @@ 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 (!strcmp(name, "")) {
> > +             igt_warn("IGT_FORCE_DRIVER flag specified without a value,"
> > +                      "ignoring force option\n");
> > +             return;
> > +     }
> > +
> > +     igt_info("Attempt to force module %s\n", name);
> > +
> > +     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:
> > @@ -250,8 +280,18 @@ static int __search_and_open(const char *base, int offset, unsigned int chipset)
> >
> >               sprintf(name, "%s%u", base, i + offset);
> >               fd = open_device(name, chipset);
> > -             if (fd != -1)
> > -                     return fd;
> > +             if (fd == -1)
> > +                     continue;
> > +
> > +             // Force module
> > +             if (chipset == DRIVER_ANY && forced_driver()) {
> > +                     if (__is_device(fd, forced_driver()))
> > +                             return fd;
> > +                     close(fd);
> > +                     continue;
> > +             }
> > +
> > +             return fd;
> >       }
> >
> >       return -1;
> > 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 23bb858f..8e65b5e3 100644
> > --- a/lib/igt_core.c
> > +++ b/lib/igt_core.c
> > @@ -647,6 +647,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.18.0
> >
> >
> > --
> > Rodrigo Siqueira
> > http://siqueira.tech
> > Graduate Student
> > Department of Computer Science
> > University of São Paulo
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev



--
Lucas De Marchi
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module
  2018-12-17 17:22   ` Lucas De Marchi
@ 2018-12-18 12:08     ` Petri Latvala
  2018-12-18 13:33       ` Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: Petri Latvala @ 2018-12-18 12:08 UTC (permalink / raw)
  To: Lucas De Marchi
  Cc: Rodrigo Siqueira, Gustavo F. Padovan, intel-gfx, igt-dev,
	Daniel Vetter

On Mon, Dec 17, 2018 at 09:22:31AM -0800, Lucas De Marchi wrote:
> On Mon, Dec 17, 2018 at 7:49 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Wed, Sep 05, 2018 at 05:38:27PM -0300, Rodrigo Siqueira wrote:
> > > This commit adds a new option for forcing the use of a specific driver
> > > indicated via an environment variable.
> > >
> > > Changes since V1:
> > >  Petri:
> > >  - Use an environment variable instead of command line
> > >  - Refactor the loop in __search_and_open to accept forced module
> > >  - Don't try to load kernel modules
> > >
> > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> >
> > Note: You can't drop the s-o-b line if your patch contains work by other
> > people, like from Petri here. Proper way to resend a patch by someone else
> > is to just add a subject prefix of "PATCH RESEND" and otherwise keep
> > everything unchanged (including author and everything).
> >
> > https://patchwork.freedesktop.org/patch/245532/
> 
> Last time I was told I have to _add_ my s-o-b nonetheless, even if
> just re-sending the patch.
> I don't think I should, but in the end I had to change the series, add
> and change patches,
> so it didn't matter.


Communication error here? Rodrigo's resend didn't have my S-o-b,
that's what Daniel was pointing at. Removing S-o-b is never
ok. Whether it's correct and/or required to add your own S-o-b to
resends is another matter.


> Maybe we need some clarification on this?
> 
> https://lists.freedesktop.org/archives/intel-gfx/2018-November/183291.html

That was about a kernel patch, and kernel patches are _very_ strict
about having to add your S-o-b.


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module
  2018-12-18 12:08     ` Petri Latvala
@ 2018-12-18 13:33       ` Daniel Vetter
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2018-12-18 13:33 UTC (permalink / raw)
  To: Lucas De Marchi, Daniel Vetter, Rodrigo Siqueira, igt-dev,
	Gustavo F. Padovan, intel-gfx, Chris Wilson

On Tue, Dec 18, 2018 at 02:08:21PM +0200, Petri Latvala wrote:
> On Mon, Dec 17, 2018 at 09:22:31AM -0800, Lucas De Marchi wrote:
> > On Mon, Dec 17, 2018 at 7:49 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> > >
> > > On Wed, Sep 05, 2018 at 05:38:27PM -0300, Rodrigo Siqueira wrote:
> > > > This commit adds a new option for forcing the use of a specific driver
> > > > indicated via an environment variable.
> > > >
> > > > Changes since V1:
> > > >  Petri:
> > > >  - Use an environment variable instead of command line
> > > >  - Refactor the loop in __search_and_open to accept forced module
> > > >  - Don't try to load kernel modules
> > > >
> > > > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > >
> > > Note: You can't drop the s-o-b line if your patch contains work by other
> > > people, like from Petri here. Proper way to resend a patch by someone else
> > > is to just add a subject prefix of "PATCH RESEND" and otherwise keep
> > > everything unchanged (including author and everything).
> > >
> > > https://patchwork.freedesktop.org/patch/245532/
> > 
> > Last time I was told I have to _add_ my s-o-b nonetheless, even if
> > just re-sending the patch.
> > I don't think I should, but in the end I had to change the series, add
> > and change patches,
> > so it didn't matter.
> 
> 
> Communication error here? Rodrigo's resend didn't have my S-o-b,
> that's what Daniel was pointing at. Removing S-o-b is never
> ok. Whether it's correct and/or required to add your own S-o-b to
> resends is another matter.

Correct.

> > Maybe we need some clarification on this?
> > 
> > https://lists.freedesktop.org/archives/intel-gfx/2018-November/183291.html
> 
> That was about a kernel patch, and kernel patches are _very_ strict
> about having to add your S-o-b.

Yes, for the kernel even just sending a patch around by someone, even if
you change nothing, needs your sob.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-12-18 13:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-05 20:38 [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module Rodrigo Siqueira
2018-09-05 22:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
2018-09-05 22:49 ` [igt-dev] [PATCH i-g-t v2] " Chris Wilson
2018-09-05 22:58 ` [igt-dev] ✗ Fi.CI.BAT: failure for Add support for forcing specific module (rev2) Patchwork
2018-12-17 15:49 ` [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module Daniel Vetter
2018-12-17 16:45   ` Rodrigo Siqueira
2018-12-17 17:22   ` Lucas De Marchi
2018-12-18 12:08     ` Petri Latvala
2018-12-18 13:33       ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox