Re: [igt-dev] [PATCH i-g-t 4/5] tests/kms_plane: Test all modifiers as well

Dhinakaran Pandiyan
2018-10-31T21:35:16Z

On Tuesday, October 30, 2018 9:20:37 AM PDT Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Instead of just testing each pixel format let's test every
> format+modifier combo.
>
> Obviously testing with solid filled fbs isn't the most effective
> way to detect tiling problems, but we can't really do much more if
> we want to keep comparing YUV vs. RGB results (unless we start to
> render the RGB content in a way that matches the YUV subsampling
> behaviour of the hardware).
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> tests/kms_plane.c | 30 +++++++++++++++++++-----------
> 1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index 45e0a304e8ce..5e498d14856f 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -406,14 +406,15 @@ static void set_legacy_lut(data_t *data, enum pipe pipe,
>
> static void test_format_plane_color(data_t *data, enum pipe pipe,
> igt_plane_t *plane,
> - uint32_t format, int width, int height,
> + uint32_t format, uint64_t modifier,
> + int width, int height,
> int color, igt_crc_t *crc, struct igt_fb *fb)
> {
> const color_t *c = &colors[color];
> struct igt_fb old_fb = *fb;
>
> igt_create_color_fb(data->drm_fd, width, height,
> - format, LOCAL_DRM_FORMAT_MOD_NONE,
> + format, modifier,
> c->red, c->green, c->blue, fb);
>
> igt_plane_set_fb(plane, fb);
> @@ -432,6 +433,7 @@ static void test_format_plane(data_t *data, enum pipe pipe,
> struct igt_fb fb = {};
> drmModeModeInfo *mode;
> uint32_t format, ref_format;
> + uint64_t modifier, ref_modifier;
> uint64_t width, height;
> igt_crc_t ref_crc[ARRAY_SIZE(colors)];
>
> @@ -440,6 +442,7 @@ static void test_format_plane(data_t *data, enum pipe pipe,
> width = mode->hdisplay;
> height = mode->vdisplay;
> ref_format = format = DRM_FORMAT_XRGB8888;
> + ref_modifier = modifier = DRM_FORMAT_MOD_NONE;
> } else {
> if (!plane->drm_plane) {
> igt_debug("Only legacy cursor ioctl supported, skipping cursor plane\n");
> @@ -448,6 +451,7 @@ static void test_format_plane(data_t *data, enum pipe pipe,
> do_or_die(drmGetCap(data->drm_fd, DRM_CAP_CURSOR_WIDTH, &width));
> do_or_die(drmGetCap(data->drm_fd, DRM_CAP_CURSOR_HEIGHT, &height));
> ref_format = format = DRM_FORMAT_ARGB8888;
> + ref_modifier = modifier = DRM_FORMAT_MOD_NONE;
> }
>
> igt_debug("Testing connector %s on %s plane %s.%u\n",
> @@ -468,22 +472,25 @@ static void test_format_plane(data_t *data, enum pipe pipe,
> test_init(data, pipe);
> igt_pipe_crc_start(data->pipe_crc);
>
> - igt_info("Testing format " IGT_FORMAT_FMT " on %s.%u\n",
> - IGT_FORMAT_ARGS(format),
> + igt_info("Testing format " IGT_FORMAT_FMT " / modifier 0x%" PRIx64 " on %s.%u\n",
> + IGT_FORMAT_ARGS(format), modifier,
> kmstest_pipe_name(pipe), plane->index);
>
> for (int i = 0; i < ARRAY_SIZE(colors); i++) {
> test_format_plane_color(data, pipe, plane,
> - format, width, height,
> + format, modifier,
Use ref_format and ref_modifier here?

I'll leave it to you if you want to address that nit.
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> + width, height,
> i, &ref_crc[i], &fb);
> }
>
> - for (int i = 0; i < plane->drm_plane->count_formats; i++) {
> + for (int i = 0; i < plane->format_mod_count; i++) {
> igt_crc_t crc;
>
> - format = plane->drm_plane->formats[i];
> + format = plane->formats[i];
> + modifier = plane->modifiers[i];
>
> - if (format == ref_format)
> + if (format == ref_format &&
> + modifier == ref_modifier)
> continue;
>
> if (!igt_fb_supported_format(format))
> @@ -499,13 +506,14 @@ static void test_format_plane(data_t *data, enum pipe pipe,
> if (format == DRM_FORMAT_XBGR8888)
> continue;
>
> - igt_info("Testing format " IGT_FORMAT_FMT " on %s.%u\n",
> - IGT_FORMAT_ARGS(format),
> + igt_info("Testing format " IGT_FORMAT_FMT " / modifier 0x%" PRIx64 " on %s.%u\n",
> + IGT_FORMAT_ARGS(format), modifier,
> kmstest_pipe_name(pipe), plane->index);
>
> for (int j = 0; j < ARRAY_SIZE(colors); j++) {
> test_format_plane_color(data, pipe, plane,
> - format, width, height,
> + format, modifier,
> + width, height,
> j, &crc, &fb);
>
> igt_assert_crc_equal(&crc, &ref_crc[j]);
>