[PATCH i-g-t 1/5] kms_frontbuffer_tracking: Increase the time we wait for PSR.

[PATCH i-g-t 1/5] kms_frontbuffer_tracking: Increase the time we wait for PSR.

[Rodrigo Vivi]

With commit (drm/i915: Delay first PSR activation.) in kernel
PSR might take a bit longer to really activate after the modeset.
The first PSR activation after modeset is taking 5 times the panel
power cycle delay time, which is 600ms for our machines here.
So timeout here needs to be a minimum of 3s. However let's use
5s as the safe value in case we find machines with higher power
cycle delay.

Since we do a lot of assert(psr_disabled), this commit is increasing
the time it takes to run the whole set of PSR tests by a few minutes,
which had been reduced by commit f4db3b18841
("kms_frontbuffer_tracking: reduce the PSR wait timeout to 2s").

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 tests/kms_frontbuffer_tracking.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 4734f25..c729cee 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -851,7 +851,7 @@ static bool fbc_wait_until_enabled(void)
 
 static bool psr_wait_until_enabled(void)
 {
-	return igt_wait(psr_is_enabled(), 2000, 1);
+	return igt_wait(psr_is_enabled(), 5000, 1);
 }
 
 #define fbc_enable() igt_set_module_param_int("enable_fbc", 1)
-- 
2.4.3

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

[PATCH i-g-t 2/5] kms_frontbuffer_tracking: Make sink crc mandatory only for PSR.

[Rodrigo Vivi]

Unfortunately Sink CRC is not 100% reliable for all platforms.
So we cannot block FBC tests nor skip them when we are getting
unreliable Sink CRC results, or not getting them at all.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 tests/kms_frontbuffer_tracking.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index c729cee..ddcec75 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1570,7 +1570,7 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
 	return flags;
 }
 
-#define do_crc_assertions(flags) do {					\
+#define do_crc_assertions(flags, mandatory_sink_crc) do {		\
 	int flags__ = (flags);						\
 	struct both_crcs crc_;						\
 									\
@@ -1582,7 +1582,11 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
 									\
 	igt_assert(wanted_crc);						\
 	igt_assert_crc_equal(&crc_.pipe, &wanted_crc->pipe);		\
-	assert_sink_crc_equal(&crc_.sink, &wanted_crc->sink);		\
+	if (mandatory_sink_crc)						\
+		assert_sink_crc_equal(&crc_.sink, &wanted_crc->sink);	\
+	else								\
+		if (!sink_crc_equal(&crc_.sink, &wanted_crc->sink))	\
+			igt_info("Sink CRC differ, but not required\n"); \
 } while (0)
 
 #define do_status_assertions(flags_) do {				\
@@ -1618,12 +1622,13 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
 
 #define do_assertions(flags) do {					\
 	int flags_ = adjust_assertion_flags(t, (flags));		\
+	bool mandatory_sink_crc = t->feature & FEATURE_PSR;		\
 									\
 	wait_user(2, "Paused before assertions.");			\
 									\
 	/* Check the CRC to make sure the drawing operations work	\
 	 * immediately, independently of the features being enabled. */	\
-	do_crc_assertions(flags_);					\
+	do_crc_assertions(flags_, mandatory_sink_crc);			\
 									\
 	/* Now we can flush things to make the test faster. */		\
 	do_flush(t);							\
@@ -1636,7 +1641,7 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
 	 * would only delay the test suite while adding no value to the	\
 	 * test suite. */						\
 	if (t->screen == SCREEN_PRIM)					\
-		do_crc_assertions(flags_);				\
+		do_crc_assertions(flags_, mandatory_sink_crc);		\
 									\
 	if (fbc.supports_last_action && opt.fbc_check_last_action) {	\
 		if (flags_ & ASSERT_LAST_ACTION_CHANGED)		\
-- 
2.4.3

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

Re: [PATCH i-g-t 2/5] kms_frontbuffer_tracking: Make sink crc mandatory only for PSR.

[Paulo Zanoni]

2015-12-03 14:39 GMT-02:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> Unfortunately Sink CRC is not 100% reliable for all platforms.
> So we cannot block FBC tests nor skip them when we are getting
> unreliable Sink CRC results, or not getting them at all.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  tests/kms_frontbuffer_tracking.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
> index c729cee..ddcec75 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -1570,7 +1570,7 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
>         return flags;
>  }
>
> -#define do_crc_assertions(flags) do {                                  \
> +#define do_crc_assertions(flags, mandatory_sink_crc) do {              \
>         int flags__ = (flags);                                          \
>         struct both_crcs crc_;                                          \
>                                                                         \
> @@ -1582,7 +1582,11 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
>                                                                         \
>         igt_assert(wanted_crc);                                         \
>         igt_assert_crc_equal(&crc_.pipe, &wanted_crc->pipe);            \
> -       assert_sink_crc_equal(&crc_.sink, &wanted_crc->sink);           \
> +       if (mandatory_sink_crc)                                         \
> +               assert_sink_crc_equal(&crc_.sink, &wanted_crc->sink);   \
> +       else                                                            \
> +               if (!sink_crc_equal(&crc_.sink, &wanted_crc->sink))     \
> +                       igt_info("Sink CRC differ, but not required\n"); \
>  } while (0)
>
>  #define do_status_assertions(flags_) do {                              \
> @@ -1618,12 +1622,13 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
>
>  #define do_assertions(flags) do {                                      \
>         int flags_ = adjust_assertion_flags(t, (flags));                \
> +       bool mandatory_sink_crc = t->feature & FEATURE_PSR;             \

You could have declared this inside the do_crc_assertions() macro,
making things a little simpler since we wouldn't need to pass the
parameter below.

