[PATCH igt 1/6] tests/kms_draw_crc: remove unnecessary mode unset calls

[PATCH igt 1/6] tests/kms_draw_crc: remove unnecessary mode unset calls

[Paulo Zanoni]

I couldn't think of a reason why we would need to unset the CRTCs
before doing the modesets on this test, so remove all the mode unset
calls.

Before:
$ time -p sudo ./kms_draw_crc
real 44.74
$ time -p for i in $(sudo ./kms_draw_crc --list-subtests); do sudo
./kms_draw_crc --run-subtest $i; done
real 121.61

After:
$ time -p sudo ./kms_draw_crc
real 7.40
$ time -p for i in $(sudo ./kms_draw_crc --list-subtests); do sudo
./kms_draw_crc --run-subtest $i; done
real 14.32

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/kms_draw_crc.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
index cb28052..e163981 100644
--- a/tests/kms_draw_crc.c
+++ b/tests/kms_draw_crc.c
@@ -143,7 +143,6 @@ static void get_method_crc(enum igt_draw_method method, uint32_t drm_format,
 
 	igt_pipe_crc_collect_crc(pipe_crc, crc);
 
-	kmstest_unset_all_crtcs(drm_fd, drm_res);
 	igt_remove_fb(drm_fd, &fb);
 }
 
@@ -152,8 +151,6 @@ static void draw_method_subtest(enum igt_draw_method method,
 {
 	igt_crc_t crc;
 
-	kmstest_unset_all_crtcs(drm_fd, drm_res);
-
 	/* Use IGT_DRAW_MMAP_GTT on an untiled buffer as the parameter for
 	 * comparison. Cache the value so we don't recompute it for every single
 	 * subtest. */
@@ -184,7 +181,6 @@ static void get_fill_crc(uint64_t tiling, igt_crc_t *crc)
 
 	igt_pipe_crc_collect_crc(pipe_crc, crc);
 
-	kmstest_unset_all_crtcs(drm_fd, drm_res);
 	igt_remove_fb(drm_fd, &fb);
 }
 
@@ -194,8 +190,6 @@ static void fill_fb_subtest(void)
 	struct igt_fb fb;
 	igt_crc_t base_crc, crc;
 
-	kmstest_unset_all_crtcs(drm_fd, drm_res);
-
 	igt_create_fb(drm_fd, ms.mode->hdisplay, ms.mode->vdisplay,
 		      DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, &fb);
 
@@ -214,7 +208,6 @@ static void fill_fb_subtest(void)
 	get_fill_crc(LOCAL_I915_FORMAT_MOD_X_TILED, &crc);
 	igt_assert_crc_equal(&crc, &base_crc);
 
-	kmstest_unset_all_crtcs(drm_fd, drm_res);
 	igt_remove_fb(drm_fd, &fb);
 }
 
-- 
2.7.4

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

[PATCH igt 2/6] kms_frontbuffer_tracking: fix sink CRC assertion

[Paulo Zanoni]

If we already detected an error, don't try to assert the size of what
we didn't read.

In machines where the sink CRC is unreliable, this was failing tests
where the sink CRC is not mandatory (FBC tests). With this change we
won't fail anymore, we'll just print error messages saying that the
sink CRC is unreliable.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/kms_frontbuffer_tracking.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

This is important. I thought I had merged this in the upstream repo a long time
ago...

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index fd3ad53..81532dc 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -943,8 +943,9 @@ static void get_sink_crc(sink_crc_t *crc, bool 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");
+	} else {
+		igt_assert(rc == SINK_CRC_SIZE);
 	}
-	igt_assert(rc == SINK_CRC_SIZE);
 }
 
 static bool sink_crc_equal(sink_crc_t *a, sink_crc_t *b)
-- 
2.7.4

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

[PATCH igt 3/6] kms_frontbuffer_tracking: move more code to get_sink_crc()

[Paulo Zanoni]

