[PATCH i-g-t] tests/kms_cursor_crc: refactoring cursor-alpha subtests

[Melissa Wen]

Considering just a fully opaque or fully transparent cursor is not enough
to check the composition of an ARGB cursor plane. For example, the cairo
ARGB32 format uses pre-multiplied alpha, and this representation may only
be evident when testing a translucent cursor.

Therefore, this patch refactors the cursor-alpha-opaque and
cursor-alpha-transparent subtests into just one subtest (cursor-alpha)
that checks the alpha blending of a white cursor, with different alpha
values, in the primary plane. This refactoring also generates some setup
stuffs savings.

Signed-off-by: Melissa Wen <melis...@gmail.com>
---
tests/kms_cursor_crc.c | 82 ++++++++++++++++++++----------------------
1 file changed, 39 insertions(+), 43 deletions(-)

diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index e9491847..e200b0b5 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -478,65 +478,66 @@ static void prepare_crtc(data_t *data, igt_output_t *output,
igt_pipe_crc_start(data->pipe_crc);
}

-static void test_cursor_alpha(data_t *data, double a)
+static void test_cursor_alpha(data_t *data)
{
igt_display_t *display = &data->display;
igt_pipe_crc_t *pipe_crc = data->pipe_crc;
- igt_crc_t crc, ref_crc;
+ igt_crc_t crc[5], ref_crc;
cairo_t *cr;
uint32_t fb_id;
int curw = data->curw;
int curh = data->curh;
+ int i;
+ double alpha;

- /* Alpha cursor fb with white color */
+ /* Alpha cursor fb */
fb_id = igt_create_fb(data->drm_fd, curw, curh,
DRM_FORMAT_ARGB8888,
LOCAL_DRM_FORMAT_MOD_NONE,
&data->fb);
igt_assert(fb_id);
+
+ /*
+ * Hardware Test
+ * With the cursor enabled, get the PF CRCs from the composition with a
+ * white cursor with different alpha values.
+ */
cr = igt_get_cairo_ctx(data->drm_fd, &data->fb);
- igt_paint_color_alpha(cr, 0, 0, curw, curh, 1.0, 1.0, 1.0, a);
- igt_put_cairo_ctx(cr);
+ cairo_set_operator (cr, CAIRO_OPERATOR_SOURCE);

- /* Hardware Test - enable cursor and get PF CRC */
cursor_enable(data);
- igt_display_commit(display);
- igt_wait_for_vblank(data->drm_fd,
- display->pipes[data->pipe].crtc_offset);
- igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &crc);
+ for (i = 0, alpha = 1.0; alpha >= 0.0; alpha -= 0.25, i++) {
+ igt_paint_color_alpha(cr, 0, 0, curw, curh, 1.0, 1.0, 1.0, alpha);
+
+ igt_display_commit(display);
+ igt_wait_for_vblank(data->drm_fd,
+ display->pipes[data->pipe].crtc_offset);
+ igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &crc[i]);
+ }
+ igt_put_cairo_ctx(cr);

cursor_disable(data);
igt_remove_fb(data->drm_fd, &data->fb);

- /* Software Test - render cursor in software, drawn it directly on PF */
+ /* Software Test - render cursor in software, drawn it directly on PF*/
cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb[FRONTBUFFER]);
- igt_paint_color_alpha(cr, 0, 0, curw, curh, 1.0, 1.0, 1.0, a);
- igt_put_cairo_ctx(cr);

- igt_display_commit(display);
- igt_wait_for_vblank(data->drm_fd,
- display->pipes[data->pipe].crtc_offset);
- igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &ref_crc);
+ for (i = 0, alpha = 1.0; alpha >= 0.0; alpha -= 0.25, i++) {
+ igt_paint_color_alpha(cr, 0, 0, curw, curh, 1.0, 1.0, 1.0, alpha);

- /* Compare CRC from Hardware/Software tests */
- igt_assert_crc_equal(&crc, &ref_crc);
-
- /*Clear Screen*/
- cr = igt_get_cairo_ctx(data->drm_fd, &data->primary_fb[FRONTBUFFER]);
- igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
- 0.0, 0.0, 0.0);
- igt_put_cairo_ctx(cr);
-}
-
-static void test_cursor_transparent(data_t *data)
-{
- test_cursor_alpha(data, 0.0);
+ igt_display_commit(display);
+ igt_wait_for_vblank(data->drm_fd,
+ display->pipes[data->pipe].crtc_offset);
+ igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &ref_crc);

-}
+ /* Compare CRC from Hardware/Software tests */
+ igt_assert_crc_equal(&crc[i], &ref_crc);

-static void test_cursor_opaque(data_t *data)
-{
- test_cursor_alpha(data, 1.0);
+ /*Clear Screen*/
+ igt_paint_color(cr, 0, 0, data->screenw, data->screenh,
+ 0.0, 0.0, 0.0);
+ }
+ igt_put_cairo_ctx(cr);
}

static void run_test(data_t *data, void (*testfunc)(data_t *), int cursor_w, int cursor_h)
@@ -705,15 +706,10 @@ static void run_tests_on_pipe(data_t *data, enum pipe pipe)
run_test(data, test_cursor_size,
data->cursor_max_w, data->cursor_max_h);

- igt_describe("Validates the composition of a fully opaque cursor "
- "plane, i.e., alpha channel equal to 1.0.");
- igt_subtest_f("pipe-%s-cursor-alpha-opaque", kmstest_pipe_name(pipe))
- run_test(data, test_cursor_opaque, data->cursor_max_w, data->cursor_max_h);
-
- igt_describe("Validates the composition of a fully transparent cursor "
- "plane, i.e., alpha channel equal to 0.0.");
- igt_subtest_f("pipe-%s-cursor-alpha-transparent", kmstest_pipe_name(pipe))
- run_test(data, test_cursor_transparent, data->cursor_max_w, data->cursor_max_h);
+ igt_describe("Validates the composition of alpha cursor plane "
+ "from fully opaque to fully transparent alpha range");
+ igt_subtest_f("pipe-%s-cursor-alpha", kmstest_pipe_name(pipe))
+ run_test(data, test_cursor_alpha, data->cursor_max_w, data->cursor_max_h);

igt_fixture
create_cursor_fb(data, data->cursor_max_w, data->cursor_max_h);
--
2.28.0

[Arkadiusz Hiler]

Hey,

Sorry it took so long for me to review this. I was taking a bit of a
break from DRM work.

The change is sound, checking multiple alpha levels makes sense, and
looks like the test mostly passes on real HW - I was a bit afraid that
there may be some discrepancies between SW and HW implementation of
alpha blending.

On Intel's CI side - GLK is misbehaving a bit:
https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4907/shards-all.html?testfilter=cursor-alpha

But that looks like it's a HW problem. It's for alpha=0.5, which wasn't
tested before and other generations of hardware are passing the test just
fine.

It would be nice to improve logging in the test a bit:
* don't fail the test early, always go through all of the alpha levels
* clearly state for which alpha levels we have failed, right now you
I have to count the number of "commits" in the debug logs

We've been doing similar improvements in other tests.

--
Cheers,
Arek

[Melissa Wen]

Hi Arek,

Thanks for the review.

Sorry for the delay, I left the context a little after GSoC and only now
I managed to wrap-up it.

I'm not sure if I got it right, when possible, could you give me
feedback on the v2 that I just sent?

Best regards,

Melissa