Re: [Intel-gfx] [PATCH i-g-t 2/2] test/kms_cursor_crc: align the start of the CRC capture to a vblank
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 > > 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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 2/2] test/kms_cursor_crc: align the start of the CRC capture to a vblank
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 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. -- 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/2] test/kms_cursor_crc: align the start of the CRC capture to a vblank
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 --- 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
