[PATCH i-g-t v2 5/6] tests/kms_atomic: Add solid fill plane subtest
Jessica Zhang
quic_jesszhan at quicinc.com
Fri Feb 2 18:14:18 UTC 2024
On 1/24/2024 1:13 AM, Pekka Paalanen wrote:
> 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?
Hi Pekka,
Unfortunately, I don't think there's a way that will let us to do that
without writeback.
>
>> + &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?
Mostly for efficiency -- I can move the set(PIXEL_SOURCE, "FB") to when
the reference FB is created, but PIXEL_SOURCE still needs to be set back
to "FB" before the rmfb is called at the very end.
I can keep the set(PIXEL_SOURCE, "FB") here and move the set(SOLID_FILL,
0) and commit to outside the loop before the remove_fb(). That way, we
don't have unnecessary commits between test cases.
Thanks,
Jessica Zhang
>
>
> 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);
>>
>
More information about the igt-dev
mailing list