[Intel-gfx] [PATCH i-g-t 2/8] kms_frontbuffer_tracking: Skip on unreliable CRC.

[Paulo Zanoni]

2015-11-05 16:53 GMT-02:00 Rodrigo Vivi <rodrigo.vivi at 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.

As much as I understand your reasoning here, "Try running this test
again" will be ignored by our future bots.

Instead of just skipping, isn't there something else we could do, such
as trying again 10 times? 60 frames doesn't seem expensive. If it
works at least sometimes, I'd say it's worth the try.

Besides, did we try the AUX_MUTEX register that was suggested here:
http://patchwork.freedesktop.org/patch/57693/ ? Maybe it would solve
all our sink CRCs problem.

Another comment: FBC doesn't really need sink CRC, but it's currently
checking sink CRC, so it may get SKIPs. Maybe instead of a SKIP for
failed sink CRC on FBC we could just ignore and move on? Maybe we
could pass some flags to collect_crcs() so it can know if sink CRCs
are ignorable.

Another problem is: what if we fail while getting the reference CRC?
We will leave garbage inside crc->data, and the other tests will
compare themselves against the garbage in case reading sink CRCs end
up working for them, so we'll have test failures that are not real
failures. Maybe we should pass some flag to collect_crtcs() signaling
that we're trying a reference CRC, so it can write something to
crtc->data, just like we have the "unsupported!" string. Then we'd
have to check this special string later.

You also probably need to fix setup_sink_crc(), because it currently
doesn't check for ETIMETDOUT.

I'm not blocking the patch, just starting the discussion :)

>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  tests/kms_frontbuffer_tracking.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/tests/kms_frontbuffer_tracking.c b/tests/kms_frontbuffer_tracking.c
> index cd2879d..606d0a9 100644
> --- a/tests/kms_frontbuffer_tracking.c
> +++ b/tests/kms_frontbuffer_tracking.c
> @@ -858,10 +858,17 @@ static bool psr_wait_until_enabled(void)
>
>  static void get_sink_crc(sink_crc_t *crc)
>  {
> +       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)
> +               igt_skip("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)
> --
> 2.4.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni