[igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Fix alpha in coverage-vs-premult-vs-constant
Srinivas, Vidya
vidya.srinivas at intel.com
Sat Jun 19 08:24:40 UTC 2021
Hello Juha-Pekka,
Thank you so much. I tried the LUT table you suggested and it worked https://patchwork.freedesktop.org/patch/440145/?series=90828&rev=4
Could you kindly check if it is right? 0.25 and 0.75 did not work. 1,1,1 too is failing once in a while.
Regards
Vidya
-----Original Message-----
From: Srinivas, Vidya
Sent: Saturday, June 19, 2021 8:40 AM
To: Juha-Pekka Heikkilä <juhapekka.heikkila at gmail.com>; igt-dev at lists.freedesktop.org
Cc: Latvala, Petri <petri.latvala at intel.com>; Lin, Charlton <Charlton.Lin at intel.com>
Subject: RE: [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Fix alpha in coverage-vs-premult-vs-constant
Thank you so much Juha-Pekka. I will try out the gamma thing you mentioned and update further.
Regards
Vidya
-----Original Message-----
From: Juha-Pekka Heikkilä <juhapekka.heikkila at gmail.com>
Sent: Saturday, June 19, 2021 1:54 AM
To: Srinivas, Vidya <vidya.srinivas at intel.com>; igt-dev at lists.freedesktop.org
Cc: Latvala, Petri <petri.latvala at intel.com>; Lin, Charlton <charlton.lin at intel.com>
Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Fix alpha in coverage-vs-premult-vs-constant
Hi Vidya,
I don't now have access to specs so I can't say if there would be something special for JSL on this case. OTOH if changing gray buffer to solid white fixes that maybe good solution, I'm trying to think how the screen will be in this case and I assume test image will remain on screen(?). If just changing gray buffer to solid white make test pass there maybe also 0.25 gray and 0.75 gray which will work. It wouldn't be the first time there is found case with specific color which causes rounding error with crc where it's just more convenient to change that color.
I don't know can it be used in this case but there might be possibility to do gamma table trick if nothing else works. This is used in kms_plane and kms_flip_scaled_crc that come to my mind, in tests/kms_flip_scaled_crc.c look for set_lut(..) function and how it is used.
I'd avoid creating special cases for some hw versions, it will lead to big mess in the end.
/Juha-Pekka
Srinivas, Vidya kirjoitti 18.6.2021 klo 21.54:
> Hello Juha-Pekka,
>
> As you said, I guess that might be the problem. But the changes of WA are already present in kernel. I tried reverting them just to check, but did not see any change.
> If I change the gray buffer (0.5 to 1,1,1) it works. If I comment first primary commit it works. If I just comment "Pre-multiplied" it works, else lower alpha works.
> Couldn’t find any other changes yet in driver. Would it be okay if I can add a patch to disable primary commit if JASPERLAKE?
>
> Thank you so much.
>
> Regards
> Vidya
>
> -----Original Message-----
> From: Srinivas, Vidya
> Sent: Friday, June 18, 2021 4:01 PM
> To: juhapekka.heikkila at gmail.com; igt-dev at lists.freedesktop.org
> Cc: Latvala, Petri <petri.latvala at intel.com>; Lin, Charlton
> <Charlton.Lin at intel.com>
> Subject: RE: [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Fix
> alpha in coverage-vs-premult-vs-constant
>
> Thank you so much Juha-Pekka. I will check on the same and update.
>
> Regards
> Vidya
>
> -----Original Message-----
> From: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> Sent: Friday, June 18, 2021 3:56 PM
> To: Srinivas, Vidya <vidya.srinivas at intel.com>;
> igt-dev at lists.freedesktop.org
> Cc: Latvala, Petri <petri.latvala at intel.com>; Lin, Charlton
> <charlton.lin at intel.com>
> Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Fix
> alpha in coverage-vs-premult-vs-constant
>
> Have you tried looking if on kernel side in intel_display.c
> icl_set_pipe_chicken(..) those icl related WAs have effect on your crc problem. Sound like setting PER_PIXEL_ALPHA_BYPASS_EN and PIXEL_ROUNDING_TRUNC_FB_PASSTHRU touch exactly the problem you are seeing. Maybe different hw revision behave differently? Try see if there's on spec some note for your HW revision for these WAs.
>
> /Juha-Pekka
>
> On 18.6.2021 11.58, Srinivas, Vidya wrote:
>> Thank you so much Juha-Pekka.
>>
>> https://patchwork.freedesktop.org/patch/436194/?series=90828&rev=2
>> just removes the DRM_PLANE_TYPE_PRIMARY fb setting from subtest That works. Would that be an okay change?
>>
>> Because without the commit after PRIMARY and alpha value 0x7e, JSL is failing. If we just don’t do the PRIMARY fb setting at start, test works as is.
>>
>> Kindly suggest. Thank you very much once again.
>>
>> Regards
>> Vidya
>>
>> -----Original Message-----
>> From: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
>> Sent: Friday, June 18, 2021 2:23 PM
>> To: Srinivas, Vidya <vidya.srinivas at intel.com>;
>> igt-dev at lists.freedesktop.org
>> Cc: Latvala, Petri <petri.latvala at intel.com>; Lin, Charlton
>> <charlton.lin at intel.com>
>> Subject: Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Fix
>> alpha in coverage-vs-premult-vs-constant
>>
>> On 11.6.2021 15.44, Vidya Srinivas wrote:
>>> Patch sets alpha to 0x7e for coverage, Pre-multiplied and constant
>>> as per the buffer being created initially in prepare_crtc. Patch
>>> also add commit after setting fb on primary plane. Without this
>>> change CRC mismatch is seen on few Gen11 systems.
>>>
>>> Signed-off-by: Vidya Srinivas <vidya.srinivas at intel.com>
>>> ---
>>> tests/kms_plane_alpha_blend.c | 9 +++++++--
>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/kms_plane_alpha_blend.c
>>> b/tests/kms_plane_alpha_blend.c index a37cb27c7d62..d3a73cf43fcd
>>> 100644
>>> --- a/tests/kms_plane_alpha_blend.c
>>> +++ b/tests/kms_plane_alpha_blend.c
>>> @@ -448,29 +448,34 @@ static void coverage_premult_constant(data_t *data, enum pipe pipe, igt_plane_t
>>> igt_crc_t ref_crc = {}, crc = {};
>>>
>>> /* Set a background color on the primary fb for testing */
>>> - if (plane->type != DRM_PLANE_TYPE_PRIMARY)
>>> + if (plane->type != DRM_PLANE_TYPE_PRIMARY) {
>>>
>>> igt_plane_set_fb(igt_pipe_get_plane_type(&display->pipes[pipe],
>>> DRM_PLANE_TYPE_PRIMARY), &data->gray_fb);
>>> + igt_display_commit2(display, COMMIT_ATOMIC);
>>
>> Why this extra commit needed? There's just below commit before crc is
>> started
>>
>>> + }
>>>
>>> igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_BLEND_MODE,
>>> "Coverage");
>>> + igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0x7e);
>>
>> On this test there's already per pixel alpha, setting plane alpha values so low will probably in this case in final rounding make these planes completely invisible. I'd suspect you can get any image pass crc doing it like this. Likely what you see on screen with these values is just pure grey instead of intended test image.
>>
>>
>>> igt_plane_set_fb(plane, &data->argb_fb_cov_7e);
>>> igt_display_commit2(display, COMMIT_ATOMIC);
>>> igt_pipe_crc_start(data->pipe_crc);
>>> igt_pipe_crc_get_single(data->pipe_crc, &ref_crc);
>>>
>>> igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_BLEND_MODE,
>>> "Pre-multiplied");
>>> + igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0x7e);
>>> igt_plane_set_fb(plane, &data->argb_fb_7e);
>>> igt_display_commit2(display, COMMIT_ATOMIC);
>>> igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &crc);
>>> igt_assert_crc_equal(&ref_crc, &crc);
>>>
>>> igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_BLEND_MODE, "None");
>>> - igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0x7e7e);
>>> + igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0x7e);
>>> igt_plane_set_fb(plane, &data->argb_fb_cov_7e);
>>> igt_display_commit2(display, COMMIT_ATOMIC);
>>> igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &crc);
>>> igt_assert_crc_equal(&ref_crc, &crc);
>>>
>>> igt_pipe_crc_stop(data->pipe_crc);
>>> +
>>
>> stray new line
>>
>>> }
>>>
>>> static void run_test_on_pipe_planes(data_t *data, enum pipe
>>> pipe, bool blend,
>>>
>>
>
More information about the igt-dev
mailing list