[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
Mon Jun 21 13:32:35 UTC 2021


Hello Juha-Pekka,

Thank you so much. I tried pulling the crc start to the beginning and used current crc same way as kms_big_fb. Whatever I do, with regular white fb, I couldn’t get it working even once (it had worked the other day). I tried normal ARGB white fb too instead of XRGB but that too wont work. I created another gray fb with alpha like the other argb buffers and this seems to work. Both gray and white work. I have kept it gray for now.

Apologies for bothering you. Kindly have a check of this version and suggest please https://patchwork.freedesktop.org/patch/440263/?series=90828&rev=6

Regards
Vidya

-----Original Message-----
From: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com> 
Sent: Monday, June 21, 2021 1:35 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

That issue where solid white is also failing *sometimes* sound like there maybe two separate issues.

I'd suggest for experimenting try using solid white, move starting of crc counting at the very beginning of the test function and place wait for vblank always before reading crcs. If this will help then the same crc issue which was earlier on kms_big_fb test is also causing problem here.

/Juha-Pekka

On 19.6.2021 11.24, Srinivas, Vidya wrote:
> 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