Make it check for the supported flag and decide what to do. This
change will make the next patches much easier, and it's probably
better to move more sink CRC logic to the sink CRC function.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/kms_frontbuffer_tracking.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 81532dc..4a46942 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -933,6 +933,11 @@ static void get_sink_crc(sink_crc_t *crc, bool mandatory)
 {
 	int rc, errno_;
 
+	if (!sink_crc.supported) {
+		memcpy(crc, "unsupported!", SINK_CRC_SIZE);
+		return;
+	}
+
 	lseek(sink_crc.fd, 0, SEEK_SET);
 
 	rc = read(sink_crc.fd, crc->data, SINK_CRC_SIZE);
@@ -1220,11 +1225,7 @@ static void print_crc(const char *str, struct both_crcs *crc)
 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, mandatory_sink_crc);
-	else
-		memcpy(&crcs->sink, "unsupported!", SINK_CRC_SIZE);
+	get_sink_crc(&crcs->sink, mandatory_sink_crc);
 }
 
 static void init_blue_crc(enum pixel_format format, bool mandatory_sink_crc)
-- 
2.7.4

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

[PATCH igt 4/6] kms_frontbuffer_tracking: refactor sink CRC reliability handling

[Paulo Zanoni]

What I'm currently seeing is that sometimes the first check during
setup_sink_crc() returns valid sink CRC, but then the subsequent
checks return ETIMEDOUT. In these cases, we keep getting flooded by
messages saying that our sink CRC is unreliable and that the results
differ. This is annoying for the FBC tests where sink CRC is not
mandatory.

Since this case shows it's useless to try to check for sink CRC
reliability before the actual tests, refactor the code in a way that
if at any point we detect that sink CRC is unreliable we'll mark it as
such and stop flooding the logs. For the tests where it's mandatory
we'll still keep the same SKIP behavior.

This refactor also allows us to just call get_sink_crc() in the
setup_sink_crc() function instead of having to reimplement the same
logic.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/kms_frontbuffer_tracking.c | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 4a46942..8aa6362 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -203,9 +203,11 @@ struct both_crcs *wanted_crc;
 struct {
 	int fd;
 	bool supported;
+	bool reliable;
 } sink_crc = {
 	.fd = -1,
-	.supported = false,
+	.supported = true,
+	.reliable = true,
 };
 
 /* The goal of this structure is to easily allow us to deal with cases where we
@@ -943,11 +945,17 @@ static void get_sink_crc(sink_crc_t *crc, bool mandatory)
 	rc = read(sink_crc.fd, crc->data, SINK_CRC_SIZE);
 	errno_ = errno;
 
-	if (rc == -1 && errno_ == ETIMEDOUT) {
+	if (rc == -1 && errno_ == ENOTTY) {
+		igt_info("Sink CRC not supported: panel doesn't support it\n");
+		sink_crc.supported = false;
+	} else if (rc == -1 && errno_ == ETIMEDOUT) {
+		if (sink_crc.reliable) {
+			igt_info("Sink CRC is unreliable on this machine.\n");
+			sink_crc.reliable = false;
+		}
+
 		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_skip("Sink CRC is unreliable on this machine.\n");
 	} else {
 		igt_assert(rc == SINK_CRC_SIZE);
 	}
@@ -1396,9 +1404,7 @@ static void teardown_modeset(void)
 
 static void setup_sink_crc(void)
 {
-	ssize_t rc;
 	sink_crc_t crc;
-	int errno_;
 	drmModeConnectorPtr c;
 
 	c = get_connector(prim_mode_params.connector_id);
@@ -1418,17 +1424,8 @@ static void setup_sink_crc(void)
 	sink_crc.fd = igt_debugfs_open("i915_sink_crc_eDP1", O_RDONLY);
 	igt_assert_lte(0, sink_crc.fd);
 
-	rc = read(sink_crc.fd, crc.data, SINK_CRC_SIZE);
-	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
-		igt_info("Unexpected sink CRC error, rc=:%zd errno:%d %s\n",
-			 rc, errno_, strerror(errno_));
+	/* Do a first read to try to detect if it's supported. */
+	get_sink_crc(&crc, false);
 }
 
 static void setup_crcs(void)
@@ -1677,9 +1674,9 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
 	igt_assert_crc_equal(&crc_.pipe, &wanted_crc->pipe);		\
 	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"); \
+	else if (sink_crc.reliable &&					\
+		 !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 {				\
-- 
2.7.4

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

[PATCH igt 5/6] kms_frontbuffer_tracking: destroy all FBs from all formats

[Paulo Zanoni]

Don't just destroy the ones from the default format.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/kms_frontbuffer_tracking.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index 8aa6362..b85f56b 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1399,7 +1399,10 @@ static void setup_modeset(void)
 
 static void teardown_modeset(void)
 {
-	destroy_fbs(FORMAT_DEFAULT);
+	enum pixel_format f;
+
+	for (f = 0; f < FORMAT_COUNT; f++)
+		destroy_fbs(f);
 }
 
 static void setup_sink_crc(void)
-- 
2.7.4

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

[PATCH igt 6/6] kms_frontbuffer_tracking: fix compression checking

[Paulo Zanoni]

Ever since Kernel's "drm/i915: don't report compression when fbc is
disabled" we've been wrongly assuming that the Kernel doesn't support
compression information due to the fact that it doesn't print that
specific line when FBC is not active. Fix this by just assuming that
the Kernel supports it, allowing us to kill some more code.

With this change, running a brand new kms_frontbuffer_tracking on
super old Kernels will result in failures, but I suppose that's fine
for IGT.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 tests/kms_frontbuffer_tracking.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index b85f56b..a3d6f1c 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -166,14 +166,12 @@ struct {
 struct {
 	bool can_test;
 
-	bool supports_compressing;
 	bool supports_last_action;
 
 	struct timespec last_action;
 } fbc = {
 	.can_test = false,
 	.supports_last_action = false,
-	.supports_compressing = false,
 };
 
 struct {
@@ -896,18 +894,6 @@ static bool fbc_wait_for_compression(void)
 	return igt_wait(fbc_is_compressing(), 2000, 1);
 }
 
-static void fbc_setup_compressing(void)
-{
-	char buf[128];
-
-	igt_debugfs_read("i915_fbc_status", buf);
-
-	if (strstr(buf, "\nCompressing:"))
-		fbc.supports_compressing = true;
-	else
-		igt_info("FBC compression information not supported\n");
-}
-
 static bool fbc_not_enough_stolen(void)
 {
 	char buf[128];
@@ -1527,7 +1513,6 @@ static void setup_fbc(void)
 	fbc.can_test = true;
 
 	fbc_setup_last_action();
-	fbc_setup_compressing();
 }
 
 static void teardown_fbc(void)
@@ -1696,8 +1681,7 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
 			igt_assert_f(false, "FBC disabled\n");		\
 		}							\
 									\
-		if (fbc.supports_compressing && 			\
-		    opt.fbc_check_compression)				\
+		if (opt.fbc_check_compression)				\
 			igt_assert(fbc_wait_for_compression());		\
 	} else if (flags_ & ASSERT_FBC_DISABLED) {			\
 		igt_assert(!fbc_wait_until_enabled());			\
-- 
2.7.4

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

Re: [PATCH igt 1/6] tests/kms_draw_crc: remove unnecessary mode unset calls

[Daniel Vetter]

On Thu, Dec 22, 2016 at 06:42:03PM -0200, Paulo Zanoni wrote:
> I couldn't think of a reason why we would need to unset the CRTCs
> before doing the modesets on this test, so remove all the mode unset
> calls.

There was (not sure whether still is) an issue with the igt_kms library
that it kept the existing config, and that then lead to impossible
configs. I think we're now clearing just the sw state correctly though, so
doing it with a full commit (and all the time-wasting implied) would be
overkill.

Anyway, that was the reason, worth to double check this is indeed working
now. You probably need multiple screens to make it go boom.
-Daniel

> 
> Before:
> $ time -p sudo ./kms_draw_crc
> real 44.74
> $ time -p for i in $(sudo ./kms_draw_crc --list-subtests); do sudo
> ./kms_draw_crc --run-subtest $i; done
> real 121.61
> 
> After:
> $ time -p sudo ./kms_draw_crc
> real 7.40
> $ time -p for i in $(sudo ./kms_draw_crc --list-subtests); do sudo
> ./kms_draw_crc --run-subtest $i; done
> real 14.32
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  tests/kms_draw_crc.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
> index cb28052..e163981 100644
> --- a/tests/kms_draw_crc.c
> +++ b/tests/kms_draw_crc.c
> @@ -143,7 +143,6 @@ static void get_method_crc(enum igt_draw_method method, uint32_t drm_format,
>  
>  	igt_pipe_crc_collect_crc(pipe_crc, crc);
>  
> -	kmstest_unset_all_crtcs(drm_fd, drm_res);
>  	igt_remove_fb(drm_fd, &fb);
>  }
>  
> @@ -152,8 +151,6 @@ static void draw_method_subtest(enum igt_draw_method method,
>  {
>  	igt_crc_t crc;
>  
> -	kmstest_unset_all_crtcs(drm_fd, drm_res);
> -
>  	/* Use IGT_DRAW_MMAP_GTT on an untiled buffer as the parameter for
>  	 * comparison. Cache the value so we don't recompute it for every single
>  	 * subtest. */
> @@ -184,7 +181,6 @@ static void get_fill_crc(uint64_t tiling, igt_crc_t *crc)
>  
>  	igt_pipe_crc_collect_crc(pipe_crc, crc);
>  
> -	kmstest_unset_all_crtcs(drm_fd, drm_res);
>  	igt_remove_fb(drm_fd, &fb);
>  }
>  
> @@ -194,8 +190,6 @@ static void fill_fb_subtest(void)
>  	struct igt_fb fb;
>  	igt_crc_t base_crc, crc;
>  
> -	kmstest_unset_all_crtcs(drm_fd, drm_res);
> -
>  	igt_create_fb(drm_fd, ms.mode->hdisplay, ms.mode->vdisplay,
>  		      DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE, &fb);
>  
> @@ -214,7 +208,6 @@ static void fill_fb_subtest(void)
>  	get_fill_crc(LOCAL_I915_FORMAT_MOD_X_TILED, &crc);
>  	igt_assert_crc_equal(&crc, &base_crc);
>  
> -	kmstest_unset_all_crtcs(drm_fd, drm_res);
>  	igt_remove_fb(drm_fd, &fb);
>  }
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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: [PATCH igt 1/6] tests/kms_draw_crc: remove unnecessary mode unset calls

[Paulo Zanoni]

Em Ter, 2016-12-27 às 15:59 +0100, Daniel Vetter escreveu:
> On Thu, Dec 22, 2016 at 06:42:03PM -0200, Paulo Zanoni wrote:
> > 
> > I couldn't think of a reason why we would need to unset the CRTCs
> > before doing the modesets on this test, so remove all the mode
> > unset
> > calls.
> 
> There was (not sure whether still is) an issue with the igt_kms
> library
> that it kept the existing config, and that then lead to impossible
> configs. I think we're now clearing just the sw state correctly
> though, so
> doing it with a full commit (and all the time-wasting implied) would
> be
> overkill.
> 
> Anyway, that was the reason, worth to double check this is indeed
> working
> now. You probably need multiple screens to make it go boom.

I don't use the igt_kms midlayer stuff... I do the libdrm KMS IOCTLs
directly because IMHO they're much simpler and predictable and easier
to use and with no side-effects, so that's definitely not a problem.
And now you just gave me another reason to not port the program to the
igt_kms API :).

I'm pretty sure the mode unsets were just to be extra-safe in
guaranteeing that whatever crtc/connector/mode we chose would work, but
that's not really a problem.


> -Daniel
> 
> > 
> > 
> > Before:
> > $ time -p sudo ./kms_draw_crc
> > real 44.74
> > $ time -p for i in $(sudo ./kms_draw_crc --list-subtests); do sudo
> > ./kms_draw_crc --run-subtest $i; done
> > real 121.61
> > 
> > After:
> > $ time -p sudo ./kms_draw_crc
> > real 7.40
> > $ time -p for i in $(sudo ./kms_draw_crc --list-subtests); do sudo
> > ./kms_draw_crc --run-subtest $i; done
> > real 14.32
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  tests/kms_draw_crc.c | 7 -------
> >  1 file changed, 7 deletions(-)
> > 
> > diff --git a/tests/kms_draw_crc.c b/tests/kms_draw_crc.c
> > index cb28052..e163981 100644
> > --- a/tests/kms_draw_crc.c
> > +++ b/tests/kms_draw_crc.c
> > @@ -143,7 +143,6 @@ static void get_method_crc(enum igt_draw_method
> > method, uint32_t drm_format,
> >  
> >  	igt_pipe_crc_collect_crc(pipe_crc, crc);
> >  
> > -	kmstest_unset_all_crtcs(drm_fd, drm_res);
> >  	igt_remove_fb(drm_fd, &fb);
> >  }
> >  
> > @@ -152,8 +151,6 @@ static void draw_method_subtest(enum
> > igt_draw_method method,
> >  {
> >  	igt_crc_t crc;
> >  
> > -	kmstest_unset_all_crtcs(drm_fd, drm_res);
> > -
> >  	/* Use IGT_DRAW_MMAP_GTT on an untiled buffer as the
> > parameter for
> >  	 * comparison. Cache the value so we don't recompute it
> > for every single
> >  	 * subtest. */
> > @@ -184,7 +181,6 @@ static void get_fill_crc(uint64_t tiling,
> > igt_crc_t *crc)
> >  
> >  	igt_pipe_crc_collect_crc(pipe_crc, crc);
> >  
> > -	kmstest_unset_all_crtcs(drm_fd, drm_res);
> >  	igt_remove_fb(drm_fd, &fb);
> >  }
> >  
> > @@ -194,8 +190,6 @@ static void fill_fb_subtest(void)
> >  	struct igt_fb fb;
> >  	igt_crc_t base_crc, crc;
> >  
> > -	kmstest_unset_all_crtcs(drm_fd, drm_res);
> > -
> >  	igt_create_fb(drm_fd, ms.mode->hdisplay, ms.mode-
> > >vdisplay,
> >  		      DRM_FORMAT_XRGB8888,
> > LOCAL_DRM_FORMAT_MOD_NONE, &fb);
> >  
> > @@ -214,7 +208,6 @@ static void fill_fb_subtest(void)
> >  	get_fill_crc(LOCAL_I915_FORMAT_MOD_X_TILED, &crc);
> >  	igt_assert_crc_equal(&crc, &base_crc);
> >  
> > -	kmstest_unset_all_crtcs(drm_fd, drm_res);
> >  	igt_remove_fb(drm_fd, &fb);
> >  }
> >  
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH igt 4/6] kms_frontbuffer_tracking: refactor sink CRC reliability handling

[Petri Latvala]

After this patch we got some failures in CI for anything not connected
to eDP. sink_crc.supported now defaults to true, so perhaps...


diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
index b91f08b..b84721f 100644
--- a/tests/kms_frontbuffer_tracking.c
+++ b/tests/kms_frontbuffer_tracking.c
@@ -1399,6 +1399,7 @@ static void setup_sink_crc(void)
        c = get_connector(prim_mode_params.connector_id);
        if (c->connector_type != DRM_MODE_CONNECTOR_eDP) {
                igt_info("Sink CRC not supported: primary screen is not eDP\n");
+               sink_crc.supported = false;
                return;
        }


Paulo?


--
Petri Latvala



On Thu, Dec 22, 2016 at 06:42:06PM -0200, Paulo Zanoni wrote:
> What I'm currently seeing is that sometimes the first check during
> setup_sink_crc() returns valid sink CRC, but then the subsequent
> checks return ETIMEDOUT. In these cases, we keep getting flooded by
> messages saying that our sink CRC is unreliable and that the results
> differ. This is annoying for the FBC tests where sink CRC is not
> mandatory.
> 
> Since this case shows it's useless to try to check for sink CRC
> reliability before the actual tests, refactor the code in a way that
> if at any point we detect that sink CRC is unreliable we'll mark it as
> such and stop flooding the logs. For the tests where it's mandatory
> we'll still keep the same SKIP behavior.
> 
> This refactor also allows us to just call get_sink_crc() in the
> setup_sink_crc() function instead of having to reimplement the same
> logic.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  tests/kms_frontbuffer_tracking.c | 39 ++++++++++++++++++---------------------
>  1 file changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
> index 4a46942..8aa6362 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -203,9 +203,11 @@ struct both_crcs *wanted_crc;
>  struct {
>  	int fd;
>  	bool supported;
> +	bool reliable;
>  } sink_crc = {
>  	.fd = -1,
> -	.supported = false,
> +	.supported = true,
> +	.reliable = true,
>  };
>  
>  /* The goal of this structure is to easily allow us to deal with cases where we
> @@ -943,11 +945,17 @@ static void get_sink_crc(sink_crc_t *crc, bool mandatory)
>  	rc = read(sink_crc.fd, crc->data, SINK_CRC_SIZE);
>  	errno_ = errno;
>  
> -	if (rc == -1 && errno_ == ETIMEDOUT) {
> +	if (rc == -1 && errno_ == ENOTTY) {
> +		igt_info("Sink CRC not supported: panel doesn't support it\n");
> +		sink_crc.supported = false;
> +	} else if (rc == -1 && errno_ == ETIMEDOUT) {
> +		if (sink_crc.reliable) {
> +			igt_info("Sink CRC is unreliable on this machine.\n");
> +			sink_crc.reliable = false;
> +		}
> +
>  		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_skip("Sink CRC is unreliable on this machine.\n");
>  	} else {
>  		igt_assert(rc == SINK_CRC_SIZE);
>  	}
> @@ -1396,9 +1404,7 @@ static void teardown_modeset(void)
>  
>  static void setup_sink_crc(void)
>  {
> -	ssize_t rc;
>  	sink_crc_t crc;
> -	int errno_;
>  	drmModeConnectorPtr c;
>  
>  	c = get_connector(prim_mode_params.connector_id);
> @@ -1418,17 +1424,8 @@ static void setup_sink_crc(void)
>  	sink_crc.fd = igt_debugfs_open("i915_sink_crc_eDP1", O_RDONLY);
>  	igt_assert_lte(0, sink_crc.fd);
>  
> -	rc = read(sink_crc.fd, crc.data, SINK_CRC_SIZE);
> -	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
> -		igt_info("Unexpected sink CRC error, rc=:%zd errno:%d %s\n",
> -			 rc, errno_, strerror(errno_));
> +	/* Do a first read to try to detect if it's supported. */
> +	get_sink_crc(&crc, false);
>  }
>  
>  static void setup_crcs(void)
> @@ -1677,9 +1674,9 @@ static int adjust_assertion_flags(const struct test_mode *t, int flags)
>  	igt_assert_crc_equal(&crc_.pipe, &wanted_crc->pipe);		\
>  	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"); \
> +	else if (sink_crc.reliable &&					\
> +		 !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 {				\
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx