[igt-dev] [PATCH i-g-t] test/kms_plane_alpha_blend: clearer validation for coverage-7efc

[Melissa Wen]

On 05/26, Kim, Sung joon wrote:
> [AMD Official Use Only - General]
> 
> Looks good to me.
> One suggestion would be adding some information (e.g. commit id, etc...) about the change that enabled Coverage alpha blend mode made in the kernel in your description?
> 
> Reviewed-by: Sung Joon Kim <sungkim at amd.com>

Hi,

Thanks for taking a look at it.

I think you are talking about those kernel changes in the AMD display
driver.  As kms_plane_alpha_blend should be a generic test for any GPU
vendors, the improvement here is not retricted to AMD, so I would prefer
to keep the message generic too.

But from your suggestion, I included the commit for alpha-7efc change,
where I exemplify the problems of the original approach for different
vendors. I just sent a v2 and included your r-b.

Let me know if you are okay with it.

BR,

Melissa
> 
> > -----Original Message-----
> > From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of
> > Melissa Wen
> > Sent: Wednesday, May 25, 2022 1:10 PM
> > To: igt-dev at lists.freedesktop.org
> > Cc: petri.latvala at intel.com; markyacoub at google.com
> > Subject: [igt-dev] [PATCH i-g-t] test/kms_plane_alpha_blend: clearer
> > validation for coverage-7efc
> >
> > Inspired by previous change in alpha-7efc, get a clearer validation for
> > the swappable property of plane_alpha and fg.alpha in the coverage blend
> > formula. Get rid of the 8-increment alpha loop by refactoring
> > coverage-7efc in favor of one-shot check format, avoiding doubts related
> > to bit precision and rounding issue.
> >
> > Signed-off-by: Melissa Wen <mwen at igalia.com>
> > ---
> >  tests/kms_plane_alpha_blend.c | 23 +++++++++++------------
> >  1 file changed, 11 insertions(+), 12 deletions(-)
> >
> > diff --git a/tests/kms_plane_alpha_blend.c
> > b/tests/kms_plane_alpha_blend.c
> > index 8e54dfd6..001a1479 100644
> > --- a/tests/kms_plane_alpha_blend.c
> > +++ b/tests/kms_plane_alpha_blend.c
> > @@ -420,20 +420,18 @@ static void coverage_7efc(data_t *data, enum pipe
> > pipe, igt_plane_t *plane)
> >       igt_pipe_crc_start(data->pipe_crc);
> >
> >       /* for coverage, plane alpha and fb alpha should be swappable, so
> > swap fb and alpha */
> > -     for (i = 0; i < 256; i += 8) {
> > -             igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, ((i/2)
> > << 8) | (i/2));
> > -             igt_plane_set_fb(plane, &data->argb_fb_cov_fc);
> > -             igt_display_commit2(display, COMMIT_ATOMIC);
> > +     igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0x7e7e);
> > +     igt_plane_set_fb(plane, &data->argb_fb_cov_fc);
> > +     igt_display_commit2(display, COMMIT_ATOMIC);
> >
> > -             igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc,
> > &ref_crc);
> > +     igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc,
> > &ref_crc);
> >
> > -             igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, (i << 8)
> > | i);
> > -             igt_plane_set_fb(plane, &data->argb_fb_cov_7e);
> > -             igt_display_commit2(display, COMMIT_ATOMIC);
> > +     igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0xfcfc);
> > +     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_get_current(display->drm_fd, data->pipe_crc, &crc);
> > +     igt_assert_crc_equal(&ref_crc, &crc);
> >
> >       igt_pipe_crc_stop(data->pipe_crc);
> >  }
> > @@ -537,7 +535,8 @@ static void run_subtests(data_t *data, enum pipe
> > pipe)
> >       igt_subtest_f("pipe-%s-alpha-7efc", kmstest_pipe_name(pipe))
> >               run_test_on_pipe_planes(data, pipe, false, true, alpha_7efc);
> >
> > -     igt_describe("Tests pipe coverage blending properties.");
> > +     igt_describe("Uses alpha values 0x7e and 0xfc to validate fg.alpha
> > and "
> > +                  "plane_alpha are swappable on coverage blend mode.");
> >       igt_subtest_f("pipe-%s-coverage-7efc", kmstest_pipe_name(pipe))
> >               run_test_on_pipe_planes(data, pipe, true, true,
> > coverage_7efc);
> >
> > --
> > 2.35.1
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20220530/754bb14d/attachment-0001.sig>