[PATCH i-g-t v2 5/6] tests/kms_atomic: Add solid fill plane subtest
Pekka Paalanen
pekka.paalanen at collabora.com
Wed Jan 24 09:13:25 UTC 2024
On Tue, 23 Jan 2024 15:28:58 -0800
Jessica Zhang <quic_jesszhan at quicinc.com> wrote:
> Add a basic test for solid fill planes.
>
> This test will first commit a single-color framebuffer plane then
> a solid fill plane with the same contents. It then validates the solid
> fill plane by comparing the resulting CRC with the CRC of the reference
> framebuffer commit.
>
> Signed-off-by: Jessica Zhang <quic_jesszhan at quicinc.com>
> ---
> tests/kms_atomic.c | 179 +++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 161 insertions(+), 18 deletions(-)
>
> diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
> old mode 100644
> new mode 100755
> index 2b6e9a8f0383..35ac5f198524
> --- a/tests/kms_atomic.c
> +++ b/tests/kms_atomic.c
> @@ -43,6 +43,7 @@
> #include <stdint.h>
> #include <string.h>
> #include <errno.h>
> +#include <math.h>
> #include <xf86drmMode.h>
> #include <cairo.h>
> #include "drm.h"
> @@ -101,6 +102,15 @@
> * @params-fence: fence parameters
> */
>
> +/**
> + * SUBTEST: plane-%s-solid-fill
> + * Description: Test atomic modesetting on %arg[1] using solid fill property.
> + *
> + * arg[1]:
> + *
> + * @primary: Primary plane
> + */
> +
> #ifndef DRM_CAP_CURSOR_WIDTH
> #define DRM_CAP_CURSOR_WIDTH 0x8
> #endif
> @@ -122,18 +132,45 @@ typedef struct {
> igt_fb_t fb;
> } data_t;
>
> +static struct {
> + uint32_t r, g, b;
> +} colors[8] = {
> + /* Basic Colors */
> + { .r = 0x00000000, .g = 0x00000000, .b = 0x00000000 },
> + { .r = 0xffffffff, .g = 0x00000000, .b = 0x00000000 },
> + { .r = 0x00000000, .g = 0xffffffff, .b = 0x00000000 },
> + { .r = 0x00000000, .g = 0x00000000, .b = 0xffffffff },
> + { .r = 0xffffffff, .g = 0xffffffff, .b = 0xffffffff },
> +
> + /* Mid-grays */
> + { .r = 0x40000000, .g = 0x40000000, .b = 0x40000000 },
> + { .r = 0x80000000, .g = 0x80000000, .b = 0x80000000 },
> + { .r = 0xcccccccc, .g = 0xcccccccc, .b = 0xcccccccc },
> +};
Hi,
the above looks good to me. More below.
> +
> enum kms_atomic_check_relax {
> ATOMIC_RELAX_NONE = 0,
> CRTC_RELAX_MODE = (1 << 0),
> PLANE_RELAX_FB = (1 << 1)
> };
>
> -static inline int damage_rect_width(struct drm_mode_rect *r)
> +/*
> + * Fixed point uint to floating point conversion based on
> + * Vulkan's UNORM conversion:
> + *
> + * https://registry.khronos.org/vulkan/specs/1.3/html/chap3.html#fundamentals-fixedconv
> + */
> +static inline double unorm_to_float(uint32_t x)
> +{
> + return x / (pow(2, 32) - 1);
> +}
> +
> +static inline int rect_width(struct drm_mode_rect *r)
> {
> return r->x2 - r->x1;
> }
>
> -static inline int damage_rect_height(struct drm_mode_rect *r)
> +static inline int rect_height(struct drm_mode_rect *r)
> {
> return r->y2 - r->y1;
> }
> @@ -1156,8 +1193,8 @@ static void atomic_plane_damage(data_t *data)
>
> cr_1 = igt_get_cairo_ctx(data->drm_fd, &fb_1);
> igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
> - damage_rect_width(&damage[0]),
> - damage_rect_height(&damage[0]), 1.0, 0, 0);
> + rect_width(&damage[0]),
> + rect_height(&damage[0]), 1.0, 0, 0);
> igt_put_cairo_ctx(cr_1);
>
> igt_plane_set_fb(data->primary, &fb_1);
> @@ -1183,8 +1220,8 @@ static void atomic_plane_damage(data_t *data)
> cairo_set_source_surface(cr_2, fb_1.cairo_surface, 0, 0);
> cairo_paint(cr_2);
> igt_paint_color(cr_2, damage[0].x1, damage[0].y1,
> - damage_rect_width(&damage[0]),
> - damage_rect_height(&damage[0]), 0, 1.0, 0);
> + rect_width(&damage[0]),
> + rect_height(&damage[0]), 0, 1.0, 0);
> igt_put_cairo_ctx(cr_1);
> igt_put_cairo_ctx(cr_2);
> igt_plane_set_fb(data->primary, &fb_2);
> @@ -1213,8 +1250,8 @@ static void atomic_plane_damage(data_t *data)
> cairo_set_source_surface(cr_1, fb_2.cairo_surface, 0, 0);
> cairo_paint(cr_1);
> igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
> - damage_rect_width(&damage[0]),
> - damage_rect_height(&damage[0]), 0, 1.0, 0);
> + rect_width(&damage[0]),
> + rect_height(&damage[0]), 0, 1.0, 0);
> igt_put_cairo_ctx(cr_2);
> igt_put_cairo_ctx(cr_1);
> igt_plane_set_fb(data->primary, &fb_1);
> @@ -1238,8 +1275,8 @@ static void atomic_plane_damage(data_t *data)
>
> cr_1 = igt_get_cairo_ctx(data->drm_fd, &fb_1);
> igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
> - damage_rect_width(&damage[0]),
> - damage_rect_height(&damage[0]), 1.0, 1.0, 0);
> + rect_width(&damage[0]),
> + rect_height(&damage[0]), 1.0, 1.0, 0);
> igt_put_cairo_ctx(cr_1);
> igt_plane_set_fb(data->primary, &fb_1);
> igt_plane_set_size(data->primary, data->fb.width, data->fb.height);
> @@ -1266,11 +1303,11 @@ static void atomic_plane_damage(data_t *data)
>
> cr_1 = igt_get_cairo_ctx(data->drm_fd, &fb_1);
> igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
> - damage_rect_width(&damage[0]),
> - damage_rect_height(&damage[0]), 0, 1.0, 1.0);
> + rect_width(&damage[0]),
> + rect_height(&damage[0]), 0, 1.0, 1.0);
> igt_paint_color(cr_1, damage[1].x1, damage[1].y1,
> - damage_rect_width(&damage[1]),
> - damage_rect_height(&damage[1]), 0, 1.0, 0);
> + rect_width(&damage[1]),
> + rect_height(&damage[1]), 0, 1.0, 0);
> igt_put_cairo_ctx(cr_1);
> igt_plane_set_fb(data->primary, &fb_1);
> igt_plane_set_size(data->primary, data->fb.width, data->fb.height);
> @@ -1299,11 +1336,11 @@ static void atomic_plane_damage(data_t *data)
>
> cr_1 = igt_get_cairo_ctx(data->drm_fd, &fb_1);
> igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
> - damage_rect_width(&damage[0]),
> - damage_rect_height(&damage[0]), 1.0, 0, 0);
> + rect_width(&damage[0]),
> + rect_height(&damage[0]), 1.0, 0, 0);
> igt_paint_color(cr_1, damage[1].x1, damage[1].y1,
> - damage_rect_width(&damage[1]),
> - damage_rect_height(&damage[1]), 1.0, 1.0, 1.0);
> + rect_width(&damage[1]),
> + rect_height(&damage[1]), 1.0, 1.0, 1.0);
> igt_put_cairo_ctx(cr_1);
> igt_plane_set_fb(data->primary, &fb_1);
> igt_plane_set_size(data->primary, data->fb.width, data->fb.height);
> @@ -1322,6 +1359,98 @@ static void atomic_plane_damage(data_t *data)
> igt_remove_fb(data->drm_fd, &fb_2);
> }
>
> +static void test_solid_fill_plane(data_t *data, igt_output_t *output, igt_plane_t *plane)
> +{
> + drmModeModeInfo *mode = igt_output_get_mode(output);
> + struct drm_mode_rect rect = { 0 };
> + struct igt_fb ref_fb;
> + igt_pipe_crc_t *pipe_crc;
> + igt_crc_t ref_crc, new_crc;
> + enum pipe pipe = data->pipe->pipe;
> + int height, width, i;
> + int ret;
> +
> + igt_require(igt_plane_has_prop(plane, IGT_PLANE_SOLID_FILL));
> + igt_require(igt_plane_has_prop(plane, IGT_PLANE_PIXEL_SOURCE));
> +
> + rect.x1 = 0;
> + rect.x2 = mode->hdisplay;
> + rect.y1 = 0;
> + rect.y2 = mode->vdisplay;
> +
> + width = rect_width(&rect);
> + height = rect_height(&rect);
> +
> + igt_plane_set_position(plane, rect.x1, rect.y1);
> + pipe_crc = igt_pipe_crc_new(data->drm_fd, pipe,
> + IGT_PIPE_CRC_SOURCE_AUTO);
> +
> +
> + for (i = 0; i < ARRAY_SIZE(colors); i++) {
> + uint32_t r = colors[i].r;
> + uint32_t g = colors[i].g;
> + uint32_t b = colors[i].b;
> +
> + struct drm_mode_create_blob c;
> + struct drm_mode_destroy_blob d;
> + struct drm_mode_solid_fill blob_data = {
> + .r = r,
> + .g = g,
> + .b = b,
> + .pad = 0x0,
> + };
> +
> + c.data = (uintptr_t) &blob_data;
> + c.length = sizeof(blob_data);
> +
> + igt_debug("Testing color r: 0x%x (%f), g: 0x%x (%f), b: 0x%x (%f)\n",
> + r, unorm_to_float(r),
> + g, unorm_to_float(g),
> + b, unorm_to_float(b));
> +
> + /* get reference CRC */
> + igt_create_color_fb(data->drm_fd, width, height,
> + DRM_FORMAT_ABGR8888, DRM_FORMAT_MOD_LINEAR,
> + unorm_to_float(r),
> + unorm_to_float(g),
> + unorm_to_float(b),
I'm slightly wary of trusting a third party library (Cairo? Pixman?)
with the float->u8 conversion. If both Cairo and the kernel do the same
mistake in conversion, the test would pass. Therefore, this is actually
testing that the kernel follows Cairo's conversion, and not the UAPI
definition, should the unthinkable happen that they disagreed.
Would it at least be possible to verify the FB value against what we
expect here?
> + &ref_fb);
> + igt_plane_set_fb(plane, &ref_fb);
> + igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +
> + igt_pipe_crc_start(pipe_crc);
> + igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &ref_crc);
> +
> + ret = drmIoctl(data->display.drm_fd, DRM_IOCTL_MODE_CREATEPROPBLOB, &c);
> + igt_assert(ret == 0);
> +
> + /* test solid fill plane */
> + igt_plane_set_solid_fill(plane, &rect, c.blob_id);
> + igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_SOURCE, "SOLID_FILL");
> +
> + igt_display_commit_atomic(&data->display,
> + DRM_MODE_ATOMIC_ALLOW_MODESET,
> + NULL);
> +
> + igt_pipe_crc_get_current(data->drm_fd, pipe_crc, &new_crc);
> +
> + igt_assert_crc_equal(&ref_crc, &new_crc);
Looking good.
> +
> + /* Switch back to FB pixel source */
> + igt_plane_set_prop_value(plane, IGT_PLANE_SOLID_FILL, 0);
> + igt_plane_set_prop_enum(plane, IGT_PLANE_PIXEL_SOURCE, "FB");
> + igt_display_commit2(&data->display, COMMIT_ATOMIC);
Just wondering, why set this at the end and not when you are actually
setting up the FB? Doesn't IGT sanitize KMS state before tests that
don't explicitly test boot-up state?
Thanks,
pq
> +
> + d.blob_id = c.blob_id;
> + ret = drmIoctl(data->drm_fd, DRM_IOCTL_MODE_DESTROYPROPBLOB, &d);
> + igt_assert(ret == 0);
> + }
> +
> + igt_pipe_crc_stop(pipe_crc);
> + igt_pipe_crc_free(pipe_crc);
> + igt_remove_fb(data->drm_fd, &ref_fb);
> +}
> +
> static void atomic_setup(data_t *data, enum pipe pipe, igt_output_t *output)
> {
> drmModeModeInfo *mode;
> @@ -1634,6 +1763,20 @@ igt_main_args("e", NULL, help_str, opt_handler, NULL)
> }
> }
>
> + igt_describe("Test case for solid fill primary planes");
> + igt_subtest_with_dynamic("plane-primary-solid-fill") {
> + for_each_pipe_with_single_output(&data.display, pipe, output) {
> + if (!pipe_output_combo_valid(&data.display, pipe, output))
> + continue;
> +
> + igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), igt_output_name(output)) {
> + atomic_setup(&data, pipe, output);
> + test_solid_fill_plane(&data, output, data.primary);
> + atomic_clear(&data, pipe, output);
> + }
> + }
> + }
> +
> igt_fixture {
> igt_display_fini(&data.display);
> drm_close_driver(data.drm_fd);
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20240124/268ba972/attachment-0001.sig>
More information about the igt-dev
mailing list