With or without this:
Acked-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>                                                                         \
>         wait_user(2, "Paused before assertions.");                      \
>                                                                         \
>         /* Check the CRC to make sure the drawing operations work       \
>          * immediately, independently of the features being enabled. */ \
> -       do_crc_assertions(flags_);                                      \
> +       do_crc_assertions(flags_, mandatory_sink_crc);                  \
>                                                                         \
>         /* Now we can flush things to make the test faster. */          \
>         do_flush(t);                                                    \
> @@ -1636,7 +1641,7 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
>          * would only delay the test suite while adding no value to the \
>          * test suite. */                                               \
>         if (t->screen == SCREEN_PRIM)                                   \
> -               do_crc_assertions(flags_);                              \
> +               do_crc_assertions(flags_, mandatory_sink_crc);          \
>                                                                         \
>         if (fbc.supports_last_action && opt.fbc_check_last_action) {    \
>                 if (flags_ & ASSERT_LAST_ACTION_CHANGED)                \
> --
> 2.4.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

[PATCH i-g-t 3/5] kms_frontbuffer_tracking: Skip on unreliable CRC.

[Rodrigo Vivi]

Even with all sink crc re-works we still have platforms
where after 6 vblanks it is unable to calculate the
sink crc. But if we don't get the sink crc it isn't true
that test failed, but that we have no ways to say test
passed or failed.

So let's print a message and move forward in case sink crc
cannot help us to know if the screen has been updated.

v2: Also include a message on setup_sink_crc and also
only skip when it is mandatory, i.e. when running for PSR.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 tests/kms_frontbuffer_tracking.c | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index ddcec75..e200975 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -859,12 +859,22 @@ static bool psr_wait_until_enabled(void)
 #define psr_enable() igt_set_module_param_int("enable_psr", 1)
 #define psr_disable() igt_set_module_param_int("enable_psr", 0)
 
-static void get_sink_crc(sink_crc_t *crc)
+static void get_sink_crc(sink_crc_t *crc, bool mandatory)
 {
+	int rc, errno_;
+
 	lseek(sink_crc.fd, 0, SEEK_SET);
 
-	igt_assert(read(sink_crc.fd, crc->data, SINK_CRC_SIZE) ==
-		   SINK_CRC_SIZE);
+	rc = read(sink_crc.fd, crc->data, SINK_CRC_SIZE);
+	errno_ = errno;
+
+	if (rc == -1 && errno_ == ETIMEDOUT) {
+		if (mandatory)
+			igt_skip("Sink CRC is unreliable on this machine. Try running this test again individually\n");
+		else
+			igt_info("Sink CRC is unreliable on this machine. Try running this test again individually\n");
+	}
+	igt_assert(rc == SINK_CRC_SIZE);
 }
 
 static bool sink_crc_equal(sink_crc_t *a, sink_crc_t *b)
@@ -1148,17 +1158,17 @@ static void print_crc(const char *str, struct both_crcs *crc)
 	free(pipe_str);
 }
 
-static void collect_crcs(struct both_crcs *crcs)
+static void collect_crcs(struct both_crcs *crcs, bool mandatory_sink_crc)
 {
 	igt_pipe_crc_collect_crc(pipe_crc, &crcs->pipe);
 
 	if (sink_crc.supported)
-		get_sink_crc(&crcs->sink);
+		get_sink_crc(&crcs->sink, mandatory_sink_crc);
 	else
 		memcpy(&crcs->sink, "unsupported!", SINK_CRC_SIZE);
 }
 
-static void init_blue_crc(enum pixel_format format)
+static void init_blue_crc(enum pixel_format format, bool mandatory_sink_crc)
 {
 	struct igt_fb blue;
 	int rc;
@@ -1176,7 +1186,7 @@ static void init_blue_crc(enum pixel_format format)
 			    blue.fb_id, 0, 0, &prim_mode_params.connector_id, 1,
 			    prim_mode_params.mode);
 	igt_assert_eq(rc, 0);
-	collect_crcs(&blue_crcs[format].crc);
+	collect_crcs(&blue_crcs[format].crc, mandatory_sink_crc);
 
 	print_crc("Blue CRC:  ", &blue_crcs[format].crc);
 
@@ -1188,7 +1198,8 @@ static void init_blue_crc(enum pixel_format format)
 }
 
 static void init_crcs(enum pixel_format format,
-		      struct draw_pattern_info *pattern)
+		      struct draw_pattern_info *pattern,
+		      bool mandatory_sink_crc)
 {
 	int r, r_, rc;
 	struct igt_fb tmp_fbs[pattern->n_rects];
@@ -1224,7 +1235,7 @@ static void init_crcs(enum pixel_format format,
 				   &prim_mode_params.connector_id, 1,
 				   prim_mode_params.mode);
 		igt_assert_eq(rc, 0);
-		collect_crcs(&pattern->crcs[format][r]);
+		collect_crcs(&pattern->crcs[format][r], mandatory_sink_crc);
 	}
 
 	for (r = 0; r < pattern->n_rects; r++) {
@@ -1345,6 +1356,8 @@ static void setup_sink_crc(void)
 	errno_ = errno;
 	if (rc == -1 && errno_ == ENOTTY)
 		igt_info("Sink CRC not supported: panel doesn't support it\n");
+	if (rc == -1 && errno_ == ETIMEDOUT)
+		igt_info("Sink CRC not reliable on this panel: skipping it\n");
 	else if (rc == SINK_CRC_SIZE)
 		sink_crc.supported = true;
 	else
@@ -1577,7 +1590,7 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
 	if (!opt.check_crc || (flags__ & DONT_ASSERT_CRC))		\
 		break;							\
 									\
-	collect_crcs(&crc_);						\
+	collect_crcs(&crc_, mandatory_sink_crc);			\
 	print_crc("Calculated CRC:", &crc_);				\
 									\
 	igt_assert(wanted_crc);						\
@@ -1797,9 +1810,9 @@ static void prepare_subtest_data(const struct test_mode *t,
 
 	unset_all_crtcs();
 
-	init_blue_crc(t->format);
+	init_blue_crc(t->format, t->feature & FEATURE_PSR);
 	if (pattern)
-		init_crcs(t->format, pattern);
+		init_crcs(t->format, pattern, t->feature & FEATURE_PSR);
 
 	enable_features_for_test(t);
 }
-- 
2.4.3

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

Re: [PATCH i-g-t 3/5] kms_frontbuffer_tracking: Skip on unreliable CRC.

[Paulo Zanoni]

2015-12-03 14:39 GMT-02:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> Even with all sink crc re-works we still have platforms
> where after 6 vblanks it is unable to calculate the
> sink crc. But if we don't get the sink crc it isn't true
> that test failed, but that we have no ways to say test
> passed or failed.
>
> So let's print a message and move forward in case sink crc
> cannot help us to know if the screen has been updated.
>
> v2: Also include a message on setup_sink_crc and also
> only skip when it is mandatory, i.e. when running for PSR.

If I assume that the idea you're implementing is what we want, then
the patch looks correct and Acked-by: Paulo Zanoni
<paulo.r.zanoni@intel.com> .

My problem is the whole idea of "silently" skipping the tests in case
the sink CRC fails. What if QA's only machines always have this
problem with sink CRCs and always skip every single PSR test during
automatic testing? We won't discover that it's skipping, and we'll end
up assuming PSR works due to no test failures, while in fact it's not
even being tested. That said, I don't really have a solution for this.
Maybe someone else could have an idea here?

>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  tests/kms_frontbuffer_tracking.c | 37 +++++++++++++++++++++++++------------
>  1 file changed, 25 insertions(+), 12 deletions(-)
>
> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
> index ddcec75..e200975 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -859,12 +859,22 @@ static bool psr_wait_until_enabled(void)
>  #define psr_enable() igt_set_module_param_int("enable_psr", 1)
>  #define psr_disable() igt_set_module_param_int("enable_psr", 0)
>
> -static void get_sink_crc(sink_crc_t *crc)
> +static void get_sink_crc(sink_crc_t *crc, bool mandatory)
>  {
> +       int rc, errno_;
> +
>         lseek(sink_crc.fd, 0, SEEK_SET);
>
> -       igt_assert(read(sink_crc.fd, crc->data, SINK_CRC_SIZE) ==
> -                  SINK_CRC_SIZE);
> +       rc = read(sink_crc.fd, crc->data, SINK_CRC_SIZE);
> +       errno_ = errno;
> +
> +       if (rc == -1 && errno_ == ETIMEDOUT) {
> +               if (mandatory)
> +                       igt_skip("Sink CRC is unreliable on this machine. Try running this test again individually\n");
> +               else
> +                       igt_info("Sink CRC is unreliable on this machine. Try running this test again individually\n");
> +       }
> +       igt_assert(rc == SINK_CRC_SIZE);
>  }
>
>  static bool sink_crc_equal(sink_crc_t *a, sink_crc_t *b)
> @@ -1148,17 +1158,17 @@ static void print_crc(const char *str, struct both_crcs *crc)
>         free(pipe_str);
>  }
>
> -static void collect_crcs(struct both_crcs *crcs)
> +static void collect_crcs(struct both_crcs *crcs, bool mandatory_sink_crc)
>  {
>         igt_pipe_crc_collect_crc(pipe_crc, &crcs->pipe);
>
>         if (sink_crc.supported)
> -               get_sink_crc(&crcs->sink);
> +               get_sink_crc(&crcs->sink, mandatory_sink_crc);
>         else
>                 memcpy(&crcs->sink, "unsupported!", SINK_CRC_SIZE);
>  }
>
> -static void init_blue_crc(enum pixel_format format)
> +static void init_blue_crc(enum pixel_format format, bool mandatory_sink_crc)
>  {
>         struct igt_fb blue;
>         int rc;
> @@ -1176,7 +1186,7 @@ static void init_blue_crc(enum pixel_format format)
>                             blue.fb_id, 0, 0, &prim_mode_params.connector_id, 1,
>                             prim_mode_params.mode);
>         igt_assert_eq(rc, 0);
> -       collect_crcs(&blue_crcs[format].crc);
> +       collect_crcs(&blue_crcs[format].crc, mandatory_sink_crc);
>
>         print_crc("Blue CRC:  ", &blue_crcs[format].crc);
>
> @@ -1188,7 +1198,8 @@ static void init_blue_crc(enum pixel_format format)
>  }
>
>  static void init_crcs(enum pixel_format format,
> -                     struct draw_pattern_info *pattern)
> +                     struct draw_pattern_info *pattern,
> +                     bool mandatory_sink_crc)
>  {
>         int r, r_, rc;
>         struct igt_fb tmp_fbs[pattern->n_rects];
> @@ -1224,7 +1235,7 @@ static void init_crcs(enum pixel_format format,
>                                    &prim_mode_params.connector_id, 1,
>                                    prim_mode_params.mode);
>                 igt_assert_eq(rc, 0);
> -               collect_crcs(&pattern->crcs[format][r]);
> +               collect_crcs(&pattern->crcs[format][r], mandatory_sink_crc);
>         }
>
>         for (r = 0; r < pattern->n_rects; r++) {
> @@ -1345,6 +1356,8 @@ static void setup_sink_crc(void)
>         errno_ = errno;
>         if (rc == -1 && errno_ == ENOTTY)
>                 igt_info("Sink CRC not supported: panel doesn't support it\n");
> +       if (rc == -1 && errno_ == ETIMEDOUT)
> +               igt_info("Sink CRC not reliable on this panel: skipping it\n");
>         else if (rc == SINK_CRC_SIZE)
>                 sink_crc.supported = true;
>         else
> @@ -1577,7 +1590,7 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
>         if (!opt.check_crc || (flags__ & DONT_ASSERT_CRC))              \
>                 break;                                                  \
>                                                                         \
> -       collect_crcs(&crc_);                                            \
> +       collect_crcs(&crc_, mandatory_sink_crc);                        \
>         print_crc("Calculated CRC:", &crc_);                            \
>                                                                         \
>         igt_assert(wanted_crc);                                         \
> @@ -1797,9 +1810,9 @@ static void prepare_subtest_data(const struct test_mode *t,
>
>         unset_all_crtcs();
>
> -       init_blue_crc(t->format);
> +       init_blue_crc(t->format, t->feature & FEATURE_PSR);
>         if (pattern)
> -               init_crcs(t->format, pattern);
> +               init_crcs(t->format, pattern, t->feature & FEATURE_PSR);
>
>         enable_features_for_test(t);
>  }
> --
> 2.4.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH i-g-t 3/5] kms_frontbuffer_tracking: Skip on unreliable CRC.

