[igt-dev] [PATCH i-g-t] tests/kms_frontbuffer_tracking: Make sure GPU is quiescent before handling assertions.

[Maarten Lankhorst]

Op 14-12-2018 om 01:22 schreef Dhinakaran Pandiyan:
> On Thu, 2018-12-13 at 16:18 -0800, Dhinakaran Pandiyan wrote:
>> On Thu, 2018-12-13 at 11:00 +0100, Daniel Vetter wrote:
>>> On Thu, Dec 13, 2018 at 10:52:34AM +0100, Maarten Lankhorst wrote:
>>>> Typically PSR enables in around 136 ms, but on some tests the we
>>>> have
>>>> to explicitly make sure that the gpu is idle before rendering.
>>>>
>>>> Otherwise, sometimes we wait for the background flusher to
>>>> finish,
>>>> which will take at least a second longer, and causes the wait for
>>>> PSR to timeout.
>>>>
>>>> On kms_frontbuffer_tracking:
>>>> Starting subtest: psr-2p-scndscrn-shrfb-plflip-blt
>>>> psr_active(debugfs_fd, true) took 136ms
>>>> psr_active(debugfs_fd, true) took 136ms
>>>> psr_active(debugfs_fd, true) took 350ms
>>>> psr_active(debugfs_fd, true) took 136ms
>>>> psr_active(debugfs_fd, true) took 1598ms
> Now that I read this again, we should be checking for PSR exit after a
> flip but instead we are waiting for PSR to become active again.
> However, this is a different issue that we should fix separately.
In that case wouldn't we better off not idling the GPU and just waiting for PSR exit?
>>>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
>>>> Cc: José Roberto de Souza <jose.souza at intel.com>
>>>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108733
>>>> Suggested-by: Chris Wilson <chris at chris-wilson.co.uk>
>>>> ---
>>>>  tests/kms_frontbuffer_tracking.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/tests/kms_frontbuffer_tracking.c
>>>> b/tests/kms_frontbuffer_tracking.c
>>>> index 276ef83ce386..7924bb902a2a 100644
>>>> --- a/tests/kms_frontbuffer_tracking.c
>>>> +++ b/tests/kms_frontbuffer_tracking.c
>>>> @@ -1642,6 +1642,9 @@ static void __do_assertions(const struct
>>>> test_mode *t, int flags,
>>>>  {
>>>>  	flags = adjust_assertion_flags(t, flags);
>>>>  
>>>> +	/* Make sure any submitted rendering is now idle. */
>>>> +	gem_quiescent_gpu(drm.fd);
>> Shouldn't this be called from draw_rect() so that the flip is done
>> after rendering is complete?

Hmm, I thought the point was to make sure the gpu was busy when flipping to trigger any race of
draw vs flip. Hence putting the idle in the assertions. :)