public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Paint FBs with cairo source operator
@ 2019-02-22 17:08 Nicholas Kazlauskas
  2019-02-22 17:39 ` Ville Syrjälä
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Nicholas Kazlauskas @ 2019-02-22 17:08 UTC (permalink / raw)
  To: igt-dev

Cairo defaults to using a blended fill, so when IGT tries to paint on
top of a framebuffer in these tests with an alpha less than 1.0 the
previous framebuffer contents will still be present underneath.

On amdgpu this is an issue because the framebuffer is created in
an unitialized state and can have contents from previous tests
still within memory.

This patch explicitly sets the cairo filling operator to SOURCE to
overwrite the contents when performing fills with alpha < 1.0 in
this test.

It also fixes the creation of the transparent FB. My guess as for why
this test previously passed on i915 is because the primary FB was
also black, so the CRC would match even if the black overlay plane
was partially transparent.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 tests/kms_plane_alpha_blend.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/kms_plane_alpha_blend.c b/tests/kms_plane_alpha_blend.c
index 1d9d8933..d7dc44ab 100644
--- a/tests/kms_plane_alpha_blend.c
+++ b/tests/kms_plane_alpha_blend.c
@@ -83,6 +83,7 @@ static void draw_squares(struct igt_fb *fb, int w, int h, double a)
 {
 	cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
 
+	cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
 	igt_paint_color_alpha(cr, 0, 0,         w / 2, h / 2, 1., 0., 0., a);
 	igt_paint_color_alpha(cr, w / 2, 0,     w / 2, h / 2, 0., 1., 0., a);
 	igt_paint_color_alpha(cr, 0, h / 2,     w / 2, h / 2, 0., 0., 1., a);
@@ -197,7 +198,8 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
 			      &data->argb_fb_0);
 
 		cr = igt_get_cairo_ctx(data->gfx_fd, &data->argb_fb_0);
-		igt_paint_color_alpha(cr, 0, 0, w, h, 0., 0., 0., 1.0);
+		cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
+		igt_paint_color_alpha(cr, 0, 0, w, h, 0., 0., 0., 0.0);
 		igt_put_cairo_ctx(data->gfx_fd, &data->argb_fb_0, cr);
 
 		igt_create_fb(data->gfx_fd, w, h,
-- 
2.17.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Paint FBs with cairo source operator
  2019-02-22 17:08 [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Paint FBs with cairo source operator Nicholas Kazlauskas
@ 2019-02-22 17:39 ` Ville Syrjälä
  2019-02-22 17:46   ` Kazlauskas, Nicholas
  2019-02-22 18:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2019-02-22 17:39 UTC (permalink / raw)
  To: Nicholas Kazlauskas; +Cc: igt-dev

On Fri, Feb 22, 2019 at 12:08:46PM -0500, Nicholas Kazlauskas wrote:
> Cairo defaults to using a blended fill, so when IGT tries to paint on
> top of a framebuffer in these tests with an alpha less than 1.0 the
> previous framebuffer contents will still be present underneath.
> 
> On amdgpu this is an issue because the framebuffer is created in
> an unitialized state and can have contents from previous tests
> still within memory.

igt_fb more or less assumes all fbs start out black. Sounds like you
need a memset() or equivalent somewhere.

> 
> This patch explicitly sets the cairo filling operator to SOURCE to
> overwrite the contents when performing fills with alpha < 1.0 in
> this test.
> 
> It also fixes the creation of the transparent FB. My guess as for why
> this test previously passed on i915 is because the primary FB was
> also black, so the CRC would match even if the black overlay plane
> was partially transparent.
> 
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> ---
>  tests/kms_plane_alpha_blend.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/kms_plane_alpha_blend.c b/tests/kms_plane_alpha_blend.c
> index 1d9d8933..d7dc44ab 100644
> --- a/tests/kms_plane_alpha_blend.c
> +++ b/tests/kms_plane_alpha_blend.c
> @@ -83,6 +83,7 @@ static void draw_squares(struct igt_fb *fb, int w, int h, double a)
>  {
>  	cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
>  
> +	cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
>  	igt_paint_color_alpha(cr, 0, 0,         w / 2, h / 2, 1., 0., 0., a);
>  	igt_paint_color_alpha(cr, w / 2, 0,     w / 2, h / 2, 0., 1., 0., a);
>  	igt_paint_color_alpha(cr, 0, h / 2,     w / 2, h / 2, 0., 0., 1., a);
> @@ -197,7 +198,8 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
>  			      &data->argb_fb_0);
>  
>  		cr = igt_get_cairo_ctx(data->gfx_fd, &data->argb_fb_0);
> -		igt_paint_color_alpha(cr, 0, 0, w, h, 0., 0., 0., 1.0);
> +		cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
> +		igt_paint_color_alpha(cr, 0, 0, w, h, 0., 0., 0., 0.0);
>  		igt_put_cairo_ctx(data->gfx_fd, &data->argb_fb_0, cr);
>  
>  		igt_create_fb(data->gfx_fd, w, h,
> -- 
> 2.17.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Paint FBs with cairo source operator
  2019-02-22 17:39 ` Ville Syrjälä
