[PATCH i-g-t v2] Add support for forcing specific module

[PATCH i-g-t v2] Add support for forcing specific module

[Rodrigo Siqueira]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module

[Chris Wilson]

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;
+}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module

[Daniel Vetter]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module

[Rodrigo Siqueira]

[-- 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: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module

[Lucas De Marchi]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module

[Petri Latvala]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t v2] Add support for forcing specific module

[Daniel Vetter]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx