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