* [igt-dev] [PATCH i-g-t v3] tests/kms_crtc_background_color: overhaul for latest ABI proposal (v3)
[not found] <20181115221346.27335-1-matthew.d.roper@intel.com>
@ 2018-11-15 22:15 ` Matt Roper
2018-11-15 23:03 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2018-11-16 1:40 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2 siblings, 0 replies; 3+ messages in thread
From: Matt Roper @ 2018-11-15 22:15 UTC (permalink / raw)
To: intel-gfx; +Cc: igt-dev
CRTC background color kernel patches were written about 2.5 years ago
and floated on the upstream mailing list, but since no opensource
userspace materialized, we never actually merged them. However the
corresponding IGT test did get merged and has basically been dead code
ever since.
A couple years later we may finally be getting closer to landing the
kernel patches (there's some interest in this functionality now from
both the ChromeOS and Weston camps), so lets update the IGT test to
match the latest proposed ABI, and to remove some of the cruft from the
original test that wouldn't actually work.
It's worth noting that we don't seem to be able to test this feature
with CRC's. Originally we wanted to draw a color into a plane's FB
(with Cairo) and then compare the CRC to turning off all planes and just
setting the CRTC background to the same color. However the precision
and rounding of the color components causes the CRC's to come out
differently, even though the end result is visually identical. So at
the moment this test is mainly useful for visual inspection in
interactive mode.
v2:
- Swap red and blue ordering in property value to reflect change
in v2 of kernel series.
v3:
- Minor updates to proposed uapi helpers (s/rgba/argb/).
Cc: igt-dev@lists.freedesktop.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
lib/igt_kms.c | 2 +-
tests/kms_crtc_background_color.c | 223 +++++++++++++++++++++-----------------
2 files changed, 122 insertions(+), 103 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index d806ccc1..33d6a6fb 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -180,7 +180,7 @@ const char * const igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
};
const char * const igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
- [IGT_CRTC_BACKGROUND] = "background_color",
+ [IGT_CRTC_BACKGROUND] = "BACKGROUND_COLOR",
[IGT_CRTC_CTM] = "CTM",
[IGT_CRTC_GAMMA_LUT] = "GAMMA_LUT",
[IGT_CRTC_GAMMA_LUT_SIZE] = "GAMMA_LUT_SIZE",
diff --git a/tests/kms_crtc_background_color.c b/tests/kms_crtc_background_color.c
index 3df3401f..a9d99864 100644
--- a/tests/kms_crtc_background_color.c
+++ b/tests/kms_crtc_background_color.c
@@ -25,164 +25,183 @@
#include "igt.h"
#include <math.h>
-
IGT_TEST_DESCRIPTION("Test crtc background color feature");
+/*
+ * The original idea was to paint a desired color into a full-screen primary
+ * plane and then compare that CRC with turning off all planes and setting the
+ * CRTC background to the same color. Unforunately, the rounding and precision
+ * of color values as rendered by cairo vs created by the display controller
+ * are slightly different and give different CRC's, even though they're
+ * visually identical.
+ *
+ * Since we can't really use CRC's for testing, this test is mainly useful for
+ * visual inspection in interactive mode at the moment.
+ */
+
typedef struct {
int gfx_fd;
- igt_display_t display;
- struct igt_fb fb;
- igt_crc_t ref_crc;
- igt_pipe_crc_t *pipe_crc;
+ igt_output_t *output;
+ drmModeModeInfo *mode;
} data_t;
-#define BLACK 0x000000 /* BGR 8bpc */
-#define CYAN 0xFFFF00 /* BGR 8bpc */
-#define PURPLE 0xFF00FF /* BGR 8bpc */
-#define WHITE 0xFFFFFF /* BGR 8bpc */
-
-#define BLACK64 0x000000000000 /* BGR 16bpc */
-#define CYAN64 0xFFFFFFFF0000 /* BGR 16bpc */
-#define PURPLE64 0xFFFF0000FFFF /* BGR 16bpc */
-#define YELLOW64 0x0000FFFFFFFF /* BGR 16bpc */
-#define WHITE64 0xFFFFFFFFFFFF /* BGR 16bpc */
-#define RED64 0x00000000FFFF /* BGR 16bpc */
-#define GREEN64 0x0000FFFF0000 /* BGR 16bpc */
-#define BLUE64 0xFFFF00000000 /* BGR 16bpc */
+/*
+ * Local copy of proposed kernel uapi
+ */
+static inline __u64
+local_argb(__u8 bpc, __u16 alpha, __u16 red, __u16 green, __u16 blue)
+{
+ int msb_shift = 16 - bpc;
+ return (__u64)alpha << msb_shift << 48 |
+ (__u64)red << msb_shift << 32 |
+ (__u64)green << msb_shift << 16 |
+ (__u64)blue << msb_shift;
+}
+#define LOCAL_ARGB_COMP(c, shift, numbits) \
+ (__u16)(((c) & 0xFFFFull << (shift)) >> ((shift) + 16 - (numbits)))
+#define LOCAL_ARGB_BLUE(c, numbits) DRM_ARGB_COMP(c, 0, numbits)
+#define LOCAL_ARGB_GREEN(c, numbits) DRM_ARGB_COMP(c, 16, numbits)
+#define LOCAL_ARGB_RED(c, numbits) DRM_ARGB_COMP(c, 32, numbits)
+#define LOCAL_ARGB_ALPHA(c, numbits) DRM_ARGB_COMP(c, 48, numbits)
+
+
+/* 8bpc values */
+#define BLACK local_argb(8, 0xff, 0, 0, 0)
+#define RED local_argb(8, 0xff, 0xff, 0, 0)
+#define GREEN local_argb(8, 0xff, 0, 0xff, 0)
+#define BLUE local_argb(8, 0xff, 0, 0, 0xff)
+#define YELLOW local_argb(8, 0xff, 0xff, 0xff, 0)
+#define WHITE local_argb(8, 0xff, 0xff, 0xff, 0xff)
+
+/* 16bpc values */
+#define BLACK64 local_argb(16, 0xffff, 0, 0, 0)
+#define RED64 local_argb(16, 0xffff, 0xffff, 0, 0)
+#define GREEN64 local_argb(16, 0xffff, 0, 0xffff, 0)
+#define BLUE64 local_argb(16, 0xffff, 0, 0, 0xffff)
+#define YELLOW64 local_argb(16, 0xffff, 0xffff, 0xffff, 0)
+#define WHITE64 local_argb(16, 0xffff, 0xffff, 0xffff, 0xffff)
+
+#if 0
static void
-paint_background(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
- uint32_t background, double alpha)
+paint_fb(data_t *data, struct igt_fb *fb, drmModeModeInfo *mode,
+ uint64_t color, int prec)
{
cairo_t *cr;
- int w, h;
+ int w = mode->hdisplay;
+ int h = mode->vdisplay;
double r, g, b;
- w = mode->hdisplay;
- h = mode->vdisplay;
+ igt_create_fb(data->gfx_fd, w, h, DRM_FORMAT_XRGB8888,
+ LOCAL_DRM_FORMAT_MOD_NONE, fb);
- cr = igt_get_cairo_ctx(data->gfx_fd, &data->fb);
+ cr = igt_get_cairo_ctx(data->gfx_fd, fb);
- /* Paint with background color */
- r = (double) (background & 0xFF) / 255.0;
- g = (double) ((background & 0xFF00) >> 8) / 255.0;
- b = (double) ((background & 0xFF0000) >> 16) / 255.0;
- igt_paint_color_alpha(cr, 0, 0, w, h, r, g, b, alpha);
+ /*
+ * Grab color (with appropriate bits of precision) and paint a
+ * framebuffer with it.
+ */
+ r = (double)LOCAL_ARGB_RED(color, prec) / ((1<<prec) - 1);
+ g = (double)LOCAL_ARGB_GREEN(color, prec) / ((1<<prec) - 1);
+ b = (double)LOCAL_ARGB_BLUE(color, prec) / ((1<<prec) - 1);
+ igt_paint_color_alpha(cr, 0, 0, w, h, r, g, b, 1.0);
- igt_put_cairo_ctx(data->gfx_fd, &data->fb, cr);
+ igt_put_cairo_ctx(data->gfx_fd, fb, cr);
}
+#endif
-static void prepare_crtc(data_t *data, igt_output_t *output, enum pipe pipe,
- igt_plane_t *plane, int opaque_buffer, int plane_color,
- uint64_t pipe_background_color)
+static void prepare_crtc(igt_display_t *display, data_t *data,
+ igt_output_t *output, enum pipe pipe)
{
- drmModeModeInfo *mode;
- igt_display_t *display = &data->display;
- int fb_id;
- double alpha;
-
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);
-
- fb_id = igt_create_fb(data->gfx_fd,
- mode->hdisplay, mode->vdisplay,
- DRM_FORMAT_XRGB8888,
- LOCAL_DRM_FORMAT_MOD_NONE, /* tiled */
- &data->fb);
- igt_assert(fb_id);
-
- /* To make FB pixel win with background color, set alpha as full opaque */
- igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, pipe_background_color);
- if (opaque_buffer)
- alpha = 1.0; /* alpha 1 is fully opque */
- else
- alpha = 0.0; /* alpha 0 is fully transparent */
- paint_background(data, &data->fb, mode, plane_color, alpha);
-
- igt_plane_set_fb(plane, &data->fb);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ data->output = output;
+ data->mode = igt_output_get_mode(output);
}
-static void cleanup_crtc(data_t *data, igt_output_t *output, igt_plane_t *plane)
+static void cleanup_crtc(igt_display_t *display, data_t *data,
+ igt_output_t *output)
{
- igt_display_t *display = &data->display;
-
- igt_pipe_crc_free(data->pipe_crc);
- data->pipe_crc = NULL;
-
- igt_remove_fb(data->gfx_fd, &data->fb);
-
- igt_pipe_obj_set_prop_value(plane->pipe, IGT_CRTC_BACKGROUND, BLACK64);
- igt_plane_set_fb(plane, NULL);
igt_output_set_pipe(output, PIPE_ANY);
-
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
}
-static void test_crtc_background(data_t *data)
+static void test_crtc_background(igt_display_t *display, data_t *data)
{
- igt_display_t *display = &data->display;
igt_output_t *output;
enum pipe pipe;
int valid_tests = 0;
- for_each_pipe_with_valid_output(display, pipe, output) {
+ for_each_pipe_with_single_output(display, pipe, output) {
igt_plane_t *plane;
- igt_output_set_pipe(output, pipe);
-
- plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
igt_require(igt_pipe_has_prop(display, pipe, IGT_CRTC_BACKGROUND));
- prepare_crtc(data, output, pipe, plane, 1, PURPLE, BLACK64);
+ prepare_crtc(display, data, output, pipe);
+ plane = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
- /* Now set background without using a plane, i.e.,
- * Disable the plane to let hw background color win blend. */
+ /*
+ * Turn off the primary plane (default bgcolor should be black
+ * unless a previous drm master changed it to something else).
+ */
igt_plane_set_fb(plane, NULL);
- igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, PURPLE64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
-
- /* Try few other background colors */
- igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, CYAN64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
-
- igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, YELLOW64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
-
+ igt_display_commit2(display, COMMIT_ATOMIC);
+
+ /* Explicitly set black as bg color */
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLACK64);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+
+ /*
+ * Test several more colors and precisions. Unfortunately the
+ * CRC's won't match between a cairo-drawn fb and a display
+ * controller bgcolor setting, but these can at least be
+ * visually verified in interactive mode to ensure the colors
+ * look good.
+ */
igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, RED64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, RED);
+ igt_display_commit2(display, COMMIT_ATOMIC);
igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, GREEN64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, GREEN);
+ igt_display_commit2(display, COMMIT_ATOMIC);
igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLUE64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLUE);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, YELLOW64);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, YELLOW);
+ igt_display_commit2(display, COMMIT_ATOMIC);
igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, WHITE64);
- igt_display_commit2(display, COMMIT_UNIVERSAL);
+ igt_display_commit2(display, COMMIT_ATOMIC);
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, WHITE);
+ igt_display_commit2(display, COMMIT_ATOMIC);
valid_tests++;
- cleanup_crtc(data, output, plane);
+
+ igt_pipe_set_prop_value(display, pipe, IGT_CRTC_BACKGROUND, BLACK64);
+ cleanup_crtc(display, data, output);
}
igt_require_f(valid_tests, "no valid crtc/connector combinations found\n");
}
igt_simple_main
{
+ igt_display_t display;
data_t data = {};
igt_skip_on_simulation();
data.gfx_fd = drm_open_driver(DRIVER_INTEL);
- igt_require_pipe_crc(data.gfx_fd);
- igt_display_require(&data.display, data.gfx_fd);
+ igt_display_require(&display, data.gfx_fd);
- test_crtc_background(&data);
+ test_crtc_background(&display, &data);
- igt_display_fini(&data.display);
+ igt_display_fini(&display);
}
--
2.14.4
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 3+ messages in thread