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

[Rodrigo Vivi]

On Thu, Nov 5, 2015 at 12:30 PM, Paulo Zanoni <przanoni at gmail.com> wrote:
> 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.

we would need to redo the test... whenever in that unreliable mode if
there is no screen update the sink crc never gets recovered.. so at
least for now we could skip, then later we can rework the tests to
fully retry whenever makes sense.

>
> 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.

We tried after this... For a moment when running it on KBL I was
excited and believed that it could help, but later when testing again
on another SKL we saw aux mutex is unreliable..  unfortunately :(

>
> 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.

good point.

>
> 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.

This is something that was happening before late sink crc rework...
now sink crc only returns something if it is reliable.

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

oh I see.. I will take a look there as well...

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

Thanks and sorry for not engaging on this discussion sooner..

>
>>
>> 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
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br