[Intel-gfx] [PATCH i-g-t 2/2] test/kms_cursor_crc: align the start of the CRC capture to a vblank

[Melissa Wen]

On 07/15, Arkadiusz Hiler wrote:
> On Mon, Jun 22, 2020 at 01:38:26PM -0300, Melissa Wen wrote:
> > When running subtests in sequence using vkms, the beginning of CRC capture
> > process does not match the simulated vblank timing. This mismatch leads to
> > an endless busy wait and, consequently, timeout failures for the remaining
> > subtests in the test sequence. This patch sets the pace by waiting for
> > vblank before starting the CRC capture.
> > 
> > Signed-off-by: Melissa Wen <melissa.srw at gmail.com>
> 
> This one is quite interetesing. The test seems to be working just fine
> on the real hardware and causes the endless busy wait on VKMS only...
> 
> DRM is bad at describing usage sequences and what's defined and what's
> undefined when it comes to behavior. We just try not to break any of the
> existing users. On top of that CRC capture is a testing/debug feature
> that doesn't have have to be stable - it's not really obvious what's the
> correct course of action here.
> 
> The vblank wait won't harm anyone, especially in the context presented
> above. You have to keep in mind that other implementations of CRC
> caputring doesn't have that requirement and you will likely find more
> similar instances of this usage pattern. There may be even more of them
> introduced over time - there's no CI on VKMS (fingers crossed that this
> is going to change).
> 
> Have you thought about what's easier here - making the current code work
> on the VKMS side or fixing the test? I would like to know your thoughts
> on this.
Hi,

Thank you very much for the review!

I've been investigating more about this with the community help and, in
fact, the problem seems to be more linked to vkms. I mean, this problem
of waiting for a vblank before starting to capture the CRC seems to
affect vkms in other igt tests too. So the most accurate thing is to
treat it over there. 

I will send a v2 only with the other patch that releases the pipe_crc
before creating a new one.

Thanks again,

Melissa
> 
> -- 
> Cheers,
> Arek
> 
> 
> 
> 
> > ---
> >  tests/kms_cursor_crc.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> > index 5976df5f..755c34ed 100644
> > --- a/tests/kms_cursor_crc.c
> > +++ b/tests/kms_cursor_crc.c
> > @@ -474,6 +474,7 @@ static void prepare_crtc(data_t *data, igt_output_t *output,
> >  		igt_assert(data->batch);
> >  	}
> >  
> > +	igt_wait_for_vblank(data->drm_fd, data->pipe);
> >  	igt_pipe_crc_start(data->pipe_crc);
> >  }
> >  
> > -- 
> > 2.27.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx