* [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
@ 2021-05-28 4:27 Vidya Srinivas
2021-06-04 18:50 ` Mark Yacoub
2021-06-08 7:01 ` Modem, Bhanuprakash
0 siblings, 2 replies; 12+ messages in thread
From: Vidya Srinivas @ 2021-05-28 4:27 UTC (permalink / raw
To: intel-gfx, igt-dev; +Cc: markyacoub, charlton.lin
Without wait for vblank, CRC mismatch is seen
between big and small CRC on few Gen11 systems.
Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
---
tests/kms_big_fb.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tests/kms_big_fb.c b/tests/kms_big_fb.c
index b35727a09bd0..f90363c3beb2 100644
--- a/tests/kms_big_fb.c
+++ b/tests/kms_big_fb.c
@@ -254,6 +254,7 @@ static void unset_lut(data_t *data)
static bool test_plane(data_t *data)
{
igt_plane_t *plane = data->plane;
+ igt_display_t *display = &data->display;
struct igt_fb *small_fb = &data->small_fb;
struct igt_fb *big_fb = &data->big_fb;
int w = data->big_fb_width - small_fb->width;
@@ -337,16 +338,17 @@ static bool test_plane(data_t *data)
igt_display_commit2(&data->display, data->display.is_atomic ?
COMMIT_ATOMIC : COMMIT_UNIVERSAL);
-
+ igt_wait_for_vblank(data->drm_fd, display->pipes[data->pipe].crtc_offset);
igt_pipe_crc_collect_crc(data->pipe_crc, &small_crc);
igt_plane_set_fb(plane, big_fb);
igt_fb_set_position(big_fb, plane, x, y);
igt_fb_set_size(big_fb, plane, small_fb->width, small_fb->height);
+
igt_plane_set_size(plane, data->width, data->height);
igt_display_commit2(&data->display, data->display.is_atomic ?
COMMIT_ATOMIC : COMMIT_UNIVERSAL);
-
+ igt_wait_for_vblank(data->drm_fd, display->pipes[data->pipe].crtc_offset);
igt_pipe_crc_collect_crc(data->pipe_crc, &big_crc);
igt_plane_set_fb(plane, NULL);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
2021-05-28 4:27 [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC Vidya Srinivas
@ 2021-06-04 18:50 ` Mark Yacoub
2021-06-05 5:46 ` Srinivas, Vidya
2021-06-08 7:01 ` Modem, Bhanuprakash
1 sibling, 1 reply; 12+ messages in thread
From: Mark Yacoub @ 2021-06-04 18:50 UTC (permalink / raw
To: Vidya Srinivas; +Cc: igt-dev, intel-gfx, charlton.lin
On Fri, May 28, 2021 at 12:36 AM Vidya Srinivas
<vidya.srinivas@intel.com> wrote:
>
> Without wait for vblank, CRC mismatch is seen
> between big and small CRC on few Gen11 systems.
>
Tested on ChromeOS on JSL (Drawlat).
Tested-by: Mark Yacoub <markyacoub@chromium.org>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
> tests/kms_big_fb.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/kms_big_fb.c b/tests/kms_big_fb.c
> index b35727a09bd0..f90363c3beb2 100644
> --- a/tests/kms_big_fb.c
> +++ b/tests/kms_big_fb.c
> @@ -254,6 +254,7 @@ static void unset_lut(data_t *data)
> static bool test_plane(data_t *data)
> {
> igt_plane_t *plane = data->plane;
> + igt_display_t *display = &data->display;
> struct igt_fb *small_fb = &data->small_fb;
> struct igt_fb *big_fb = &data->big_fb;
> int w = data->big_fb_width - small_fb->width;
> @@ -337,16 +338,17 @@ static bool test_plane(data_t *data)
> igt_display_commit2(&data->display, data->display.is_atomic ?
> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>
> -
> + igt_wait_for_vblank(data->drm_fd, display->pipes[data->pipe].crtc_offset);
> igt_pipe_crc_collect_crc(data->pipe_crc, &small_crc);
>
> igt_plane_set_fb(plane, big_fb);
> igt_fb_set_position(big_fb, plane, x, y);
> igt_fb_set_size(big_fb, plane, small_fb->width, small_fb->height);
> +
> igt_plane_set_size(plane, data->width, data->height);
> igt_display_commit2(&data->display, data->display.is_atomic ?
> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
> -
> + igt_wait_for_vblank(data->drm_fd, display->pipes[data->pipe].crtc_offset);
> igt_pipe_crc_collect_crc(data->pipe_crc, &big_crc);
>
> igt_plane_set_fb(plane, NULL);
> --
> 2.7.4
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
2021-06-04 18:50 ` Mark Yacoub
@ 2021-06-05 5:46 ` Srinivas, Vidya
0 siblings, 0 replies; 12+ messages in thread
From: Srinivas, Vidya @ 2021-06-05 5:46 UTC (permalink / raw
To: Mark Yacoub
Cc: igt-dev@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
Lin, Charlton
Thank you very much Mark, for testing the patch and providing the "Tested-by" tag.
Regards
Vidya
-----Original Message-----
From: Mark Yacoub <markyacoub@chromium.org>
Sent: Saturday, June 5, 2021 12:20 AM
To: Srinivas, Vidya <vidya.srinivas@intel.com>
Cc: intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org; Almahallawy, Khaled <khaled.almahallawy@intel.com>; Lin, Charlton <charlton.lin@intel.com>
Subject: Re: [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
On Fri, May 28, 2021 at 12:36 AM Vidya Srinivas <vidya.srinivas@intel.com> wrote:
>
> Without wait for vblank, CRC mismatch is seen between big and small
> CRC on few Gen11 systems.
>
Tested on ChromeOS on JSL (Drawlat).
Tested-by: Mark Yacoub <markyacoub@chromium.org>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
> tests/kms_big_fb.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/kms_big_fb.c b/tests/kms_big_fb.c index
> b35727a09bd0..f90363c3beb2 100644
> --- a/tests/kms_big_fb.c
> +++ b/tests/kms_big_fb.c
> @@ -254,6 +254,7 @@ static void unset_lut(data_t *data) static bool
> test_plane(data_t *data) {
> igt_plane_t *plane = data->plane;
> + igt_display_t *display = &data->display;
> struct igt_fb *small_fb = &data->small_fb;
> struct igt_fb *big_fb = &data->big_fb;
> int w = data->big_fb_width - small_fb->width; @@ -337,16
> +338,17 @@ static bool test_plane(data_t *data)
> igt_display_commit2(&data->display, data->display.is_atomic ?
> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>
> -
> + igt_wait_for_vblank(data->drm_fd,
> + display->pipes[data->pipe].crtc_offset);
> igt_pipe_crc_collect_crc(data->pipe_crc, &small_crc);
>
> igt_plane_set_fb(plane, big_fb);
> igt_fb_set_position(big_fb, plane, x, y);
> igt_fb_set_size(big_fb, plane, small_fb->width,
> small_fb->height);
> +
> igt_plane_set_size(plane, data->width, data->height);
> igt_display_commit2(&data->display, data->display.is_atomic ?
> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
> -
> + igt_wait_for_vblank(data->drm_fd,
> + display->pipes[data->pipe].crtc_offset);
> igt_pipe_crc_collect_crc(data->pipe_crc, &big_crc);
>
> igt_plane_set_fb(plane, NULL);
> --
> 2.7.4
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
2021-05-28 4:27 [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC Vidya Srinivas
2021-06-04 18:50 ` Mark Yacoub
@ 2021-06-08 7:01 ` Modem, Bhanuprakash
2021-06-08 7:34 ` [Intel-gfx] [igt-dev] " Juha-Pekka Heikkila
1 sibling, 1 reply; 12+ messages in thread
From: Modem, Bhanuprakash @ 2021-06-08 7:01 UTC (permalink / raw
To: Srinivas, Vidya, intel-gfx@lists.freedesktop.org,
igt-dev@lists.freedesktop.org
Cc: markyacoub@chromium.org, Lin, Charlton
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Vidya
> Srinivas
> Sent: Friday, May 28, 2021 9:57 AM
> To: intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
> Cc: markyacoub@chromium.org; Lin, Charlton <charlton.lin@intel.com>
> Subject: [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank
> before collecting CRC
>
> Without wait for vblank, CRC mismatch is seen
> between big and small CRC on few Gen11 systems.
>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
> tests/kms_big_fb.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/kms_big_fb.c b/tests/kms_big_fb.c
> index b35727a09bd0..f90363c3beb2 100644
> --- a/tests/kms_big_fb.c
> +++ b/tests/kms_big_fb.c
> @@ -254,6 +254,7 @@ static void unset_lut(data_t *data)
> static bool test_plane(data_t *data)
> {
> igt_plane_t *plane = data->plane;
> + igt_display_t *display = &data->display;
For code readability purpose, I think we need to update to use this variable
wherever we are using "&data->display" in this function.
s/"&data->display"/"display"/
With above change, this patch LGTM
Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> struct igt_fb *small_fb = &data->small_fb;
> struct igt_fb *big_fb = &data->big_fb;
> int w = data->big_fb_width - small_fb->width;
> @@ -337,16 +338,17 @@ static bool test_plane(data_t *data)
> igt_display_commit2(&data->display, data->display.is_atomic ?
> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>
> -
> + igt_wait_for_vblank(data->drm_fd, display->pipes[data->pipe].crtc_offset);
> igt_pipe_crc_collect_crc(data->pipe_crc, &small_crc);
>
> igt_plane_set_fb(plane, big_fb);
> igt_fb_set_position(big_fb, plane, x, y);
> igt_fb_set_size(big_fb, plane, small_fb->width, small_fb->height);
> +
> igt_plane_set_size(plane, data->width, data->height);
> igt_display_commit2(&data->display, data->display.is_atomic ?
> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
> -
> + igt_wait_for_vblank(data->drm_fd, display->pipes[data->pipe].crtc_offset);
> igt_pipe_crc_collect_crc(data->pipe_crc, &big_crc);
>
> igt_plane_set_fb(plane, NULL);
> --
> 2.7.4
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
2021-06-08 7:01 ` Modem, Bhanuprakash
@ 2021-06-08 7:34 ` Juha-Pekka Heikkila
2021-06-08 7:48 ` Srinivas, Vidya
0 siblings, 1 reply; 12+ messages in thread
From: Juha-Pekka Heikkila @ 2021-06-08 7:34 UTC (permalink / raw
To: Modem, Bhanuprakash, Srinivas, Vidya,
intel-gfx@lists.freedesktop.org, igt-dev@lists.freedesktop.org
Cc: Lin, Charlton
On 8.6.2021 10.01, Modem, Bhanuprakash wrote:
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Vidya
>> Srinivas
>> Sent: Friday, May 28, 2021 9:57 AM
>> To: intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
>> Cc: markyacoub@chromium.org; Lin, Charlton <charlton.lin@intel.com>
>> Subject: [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank
>> before collecting CRC
>>
>> Without wait for vblank, CRC mismatch is seen
>> between big and small CRC on few Gen11 systems.
>>
>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>> ---
>> tests/kms_big_fb.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/kms_big_fb.c b/tests/kms_big_fb.c
>> index b35727a09bd0..f90363c3beb2 100644
>> --- a/tests/kms_big_fb.c
>> +++ b/tests/kms_big_fb.c
>> @@ -254,6 +254,7 @@ static void unset_lut(data_t *data)
>> static bool test_plane(data_t *data)
>> {
>> igt_plane_t *plane = data->plane;
>> + igt_display_t *display = &data->display;
>
> For code readability purpose, I think we need to update to use this variable
> wherever we are using "&data->display" in this function.
> s/"&data->display"/"display"/
>
> With above change, this patch LGTM
> Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>
I still don't see benefit in this patch. For now all this look like is
doing is slow down the test and if this actually helps there's a real
bug somewhere which is not here. My earlier review comments were not
addressed hence repeat here, see below.
>> struct igt_fb *small_fb = &data->small_fb;
>> struct igt_fb *big_fb = &data->big_fb;
>> int w = data->big_fb_width - small_fb->width;
>> @@ -337,16 +338,17 @@ static bool test_plane(data_t *data)
>> igt_display_commit2(&data->display, data->display.is_atomic ?
>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>>
>> -
>> + igt_wait_for_vblank(data->drm_fd, display->pipes[data->pipe].crtc_offset);
Above this line there's flip to different fb. Below this line crc
calculation is restarted, get one crc and stop crc. There's several
vblanks already spent here, if now adding one more somehow helps it
sound like there's problems in crc calculation on some platform or
kernel is saying too early framebuffer is ready to be shown. Am I
missing something here?
/Juha-Pekka
>> igt_pipe_crc_collect_crc(data->pipe_crc, &small_crc);
>>
>> igt_plane_set_fb(plane, big_fb);
>> igt_fb_set_position(big_fb, plane, x, y);
>> igt_fb_set_size(big_fb, plane, small_fb->width, small_fb->height);
>> +
spurious empty line need to be removed.
>> igt_plane_set_size(plane, data->width, data->height);
>> igt_display_commit2(&data->display, data->display.is_atomic ?
>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>> -
>> + igt_wait_for_vblank(data->drm_fd, display->pipes[data->pipe].crtc_offset);
>> igt_pipe_crc_collect_crc(data->pipe_crc, &big_crc);
>>
>> igt_plane_set_fb(plane, NULL);
>> --
>> 2.7.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
2021-06-08 7:34 ` [Intel-gfx] [igt-dev] " Juha-Pekka Heikkila
@ 2021-06-08 7:48 ` Srinivas, Vidya
2021-06-08 9:19 ` Srinivas, Vidya
0 siblings, 1 reply; 12+ messages in thread
From: Srinivas, Vidya @ 2021-06-08 7:48 UTC (permalink / raw
To: juhapekka.heikkila@gmail.com, Modem, Bhanuprakash,
intel-gfx@lists.freedesktop.org, igt-dev@lists.freedesktop.org
Cc: Lin, Charlton
Hello Juha-Pekka and Bhanu
Thank you for the review comments. Apologies Juha-Pekka, I will incorporate your review comments and try out.
Regards
Vidya
-----Original Message-----
From: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Sent: Tuesday, June 8, 2021 1:04 PM
To: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; Srinivas, Vidya <vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
Cc: Lin, Charlton <charlton.lin@intel.com>
Subject: Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
On 8.6.2021 10.01, Modem, Bhanuprakash wrote:
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
>> Of Vidya Srinivas
>> Sent: Friday, May 28, 2021 9:57 AM
>> To: intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
>> Cc: markyacoub@chromium.org; Lin, Charlton <charlton.lin@intel.com>
>> Subject: [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for
>> vblank before collecting CRC
>>
>> Without wait for vblank, CRC mismatch is seen between big and small
>> CRC on few Gen11 systems.
>>
>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>> ---
>> tests/kms_big_fb.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/kms_big_fb.c b/tests/kms_big_fb.c index
>> b35727a09bd0..f90363c3beb2 100644
>> --- a/tests/kms_big_fb.c
>> +++ b/tests/kms_big_fb.c
>> @@ -254,6 +254,7 @@ static void unset_lut(data_t *data)
>> static bool test_plane(data_t *data)
>> {
>> igt_plane_t *plane = data->plane;
>> + igt_display_t *display = &data->display;
>
> For code readability purpose, I think we need to update to use this
> variable wherever we are using "&data->display" in this function.
> s/"&data->display"/"display"/
>
> With above change, this patch LGTM
> Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>
I still don't see benefit in this patch. For now all this look like is doing is slow down the test and if this actually helps there's a real bug somewhere which is not here. My earlier review comments were not addressed hence repeat here, see below.
>> struct igt_fb *small_fb = &data->small_fb;
>> struct igt_fb *big_fb = &data->big_fb;
>> int w = data->big_fb_width - small_fb->width; @@ -337,16 +338,17
>> @@ static bool test_plane(data_t *data)
>> igt_display_commit2(&data->display, data->display.is_atomic ?
>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>>
>> -
>> + igt_wait_for_vblank(data->drm_fd,
>> +display->pipes[data->pipe].crtc_offset);
Above this line there's flip to different fb. Below this line crc calculation is restarted, get one crc and stop crc. There's several vblanks already spent here, if now adding one more somehow helps it sound like there's problems in crc calculation on some platform or kernel is saying too early framebuffer is ready to be shown. Am I missing something here?
/Juha-Pekka
>> igt_pipe_crc_collect_crc(data->pipe_crc, &small_crc);
>>
>> igt_plane_set_fb(plane, big_fb);
>> igt_fb_set_position(big_fb, plane, x, y);
>> igt_fb_set_size(big_fb, plane, small_fb->width,
>> small_fb->height);
>> +
spurious empty line need to be removed.
>> igt_plane_set_size(plane, data->width, data->height);
>> igt_display_commit2(&data->display, data->display.is_atomic ?
>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>> -
>> + igt_wait_for_vblank(data->drm_fd,
>> +display->pipes[data->pipe].crtc_offset);
>> igt_pipe_crc_collect_crc(data->pipe_crc, &big_crc);
>>
>> igt_plane_set_fb(plane, NULL);
>> --
>> 2.7.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
2021-06-08 7:48 ` Srinivas, Vidya
@ 2021-06-08 9:19 ` Srinivas, Vidya
2021-06-08 11:50 ` Juha-Pekka Heikkila
0 siblings, 1 reply; 12+ messages in thread
From: Srinivas, Vidya @ 2021-06-08 9:19 UTC (permalink / raw
To: juhapekka.heikkila@gmail.com, Modem, Bhanuprakash,
intel-gfx@lists.freedesktop.org, igt-dev@lists.freedesktop.org
Cc: Lin, Charlton
Hello Juha-Pekka
Instead of wait for vblank, this also works
igt_pipe_crc_start-> igt_pipe_crc_get_current for small fb after commit -> then igt_pipe_crc_get_current For big fb -> compare -> igt_pipe_crc_stop
Would this change be okay? Kindly suggest. igt_pipe_crc_collect_crc is not working. It gives CRC mismatch for few subtests like subtest y-tiled-32bpp-rotate-0
Have submitted the change here https://patchwork.freedesktop.org/patch/437657/?series=90389&rev=6
Thank you so much.
Regards
Vidya
-----Original Message-----
From: Srinivas, Vidya
Sent: Tuesday, June 8, 2021 1:19 PM
To: juhapekka.heikkila@gmail.com; Modem, Bhanuprakash <Bhanuprakash.Modem@intel.com>; intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
Cc: Lin, Charlton <Charlton.Lin@intel.com>
Subject: RE: [igt-dev] [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
Hello Juha-Pekka and Bhanu
Thank you for the review comments. Apologies Juha-Pekka, I will incorporate your review comments and try out.
Regards
Vidya
-----Original Message-----
From: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Sent: Tuesday, June 8, 2021 1:04 PM
To: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; Srinivas, Vidya <vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
Cc: Lin, Charlton <charlton.lin@intel.com>
Subject: Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
On 8.6.2021 10.01, Modem, Bhanuprakash wrote:
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
>> Of Vidya Srinivas
>> Sent: Friday, May 28, 2021 9:57 AM
>> To: intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
>> Cc: markyacoub@chromium.org; Lin, Charlton <charlton.lin@intel.com>
>> Subject: [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for
>> vblank before collecting CRC
>>
>> Without wait for vblank, CRC mismatch is seen between big and small
>> CRC on few Gen11 systems.
>>
>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>> ---
>> tests/kms_big_fb.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/kms_big_fb.c b/tests/kms_big_fb.c index
>> b35727a09bd0..f90363c3beb2 100644
>> --- a/tests/kms_big_fb.c
>> +++ b/tests/kms_big_fb.c
>> @@ -254,6 +254,7 @@ static void unset_lut(data_t *data)
>> static bool test_plane(data_t *data)
>> {
>> igt_plane_t *plane = data->plane;
>> + igt_display_t *display = &data->display;
>
> For code readability purpose, I think we need to update to use this
> variable wherever we are using "&data->display" in this function.
> s/"&data->display"/"display"/
>
> With above change, this patch LGTM
> Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>
I still don't see benefit in this patch. For now all this look like is doing is slow down the test and if this actually helps there's a real bug somewhere which is not here. My earlier review comments were not addressed hence repeat here, see below.
>> struct igt_fb *small_fb = &data->small_fb;
>> struct igt_fb *big_fb = &data->big_fb;
>> int w = data->big_fb_width - small_fb->width; @@ -337,16 +338,17
>> @@ static bool test_plane(data_t *data)
>> igt_display_commit2(&data->display, data->display.is_atomic ?
>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>>
>> -
>> + igt_wait_for_vblank(data->drm_fd,
>> +display->pipes[data->pipe].crtc_offset);
Above this line there's flip to different fb. Below this line crc calculation is restarted, get one crc and stop crc. There's several vblanks already spent here, if now adding one more somehow helps it sound like there's problems in crc calculation on some platform or kernel is saying too early framebuffer is ready to be shown. Am I missing something here?
/Juha-Pekka
>> igt_pipe_crc_collect_crc(data->pipe_crc, &small_crc);
>>
>> igt_plane_set_fb(plane, big_fb);
>> igt_fb_set_position(big_fb, plane, x, y);
>> igt_fb_set_size(big_fb, plane, small_fb->width,
>> small_fb->height);
>> +
spurious empty line need to be removed.
>> igt_plane_set_size(plane, data->width, data->height);
>> igt_display_commit2(&data->display, data->display.is_atomic ?
>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>> -
>> + igt_wait_for_vblank(data->drm_fd,
>> +display->pipes[data->pipe].crtc_offset);
>> igt_pipe_crc_collect_crc(data->pipe_crc, &big_crc);
>>
>> igt_plane_set_fb(plane, NULL);
>> --
>> 2.7.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
2021-06-08 9:19 ` Srinivas, Vidya
@ 2021-06-08 11:50 ` Juha-Pekka Heikkila
2021-06-08 11:54 ` Srinivas, Vidya
2021-06-10 8:38 ` Srinivas, Vidya
0 siblings, 2 replies; 12+ messages in thread
From: Juha-Pekka Heikkila @ 2021-06-08 11:50 UTC (permalink / raw
To: Srinivas, Vidya, Modem, Bhanuprakash,
intel-gfx@lists.freedesktop.org, igt-dev@lists.freedesktop.org
Cc: Lin, Charlton
On 8.6.2021 12.19, Srinivas, Vidya wrote:
> Hello Juha-Pekka
>
> Instead of wait for vblank, this also works
> igt_pipe_crc_start-> igt_pipe_crc_get_current for small fb after commit -> then igt_pipe_crc_get_current For big fb -> compare -> igt_pipe_crc_stop
>
> Would this change be okay? Kindly suggest. igt_pipe_crc_collect_crc is not working. It gives CRC mismatch for few subtests like subtest y-tiled-32bpp-rotate-0
This change is ok. It kind of implies there maybe is some problem on
your platform with starting of crc calculation but if this is only place
where it will show I'm ok with that since crc will not affect normal
users in any way.
I noticed your new patch, lets see how all ci machines behave on that
before doing anything else.
/Juha-Pekka
>
> Have submitted the change here https://patchwork.freedesktop.org/patch/437657/?series=90389&rev=6
>
> Thank you so much.
>
> Regards
> Vidya
>
> -----Original Message-----
> From: Srinivas, Vidya
> Sent: Tuesday, June 8, 2021 1:19 PM
> To: juhapekka.heikkila@gmail.com; Modem, Bhanuprakash <Bhanuprakash.Modem@intel.com>; intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
> Cc: Lin, Charlton <Charlton.Lin@intel.com>
> Subject: RE: [igt-dev] [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
>
> Hello Juha-Pekka and Bhanu
>
> Thank you for the review comments. Apologies Juha-Pekka, I will incorporate your review comments and try out.
>
> Regards
> Vidya
>
>
> -----Original Message-----
> From: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Sent: Tuesday, June 8, 2021 1:04 PM
> To: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; Srinivas, Vidya <vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
> Cc: Lin, Charlton <charlton.lin@intel.com>
> Subject: Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
>
> On 8.6.2021 10.01, Modem, Bhanuprakash wrote:
>>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
>>> Of Vidya Srinivas
>>> Sent: Friday, May 28, 2021 9:57 AM
>>> To: intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
>>> Cc: markyacoub@chromium.org; Lin, Charlton <charlton.lin@intel.com>
>>> Subject: [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for
>>> vblank before collecting CRC
>>>
>>> Without wait for vblank, CRC mismatch is seen between big and small
>>> CRC on few Gen11 systems.
>>>
>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>>> ---
>>> tests/kms_big_fb.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/kms_big_fb.c b/tests/kms_big_fb.c index
>>> b35727a09bd0..f90363c3beb2 100644
>>> --- a/tests/kms_big_fb.c
>>> +++ b/tests/kms_big_fb.c
>>> @@ -254,6 +254,7 @@ static void unset_lut(data_t *data)
>>> static bool test_plane(data_t *data)
>>> {
>>> igt_plane_t *plane = data->plane;
>>> + igt_display_t *display = &data->display;
>>
>> For code readability purpose, I think we need to update to use this
>> variable wherever we are using "&data->display" in this function.
>> s/"&data->display"/"display"/
>>
>> With above change, this patch LGTM
>> Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>
>
> I still don't see benefit in this patch. For now all this look like is doing is slow down the test and if this actually helps there's a real bug somewhere which is not here. My earlier review comments were not addressed hence repeat here, see below.
>
>
>>> struct igt_fb *small_fb = &data->small_fb;
>>> struct igt_fb *big_fb = &data->big_fb;
>>> int w = data->big_fb_width - small_fb->width; @@ -337,16 +338,17
>>> @@ static bool test_plane(data_t *data)
>>> igt_display_commit2(&data->display, data->display.is_atomic ?
>>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>>>
>>> -
>>> + igt_wait_for_vblank(data->drm_fd,
>>> +display->pipes[data->pipe].crtc_offset);
>
> Above this line there's flip to different fb. Below this line crc calculation is restarted, get one crc and stop crc. There's several vblanks already spent here, if now adding one more somehow helps it sound like there's problems in crc calculation on some platform or kernel is saying too early framebuffer is ready to be shown. Am I missing something here?
>
> /Juha-Pekka
>
>>> igt_pipe_crc_collect_crc(data->pipe_crc, &small_crc);
>>>
>>> igt_plane_set_fb(plane, big_fb);
>>> igt_fb_set_position(big_fb, plane, x, y);
>>> igt_fb_set_size(big_fb, plane, small_fb->width,
>>> small_fb->height);
>>> +
>
> spurious empty line need to be removed.
>
>>> igt_plane_set_size(plane, data->width, data->height);
>>> igt_display_commit2(&data->display, data->display.is_atomic ?
>>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>>> -
>>> + igt_wait_for_vblank(data->drm_fd,
>>> +display->pipes[data->pipe].crtc_offset);
>>> igt_pipe_crc_collect_crc(data->pipe_crc, &big_crc);
>>>
>>> igt_plane_set_fb(plane, NULL);
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
2021-06-08 11:50 ` Juha-Pekka Heikkila
@ 2021-06-08 11:54 ` Srinivas, Vidya
2021-06-10 8:38 ` Srinivas, Vidya
1 sibling, 0 replies; 12+ messages in thread
From: Srinivas, Vidya @ 2021-06-08 11:54 UTC (permalink / raw
To: juhapekka.heikkila@gmail.com, Modem, Bhanuprakash,
intel-gfx@lists.freedesktop.org, igt-dev@lists.freedesktop.org
Cc: Lin, Charlton
Thank you so much Juha-Pekka.
Regards
Vidya
-----Original Message-----
From: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Sent: Tuesday, June 8, 2021 5:21 PM
To: Srinivas, Vidya <vidya.srinivas@intel.com>; Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
Cc: Lin, Charlton <charlton.lin@intel.com>
Subject: Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
On 8.6.2021 12.19, Srinivas, Vidya wrote:
> Hello Juha-Pekka
>
> Instead of wait for vblank, this also works igt_pipe_crc_start->
> igt_pipe_crc_get_current for small fb after commit -> then
> igt_pipe_crc_get_current For big fb -> compare -> igt_pipe_crc_stop
>
> Would this change be okay? Kindly suggest. igt_pipe_crc_collect_crc is
> not working. It gives CRC mismatch for few subtests like subtest
> y-tiled-32bpp-rotate-0
This change is ok. It kind of implies there maybe is some problem on your platform with starting of crc calculation but if this is only place where it will show I'm ok with that since crc will not affect normal users in any way.
I noticed your new patch, lets see how all ci machines behave on that before doing anything else.
/Juha-Pekka
>
> Have submitted the change here
> https://patchwork.freedesktop.org/patch/437657/?series=90389&rev=6
>
> Thank you so much.
>
> Regards
> Vidya
>
> -----Original Message-----
> From: Srinivas, Vidya
> Sent: Tuesday, June 8, 2021 1:19 PM
> To: juhapekka.heikkila@gmail.com; Modem, Bhanuprakash
> <Bhanuprakash.Modem@intel.com>; intel-gfx@lists.freedesktop.org;
> igt-dev@lists.freedesktop.org
> Cc: Lin, Charlton <Charlton.Lin@intel.com>
> Subject: RE: [igt-dev] [Intel-gfx] [PATCH i-g-t] [RFC]
> tests/kms_big_fb: Wait for vblank before collecting CRC
>
> Hello Juha-Pekka and Bhanu
>
> Thank you for the review comments. Apologies Juha-Pekka, I will incorporate your review comments and try out.
>
> Regards
> Vidya
>
>
> -----Original Message-----
> From: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Sent: Tuesday, June 8, 2021 1:04 PM
> To: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; Srinivas,
> Vidya <vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org;
> igt-dev@lists.freedesktop.org
> Cc: Lin, Charlton <charlton.lin@intel.com>
> Subject: Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] [RFC]
> tests/kms_big_fb: Wait for vblank before collecting CRC
>
> On 8.6.2021 10.01, Modem, Bhanuprakash wrote:
>>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
>>> Of Vidya Srinivas
>>> Sent: Friday, May 28, 2021 9:57 AM
>>> To: intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
>>> Cc: markyacoub@chromium.org; Lin, Charlton <charlton.lin@intel.com>
>>> Subject: [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for
>>> vblank before collecting CRC
>>>
>>> Without wait for vblank, CRC mismatch is seen between big and small
>>> CRC on few Gen11 systems.
>>>
>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>>> ---
>>> tests/kms_big_fb.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/kms_big_fb.c b/tests/kms_big_fb.c index
>>> b35727a09bd0..f90363c3beb2 100644
>>> --- a/tests/kms_big_fb.c
>>> +++ b/tests/kms_big_fb.c
>>> @@ -254,6 +254,7 @@ static void unset_lut(data_t *data)
>>> static bool test_plane(data_t *data)
>>> {
>>> igt_plane_t *plane = data->plane;
>>> + igt_display_t *display = &data->display;
>>
>> For code readability purpose, I think we need to update to use this
>> variable wherever we are using "&data->display" in this function.
>> s/"&data->display"/"display"/
>>
>> With above change, this patch LGTM
>> Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>
>
> I still don't see benefit in this patch. For now all this look like is doing is slow down the test and if this actually helps there's a real bug somewhere which is not here. My earlier review comments were not addressed hence repeat here, see below.
>
>
>>> struct igt_fb *small_fb = &data->small_fb;
>>> struct igt_fb *big_fb = &data->big_fb;
>>> int w = data->big_fb_width - small_fb->width; @@ -337,16 +338,17
>>> @@ static bool test_plane(data_t *data)
>>> igt_display_commit2(&data->display, data->display.is_atomic ?
>>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>>>
>>> -
>>> + igt_wait_for_vblank(data->drm_fd,
>>> +display->pipes[data->pipe].crtc_offset);
>
> Above this line there's flip to different fb. Below this line crc calculation is restarted, get one crc and stop crc. There's several vblanks already spent here, if now adding one more somehow helps it sound like there's problems in crc calculation on some platform or kernel is saying too early framebuffer is ready to be shown. Am I missing something here?
>
> /Juha-Pekka
>
>>> igt_pipe_crc_collect_crc(data->pipe_crc, &small_crc);
>>>
>>> igt_plane_set_fb(plane, big_fb);
>>> igt_fb_set_position(big_fb, plane, x, y);
>>> igt_fb_set_size(big_fb, plane, small_fb->width,
>>> small_fb->height);
>>> +
>
> spurious empty line need to be removed.
>
>>> igt_plane_set_size(plane, data->width, data->height);
>>> igt_display_commit2(&data->display, data->display.is_atomic ?
>>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>>> -
>>> + igt_wait_for_vblank(data->drm_fd,
>>> +display->pipes[data->pipe].crtc_offset);
>>> igt_pipe_crc_collect_crc(data->pipe_crc, &big_crc);
>>>
>>> igt_plane_set_fb(plane, NULL);
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
2021-06-08 11:50 ` Juha-Pekka Heikkila
2021-06-08 11:54 ` Srinivas, Vidya
@ 2021-06-10 8:38 ` Srinivas, Vidya
2021-06-10 8:51 ` Petri Latvala
1 sibling, 1 reply; 12+ messages in thread
From: Srinivas, Vidya @ 2021-06-10 8:38 UTC (permalink / raw
To: juhapekka.heikkila@gmail.com, Modem, Bhanuprakash,
intel-gfx@lists.freedesktop.org, igt-dev@lists.freedesktop.org
Cc: Lin, Charlton
Hello Juha-Pekka,
https://patchwork.freedesktop.org/series/90389/#rev7 shows PASS for all CI.
However I don’t see kms_big_fb all the subtests running in CI. In the logs I see pass for linear-32bpp-rotate-0
May I include your Reviewed by to take it further? Please suggest. Thank you so much.
Regards
Vidya
-----Original Message-----
From: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Sent: Tuesday, June 8, 2021 5:21 PM
To: Srinivas, Vidya <vidya.srinivas@intel.com>; Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
Cc: Lin, Charlton <charlton.lin@intel.com>
Subject: Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
On 8.6.2021 12.19, Srinivas, Vidya wrote:
> Hello Juha-Pekka
>
> Instead of wait for vblank, this also works igt_pipe_crc_start->
> igt_pipe_crc_get_current for small fb after commit -> then
> igt_pipe_crc_get_current For big fb -> compare -> igt_pipe_crc_stop
>
> Would this change be okay? Kindly suggest. igt_pipe_crc_collect_crc is
> not working. It gives CRC mismatch for few subtests like subtest
> y-tiled-32bpp-rotate-0
This change is ok. It kind of implies there maybe is some problem on your platform with starting of crc calculation but if this is only place where it will show I'm ok with that since crc will not affect normal users in any way.
I noticed your new patch, lets see how all ci machines behave on that before doing anything else.
/Juha-Pekka
>
> Have submitted the change here
> https://patchwork.freedesktop.org/patch/437657/?series=90389&rev=6
>
> Thank you so much.
>
> Regards
> Vidya
>
> -----Original Message-----
> From: Srinivas, Vidya
> Sent: Tuesday, June 8, 2021 1:19 PM
> To: juhapekka.heikkila@gmail.com; Modem, Bhanuprakash
> <Bhanuprakash.Modem@intel.com>; intel-gfx@lists.freedesktop.org;
> igt-dev@lists.freedesktop.org
> Cc: Lin, Charlton <Charlton.Lin@intel.com>
> Subject: RE: [igt-dev] [Intel-gfx] [PATCH i-g-t] [RFC]
> tests/kms_big_fb: Wait for vblank before collecting CRC
>
> Hello Juha-Pekka and Bhanu
>
> Thank you for the review comments. Apologies Juha-Pekka, I will incorporate your review comments and try out.
>
> Regards
> Vidya
>
>
> -----Original Message-----
> From: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Sent: Tuesday, June 8, 2021 1:04 PM
> To: Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; Srinivas,
> Vidya <vidya.srinivas@intel.com>; intel-gfx@lists.freedesktop.org;
> igt-dev@lists.freedesktop.org
> Cc: Lin, Charlton <charlton.lin@intel.com>
> Subject: Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] [RFC]
> tests/kms_big_fb: Wait for vblank before collecting CRC
>
> On 8.6.2021 10.01, Modem, Bhanuprakash wrote:
>>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
>>> Of Vidya Srinivas
>>> Sent: Friday, May 28, 2021 9:57 AM
>>> To: intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org
>>> Cc: markyacoub@chromium.org; Lin, Charlton <charlton.lin@intel.com>
>>> Subject: [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for
>>> vblank before collecting CRC
>>>
>>> Without wait for vblank, CRC mismatch is seen between big and small
>>> CRC on few Gen11 systems.
>>>
>>> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
>>> ---
>>> tests/kms_big_fb.c | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/kms_big_fb.c b/tests/kms_big_fb.c index
>>> b35727a09bd0..f90363c3beb2 100644
>>> --- a/tests/kms_big_fb.c
>>> +++ b/tests/kms_big_fb.c
>>> @@ -254,6 +254,7 @@ static void unset_lut(data_t *data)
>>> static bool test_plane(data_t *data)
>>> {
>>> igt_plane_t *plane = data->plane;
>>> + igt_display_t *display = &data->display;
>>
>> For code readability purpose, I think we need to update to use this
>> variable wherever we are using "&data->display" in this function.
>> s/"&data->display"/"display"/
>>
>> With above change, this patch LGTM
>> Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>
>
> I still don't see benefit in this patch. For now all this look like is doing is slow down the test and if this actually helps there's a real bug somewhere which is not here. My earlier review comments were not addressed hence repeat here, see below.
>
>
>>> struct igt_fb *small_fb = &data->small_fb;
>>> struct igt_fb *big_fb = &data->big_fb;
>>> int w = data->big_fb_width - small_fb->width; @@ -337,16 +338,17
>>> @@ static bool test_plane(data_t *data)
>>> igt_display_commit2(&data->display, data->display.is_atomic ?
>>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>>>
>>> -
>>> + igt_wait_for_vblank(data->drm_fd,
>>> +display->pipes[data->pipe].crtc_offset);
>
> Above this line there's flip to different fb. Below this line crc calculation is restarted, get one crc and stop crc. There's several vblanks already spent here, if now adding one more somehow helps it sound like there's problems in crc calculation on some platform or kernel is saying too early framebuffer is ready to be shown. Am I missing something here?
>
> /Juha-Pekka
>
>>> igt_pipe_crc_collect_crc(data->pipe_crc, &small_crc);
>>>
>>> igt_plane_set_fb(plane, big_fb);
>>> igt_fb_set_position(big_fb, plane, x, y);
>>> igt_fb_set_size(big_fb, plane, small_fb->width,
>>> small_fb->height);
>>> +
>
> spurious empty line need to be removed.
>
>>> igt_plane_set_size(plane, data->width, data->height);
>>> igt_display_commit2(&data->display, data->display.is_atomic ?
>>> COMMIT_ATOMIC : COMMIT_UNIVERSAL);
>>> -
>>> + igt_wait_for_vblank(data->drm_fd,
>>> +display->pipes[data->pipe].crtc_offset);
>>> igt_pipe_crc_collect_crc(data->pipe_crc, &big_crc);
>>>
>>> igt_plane_set_fb(plane, NULL);
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
2021-06-10 8:38 ` Srinivas, Vidya
@ 2021-06-10 8:51 ` Petri Latvala
2021-06-10 8:52 ` Srinivas, Vidya
0 siblings, 1 reply; 12+ messages in thread
From: Petri Latvala @ 2021-06-10 8:51 UTC (permalink / raw
To: Srinivas, Vidya
Cc: igt-dev@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
Lin, Charlton
On Thu, Jun 10, 2021 at 08:38:42AM +0000, Srinivas, Vidya wrote:
> Hello Juha-Pekka,
>
> https://patchwork.freedesktop.org/series/90389/#rev7 shows PASS for all CI.
> However I don’t see kms_big_fb all the subtests running in CI. In the logs I see pass for linear-32bpp-rotate-0
The default view in the CI results only shows tests that have
issues. "view -> shards all" from the top shows all tests.
https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5907/shards-all.html?testfilter=kms_big_fb
--
Petri Latvala
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
2021-06-10 8:51 ` Petri Latvala
@ 2021-06-10 8:52 ` Srinivas, Vidya
0 siblings, 0 replies; 12+ messages in thread
From: Srinivas, Vidya @ 2021-06-10 8:52 UTC (permalink / raw
To: Latvala, Petri
Cc: igt-dev@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
Lin, Charlton
Thank you so much Petri. I see it’s a PASS for almost everything except on one GLK.
Regards
Vidya
-----Original Message-----
From: Latvala, Petri <petri.latvala@intel.com>
Sent: Thursday, June 10, 2021 2:21 PM
To: Srinivas, Vidya <vidya.srinivas@intel.com>
Cc: juhapekka.heikkila@gmail.com; Modem, Bhanuprakash <bhanuprakash.modem@intel.com>; intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org; Lin, Charlton <charlton.lin@intel.com>
Subject: Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC
On Thu, Jun 10, 2021 at 08:38:42AM +0000, Srinivas, Vidya wrote:
> Hello Juha-Pekka,
>
> https://patchwork.freedesktop.org/series/90389/#rev7 shows PASS for all CI.
> However I don’t see kms_big_fb all the subtests running in CI. In the
> logs I see pass for linear-32bpp-rotate-0
The default view in the CI results only shows tests that have issues. "view -> shards all" from the top shows all tests.
https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5907/shards-all.html?testfilter=kms_big_fb
--
Petri Latvala
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-06-10 8:52 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-05-28 4:27 [Intel-gfx] [PATCH i-g-t] [RFC] tests/kms_big_fb: Wait for vblank before collecting CRC Vidya Srinivas
2021-06-04 18:50 ` Mark Yacoub
2021-06-05 5:46 ` Srinivas, Vidya
2021-06-08 7:01 ` Modem, Bhanuprakash
2021-06-08 7:34 ` [Intel-gfx] [igt-dev] " Juha-Pekka Heikkila
2021-06-08 7:48 ` Srinivas, Vidya
2021-06-08 9:19 ` Srinivas, Vidya
2021-06-08 11:50 ` Juha-Pekka Heikkila
2021-06-08 11:54 ` Srinivas, Vidya
2021-06-10 8:38 ` Srinivas, Vidya
2021-06-10 8:51 ` Petri Latvala
2021-06-10 8:52 ` Srinivas, Vidya
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).