[Vivi, Rodrigo]

On Thu, 2015-12-03 at 15:05 -0200, Paulo Zanoni wrote:
> 2015-12-03 14:39 GMT-02:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> > Even with all sink crc re-works we still have platforms
> > where after 6 vblanks it is unable to calculate the
> > sink crc. But if we don't get the sink crc it isn't true
> > that test failed, but that we have no ways to say test
> > passed or failed.
> > 
> > So let's print a message and move forward in case sink crc
> > cannot help us to know if the screen has been updated.
> > 
> > v2: Also include a message on setup_sink_crc and also
> > only skip when it is mandatory, i.e. when running for PSR.
> 
> If I assume that the idea you're implementing is what we want, then
> the patch looks correct and Acked-by: Paulo Zanoni
> <paulo.r.zanoni@intel.com> .
> 
> My problem is the whole idea of "silently" skipping the tests in case
> the sink CRC fails. What if QA's only machines always have this
> problem with sink CRCs and always skip every single PSR test during
> automatic testing? We won't discover that it's skipping, and we'll 
> end
> up assuming PSR works due to no test failures, while in fact it's not
> even being tested. That said, I don't really have a solution for 
> this.

Yeah, this is a good point. This is one of the reasons I want to add
that aux fix soon as well. With that in place the sink CRC is not that
unreliable and if skip will be rare and random.
I run the tests in many different platforms here and many times and it
is really rare to skip so I prefer to skip when that happens than have
new bug entry for every false positive that might randomly happen.
Also we know it is a sink CRC issue and not a PSR one, but people
looking to the test will believe that PSR failed and not sink crc...

> Maybe someone else could have an idea here?
> 
> > 
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  tests/kms_frontbuffer_tracking.c | 37 +++++++++++++++++++++++++---
> > ---------
> >  1 file changed, 25 insertions(+), 12 deletions(-)
> > 
> > diff --git a/tests/kms_frontbuffer_tracking.c 
> > b/tests/kms_frontbuffer_tracking.c
> > index ddcec75..e200975 100644
> > --- a/tests/kms_frontbuffer_tracking.c
> > +++ b/tests/kms_frontbuffer_tracking.c
> > @@ -859,12 +859,22 @@ static bool psr_wait_until_enabled(void)
> >  #define psr_enable() igt_set_module_param_int("enable_psr", 1)
> >  #define psr_disable() igt_set_module_param_int("enable_psr", 0)
> > 
> > -static void get_sink_crc(sink_crc_t *crc)
> > +static void get_sink_crc(sink_crc_t *crc, bool mandatory)
> >  {
> > +       int rc, errno_;
> > +
> >         lseek(sink_crc.fd, 0, SEEK_SET);
> > 
> > -       igt_assert(read(sink_crc.fd, crc->data, SINK_CRC_SIZE) ==
> > -                  SINK_CRC_SIZE);
> > +       rc = read(sink_crc.fd, crc->data, SINK_CRC_SIZE);
> > +       errno_ = errno;
> > +
> > +       if (rc == -1 && errno_ == ETIMEDOUT) {
> > +               if (mandatory)
> > +                       igt_skip("Sink CRC is unreliable on this 
> > machine. Try running this test again individually\n");
> > +               else
> > +                       igt_info("Sink CRC is unreliable on this 
> > machine. Try running this test again individually\n");
> > +       }
> > +       igt_assert(rc == SINK_CRC_SIZE);
> >  }
> > 
> >  static bool sink_crc_equal(sink_crc_t *a, sink_crc_t *b)
> > @@ -1148,17 +1158,17 @@ static void print_crc(const char *str, 
> > struct both_crcs *crc)
> >         free(pipe_str);
> >  }
> > 
> > -static void collect_crcs(struct both_crcs *crcs)
> > +static void collect_crcs(struct both_crcs *crcs, bool 
> > mandatory_sink_crc)
> >  {
> >         igt_pipe_crc_collect_crc(pipe_crc, &crcs->pipe);
> > 
> >         if (sink_crc.supported)
> > -               get_sink_crc(&crcs->sink);
> > +               get_sink_crc(&crcs->sink, mandatory_sink_crc);
> >         else
> >                 memcpy(&crcs->sink, "unsupported!", SINK_CRC_SIZE);
> >  }
> > 
> > -static void init_blue_crc(enum pixel_format format)
> > +static void init_blue_crc(enum pixel_format format, bool 
> > mandatory_sink_crc)
> >  {
> >         struct igt_fb blue;
> >         int rc;
> > @@ -1176,7 +1186,7 @@ static void init_blue_crc(enum pixel_format 
> > format)
> >                             blue.fb_id, 0, 0, 
> > &prim_mode_params.connector_id, 1,
> >                             prim_mode_params.mode);
> >         igt_assert_eq(rc, 0);
> > -       collect_crcs(&blue_crcs[format].crc);
> > +       collect_crcs(&blue_crcs[format].crc, mandatory_sink_crc);
> > 
> >         print_crc("Blue CRC:  ", &blue_crcs[format].crc);
> > 
> > @@ -1188,7 +1198,8 @@ static void init_blue_crc(enum pixel_format 
> > format)
> >  }
> > 
> >  static void init_crcs(enum pixel_format format,
> > -                     struct draw_pattern_info *pattern)
> > +                     struct draw_pattern_info *pattern,
> > +                     bool mandatory_sink_crc)
> >  {
> >         int r, r_, rc;
> >         struct igt_fb tmp_fbs[pattern->n_rects];
> > @@ -1224,7 +1235,7 @@ static void init_crcs(enum pixel_format 
> > format,
> >                                    &prim_mode_params.connector_id, 
> > 1,
> >                                    prim_mode_params.mode);
> >                 igt_assert_eq(rc, 0);
> > -               collect_crcs(&pattern->crcs[format][r]);
> > +               collect_crcs(&pattern->crcs[format][r], 
> > mandatory_sink_crc);
> >         }
> > 
> >         for (r = 0; r < pattern->n_rects; r++) {
> > @@ -1345,6 +1356,8 @@ static void setup_sink_crc(void)
> >         errno_ = errno;
> >         if (rc == -1 && errno_ == ENOTTY)
> >                 igt_info("Sink CRC not supported: panel doesn't 
> > support it\n");
> > +       if (rc == -1 && errno_ == ETIMEDOUT)
> > +               igt_info("Sink CRC not reliable on this panel: 
> > skipping it\n");
> >         else if (rc == SINK_CRC_SIZE)
> >                 sink_crc.supported = true;
> >         else
> > @@ -1577,7 +1590,7 @@ static int adjust_assertion_flags(const 
> > struct test_mode *t, int flags)
> >         if (!opt.check_crc || (flags__ & DONT_ASSERT_CRC))         
> >      \
> >                 break;                                             
> >      \
> >                                                                    
> >      \
> > -       collect_crcs(&crc_);                                       
> >      \
> > +       collect_crcs(&crc_, mandatory_sink_crc);                   
> >      \
> >         print_crc("Calculated CRC:", &crc_);                       
> >      \
> >                                                                    
> >      \
> >         igt_assert(wanted_crc);                                    
> >      \
> > @@ -1797,9 +1810,9 @@ static void prepare_subtest_data(const struct 
> > test_mode *t,
> > 
> >         unset_all_crtcs();
> > 
> > -       init_blue_crc(t->format);
> > +       init_blue_crc(t->format, t->feature & FEATURE_PSR);
> >         if (pattern)
> > -               init_crcs(t->format, pattern);
> > +               init_crcs(t->format, pattern, t->feature & 
> > FEATURE_PSR);
> > 
> >         enable_features_for_test(t);
> >  }
> > --
> > 2.4.3
> > 
> > _______________________________________________
> > 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

Re: [PATCH i-g-t 3/5] kms_frontbuffer_tracking: Skip on unreliable CRC.

[Paulo Zanoni]

2015-12-03 17:44 GMT-02:00 Vivi, Rodrigo <rodrigo.vivi@intel.com>:
> On Thu, 2015-12-03 at 15:05 -0200, Paulo Zanoni wrote:
>> 2015-12-03 14:39 GMT-02:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
>> > Even with all sink crc re-works we still have platforms
>> > where after 6 vblanks it is unable to calculate the
>> > sink crc. But if we don't get the sink crc it isn't true
>> > that test failed, but that we have no ways to say test
>> > passed or failed.
>> >
>> > So let's print a message and move forward in case sink crc
>> > cannot help us to know if the screen has been updated.
>> >
>> > v2: Also include a message on setup_sink_crc and also
>> > only skip when it is mandatory, i.e. when running for PSR.
>>
>> If I assume that the idea you're implementing is what we want, then
>> the patch looks correct and Acked-by: Paulo Zanoni
>> <paulo.r.zanoni@intel.com> .
>>
>> My problem is the whole idea of "silently" skipping the tests in case
>> the sink CRC fails. What if QA's only machines always have this
>> problem with sink CRCs and always skip every single PSR test during
>> automatic testing? We won't discover that it's skipping, and we'll
>> end
>> up assuming PSR works due to no test failures, while in fact it's not
>> even being tested. That said, I don't really have a solution for
>> this.
>
> Yeah, this is a good point. This is one of the reasons I want to add
> that aux fix soon as well. With that in place the sink CRC is not that
> unreliable and if skip will be rare and random.
> I run the tests in many different platforms here and many times and it
> is really rare to skip so I prefer to skip when that happens than have
> new bug entry for every false positive that might randomly happen.
> Also we know it is a sink CRC issue and not a PSR one, but people
> looking to the test will believe that PSR failed and not sink crc...

If the current failures don't happen 100% of the time, I wonder if it
wouldn't be better to just re-run the same subtest all over again
(including the mode unset/sets) until the CRCs work.

>
>> Maybe someone else could have an idea here?
>>
>> >
>> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > ---
>> >  tests/kms_frontbuffer_tracking.c | 37 +++++++++++++++++++++++++---
>> > ---------
>> >  1 file changed, 25 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/tests/kms_frontbuffer_tracking.c
>> > b/tests/kms_frontbuffer_tracking.c
>> > index ddcec75..e200975 100644
>> > --- a/tests/kms_frontbuffer_tracking.c
>> > +++ b/tests/kms_frontbuffer_tracking.c
>> > @@ -859,12 +859,22 @@ static bool psr_wait_until_enabled(void)
>> >  #define psr_enable() igt_set_module_param_int("enable_psr", 1)
>> >  #define psr_disable() igt_set_module_param_int("enable_psr", 0)
>> >
>> > -static void get_sink_crc(sink_crc_t *crc)
>> > +static void get_sink_crc(sink_crc_t *crc, bool mandatory)
>> >  {
>> > +       int rc, errno_;
>> > +
>> >         lseek(sink_crc.fd, 0, SEEK_SET);
>> >
>> > -       igt_assert(read(sink_crc.fd, crc->data, SINK_CRC_SIZE) ==
>> > -                  SINK_CRC_SIZE);
>> > +       rc = read(sink_crc.fd, crc->data, SINK_CRC_SIZE);
>> > +       errno_ = errno;
>> > +
>> > +       if (rc == -1 && errno_ == ETIMEDOUT) {
>> > +               if (mandatory)
>> > +                       igt_skip("Sink CRC is unreliable on this
>> > machine. Try running this test again individually\n");
>> > +               else
>> > +                       igt_info("Sink CRC is unreliable on this
>> > machine. Try running this test again individually\n");
>> > +       }
>> > +       igt_assert(rc == SINK_CRC_SIZE);
>> >  }
>> >
>> >  static bool sink_crc_equal(sink_crc_t *a, sink_crc_t *b)
>> > @@ -1148,17 +1158,17 @@ static void print_crc(const char *str,
>> > struct both_crcs *crc)
>> >         free(pipe_str);
>> >  }
>> >
>> > -static void collect_crcs(struct both_crcs *crcs)
>> > +static void collect_crcs(struct both_crcs *crcs, bool
>> > mandatory_sink_crc)
>> >  {
>> >         igt_pipe_crc_collect_crc(pipe_crc, &crcs->pipe);
>> >
>> >         if (sink_crc.supported)
>> > -               get_sink_crc(&crcs->sink);
>> > +               get_sink_crc(&crcs->sink, mandatory_sink_crc);
>> >         else
>> >                 memcpy(&crcs->sink, "unsupported!", SINK_CRC_SIZE);
>> >  }
>> >
>> > -static void init_blue_crc(enum pixel_format format)
>> > +static void init_blue_crc(enum pixel_format format, bool
>> > mandatory_sink_crc)
>> >  {
>> >         struct igt_fb blue;
>> >         int rc;
>> > @@ -1176,7 +1186,7 @@ static void init_blue_crc(enum pixel_format
>> > format)
>> >                             blue.fb_id, 0, 0,
>> > &prim_mode_params.connector_id, 1,
>> >                             prim_mode_params.mode);
>> >         igt_assert_eq(rc, 0);
>> > -       collect_crcs(&blue_crcs[format].crc);
>> > +       collect_crcs(&blue_crcs[format].crc, mandatory_sink_crc);
>> >
>> >         print_crc("Blue CRC:  ", &blue_crcs[format].crc);
>> >
>> > @@ -1188,7 +1198,8 @@ static void init_blue_crc(enum pixel_format
>> > format)
>> >  }
>> >
>> >  static void init_crcs(enum pixel_format format,
>> > -                     struct draw_pattern_info *pattern)
>> > +                     struct draw_pattern_info *pattern,
>> > +                     bool mandatory_sink_crc)
>> >  {
>> >         int r, r_, rc;
>> >         struct igt_fb tmp_fbs[pattern->n_rects];
>> > @@ -1224,7 +1235,7 @@ static void init_crcs(enum pixel_format
>> > format,
>> >                                    &prim_mode_params.connector_id,
>> > 1,
>> >                                    prim_mode_params.mode);
>> >                 igt_assert_eq(rc, 0);
>> > -               collect_crcs(&pattern->crcs[format][r]);
>> > +               collect_crcs(&pattern->crcs[format][r],
>> > mandatory_sink_crc);
>> >         }
>> >
>> >         for (r = 0; r < pattern->n_rects; r++) {
>> > @@ -1345,6 +1356,8 @@ static void setup_sink_crc(void)
>> >         errno_ = errno;
>> >         if (rc == -1 && errno_ == ENOTTY)
>> >                 igt_info("Sink CRC not supported: panel doesn't
>> > support it\n");
>> > +       if (rc == -1 && errno_ == ETIMEDOUT)
>> > +               igt_info("Sink CRC not reliable on this panel:
>> > skipping it\n");
>> >         else if (rc == SINK_CRC_SIZE)
>> >                 sink_crc.supported = true;
>> >         else
>> > @@ -1577,7 +1590,7 @@ static int adjust_assertion_flags(const
>> > struct test_mode *t, int flags)
>> >         if (!opt.check_crc || (flags__ & DONT_ASSERT_CRC))
>> >      \
>> >                 break;
>> >      \
>> >
>> >      \
>> > -       collect_crcs(&crc_);
>> >      \
>> > +       collect_crcs(&crc_, mandatory_sink_crc);
>> >      \
>> >         print_crc("Calculated CRC:", &crc_);
>> >      \
>> >
>> >      \
>> >         igt_assert(wanted_crc);
>> >      \
>> > @@ -1797,9 +1810,9 @@ static void prepare_subtest_data(const struct
>> > test_mode *t,
>> >
>> >         unset_all_crtcs();
>> >
>> > -       init_blue_crc(t->format);
>> > +       init_blue_crc(t->format, t->feature & FEATURE_PSR);
>> >         if (pattern)
>> > -               init_crcs(t->format, pattern);
>> > +               init_crcs(t->format, pattern, t->feature &
>> > FEATURE_PSR);
>> >
>> >         enable_features_for_test(t);
>> >  }
>> > --
>> > 2.4.3
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>>



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

Re: [PATCH i-g-t 3/5] kms_frontbuffer_tracking: Skip on unreliable CRC.

[Vivi, Rodrigo]

On Thu, 2015-12-03 at 17:59 -0200, Paulo Zanoni wrote:
> 2015-12-03 17:44 GMT-02:00 Vivi, Rodrigo <rodrigo.vivi@intel.com>:
> > On Thu, 2015-12-03 at 15:05 -0200, Paulo Zanoni wrote:
> > > 2015-12-03 14:39 GMT-02:00 Rodrigo Vivi <rodrigo.vivi@intel.com>:
> > > > Even with all sink crc re-works we still have platforms
> > > > where after 6 vblanks it is unable to calculate the
> > > > sink crc. But if we don't get the sink crc it isn't true
> > > > that test failed, but that we have no ways to say test
> > > > passed or failed.
> > > > 
> > > > So let's print a message and move forward in case sink crc
> > > > cannot help us to know if the screen has been updated.
> > > > 
> > > > v2: Also include a message on setup_sink_crc and also
> > > > only skip when it is mandatory, i.e. when running for PSR.
> > > 
> > > If I assume that the idea you're implementing is what we want, 
> > > then
> > > the patch looks correct and Acked-by: Paulo Zanoni
> > > <paulo.r.zanoni@intel.com> .
> > > 
> > > My problem is the whole idea of "silently" skipping the tests in 
> > > case
> > > the sink CRC fails. What if QA's only machines always have this
> > > problem with sink CRCs and always skip every single PSR test 
> > > during
> > > automatic testing? We won't discover that it's skipping, and 
> > > we'll
> > > end
> > > up assuming PSR works due to no test failures, while in fact it's 
> > > not
> > > even being tested. That said, I don't really have a solution for
> > > this.
> > 
> > Yeah, this is a good point. This is one of the reasons I want to 
> > add
> > that aux fix soon as well. With that in place the sink CRC is not 
> > that
> > unreliable and if skip will be rare and random.
> > I run the tests in many different platforms here and many times and 
> > it
> > is really rare to skip so I prefer to skip when that happens than 
> > have
> > new bug entry for every false positive that might randomly happen.
> > Also we know it is a sink CRC issue and not a PSR one, but people
> > looking to the test will believe that PSR failed and not sink 
> > crc...
> 
> If the current failures don't happen 100% of the time, I wonder if it
> wouldn't be better to just re-run the same subtest all over again
> (including the mode unset/sets) until the CRCs work.

Yeah, I agree this is a best approach. Let's stay with this skip for
now and apply this retry on top

> 
> > 
> > > Maybe someone else could have an idea here?
> > > 
> > > > 
> > > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > ---
> > > >  tests/kms_frontbuffer_tracking.c | 37 
> > > > +++++++++++++++++++++++++---
> > > > ---------
> > > >  1 file changed, 25 insertions(+), 12 deletions(-)
> > > > 
> > > > diff --git a/tests/kms_frontbuffer_tracking.c
> > > > b/tests/kms_frontbuffer_tracking.c
> > > > index ddcec75..e200975 100644
> > > > --- a/tests/kms_frontbuffer_tracking.c
> > > > +++ b/tests/kms_frontbuffer_tracking.c
> > > > @@ -859,12 +859,22 @@ static bool psr_wait_until_enabled(void)
> > > >  #define psr_enable() igt_set_module_param_int("enable_psr", 1)
> > > >  #define psr_disable() igt_set_module_param_int("enable_psr", 
> > > > 0)
> > > > 
> > > > -static void get_sink_crc(sink_crc_t *crc)
> > > > +static void get_sink_crc(sink_crc_t *crc, bool mandatory)
> > > >  {
> > > > +       int rc, errno_;
> > > > +
> > > >         lseek(sink_crc.fd, 0, SEEK_SET);
> > > > 
> > > > -       igt_assert(read(sink_crc.fd, crc->data, SINK_CRC_SIZE) 
> > > > ==
> > > > -                  SINK_CRC_SIZE);
> > > > +       rc = read(sink_crc.fd, crc->data, SINK_CRC_SIZE);
> > > > +       errno_ = errno;
> > > > +
> > > > +       if (rc == -1 && errno_ == ETIMEDOUT) {
> > > > +               if (mandatory)
> > > > +                       igt_skip("Sink CRC is unreliable on 
> > > > this
> > > > machine. Try running this test again individually\n");
> > > > +               else
> > > > +                       igt_info("Sink CRC is unreliable on 
> > > > this
> > > > machine. Try running this test again individually\n");
> > > > +       }
> > > > +       igt_assert(rc == SINK_CRC_SIZE);
> > > >  }
> > > > 
> > > >  static bool sink_crc_equal(sink_crc_t *a, sink_crc_t *b)
> > > > @@ -1148,17 +1158,17 @@ static void print_crc(const char *str,
> > > > struct both_crcs *crc)
> > > >         free(pipe_str);
> > > >  }
> > > > 
> > > > -static void collect_crcs(struct both_crcs *crcs)
> > > > +static void collect_crcs(struct both_crcs *crcs, bool
> > > > mandatory_sink_crc)
> > > >  {
> > > >         igt_pipe_crc_collect_crc(pipe_crc, &crcs->pipe);
> > > > 
> > > >         if (sink_crc.supported)
> > > > -               get_sink_crc(&crcs->sink);
> > > > +               get_sink_crc(&crcs->sink, mandatory_sink_crc);
> > > >         else
> > > >                 memcpy(&crcs->sink, "unsupported!", 
> > > > SINK_CRC_SIZE);
> > > >  }
> > > > 
> > > > -static void init_blue_crc(enum pixel_format format)
> > > > +static void init_blue_crc(enum pixel_format format, bool
> > > > mandatory_sink_crc)
> > > >  {
> > > >         struct igt_fb blue;
> > > >         int rc;
> > > > @@ -1176,7 +1186,7 @@ static void init_blue_crc(enum 
> > > > pixel_format
> > > > format)
> > > >                             blue.fb_id, 0, 0,
> > > > &prim_mode_params.connector_id, 1,
> > > >                             prim_mode_params.mode);
> > > >         igt_assert_eq(rc, 0);
> > > > -       collect_crcs(&blue_crcs[format].crc);
> > > > +       collect_crcs(&blue_crcs[format].crc, 
> > > > mandatory_sink_crc);
> > > > 
> > > >         print_crc("Blue CRC:  ", &blue_crcs[format].crc);
> > > > 
> > > > @@ -1188,7 +1198,8 @@ static void init_blue_crc(enum 
> > > > pixel_format
> > > > format)
> > > >  }
> > > > 
> > > >  static void init_crcs(enum pixel_format format,
> > > > -                     struct draw_pattern_info *pattern)
> > > > +                     struct draw_pattern_info *pattern,
> > > > +                     bool mandatory_sink_crc)
> > > >  {
> > > >         int r, r_, rc;
> > > >         struct igt_fb tmp_fbs[pattern->n_rects];
> > > > @@ -1224,7 +1235,7 @@ static void init_crcs(enum pixel_format
> > > > format,
> > > >                                   
> > > >  &prim_mode_params.connector_id,
> > > > 1,
> > > >                                    prim_mode_params.mode);
> > > >                 igt_assert_eq(rc, 0);
> > > > -               collect_crcs(&pattern->crcs[format][r]);
> > > > +               collect_crcs(&pattern->crcs[format][r],
> > > > mandatory_sink_crc);
> > > >         }
> > > > 
> > > >         for (r = 0; r < pattern->n_rects; r++) {
> > > > @@ -1345,6 +1356,8 @@ static void setup_sink_crc(void)
> > > >         errno_ = errno;
> > > >         if (rc == -1 && errno_ == ENOTTY)
> > > >                 igt_info("Sink CRC not supported: panel doesn't
> > > > support it\n");
> > > > +       if (rc == -1 && errno_ == ETIMEDOUT)
> > > > +               igt_info("Sink CRC not reliable on this panel:
> > > > skipping it\n");
> > > >         else if (rc == SINK_CRC_SIZE)
> > > >                 sink_crc.supported = true;
> > > >         else
> > > > @@ -1577,7 +1590,7 @@ static int adjust_assertion_flags(const
> > > > struct test_mode *t, int flags)
> > > >         if (!opt.check_crc || (flags__ & DONT_ASSERT_CRC))
> > > >      \
> > > >                 break;
> > > >      \
> > > > 
> > > >      \
> > > > -       collect_crcs(&crc_);
> > > >      \
> > > > +       collect_crcs(&crc_, mandatory_sink_crc);
> > > >      \
> > > >         print_crc("Calculated CRC:", &crc_);
> > > >      \
> > > > 
> > > >      \
> > > >         igt_assert(wanted_crc);
> > > >      \
> > > > @@ -1797,9 +1810,9 @@ static void prepare_subtest_data(const 
> > > > struct
> > > > test_mode *t,
> > > > 
> > > >         unset_all_crtcs();
> > > > 
> > > > -       init_blue_crc(t->format);
> > > > +       init_blue_crc(t->format, t->feature & FEATURE_PSR);
> > > >         if (pattern)
> > > > -               init_crcs(t->format, pattern);
> > > > +               init_crcs(t->format, pattern, t->feature &
> > > > FEATURE_PSR);
> > > > 
> > > >         enable_features_for_test(t);
> > > >  }
> > > > --
> > > > 2.4.3
> > > > 
> > > > _______________________________________________
> > > > 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

[PATCH i-g-t 4/5] kms_psr_sink_crc: Fix no-psr option.

[Rodrigo Vivi]

commit 75b286e821 ("tests/kms_psr_sink_crc: test even
if PSR is disabled by default")' force PSR enabling without
respecting the no-psr (running-with-psr-disabled) option.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 tests/kms_psr_sink_crc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index 387c615..b2f88ea 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -557,7 +557,8 @@ int main(int argc, char *argv[])
 		kmstest_set_vt_graphics_mode();
 		data.devid = intel_get_drm_devid(data.drm_fd);
 
-		igt_set_module_param_int("enable_psr", 1);
+		igt_set_module_param_int("enable_psr", running_with_psr_disabled ?
+					 0 : 1);
 
 		igt_skip_on(!psr_possible(&data));
 
-- 
2.4.3

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

[PATCH i-g-t 5/5] kms_psr_sink_crc: Add suspend/resume sub test.

[Rodrigo Vivi]

Although kms_frontbuffer_tracking already has psr-suspend testcase
this one here can complement it by testing different combination
and mainly covering 2 different cases individually:

1. wait-for-psr, suspend-resume tehn run 1 operation.

2. suspend-resume, wait-for-psr then run 1 operation.

v2: Remove no-suspend option since this should be done with piglit
if necessary for now.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 tests/kms_psr_sink_crc.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index b2f88ea..7768a60 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -626,6 +626,36 @@ int main(int argc, char *argv[])
 		test_cleanup(&data);
 	}
 
+	igt_subtest_f("suspend_psr_active") {
+
+		igt_skip_on(no_suspend);
+
+		data.test_plane = PRIMARY;
+		data.op = PAGE_FLIP;
+		setup_test_plane(&data);
+		igt_assert(wait_psr_entry(&data));
+
+		igt_system_suspend_autoresume();
+
+		run_test(&data);
+		test_cleanup(&data);
+	}
+
+	igt_subtest_f("suspend_psr_exit") {
+
+		igt_skip_on(no_suspend);
+
+		data.test_plane = CURSOR;
+		data.op = PLANE_ONOFF;
+		setup_test_plane(&data);
+
+		igt_system_suspend_autoresume();
+
+		igt_assert(wait_psr_entry(&data));
+		run_test(&data);
+		test_cleanup(&data);
+	}
+
 	igt_fixture {
 		drm_intel_bufmgr_destroy(data.bufmgr);
 		display_fini(&data);
-- 
2.4.3

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

[PATCH i-g-t] kms_psr_sink_crc: Add suspend/resume sub test.

[Rodrigo Vivi]

Although kms_frontbuffer_tracking already has psr-suspend testcase
this one here can complement it by testing different combination
and mainly covering 2 different cases individually:

1. wait-for-psr, suspend-resume tehn run 1 operation.

2. suspend-resume, wait-for-psr then run 1 operation.

v2: Remove no-suspend option since this should be done with piglit
if necessary for now.

v3: argh! remove remaining no-suspend checks...

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 tests/kms_psr_sink_crc.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
index b2f88ea..8843ed8 100644
--- a/tests/kms_psr_sink_crc.c
+++ b/tests/kms_psr_sink_crc.c
@@ -626,6 +626,30 @@ int main(int argc, char *argv[])
 		test_cleanup(&data);
 	}
 
+	igt_subtest_f("suspend_psr_active") {
+		data.test_plane = PRIMARY;
+		data.op = PAGE_FLIP;
+		setup_test_plane(&data);
+		igt_assert(wait_psr_entry(&data));
+
+		igt_system_suspend_autoresume();
+
+		run_test(&data);
+		test_cleanup(&data);
+	}
+
+	igt_subtest_f("suspend_psr_exit") {
+		data.test_plane = CURSOR;
+		data.op = PLANE_ONOFF;
+		setup_test_plane(&data);
+
+		igt_system_suspend_autoresume();
+
+		igt_assert(wait_psr_entry(&data));
+		run_test(&data);
+		test_cleanup(&data);
+	}
+
 	igt_fixture {
 		drm_intel_bufmgr_destroy(data.bufmgr);
 		display_fini(&data);
-- 
2.4.3

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