@ 2019-02-22 17:46   ` Kazlauskas, Nicholas
  2019-02-22 18:57     ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Kazlauskas, Nicholas @ 2019-02-22 17:46 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: igt-dev@lists.freedesktop.org

On 2/22/19 12:39 PM, Ville Syrjälä wrote:
> On Fri, Feb 22, 2019 at 12:08:46PM -0500, Nicholas Kazlauskas wrote:
>> Cairo defaults to using a blended fill, so when IGT tries to paint on
>> top of a framebuffer in these tests with an alpha less than 1.0 the
>> previous framebuffer contents will still be present underneath.
>>
>> On amdgpu this is an issue because the framebuffer is created in
>> an unitialized state and can have contents from previous tests
>> still within memory.
> 
> igt_fb more or less assumes all fbs start out black. Sounds like you
> need a memset() or equivalent somewhere.

The buffer could also be painted to 0 implicitly using cairo. We also 
need a patch at some point to not generate the suspicious CRC read 
warning since our black screens are (0, 0, 0) and we rely on tests 
leaving garbage in the framebuffer to avoid it.

However, this patch would still be needed since the fill will still be 
blended by default. When creating transparent or semi-transparent 
buffers they should be filled in with the cairo source operator or the 
alpha channel won't be set to the correct value.

Nicholas Kazlauskas

> 
>>
>> This patch explicitly sets the cairo filling operator to SOURCE to
>> overwrite the contents when performing fills with alpha < 1.0 in
>> this test.
>>
>> It also fixes the creation of the transparent FB. My guess as for why
>> this test previously passed on i915 is because the primary FB was
>> also black, so the CRC would match even if the black overlay plane
>> was partially transparent.
>>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>> ---
>>   tests/kms_plane_alpha_blend.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/kms_plane_alpha_blend.c b/tests/kms_plane_alpha_blend.c
>> index 1d9d8933..d7dc44ab 100644
>> --- a/tests/kms_plane_alpha_blend.c
>> +++ b/tests/kms_plane_alpha_blend.c
>> @@ -83,6 +83,7 @@ static void draw_squares(struct igt_fb *fb, int w, int h, double a)
>>   {
>>   	cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
>>   
>> +	cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
>>   	igt_paint_color_alpha(cr, 0, 0,         w / 2, h / 2, 1., 0., 0., a);
>>   	igt_paint_color_alpha(cr, w / 2, 0,     w / 2, h / 2, 0., 1., 0., a);
>>   	igt_paint_color_alpha(cr, 0, h / 2,     w / 2, h / 2, 0., 0., 1., a);
>> @@ -197,7 +198,8 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
>>   			      &data->argb_fb_0);
>>   
>>   		cr = igt_get_cairo_ctx(data->gfx_fd, &data->argb_fb_0);
>> -		igt_paint_color_alpha(cr, 0, 0, w, h, 0., 0., 0., 1.0);
>> +		cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
>> +		igt_paint_color_alpha(cr, 0, 0, w, h, 0., 0., 0., 0.0);
>>   		igt_put_cairo_ctx(data->gfx_fd, &data->argb_fb_0, cr);
>>   
>>   		igt_create_fb(data->gfx_fd, w, h,
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> igt-dev mailing list
>> igt-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [igt-dev] ✗ Fi.CI.BAT: failure for tests/kms_plane_alpha_blend: Paint FBs with cairo source operator
  2019-02-22 17:08 [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Paint FBs with cairo source operator Nicholas Kazlauskas
  2019-02-22 17:39 ` Ville Syrjälä
@ 2019-02-22 18:27 ` Patchwork
  2019-02-25 16:47 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_alpha_blend: Paint FBs with cairo source operator (rev2) Patchwork
  2019-02-25 23:08 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2019-02-22 18:27 UTC (permalink / raw)
  To: igt-dev

== Series Details ==

Series: tests/kms_plane_alpha_blend: Paint FBs with cairo source operator
URL   : https://patchwork.freedesktop.org/series/57105/
State : failure

== Summary ==

CI Bug Log - changes from IGT_4853 -> IGTPW_2493
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_2493 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_2493, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57105/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_2493:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-kbl-7500u:       PASS -> FAIL

  
#### Warnings ####

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7500u:       DMESG-WARN [fdo#102505] / [fdo#103558] / [fdo#105079] / [fdo#105602] -> FAIL

  
Known issues
------------

  Here are the changes found in IGTPW_2493 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       PASS -> SKIP [fdo#109271]

  * igt@i915_pm_rpm@basic-rte:
    - fi-bsw-kefka:       PASS -> FAIL [fdo#108800]

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       PASS -> SKIP [fdo#109271] / [fdo#109278] +2

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-kbl-7500u:       PASS -> FAIL [fdo#109495]

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-icl-u2:          FAIL [fdo#103375] -> PASS

  * igt@kms_busy@basic-flip-b:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       WARN [fdo#109380] -> PASS

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       FAIL [fdo#109485] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
    - fi-kbl-7567u:       SKIP [fdo#109271] -> PASS +33

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109380]: https://bugs.freedesktop.org/show_bug.cgi?id=109380
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
  [fdo#109495]: https://bugs.freedesktop.org/show_bug.cgi?id=109495


Participating hosts (45 -> 40)
------------------------------

  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

    * IGT: IGT_4853 -> IGTPW_2493

  CI_DRM_5654: 30c7f283790b433aa311ef7a7d2b6b428886fb9a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2493: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2493/
  IGT_4853: 8afdfd8fa9ce17043d9105dedca46ad4555fdcdb @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2493/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Paint FBs with cairo source operator
  2019-02-22 17:46   ` Kazlauskas, Nicholas
@ 2019-02-22 18:57     ` Ville Syrjälä
  2019-02-25 14:32       ` Kazlauskas, Nicholas
  0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2019-02-22 18:57 UTC (permalink / raw)
  To: Kazlauskas, Nicholas; +Cc: igt-dev@lists.freedesktop.org

On Fri, Feb 22, 2019 at 05:46:08PM +0000, Kazlauskas, Nicholas wrote:
> On 2/22/19 12:39 PM, Ville Syrjälä wrote:
> > On Fri, Feb 22, 2019 at 12:08:46PM -0500, Nicholas Kazlauskas wrote:
> >> Cairo defaults to using a blended fill, so when IGT tries to paint on
> >> top of a framebuffer in these tests with an alpha less than 1.0 the
> >> previous framebuffer contents will still be present underneath.
> >>
> >> On amdgpu this is an issue because the framebuffer is created in
> >> an unitialized state and can have contents from previous tests
> >> still within memory.
> > 
> > igt_fb more or less assumes all fbs start out black. Sounds like you
> > need a memset() or equivalent somewhere.
> 
> The buffer could also be painted to 0 implicitly using cairo. We also 
> need a patch at some point to not generate the suspicious CRC read 
> warning since our black screens are (0, 0, 0) and we rely on tests 
> leaving garbage in the framebuffer to avoid it.

I guess just 'if (is_i915) check_suspicious_crc()' should help.
Though I'm not 100% sure we can't get those on some i915 platforms too.

> 
> However, this patch would still be needed since the fill will still be 
> blended by default. When creating transparent or semi-transparent 
> buffers they should be filled in with the cairo source operator or the 
> alpha channel won't be set to the correct value.

AFAICS if the dest alpha and color are both 0.0 the only fb that is
wrong is argb_fb_0 since the current code will in fact give us
alpha of 1.0. draw_squares() with OVER should still give us the
same results as we'd get with SOURCE.

I guess this means any test which is currently using argb_fb_0
is definitely not testing per-pixel alpha. 

So argb_transparant() and basic_alpha() are simply useless atm?

constant_alpha_min() sets pixel_blend_mode to None anyway so
doesn't really matter what we have in the alpha channel.

Not sure why those tests aren't doing the gray_fb thing the other tests
are using...

Anyways, no point in blending when you don't have to so
the patch seems to make sense to me either way:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Nicholas Kazlauskas
> 
> > 
> >>
> >> This patch explicitly sets the cairo filling operator to SOURCE to
> >> overwrite the contents when performing fills with alpha < 1.0 in
> >> this test.
> >>
> >> It also fixes the creation of the transparent FB. My guess as for why
> >> this test previously passed on i915 is because the primary FB was
> >> also black, so the CRC would match even if the black overlay plane
> >> was partially transparent.
> >>
> >> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> >> ---
> >>   tests/kms_plane_alpha_blend.c | 4 +++-
> >>   1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tests/kms_plane_alpha_blend.c b/tests/kms_plane_alpha_blend.c
> >> index 1d9d8933..d7dc44ab 100644
> >> --- a/tests/kms_plane_alpha_blend.c
> >> +++ b/tests/kms_plane_alpha_blend.c
> >> @@ -83,6 +83,7 @@ static void draw_squares(struct igt_fb *fb, int w, int h, double a)
> >>   {
> >>   	cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
> >>   
> >> +	cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
> >>   	igt_paint_color_alpha(cr, 0, 0,         w / 2, h / 2, 1., 0., 0., a);
> >>   	igt_paint_color_alpha(cr, w / 2, 0,     w / 2, h / 2, 0., 1., 0., a);
> >>   	igt_paint_color_alpha(cr, 0, h / 2,     w / 2, h / 2, 0., 0., 1., a);
> >> @@ -197,7 +198,8 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
> >>   			      &data->argb_fb_0);
> >>   
> >>   		cr = igt_get_cairo_ctx(data->gfx_fd, &data->argb_fb_0);
> >> -		igt_paint_color_alpha(cr, 0, 0, w, h, 0., 0., 0., 1.0);
> >> +		cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
> >> +		igt_paint_color_alpha(cr, 0, 0, w, h, 0., 0., 0., 0.0);
> >>   		igt_put_cairo_ctx(data->gfx_fd, &data->argb_fb_0, cr);
> >>   
> >>   		igt_create_fb(data->gfx_fd, w, h,
> >> -- 
> >> 2.17.1
> >>
> >> _______________________________________________
> >> igt-dev mailing list
> >> igt-dev@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > 
> 

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Paint FBs with cairo source operator
  2019-02-22 18:57     ` Ville Syrjälä
@ 2019-02-25 14:32       ` Kazlauskas, Nicholas
  2019-02-27 19:49         ` Wentland, Harry
  0 siblings, 1 reply; 9+ messages in thread
From: Kazlauskas, Nicholas @ 2019-02-25 14:32 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: igt-dev@lists.freedesktop.org

On 2/22/19 1:57 PM, Ville Syrjälä wrote:
> On Fri, Feb 22, 2019 at 05:46:08PM +0000, Kazlauskas, Nicholas wrote:
>> On 2/22/19 12:39 PM, Ville Syrjälä wrote:
>>> On Fri, Feb 22, 2019 at 12:08:46PM -0500, Nicholas Kazlauskas wrote:
>>>> Cairo defaults to using a blended fill, so when IGT tries to paint on
>>>> top of a framebuffer in these tests with an alpha less than 1.0 the
>>>> previous framebuffer contents will still be present underneath.
>>>>
>>>> On amdgpu this is an issue because the framebuffer is created in
>>>> an unitialized state and can have contents from previous tests
>>>> still within memory.
>>>
>>> igt_fb more or less assumes all fbs start out black. Sounds like you
>>> need a memset() or equivalent somewhere.
>>
>> The buffer could also be painted to 0 implicitly using cairo. We also
>> need a patch at some point to not generate the suspicious CRC read
>> warning since our black screens are (0, 0, 0) and we rely on tests
>> leaving garbage in the framebuffer to avoid it.
> 
> I guess just 'if (is_i915) check_suspicious_crc()' should help.
> Though I'm not 100% sure we can't get those on some i915 platforms too.

Another solution here would just be to modify the CRC in amdgpu before 
returning it to userspace. Like XOR the value with some constant. Though 
that would be a little less elegant since there could still be the 
chance we get a CRC value back with that exact constant.

> 
>>
>> However, this patch would still be needed since the fill will still be
>> blended by default. When creating transparent or semi-transparent
>> buffers they should be filled in with the cairo source operator or the
>> alpha channel won't be set to the correct value.
> 
> AFAICS if the dest alpha and color are both 0.0 the only fb that is
> wrong is argb_fb_0 since the current code will in fact give us
> alpha of 1.0. draw_squares() with OVER should still give us the
> same results as we'd get with SOURCE.
> 
> I guess this means any test which is currently using argb_fb_0
> is definitely not testing per-pixel alpha.
> 
> So argb_transparant() and basic_alpha() are simply useless atm?
> 
> constant_alpha_min() sets pixel_blend_mode to None anyway so
> doesn't really matter what we have in the alpha channel.
> 
> Not sure why those tests aren't doing the gray_fb thing the other tests
> are using...
> 
> Anyways, no point in blending when you don't have to so
> the patch seems to make sense to me either way:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks for the review. Like you said, the main problem is with argb_fb_0 
and argb_transparent and basic_alpha but it's better to be explicit 
about intent with the square filling too IMO.

Nicholas Kazlauskas

> 
>>
>> Nicholas Kazlauskas
>>
>>>
>>>>
>>>> This patch explicitly sets the cairo filling operator to SOURCE to
>>>> overwrite the contents when performing fills with alpha < 1.0 in
>>>> this test.
>>>>
>>>> It also fixes the creation of the transparent FB. My guess as for why
>>>> this test previously passed on i915 is because the primary FB was
>>>> also black, so the CRC would match even if the black overlay plane
>>>> was partially transparent.
>>>>
>>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>>>> ---
>>>>    tests/kms_plane_alpha_blend.c | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/kms_plane_alpha_blend.c b/tests/kms_plane_alpha_blend.c
>>>> index 1d9d8933..d7dc44ab 100644
>>>> --- a/tests/kms_plane_alpha_blend.c
>>>> +++ b/tests/kms_plane_alpha_blend.c
>>>> @@ -83,6 +83,7 @@ static void draw_squares(struct igt_fb *fb, int w, int h, double a)
>>>>    {
>>>>    	cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
>>>>    
>>>> +	cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
>>>>    	igt_paint_color_alpha(cr, 0, 0,         w / 2, h / 2, 1., 0., 0., a);
>>>>    	igt_paint_color_alpha(cr, w / 2, 0,     w / 2, h / 2, 0., 1., 0., a);
>>>>    	igt_paint_color_alpha(cr, 0, h / 2,     w / 2, h / 2, 0., 0., 1., a);
>>>> @@ -197,7 +198,8 @@ static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
>>>>    			      &data->argb_fb_0);
>>>>    
>>>>    		cr = igt_get_cairo_ctx(data->gfx_fd, &data->argb_fb_0);
>>>> -		igt_paint_color_alpha(cr, 0, 0, w, h, 0., 0., 0., 1.0);
>>>> +		cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
>>>> +		igt_paint_color_alpha(cr, 0, 0, w, h, 0., 0., 0., 0.0);
>>>>    		igt_put_cairo_ctx(data->gfx_fd, &data->argb_fb_0, cr);
>>>>    
>>>>    		igt_create_fb(data->gfx_fd, w, h,
>>>> -- 
>>>> 2.17.1
>>>>
>>>> _______________________________________________
>>>> igt-dev mailing list
>>>> igt-dev@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/igt-dev
>>>
>>
> 

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_alpha_blend: Paint FBs with cairo source operator (rev2)
  2019-02-22 17:08 [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Paint FBs with cairo source operator Nicholas Kazlauskas
  2019-02-22 17:39 ` Ville Syrjälä
  2019-02-22 18:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
@ 2019-02-25 16:47 ` Patchwork
  2019-02-25 23:08 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2019-02-25 16:47 UTC (permalink / raw)
  To: igt-dev

== Series Details ==

Series: tests/kms_plane_alpha_blend: Paint FBs with cairo source operator (rev2)
URL   : https://patchwork.freedesktop.org/series/57105/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5658 -> IGTPW_2514
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57105/revisions/2/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2514 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-compute:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#108094]

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       PASS -> FAIL [fdo#109485]

  
#### Possible fixes ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       DMESG-WARN [fdo#108965] -> PASS

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       WARN [fdo#109380] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
    - fi-kbl-7567u:       SKIP [fdo#109271] -> PASS +33

  
  [fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109380]: https://bugs.freedesktop.org/show_bug.cgi?id=109380
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485


Participating hosts (44 -> 36)
------------------------------

  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-gdg-551 fi-pnv-d510 fi-bsw-kefka 


Build changes
-------------

    * IGT: IGT_4854 -> IGTPW_2514

  CI_DRM_5658: dc6f5e9c1239d7a4b77e31cfaca48873692d579f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2514: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2514/
  IGT_4854: 06b0830fb948b9b632342cd26100342aa01cbc79 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2514/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for tests/kms_plane_alpha_blend: Paint FBs with cairo source operator (rev2)
  2019-02-22 17:08 [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Paint FBs with cairo source operator Nicholas Kazlauskas
                   ` (2 preceding siblings ...)
  2019-02-25 16:47 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_alpha_blend: Paint FBs with cairo source operator (rev2) Patchwork
@ 2019-02-25 23:08 ` Patchwork
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2019-02-25 23:08 UTC (permalink / raw)
  To: igt-dev

== Series Details ==

Series: tests/kms_plane_alpha_blend: Paint FBs with cairo source operator (rev2)
URL   : https://patchwork.freedesktop.org/series/57105/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5658_full -> IGTPW_2514_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57105/revisions/2/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2514_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@reset-stress:
    - shard-snb:          PASS -> FAIL [fdo#109661]

  * igt@gem_exec_params@secure-non-master:
    - shard-snb:          PASS -> INCOMPLETE [fdo#105411]

  * igt@gem_exec_schedule@preemptive-hang-bsd2:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] +32

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
    - shard-apl:          PASS -> FAIL [fdo#109660]

  * igt@kms_busy@basic-modeset-e:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-snb:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-hang-oldfb-render-d:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_chamelium@hdmi-crc-single:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +3

  * igt@kms_color@pipe-a-degamma:
    - shard-kbl:          PASS -> FAIL [fdo#104782] / [fdo#108145]

  * igt@kms_color@pipe-a-legacy-gamma:
    - shard-apl:          PASS -> FAIL [fdo#104782] / [fdo#108145]

  * igt@kms_cursor_crc@cursor-128x128-dpms:
    - shard-kbl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-256x85-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232] +4

  * igt@kms_cursor_crc@cursor-64x64-onscreen:
    - shard-apl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-mmap-gtt:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +2

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          PASS -> FAIL [fdo#103167] +4

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
    - shard-kbl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
    - shard-glk:          PASS -> FAIL [fdo#103167] +3

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-pri-indfb-draw-render:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +15

  * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2

  * igt@kms_plane_alpha_blend@pipe-a-alpha-transparant-fb:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145]
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-apl:          PASS -> FAIL [fdo#103166] +7

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
    - shard-glk:          PASS -> FAIL [fdo#103166] +4

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-kbl:          PASS -> FAIL [fdo#103166] +3

  * igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3

  * igt@kms_setmode@basic:
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@kms_vblank@pipe-c-ts-continuation-modeset-rpm:
    - shard-kbl:          PASS -> FAIL [fdo#104894] +1
    - shard-apl:          PASS -> FAIL [fdo#104894] +1

  * igt@perf_pmu@busy-check-all-vecs0:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +31

  * igt@prime_vgem@fence-write-hang:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +31

  
#### Possible fixes ####

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-snb:          SKIP [fdo#109271] -> PASS

  * igt@kms_atomic_transition@plane-all-modeset-transition:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-render-c:
    - shard-kbl:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_busy@extended-pageflip-hang-newfb-render-b:
    - shard-glk:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-apl:          FAIL [fdo#106510] / [fdo#108145] -> PASS

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-apl:          FAIL [fdo#104782] -> PASS
    - shard-kbl:          FAIL [fdo#104782] -> PASS

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-kbl:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-apl:          FAIL [fdo#103232] -> PASS +3

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          FAIL [fdo#104873] -> PASS

  * igt@kms_cursor_legacy@pipe-c-torture-bo:
    - shard-hsw:          DMESG-WARN [fdo#107122] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-apl:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-glk:          FAIL [fdo#103167] -> PASS +5

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-apl:          FAIL [fdo#108948] -> PASS +1

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-glk:          FAIL [fdo#108948] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-glk:          FAIL [fdo#103166] -> PASS +3

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#106510]: https://bugs.freedesktop.org/show_bug.cgi?id=106510
  [fdo#107122]: https://bugs.freedesktop.org/show_bug.cgi?id=107122
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * IGT: IGT_4854 -> IGTPW_2514
    * Piglit: piglit_4509 -> None

  CI_DRM_5658: dc6f5e9c1239d7a4b77e31cfaca48873692d579f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2514: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2514/
  IGT_4854: 06b0830fb948b9b632342cd26100342aa01cbc79 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2514/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Paint FBs with cairo source operator
  2019-02-25 14:32       ` Kazlauskas, Nicholas
@ 2019-02-27 19:49         ` Wentland, Harry
  0 siblings, 0 replies; 9+ messages in thread
From: Wentland, Harry @ 2019-02-27 19:49 UTC (permalink / raw)
  To: Kazlauskas, Nicholas, Ville Syrjälä
  Cc: igt-dev@lists.freedesktop.org

snip

>>
>>>
>>> However, this patch would still be needed since the fill will still be
>>> blended by default. When creating transparent or semi-transparent
>>> buffers they should be filled in with the cairo source operator or the
>>> alpha channel won't be set to the correct value.
>>
>> AFAICS if the dest alpha and color are both 0.0 the only fb that is
>> wrong is argb_fb_0 since the current code will in fact give us
>> alpha of 1.0. draw_squares() with OVER should still give us the
>> same results as we'd get with SOURCE.
>>
>> I guess this means any test which is currently using argb_fb_0
>> is definitely not testing per-pixel alpha.
>>
>> So argb_transparant() and basic_alpha() are simply useless atm?
>>
>> constant_alpha_min() sets pixel_blend_mode to None anyway so
>> doesn't really matter what we have in the alpha channel.
>>
>> Not sure why those tests aren't doing the gray_fb thing the other tests
>> are using...
>>
>> Anyways, no point in blending when you don't have to so
>> the patch seems to make sense to me either way:
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Thanks for the review. Like you said, the main problem is with argb_fb_0 
> and argb_transparent and basic_alpha but it's better to be explicit 
> about intent with the square filling too IMO.
> 

Merged.

> Nicholas Kazlauskas
> 

snip
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-02-27 19:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-22 17:08 [igt-dev] [PATCH i-g-t] tests/kms_plane_alpha_blend: Paint FBs with cairo source operator Nicholas Kazlauskas
2019-02-22 17:39 ` Ville Syrjälä
2019-02-22 17:46   ` Kazlauskas, Nicholas
2019-02-22 18:57     ` Ville Syrjälä
2019-02-25 14:32       ` Kazlauskas, Nicholas
2019-02-27 19:49         ` Wentland, Harry
2019-02-22 18:27 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
2019-02-25 16:47 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_plane_alpha_blend: Paint FBs with cairo source operator (rev2) Patchwork
2019-02-25 23:08 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox