2015-11-05 16:53 GMT-02:00 Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>: > 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@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
