[PATCH i-g-t] remove tests dependency on VT /dev/tty0

[PATCH i-g-t] remove tests dependency on VT /dev/tty0

[Marc Herbert]

Hi,

  In a private communication Thomas Wood recommended the use of
igt_warn() over my initial igt_debug(), since the lack of /dev/tty0
"could indicate a problem". Unfortunately I have no idea what kind of
problem this could indicate outside ChromeOS. On the other hand, I'm
fairly sure that systematically printing a warning on every single
ChromeOS release going forward will create a "cry wolf" problem,
reducing users' attention to warnings and discouraging QA from collecting
them.

The top priority is obviously the ability to run the tests at all -
thus the current patch. But if someone can think of a silver and
relatively cheap bullet that would suit all usecases then please let
me/us know ASAP.

Cheers,

Marc

Marc Herbert (1):
  lib/igt_kms.c: remove tests dependency on VT /dev/tty0

 lib/igt_kms.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

-- 
1.9.3

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

[PATCH i-g-t] lib/igt_kms.c: remove tests dependency on VT /dev/tty0

[Marc Herbert]

Required to run on any recent, freon-based and X11-free ChromeOS release.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
---
 lib/igt_kms.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index d0c3690..27cff86 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -299,12 +299,24 @@ int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id)
 	return pfci.pipe;
 }
 
+static unsigned long orig_vt_mode = -1UL;
+
 static signed long set_vt_mode(unsigned long mode)
 {
 	int fd;
 	unsigned long prev_mode;
+	static const char TTY0[] = "/dev/tty0";
+
+	if (access(TTY0, F_OK)) {
+		/* errno message should be "No such file". Do not
+		   hardcode but ask strerror() in the very unlikely
+		   case something else happened. */
+		igt_warn("VT: %s: %s, cannot change its mode\n",
+			  TTY0, strerror(errno));
+		return orig_vt_mode;
+	}
 
-	fd = open("/dev/tty0", O_RDONLY);
+	fd = open(TTY0, O_RDONLY);
 	if (fd < 0)
 		return -errno;
 
@@ -323,8 +335,6 @@ err:
 	return -errno;
 }
 
-static unsigned long orig_vt_mode = -1UL;
-
 /**
  * kmstest_restore_vt_mode:
  *
@@ -363,6 +373,10 @@ void kmstest_set_vt_graphics_mode(void)
 	ret = set_vt_mode(KD_GRAPHICS);
 	igt_enable_exit_handler();
 
+	/* Return if set_vt_mode has successfully not changed anything */
+	if (ret == orig_vt_mode)
+		return;
+
 	igt_assert(ret >= 0);
 	orig_vt_mode = ret;
 
-- 
1.9.3

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

Re: [PATCH i-g-t] lib/igt_kms.c: remove tests dependency on VT /dev/tty0

[Daniel Vetter]

On Thu, Feb 19, 2015 at 04:38:43PM -0800, Marc Herbert wrote:
> Required to run on any recent, freon-based and X11-free ChromeOS release.
> 
> Signed-off-by: Marc Herbert <marc.herbert@intel.com>
> ---
>  lib/igt_kms.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index d0c3690..27cff86 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -299,12 +299,24 @@ int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id)
>  	return pfci.pipe;
>  }
>  
> +static unsigned long orig_vt_mode = -1UL;
> +
>  static signed long set_vt_mode(unsigned long mode)
>  {
>  	int fd;
>  	unsigned long prev_mode;
> +	static const char TTY0[] = "/dev/tty0";
> +
> +	if (access(TTY0, F_OK)) {
> +		/* errno message should be "No such file". Do not
> +		   hardcode but ask strerror() in the very unlikely
> +		   case something else happened. */
> +		igt_warn("VT: %s: %s, cannot change its mode\n",
> +			  TTY0, strerror(errno));
> +		return orig_vt_mode;

-1 collides with the negative errno space. Imo it's clearer to just return
KD_GRAPHICS since a system without vt is kinda always in gfx mode. This
means we'll run the exit handler but that will again be a noop because
there's still no tty.

With that change the patch looks good I think.
-Daniel

> +	}
>  
> -	fd = open("/dev/tty0", O_RDONLY);
> +	fd = open(TTY0, O_RDONLY);
>  	if (fd < 0)
>  		return -errno;
>  
> @@ -323,8 +335,6 @@ err:
>  	return -errno;
>  }
>  
> -static unsigned long orig_vt_mode = -1UL;
> -
>  /**
>   * kmstest_restore_vt_mode:
>   *
> @@ -363,6 +373,10 @@ void kmstest_set_vt_graphics_mode(void)
>  	ret = set_vt_mode(KD_GRAPHICS);
>  	igt_enable_exit_handler();
>  
> +	/* Return if set_vt_mode has successfully not changed anything */
> +	if (ret == orig_vt_mode)
> +		return;
> +
>  	igt_assert(ret >= 0);
>  	orig_vt_mode = ret;
>  
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] lib/igt_kms.c: remove tests dependency on VT /dev/tty0

[Thomas Wood]

On 24 February 2015 at 00:03, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Feb 19, 2015 at 04:38:43PM -0800, Marc Herbert wrote:
>> Required to run on any recent, freon-based and X11-free ChromeOS release.
>>
>> Signed-off-by: Marc Herbert <marc.herbert@intel.com>
>> ---
>>  lib/igt_kms.c | 20 +++++++++++++++++---
>>  1 file changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index d0c3690..27cff86 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -299,12 +299,24 @@ int kmstest_get_pipe_from_crtc_id(int fd, int crtc_id)
>>       return pfci.pipe;
>>  }
>>
>> +static unsigned long orig_vt_mode = -1UL;
>> +
>>  static signed long set_vt_mode(unsigned long mode)
>>  {
>>       int fd;
>>       unsigned long prev_mode;
>> +     static const char TTY0[] = "/dev/tty0";
>> +
>> +     if (access(TTY0, F_OK)) {
>> +             /* errno message should be "No such file". Do not
>> +                hardcode but ask strerror() in the very unlikely
>> +                case something else happened. */
>> +             igt_warn("VT: %s: %s, cannot change its mode\n",
>> +                       TTY0, strerror(errno));
>> +             return orig_vt_mode;
>
> -1 collides with the negative errno space. Imo it's clearer to just return
> KD_GRAPHICS since a system without vt is kinda always in gfx mode. This
> means we'll run the exit handler but that will again be a noop because
> there's still no tty.
>
> With that change the patch looks good I think.

It seems that missing tty0 isn't a major cause for concern, so the
igt_warn can be changed back to igt_debug as well.

> -Daniel
>
>> +     }
>>
>> -     fd = open("/dev/tty0", O_RDONLY);
>> +     fd = open(TTY0, O_RDONLY);
>>       if (fd < 0)
>>               return -errno;
>>
>> @@ -323,8 +335,6 @@ err:
>>       return -errno;
>>  }
>>
>> -static unsigned long orig_vt_mode = -1UL;
>> -
>>  /**
>>   * kmstest_restore_vt_mode:
>>   *
>> @@ -363,6 +373,10 @@ void kmstest_set_vt_graphics_mode(void)
>>       ret = set_vt_mode(KD_GRAPHICS);
>>       igt_enable_exit_handler();
>>
>> +     /* Return if set_vt_mode has successfully not changed anything */
>> +     if (ret == orig_vt_mode)
>> +             return;
>> +
>>       igt_assert(ret >= 0);
>>       orig_vt_mode = ret;
>>
>> --
>> 1.9.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx