Re: [igt-dev] [PATCH i-g-t] tests: Add kms plane alpha blending test, v2.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Oct 02, 2018 at 10:49:58AM +0200, Maarten Lankhorst wrote:
> Op 01-10-18 om 20:19 schreef Ville Syrjälä:
> > On Mon, Oct 01, 2018 at 05:48:12PM +0200, Maarten Lankhorst wrote:
> >> Add a few tests to test various blending modes.
> >>
> >> Some of the tests will skip if pixel mode alpha cannot be enabled
> >> with plane alpha at the same time. This is for mali-dp. I didn't
> >> test on that platform, but tested with the same check on i915.
> >>
> >> The tests won't pass i915 on pre-gen11 hw. i915 has small rounding
> >> errors with 0xff and 0x00 alpha, which gives CRC mismatches.
> >>
> >> Changes since v1:
> >> - Send the correct version, with the skips for mali-dp in place.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> ---
> >>  lib/igt_kms.c                 |   2 +
> >>  lib/igt_kms.h                 |   2 +
> >>  tests/Makefile.sources        |   1 +
> >>  tests/kms_plane_alpha_blend.c | 571 ++++++++++++++++++++++++++++++++++
> >>  tests/meson.build             |   1 +
> >>  5 files changed, 577 insertions(+)
> >>  create mode 100644 tests/kms_plane_alpha_blend.c
> >>
> >> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> >> index 4563bfd9d25b..e88e96149957 100644
> >> --- a/lib/igt_kms.c
> >> +++ b/lib/igt_kms.c
> >> @@ -175,6 +175,8 @@ const char * const igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> >>  	[IGT_PLANE_IN_FORMATS] = "IN_FORMATS",
> >>  	[IGT_PLANE_COLOR_ENCODING] = "COLOR_ENCODING",
> >>  	[IGT_PLANE_COLOR_RANGE] = "COLOR_RANGE",
> >> +	[IGT_PLANE_PIXEL_BLEND_MODE] = "pixel blend mode",
> >> +	[IGT_PLANE_ALPHA] = "alpha",
> >>  };
> >>  
> >>  const char * const igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
> >> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> >> index 3862efa28be0..3af336f43020 100644
> >> --- a/lib/igt_kms.h
> >> +++ b/lib/igt_kms.h
> >> @@ -266,6 +266,8 @@ enum igt_atomic_plane_properties {
> >>         IGT_PLANE_IN_FORMATS,
> >>         IGT_PLANE_COLOR_ENCODING,
> >>         IGT_PLANE_COLOR_RANGE,
> >> +       IGT_PLANE_PIXEL_BLEND_MODE,
> >> +       IGT_PLANE_ALPHA,
> >>         IGT_NUM_PLANE_PROPS
> >>  };
> >>  
> >> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> >> index 109c5397de85..e6f7a364b4b3 100644
> >> --- a/tests/Makefile.sources
> >> +++ b/tests/Makefile.sources
> >> @@ -194,6 +194,7 @@ TESTS_progs = \
> >>  	kms_pipe_b_c_ivb \
> >>  	kms_pipe_crc_basic \
> >>  	kms_plane \
> >> +	kms_plane_alpha_blend \
> >>  	kms_plane_lowres \
> >>  	kms_plane_multiple \
> >>  	kms_plane_scaling \
> >> diff --git a/tests/kms_plane_alpha_blend.c b/tests/kms_plane_alpha_blend.c
> >> new file mode 100644
> >> index 000000000000..81c8cb916a63
> >> --- /dev/null
> >> +++ b/tests/kms_plane_alpha_blend.c
> >> @@ -0,0 +1,571 @@
> >> +/*
> >> + * Copyright © 2018 Intel Corporation
> >> + *
> >> + * Permission is hereby granted, free of charge, to any person obtaining a
> >> + * copy of this software and associated documentation files (the "Software"),
> >> + * to deal in the Software without restriction, including without limitation
> >> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> >> + * and/or sell copies of the Software, and to permit persons to whom the
> >> + * Software is furnished to do so, subject to the following conditions:
> >> + *
> >> + * The above copyright notice and this permission notice (including the next
> >> + * paragraph) shall be included in all copies or substantial portions of the
> >> + * Software.
> >> + *
> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> >> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> >> + * IN THE SOFTWARE.
> >> + *
> >> + * Authors:
> >> + *   Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> + */
> >> +
> >> +#include "igt.h"
> >> +
> >> +IGT_TEST_DESCRIPTION("Test plane alpha and blending mode properties");
> >> +
> >> +typedef struct {
> >> +	int gfx_fd;
> >> +	igt_display_t display;
> >> +	struct igt_fb xrgb_fb, argb_fb_0, argb_fb_cov_0, argb_fb_7e, argb_fb_cov_7e, argb_fb_fc, argb_fb_cov_fc, argb_fb_100, black_fb, gray_fb;
> >> +	igt_crc_t ref_crc;
> >> +	igt_pipe_crc_t *pipe_crc;
> >> +} data_t;
> >> +
> >> +static void __draw_gradient(struct igt_fb *fb, int w, int h, double a, cairo_t *cr)
> >> +{
> >> +	cairo_pattern_t *pat;
> >> +
> >> +	pat = cairo_pattern_create_linear(0, 0, w, h);
> >> +	cairo_pattern_add_color_stop_rgba(pat, 0.00, 0.00, 0.00, 0.00, 1.);
> >> +	cairo_pattern_add_color_stop_rgba(pat, 0.25, 1.00, 1.00, 0.00, 1.);
> >> +	cairo_pattern_add_color_stop_rgba(pat, 0.50, 0.00, 1.00, 1.00, 1.);
> >> +	cairo_pattern_add_color_stop_rgba(pat, 0.75, 1.00, 0.00, 1.00, 1.);
> >> +	cairo_pattern_add_color_stop_rgba(pat, 1.00, 1.00, 1.00, 1.00, 1.);
> >> +
> >> +	cairo_rectangle(cr, 0, 0, w, h);
> >> +	cairo_set_source(cr, pat);
> >> +	cairo_set_operator(cr, CAIRO_OPERATOR_SOURCE);
> >> +	cairo_paint_with_alpha(cr, a);
> >> +	cairo_pattern_destroy(pat);
> >> +}
> >> +
> >> +static void draw_gradient(struct igt_fb *fb, int w, int h, double a)
> >> +{
> >> +	cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
> >> +
> >> +	__draw_gradient(fb, w, h, a, cr);
> >> +
> >> +	igt_put_cairo_ctx(fb->fd, fb, cr);
> >> +}
> >> +
> >> +static void draw_gradient_coverage(struct igt_fb *fb, int w, int h, uint8_t a)
> >> +{
> >> +	cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
> >> +	uint8_t *data = cairo_image_surface_get_data(fb->cairo_surface);
> >> +	uint32_t stride = fb->strides[0];
> >> +	int i;
> >> +
> >> +	__draw_gradient(fb, w, h, 1., cr);
> >> +
> >> +	for (; h--; data += stride)
> >> +		for (i = 0; i < w; i++)
> >> +			data[i * 4 + 3] = a;
> > Hrm. I guess no way to make cairo paint non-premultiplied. It's also
> > interesting that when you specify a source/pattern color you apparently
> > give in non-premultiplied form even though surface source would be in
> > premultiplied form. Cairo seems a bit inconsistent here.
> >
> >> +
> >> +	igt_put_cairo_ctx(fb->fd, fb, cr);
> >> +}
> >> +
> >> +static void draw_squares(struct igt_fb *fb, int w, int h, double a)
> >> +{
> >> +	cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
> >> +
> >> +	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);
> >> +	igt_paint_color_alpha(cr, w / 2, h / 2, w / 4, h / 2, 1., 1., 1., a);
> >> +	igt_paint_color_alpha(cr, 3 * w / 4, h / 2, w / 4, h / 2, 0., 0., 0., a);
> >> +
> >> +	igt_put_cairo_ctx(fb->fd, fb, cr);
> >> +}
> >> +
> >> +static void draw_squares_coverage(struct igt_fb *fb, int w, int h, uint8_t as)
> >> +{
> >> +	cairo_t *cr = igt_get_cairo_ctx(fb->fd, fb);
> >> +	int i, j;
> >> +	uint32_t *data = (void *)cairo_image_surface_get_data(fb->cairo_surface);
> >> +	uint32_t stride = fb->strides[0] / 4;
> >> +	uint32_t a = as << 24;
> >> +
> >> +	for (j = 0; j < h / 2; j++) {
> >> +		for (i = 0; i < w / 2; i++)
> >> +			data[j * stride + i] = a | 0xff0000;
> >> +
> >> +		for (; i < w; i++)
> >> +			data[j * stride + i] = a | 0xff00;
> >> +	}
> >> +
> >> +	for (j = h / 2; j < h; j++) {
> >> +		for (i = 0; i < w / 2; i++)
> >> +			data[j * stride + i] = a | 0xff;
> >> +
> >> +		for (; i < 3 * w / 4; i++)
> >> +			data[j * stride + i] = a | 0xffffff;
> >> +
> >> +		for (; i < w; i++)
> >> +			data[j * stride + i] = a;
> >> +	}
> >> +
> >> +	igt_put_cairo_ctx(fb->fd, fb, cr);
> >> +}
> >> +
> >> +static void reset_alpha(igt_display_t *display, enum pipe pipe)
> >> +{
> >> +	igt_plane_t *plane;
> >> +
> >> +	for_each_plane_on_pipe(display, pipe, plane) {
> >> +		if (igt_plane_has_prop(plane, IGT_PLANE_ALPHA))
> >> +			igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0xffff);
> >> +
> >> +		if (igt_plane_has_prop(plane, IGT_PLANE_PIXEL_BLEND_MODE))
> >> +			igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_BLEND_MODE, "Pre-multiplied");
> >> +	}
> >> +}
> >> +
> >> +static bool has_multiplied_alpha(data_t *data, igt_plane_t *plane)
> > What's this "multiplied alpha"? Is that the constant alpha prop?
> > Why can't we just look for the prop?
> >
> >> +{
> >> +	int ret;
> >> +
> >> +	igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0x8080);
> >> +	igt_plane_set_fb(plane, &data->argb_fb_100);
> >> +	ret = igt_display_try_commit_atomic(&data->display,
> >> +		DRM_MODE_ATOMIC_TEST_ONLY | DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> >> +	igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0xffff);
> >> +	igt_plane_set_fb(plane, NULL);
> >> +
> >> +	return ret == 0;
> >> +}
> >> +
> >> +static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe)
> >> +{
> >> +	drmModeModeInfo *mode;
> >> +	igt_display_t *display = &data->display;
> >> +	int w, h;
> >> +	igt_plane_t *primary = igt_pipe_get_plane_type(&display->pipes[pipe], DRM_PLANE_TYPE_PRIMARY);
> >> +
> >> +	igt_display_reset(display);
> >> +	igt_output_set_pipe(output, pipe);
> >> +
> >> +	/* create the pipe_crc object for this pipe */
> >> +	igt_pipe_crc_free(data->pipe_crc);
> >> +	data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
> >> +
> >> +	mode = igt_output_get_mode(output);
> >> +	w = mode->hdisplay;
> >> +	h = mode->vdisplay;
> >> +
> >> +	/* recreate all fbs if incompatible */
> >> +	if (data->xrgb_fb.width != w || data->xrgb_fb.height != h) {
> >> +		cairo_t *cr;
> >> +
> >> +		igt_remove_fb(data->gfx_fd, &data->xrgb_fb);
> >> +		igt_remove_fb(data->gfx_fd, &data->argb_fb_0);
> >> +		igt_remove_fb(data->gfx_fd, &data->argb_fb_cov_0);
> >> +		igt_remove_fb(data->gfx_fd, &data->argb_fb_7e);
> >> +		igt_remove_fb(data->gfx_fd, &data->argb_fb_fc);
> >> +		igt_remove_fb(data->gfx_fd, &data->argb_fb_cov_7e);
> >> +		igt_remove_fb(data->gfx_fd, &data->argb_fb_cov_fc);
> >> +		igt_remove_fb(data->gfx_fd, &data->argb_fb_100);
> >> +		igt_remove_fb(data->gfx_fd, &data->black_fb);
> >> +		igt_remove_fb(data->gfx_fd, &data->gray_fb);
> >> +
> >> +		igt_create_fb(data->gfx_fd, w, h,
> >> +			      DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> >> +			      &data->xrgb_fb);
> >> +		draw_gradient(&data->xrgb_fb, w, h, 1.);
> >> +
> >> +		igt_create_fb(data->gfx_fd, w, h,
> >> +			      DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> >> +			      &data->argb_fb_cov_0);
> >> +		draw_gradient_coverage(&data->argb_fb_cov_0, w, h, 0);
> >> +
> >> +		igt_create_fb(data->gfx_fd, w, h,
> >> +			      DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> >> +			      &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);
> >> +		igt_put_cairo_ctx(data->gfx_fd, &data->argb_fb_0, cr);
> >> +
> >> +		igt_create_fb(data->gfx_fd, w, h,
> >> +			      DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> >> +			      &data->argb_fb_7e);
> >> +		draw_squares(&data->argb_fb_7e, w, h, 126. / 255.);
> >> +
> >> +		igt_create_fb(data->gfx_fd, w, h,
> >> +			      DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> >> +			      &data->argb_fb_cov_7e);
> >> +		draw_squares_coverage(&data->argb_fb_cov_7e, w, h, 0x7e);
> >> +
> >> +		igt_create_fb(data->gfx_fd, w, h,
> >> +			      DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> >> +			      &data->argb_fb_fc);
> >> +		draw_squares(&data->argb_fb_fc, w, h, 252. / 255.);
> >> +
> >> +		igt_create_fb(data->gfx_fd, w, h,
> >> +			      DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> >> +			      &data->argb_fb_cov_fc);
> >> +		draw_squares_coverage(&data->argb_fb_cov_fc, w, h, 0xfc);
> >> +
> >> +		igt_create_fb(data->gfx_fd, w, h,
> >> +			      DRM_FORMAT_ARGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> >> +			      &data->argb_fb_100);
> >> +		draw_gradient(&data->argb_fb_100, w, h, 1.);
> >> +
> >> +		igt_create_fb(data->gfx_fd, w, h,
> >> +			      DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> >> +			      &data->black_fb);
> >> +
> >> +		igt_create_color_fb(data->gfx_fd, w, h,
> >> +				    DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> >> +				    .5, .5, .5, &data->gray_fb);
> >> +	}
> >> +
> >> +	igt_plane_set_fb(primary, &data->black_fb);
> >> +	/* reset alpha property to default */
> >> +	reset_alpha(display, pipe);
> >> +}
> >> +
> >> +static void basic_alpha(data_t *data, enum pipe pipe, igt_plane_t *plane)
> >> +{
> >> +	igt_display_t *display = &data->display;
> >> +	igt_crc_t ref_crc, crc;
> >> +	int i;
> >> +
> >> +	/* Testcase 1: alpha = 0.0, plane should be transparant. */
> >> +	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_fb(plane, &data->argb_fb_0);
> >> +
> >> +	/* transparant fb should be transparant, no matter what.. */
> >> +	for (i = 7; i < 256; i += 8) {
> >> +		igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, i | (i << 8));
> >> +		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);
> >> +	}
> >> +
> >> +	/* And test alpha = 0, should give same CRC, but doesn't on some i915 platforms. */
> >> +	igt_plane_set_prop_value(plane, IGT_PLANE_ALPHA, 0);
> >> +	igt_display_commit2(display, COMMIT_ATOMIC);
> >> +
> >> +	igt_pipe_crc_get_current(display->drm_fd, data->pipe_crc, &crc);
> >> +	igt_pipe_crc_stop(data->pipe_crc);
> >> +	igt_assert_crc_equal(&ref_crc, &crc);
> >> +}
> >> +
> >> +static void argb_opaque(data_t *data, enum pipe pipe, igt_plane_t *plane)
> >> +{
> >> +	igt_display_t *display = &data->display;
> >> +	igt_crc_t ref_crc, crc;
> >> +
> >> +	/* alpha = 1.0, plane should be fully opaque, test with an opaque fb */
> >> +	igt_plane_set_fb(plane, &data->xrgb_fb);
> >> +	igt_display_commit2(display, COMMIT_ATOMIC);
> >> +	igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc);
> > Could probably speed up the test quite a bit with crc_get_current()?
> >
> >> +
> >> +	igt_plane_set_fb(plane, &data->argb_fb_100);
> >> +	igt_display_commit2(display, COMMIT_ATOMIC);
> >> +	igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
> >> +
> >> +	igt_assert_crc_equal(&ref_crc, &crc);
> >> +}
> >> +
> >> +static void argb_transparant(data_t *data, enum pipe pipe, igt_plane_t *plane)
> >> +{
> >> +	igt_display_t *display = &data->display;
> >> +	igt_crc_t ref_crc, crc;
> >> +
> >> +	/* alpha = 1.0, plane should be fully opaque, test with a transparant fb */
> >> +	igt_plane_set_fb(plane, NULL);
> >> +	igt_display_commit2(display, COMMIT_ATOMIC);
> >> +	igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc);
> >> +
> >> +	igt_plane_set_fb(plane, &data->argb_fb_0);
> >> +	igt_display_commit2(display, COMMIT_ATOMIC);
> >> +	igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
> > Might make it a bit easier to parse these if you didn't rely on the
> > default state of the props and instead just set them explicitly.
> >
> > In general this a bit repetitive. I wonder if some kind of declarative
> > approach might increase the signal to noise ratio a bit.
> >
> > To me it looks like the weak point of this test is that it mostly relies
> > on comparins the different blend modes with each other. I guess there
> > are enough ways to combine those to get decent assurances that they
> > aren't just misbehaving in ways that make the crcs match by accident.
> > Still, comparing against software rendered results would probably be
> > how I'd have tried to approach this. That would also eliminate the
> > dependency on having all the blend modes supported in the hardware.
> >
> > Looks decent enough anyway, so
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> This is deliberate. HW might round in different ways then sw. So to compare hw rendering against sw,
> would assume a specific way of rounding.

I would expect we should be able to get close enough with certain alpha
values. And there's always the "let's throw away some low bits with the
lut" trick.

> Some tests compare preblended 0xfc vs 0x7e. This way at least we know hw will probably round in the
> same way when sw does things in a certain way. Even if we don't know the exact rounding.
> Because 0xfc * 0x7e should be same as 0x7e * 0xfc.

Assuming the hw expands the per-pixel and constant alphas the same way.
Not sure I'd want to take that bet.

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