[igt-dev] [PATCH 1/2] lib: prepare helpers for generic prime test

[igt-dev] [PATCH 1/2] lib: prepare helpers for generic prime test

[Oleg Vasilev]

- Add vkms to gpu providers list
- Expose fb_init

Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
---
 lib/drmtest.c |  1 +
 lib/drmtest.h |  1 +
 lib/igt_fb.c  | 38 ++++++++++++++++++--------------------
 lib/igt_fb.h  |  4 ++++
 4 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index 25f20353..81f4bf32 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -210,6 +210,7 @@ static const struct module {
 	{ DRIVER_V3D, "v3d" },
 	{ DRIVER_VC4, "vc4" },
 	{ DRIVER_VGEM, "vgem" },
+	{ DRIVER_VKMS, "vkms" },
 	{ DRIVER_VIRTIO, "virtio_gpu", modprobe_virtio },
 	{}
 };
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 6c4c3899..d4043ff1 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -45,6 +45,7 @@
 #define DRIVER_AMDGPU	(1 << 4)
 #define DRIVER_V3D	(1 << 5)
 #define DRIVER_PANFROST	(1 << 6)
+#define DRIVER_VKMS	(1 << 7)
 /*
  * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
  * with vgem as well as a supported driver, you can end up with a
diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 415a3d65..49c1dbe4 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -485,12 +485,10 @@ static int fb_num_planes(const struct igt_fb *fb)
 		return format->num_planes;
 }
 
-static void fb_init(struct igt_fb *fb,
-		    int fd, int width, int height,
-		    uint32_t drm_format,
-		    uint64_t modifier,
-		    enum igt_color_encoding color_encoding,
-		    enum igt_color_range color_range)
+void igt_init_fb(struct igt_fb *fb, int fd, int width, int height,
+		 uint32_t drm_format, uint64_t modifier,
+		 enum igt_color_encoding color_encoding,
+		 enum igt_color_range color_range)
 {
 	const struct format_desc_struct *f = lookup_drm_format(drm_format);
 
@@ -627,8 +625,8 @@ void igt_calc_fb_size(int fd, int width, int height, uint32_t drm_format, uint64
 {
 	struct igt_fb fb;
 
-	fb_init(&fb, fd, width, height, drm_format, modifier,
-		IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
+	igt_init_fb(&fb, fd, width, height, drm_format, modifier,
+		    IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
 
 	fb.size = calc_fb_size(&fb);
 
@@ -855,8 +853,8 @@ void igt_create_bo_for_fb(int fd, int width, int height,
 			  uint32_t format, uint64_t modifier,
 			  struct igt_fb *fb /* out */)
 {
-	fb_init(fb, fd, width, height, format, modifier,
-		IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
+	igt_init_fb(fb, fd, width, height, format, modifier,
+		    IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
 	create_bo_for_fb(fb);
 }
 
@@ -885,8 +883,8 @@ int igt_create_bo_with_dimensions(int fd, int width, int height,
 {
 	struct igt_fb fb;
 
-	fb_init(&fb, fd, width, height, format, modifier,
-		IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
+	igt_init_fb(&fb, fd, width, height, format, modifier,
+		    IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
 
 	for (int i = 0; i < fb.num_planes; i++)
 		fb.strides[i] = stride;
@@ -1440,8 +1438,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
 	enum igt_color_range color_range = IGT_COLOR_YCBCR_LIMITED_RANGE;
 	uint32_t flags = 0;
 
-	fb_init(fb, fd, width, height, format, modifier,
-		color_encoding, color_range);
+	igt_init_fb(fb, fd, width, height, format, modifier,
+		    color_encoding, color_range);
 
 	for (int i = 0; i < fb->num_planes; i++)
 		fb->strides[i] = bo_stride;
@@ -1971,9 +1969,9 @@ static void setup_linear_mapping(struct fb_blit_upload *blit)
 	 * destination, tiling it at the same time.
 	 */
 
-	fb_init(&linear->fb, fb->fd, fb->width, fb->height,
-		fb->drm_format, LOCAL_DRM_FORMAT_MOD_NONE,
-		fb->color_encoding, fb->color_range);
+	igt_init_fb(&linear->fb, fb->fd, fb->width, fb->height,
+		    fb->drm_format, LOCAL_DRM_FORMAT_MOD_NONE,
+		    fb->color_encoding, fb->color_range);
 
 	create_bo_for_fb(&linear->fb);
 
@@ -2127,9 +2125,9 @@ static void *igt_fb_create_cairo_shadow_buffer(int fd,
 
 	igt_assert(shadow);
 
-	fb_init(shadow, fd, width, height,
-		drm_format, LOCAL_DRM_FORMAT_MOD_NONE,
-		IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
+	igt_init_fb(shadow, fd, width, height,
+		    drm_format, LOCAL_DRM_FORMAT_MOD_NONE,
+		    IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
 
 	shadow->strides[0] = ALIGN(width * (shadow->plane_bpp[0] / 8), 16);
 	shadow->size = ALIGN((uint64_t)shadow->strides[0] * height,
diff --git a/lib/igt_fb.h b/lib/igt_fb.h
index be786911..3746fa1a 100644
--- a/lib/igt_fb.h
+++ b/lib/igt_fb.h
@@ -117,6 +117,10 @@ void igt_get_fb_tile_size(int fd, uint64_t modifier, int fb_bpp,
 			  unsigned *width_ret, unsigned *height_ret);
 void igt_calc_fb_size(int fd, int width, int height, uint32_t format, uint64_t modifier,
 		      uint64_t *size_ret, unsigned *stride_ret);
+void igt_init_fb(struct igt_fb *fb, int fd, int width, int height,
+		 uint32_t drm_format, uint64_t modifier,
+		 enum igt_color_encoding color_encoding,
+		 enum igt_color_range color_range);
 unsigned int
 igt_create_fb_with_bo_size(int fd, int width, int height,
 			   uint32_t format, uint64_t modifier,
-- 
2.22.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH 2/2] tests/prime_generic: add vendor-agnostic prime tests

[Oleg Vasilev]

Currently, we have different sets of prime tests:
 - vgem+i915
 - amdgpu+i915
 - nv+i915

Those tests use vendor-specific ioctls, therefore, not interchangeable.
The idea is to create a set of tests which are expected to work on any
prime-compatible driver. It can be run with any combination of
exporter+importer, where

The first test is simple:
1. Exporter creates a framebuffer and fills it with a plain color
2. Fb is transferred to the importer
3. Importer modesets and computes pipe CRC
4. Importer draws the same color through cairo and compares CRC

The initial motivation comes from the need to test prime support in
vkms.

Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
---
 tests/meson.build     |   1 +
 tests/prime_generic.c | 238 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 239 insertions(+)
 create mode 100644 tests/prime_generic.c

diff --git a/tests/meson.build b/tests/meson.build
index 34a74025..1c938e95 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -76,6 +76,7 @@ test_progs = [
 	'prime_self_import',
 	'prime_udl',
 	'prime_vgem',
+	'prime_generic',
 	'syncobj_basic',
 	'syncobj_wait',
 	'template',
diff --git a/tests/prime_generic.c b/tests/prime_generic.c
new file mode 100644
index 00000000..65e48763
--- /dev/null
+++ b/tests/prime_generic.c
@@ -0,0 +1,238 @@
+/*
+ * Copyright © 2019 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.
+ */
+
+#include "igt.h"
+#include "igt_vgem.h"
+
+#include <sys/ioctl.h>
+#include <sys/poll.h>
+#include <time.h>
+
+static struct {
+	double r, g, b;
+	uint32_t color;
+	igt_crc_t prime_crc;
+	igt_crc_t direct_crc;
+	char *prime_str;
+	char *direct_str;
+} colors[3] = {
+	{ .r = 0.0, .g = 0.0, .b = 0.0, .color = 0xff000000},
+	{ .r = 1.0, .g = 1.0, .b = 1.0, .color = 0xffffffff},
+	{ .r = 1.0, .g = 0.0, .b = 0.0, .color = 0xffff0000},
+};
+
+IGT_TEST_DESCRIPTION("Generic tests, working with any prime device");
+
+static bool has_prime_import(int fd)
+{
+	uint64_t value;
+
+	if (drmGetCap(fd, DRM_CAP_PRIME, &value))
+		return false;
+
+	return value & DRM_PRIME_CAP_IMPORT;
+}
+
+static bool has_prime_export(int fd)
+{
+	uint64_t value;
+
+	if (drmGetCap(fd, DRM_CAP_PRIME, &value))
+		return false;
+
+	return value & DRM_PRIME_CAP_EXPORT;
+}
+
+static void prepare_scratch(int exporter, struct vgem_bo *scratch,
+			    drmModeModeInfo *mode, uint32_t color)
+{
+	int i;
+	uint32_t *ptr;
+
+	scratch->width = mode->hdisplay;
+	scratch->height = mode->vdisplay;
+	scratch->bpp = 32;
+	vgem_create(exporter, scratch);
+
+	ptr = vgem_mmap(exporter, scratch, PROT_WRITE);
+	for (i = 0; i < scratch->size / sizeof(*ptr); ++i)
+		ptr[i] = color;
+
+	munmap(ptr, scratch->size);
+}
+
+static void prepare_fb(int importer, struct vgem_bo *scratch, struct igt_fb *fb)
+{
+	enum igt_color_encoding color_encoding = IGT_COLOR_YCBCR_BT709;
+	enum igt_color_range color_range = IGT_COLOR_YCBCR_LIMITED_RANGE;
+
+	igt_init_fb(fb, importer, scratch->width, scratch->height,
+		    DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
+		    color_encoding, color_range);
+}
+
+static void import_fb(int importer, struct igt_fb *fb,
+		      int dmabuf_fd, uint32_t pitch)
+{
+	uint32_t offsets[4], pitches[4], handles[4];
+	int ret;
+
+	fb->gem_handle = prime_fd_to_handle(importer, dmabuf_fd);
+
+	handles[0] = fb->gem_handle;
+	pitches[0] = pitch;
+	offsets[0] = 0;
+
+	ret = drmModeAddFB2(importer, fb->width, fb->height,
+			    DRM_FORMAT_XRGB8888,
+			    handles, pitches, offsets,
+			    &fb->fb_id, 0);
+	igt_assert(ret == 0);
+}
+
+static void set_fb(struct igt_fb *fb,
+		   igt_display_t *display,
+		   igt_output_t *output)
+{
+	igt_plane_t *primary;
+	int ret;
+
+	primary = igt_output_get_plane(output, 0);
+	igt_plane_set_fb(primary, fb);
+	ret = igt_display_commit(display);
+
+	igt_assert(ret == 0);
+}
+
+static void test_crc(int exporter, int importer)
+{
+	enum pipe pipe = PIPE_A;
+	igt_display_t display;
+	igt_output_t *output;
+	igt_pipe_crc_t *pipe_crc;
+	struct igt_fb fb;
+	int dmabuf_fd;
+	struct vgem_bo scratch; /* despite the name, it suits for any
+				   gem-compatible device */
+	int i, j;
+	drmModeModeInfo *mode;
+
+	bool crc_equal;
+
+	igt_display_require(&display, importer);
+	igt_skip_on(pipe >= display.n_pipes);
+	output = igt_get_single_output_for_pipe(&display, pipe);
+	igt_require_f(output, "No connector found for pipe %s\n",
+		      kmstest_pipe_name(pipe));
+
+	igt_display_reset(&display);
+	igt_output_set_pipe(output, pipe);
+
+	mode = igt_output_get_mode(output);
+	pipe_crc = igt_pipe_crc_new(importer, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
+
+	for (i = 0; i < ARRAY_SIZE(colors); i++) {
+		prepare_scratch(exporter, &scratch, mode, colors[i].color);
+		dmabuf_fd = prime_handle_to_fd(exporter, scratch.handle);
+		gem_close(exporter, scratch.handle);
+
+		prepare_fb(importer, &scratch, &fb);
+		import_fb(importer, &fb, dmabuf_fd, scratch.pitch);
+		close(dmabuf_fd);
+
+		set_fb(&fb, &display, output);
+		igt_pipe_crc_collect_crc(pipe_crc, &colors[i].prime_crc);
+		colors[i].prime_str = igt_crc_to_string(&colors[i].prime_crc);
+		igt_debug("Prime CRC for %#08x is %s\n",
+			  colors[i].color, colors[i].prime_str);
+		igt_remove_fb(importer, &fb);
+
+		igt_create_color_fb(importer,
+				    mode->hdisplay, mode->vdisplay,
+				    DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
+				    colors[i].r, colors[i].g, colors[i].b,
+				    &fb);
+
+		set_fb(&fb, &display, output);
+		igt_pipe_crc_collect_crc(pipe_crc, &colors[i].direct_crc);
+		colors[i].direct_str = igt_crc_to_string(&colors[i].direct_crc);
+		igt_debug("Direct CRC for %#08x is %s\n",
+			  colors[i].color, colors[i].direct_str);
+		igt_remove_fb(importer, &fb);
+	}
+	igt_pipe_crc_free(pipe_crc);
+
+	igt_debug("CRC table:\n");
+	igt_debug("Color\t\tPrime\t\tDirect\n");
+	for (i = 0; i < ARRAY_SIZE(colors); i++) {
+		igt_debug("%#08x\t%.8s\t%.8s\n", colors[i].color,
+			  colors[i].prime_str, colors[i].direct_str);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(colors); i++) {
+		for (j = 0; j < ARRAY_SIZE(colors); j++) {
+			if (i == j) {
+				igt_assert_crc_equal(&colors[i].prime_crc,
+						     &colors[j].direct_crc);
+			} else {
+				crc_equal = igt_check_crc_equal(&colors[i].prime_crc,
+								&colors[j].direct_crc);
+				igt_assert_f(!crc_equal, "CRC should be different");
+			}
+		}
+	}
+}
+
+static void run_test_crc(int export_chipset, int import_chipset)
+{
+	int importer = -1;
+	int exporter = -1;
+
+	exporter = drm_open_driver(export_chipset);
+	importer = drm_open_driver_master(import_chipset);
+
+	igt_require(has_prime_export(exporter));
+	igt_require(has_prime_import(importer));
+	igt_require_pipe_crc(importer);
+
+	test_crc(exporter, importer);
+	close(importer);
+	close(exporter);
+}
+
+igt_main
+{
+	igt_fixture {
+		kmstest_set_vt_graphics_mode();
+	}
+	igt_subtest("crc-vgem-vkms")
+		run_test_crc(DRIVER_VGEM, DRIVER_VKMS);
+	igt_subtest("crc-i915-vkms")
+		run_test_crc(DRIVER_INTEL, DRIVER_VKMS);
+	igt_subtest("crc-vgem-i915")
+		run_test_crc(DRIVER_VGEM, DRIVER_INTEL);
+	igt_subtest("crc-amd-i915")
+		run_test_crc(DRIVER_AMDGPU, DRIVER_INTEL);
+	igt_subtest("crc-i915-amd")
+		run_test_crc(DRIVER_INTEL, DRIVER_AMDGPU);
+}
-- 
2.22.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH 2/2] tests/prime_generic: add vendor-agnostic prime tests

[Rodrigo Siqueira]

Hi Oleg,

I really liked this patch since it looks like documentation, which
means that someone like me can use it for understanding how to use
prime.

Btw, a few comments inline

On Thu, Jul 4, 2019 at 6:56 AM Oleg Vasilev <oleg.vasilev@intel.com> wrote:
>
> Currently, we have different sets of prime tests:
>  - vgem+i915
>  - amdgpu+i915
>  - nv+i915
>
> Those tests use vendor-specific ioctls, therefore, not interchangeable.
> The idea is to create a set of tests which are expected to work on any
> prime-compatible driver. It can be run with any combination of
> exporter+importer, where
>
> The first test is simple:
> 1. Exporter creates a framebuffer and fills it with a plain color
> 2. Fb is transferred to the importer
> 3. Importer modesets and computes pipe CRC
> 4. Importer draws the same color through cairo and compares CRC
>
> The initial motivation comes from the need to test prime support in
> vkms.
>
> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> ---
>  tests/meson.build     |   1 +
>  tests/prime_generic.c | 238 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 239 insertions(+)
>  create mode 100644 tests/prime_generic.c
>
> diff --git a/tests/meson.build b/tests/meson.build
> index 34a74025..1c938e95 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -76,6 +76,7 @@ test_progs = [
>         'prime_self_import',
>         'prime_udl',
>         'prime_vgem',
> +       'prime_generic',
>         'syncobj_basic',
>         'syncobj_wait',
>         'template',
> diff --git a/tests/prime_generic.c b/tests/prime_generic.c
> new file mode 100644
> index 00000000..65e48763
> --- /dev/null
> +++ b/tests/prime_generic.c
> @@ -0,0 +1,238 @@
> +/*
> + * Copyright © 2019 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.
> + */
> +
> +#include "igt.h"
> +#include "igt_vgem.h"
> +
> +#include <sys/ioctl.h>
> +#include <sys/poll.h>
> +#include <time.h>
> +
> +static struct {
> +       double r, g, b;
> +       uint32_t color;
> +       igt_crc_t prime_crc;
> +       igt_crc_t direct_crc;
> +       char *prime_str;
> +       char *direct_str;
> +} colors[3] = {
> +       { .r = 0.0, .g = 0.0, .b = 0.0, .color = 0xff000000},
> +       { .r = 1.0, .g = 1.0, .b = 1.0, .color = 0xffffffff},
> +       { .r = 1.0, .g = 0.0, .b = 0.0, .color = 0xffff0000},
> +};
> +
> +IGT_TEST_DESCRIPTION("Generic tests, working with any prime device");
> +
> +static bool has_prime_import(int fd)
> +{
> +       uint64_t value;
> +
> +       if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> +               return false;
> +
> +       return value & DRM_PRIME_CAP_IMPORT;
> +}
> +
> +static bool has_prime_export(int fd)
> +{
> +       uint64_t value;
> +
> +       if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> +               return false;
> +
> +       return value & DRM_PRIME_CAP_EXPORT;
> +}
> +
> +static void prepare_scratch(int exporter, struct vgem_bo *scratch,
> +                           drmModeModeInfo *mode, uint32_t color)
> +{
> +       int i;
> +       uint32_t *ptr;
> +
> +       scratch->width = mode->hdisplay;
> +       scratch->height = mode->vdisplay;
> +       scratch->bpp = 32;
> +       vgem_create(exporter, scratch);
> +
> +       ptr = vgem_mmap(exporter, scratch, PROT_WRITE);
> +       for (i = 0; i < scratch->size / sizeof(*ptr); ++i)
> +               ptr[i] = color;
> +
> +       munmap(ptr, scratch->size);
> +}
> +
> +static void prepare_fb(int importer, struct vgem_bo *scratch, struct igt_fb *fb)
> +{
> +       enum igt_color_encoding color_encoding = IGT_COLOR_YCBCR_BT709;
> +       enum igt_color_range color_range = IGT_COLOR_YCBCR_LIMITED_RANGE;
> +
> +       igt_init_fb(fb, importer, scratch->width, scratch->height,
> +                   DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> +                   color_encoding, color_range);
> +}
> +
> +static void import_fb(int importer, struct igt_fb *fb,
> +                     int dmabuf_fd, uint32_t pitch)
> +{
> +       uint32_t offsets[4], pitches[4], handles[4];
> +       int ret;
> +
> +       fb->gem_handle = prime_fd_to_handle(importer, dmabuf_fd);
> +
> +       handles[0] = fb->gem_handle;
> +       pitches[0] = pitch;
> +       offsets[0] = 0;
> +
> +       ret = drmModeAddFB2(importer, fb->width, fb->height,
> +                           DRM_FORMAT_XRGB8888,
> +                           handles, pitches, offsets,
> +                           &fb->fb_id, 0);
> +       igt_assert(ret == 0);
> +}
> +
> +static void set_fb(struct igt_fb *fb,
> +                  igt_display_t *display,
> +                  igt_output_t *output)
> +{
> +       igt_plane_t *primary;
> +       int ret;
> +
> +       primary = igt_output_get_plane(output, 0);
> +       igt_plane_set_fb(primary, fb);
> +       ret = igt_display_commit(display);
> +
> +       igt_assert(ret == 0);
> +}
> +
> +static void test_crc(int exporter, int importer)
> +{
> +       enum pipe pipe = PIPE_A;
> +       igt_display_t display;
> +       igt_output_t *output;
> +       igt_pipe_crc_t *pipe_crc;
> +       struct igt_fb fb;
> +       int dmabuf_fd;
> +       struct vgem_bo scratch; /* despite the name, it suits for any
> +                                  gem-compatible device */

Since vgem_bo is suiting for any gem-compatible device, how about send
a patch that added a better name for it? IMHO this is good for
readability sake - anyway, just an idea.

> +       int i, j;
> +       drmModeModeInfo *mode;
> +
> +       bool crc_equal;
> +
> +       igt_display_require(&display, importer);
> +       igt_skip_on(pipe >= display.n_pipes);
> +       output = igt_get_single_output_for_pipe(&display, pipe);
> +       igt_require_f(output, "No connector found for pipe %s\n",
> +                     kmstest_pipe_name(pipe));
> +
> +       igt_display_reset(&display);
> +       igt_output_set_pipe(output, pipe);

The above code looks like a setup, how about move it for a static
function responsible for this action? IMHO, it could be easier to
understand test_crc, if it's focused on only on CRC test.

Additionally, I want to suggest that igt_display_fini() gets invoked
at the end of this function because you initialized some display stuff
and did not clean up them at the end.

> +
> +       mode = igt_output_get_mode(output);
> +       pipe_crc = igt_pipe_crc_new(importer, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
> +
> +       for (i = 0; i < ARRAY_SIZE(colors); i++) {
> +               prepare_scratch(exporter, &scratch, mode, colors[i].color);
> +               dmabuf_fd = prime_handle_to_fd(exporter, scratch.handle);
> +               gem_close(exporter, scratch.handle);
> +
> +               prepare_fb(importer, &scratch, &fb);
> +               import_fb(importer, &fb, dmabuf_fd, scratch.pitch);
> +               close(dmabuf_fd);
> +
> +               set_fb(&fb, &display, output);
> +               igt_pipe_crc_collect_crc(pipe_crc, &colors[i].prime_crc);
> +               colors[i].prime_str = igt_crc_to_string(&colors[i].prime_crc);

Maybe I missed something, but I think you forgot to free the buffer
allocated by igt_crc_to_string().

> +               igt_debug("Prime CRC for %#08x is %s\n",
> +                         colors[i].color, colors[i].prime_str);
> +               igt_remove_fb(importer, &fb);
> +
> +               igt_create_color_fb(importer,
> +                                   mode->hdisplay, mode->vdisplay,
> +                                   DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> +                                   colors[i].r, colors[i].g, colors[i].b,
> +                                   &fb);
> +
> +               set_fb(&fb, &display, output);
> +               igt_pipe_crc_collect_crc(pipe_crc, &colors[i].direct_crc);
> +               colors[i].direct_str = igt_crc_to_string(&colors[i].direct_crc);

Ditto.

> +               igt_debug("Direct CRC for %#08x is %s\n",
> +                         colors[i].color, colors[i].direct_str);
> +               igt_remove_fb(importer, &fb);

How about creating a static function with a meaningful name that makes
the following steps:

 set_fb() -> igt_pipe_crc_collect_crc() -> colors[i]* -> igt_debug()
-> igt_remove_fb()

The above steps are similar for 'prime' and 'direct'. Maybe a function
that only handles these steps makes it easy to understand the code and
also improve the maintainability.

Best regards

> +       }
> +       igt_pipe_crc_free(pipe_crc);
> +
> +       igt_debug("CRC table:\n");
> +       igt_debug("Color\t\tPrime\t\tDirect\n");
> +       for (i = 0; i < ARRAY_SIZE(colors); i++) {
> +               igt_debug("%#08x\t%.8s\t%.8s\n", colors[i].color,
> +                         colors[i].prime_str, colors[i].direct_str);
> +       }
> +
> +       for (i = 0; i < ARRAY_SIZE(colors); i++) {
> +               for (j = 0; j < ARRAY_SIZE(colors); j++) {
> +                       if (i == j) {
> +                               igt_assert_crc_equal(&colors[i].prime_crc,
> +                                                    &colors[j].direct_crc);
> +                       } else {
> +                               crc_equal = igt_check_crc_equal(&colors[i].prime_crc,
> +                                                               &colors[j].direct_crc);
> +                               igt_assert_f(!crc_equal, "CRC should be different");
> +                       }
> +               }
> +       }
> +}
> +
> +static void run_test_crc(int export_chipset, int import_chipset)
> +{
> +       int importer = -1;
> +       int exporter = -1;
> +
> +       exporter = drm_open_driver(export_chipset);
> +       importer = drm_open_driver_master(import_chipset);
> +
> +       igt_require(has_prime_export(exporter));
> +       igt_require(has_prime_import(importer));
> +       igt_require_pipe_crc(importer);
> +
> +       test_crc(exporter, importer);
> +       close(importer);
> +       close(exporter);
> +}
> +
> +igt_main
> +{
> +       igt_fixture {
> +               kmstest_set_vt_graphics_mode();
> +       }
> +       igt_subtest("crc-vgem-vkms")
> +               run_test_crc(DRIVER_VGEM, DRIVER_VKMS);
> +       igt_subtest("crc-i915-vkms")
> +               run_test_crc(DRIVER_INTEL, DRIVER_VKMS);
> +       igt_subtest("crc-vgem-i915")
> +               run_test_crc(DRIVER_VGEM, DRIVER_INTEL);
> +       igt_subtest("crc-amd-i915")
> +               run_test_crc(DRIVER_AMDGPU, DRIVER_INTEL);
> +       igt_subtest("crc-i915-amd")
> +               run_test_crc(DRIVER_INTEL, DRIVER_AMDGPU);
> +}
> --
> 2.22.0
>


-- 

Rodrigo Siqueira
https://siqueira.tech
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH 2/2] tests/prime_generic: add vendor-agnostic prime tests

[Daniel Vetter]

On Thu, Jul 04, 2019 at 12:56:04PM +0300, Oleg Vasilev wrote:
> Currently, we have different sets of prime tests:
>  - vgem+i915
>  - amdgpu+i915
>  - nv+i915
> 
> Those tests use vendor-specific ioctls, therefore, not interchangeable.
> The idea is to create a set of tests which are expected to work on any
> prime-compatible driver. It can be run with any combination of
> exporter+importer, where
> 
> The first test is simple:
> 1. Exporter creates a framebuffer and fills it with a plain color
> 2. Fb is transferred to the importer
> 3. Importer modesets and computes pipe CRC
> 4. Importer draws the same color through cairo and compares CRC
> 
> The initial motivation comes from the need to test prime support in
> vkms.
> 
> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> ---
>  tests/meson.build     |   1 +
>  tests/prime_generic.c | 238 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 239 insertions(+)
>  create mode 100644 tests/prime_generic.c
> 
> diff --git a/tests/meson.build b/tests/meson.build
> index 34a74025..1c938e95 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -76,6 +76,7 @@ test_progs = [
>  	'prime_self_import',
>  	'prime_udl',
>  	'prime_vgem',
> +	'prime_generic',
>  	'syncobj_basic',
>  	'syncobj_wait',
>  	'template',
> diff --git a/tests/prime_generic.c b/tests/prime_generic.c
> new file mode 100644
> index 00000000..65e48763
> --- /dev/null
> +++ b/tests/prime_generic.c
> @@ -0,0 +1,238 @@
> +/*
> + * Copyright © 2019 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.
> + */
> +
> +#include "igt.h"
> +#include "igt_vgem.h"
> +
> +#include <sys/ioctl.h>
> +#include <sys/poll.h>
> +#include <time.h>
> +
> +static struct {
> +	double r, g, b;
> +	uint32_t color;
> +	igt_crc_t prime_crc;
> +	igt_crc_t direct_crc;
> +	char *prime_str;
> +	char *direct_str;
> +} colors[3] = {
> +	{ .r = 0.0, .g = 0.0, .b = 0.0, .color = 0xff000000},
> +	{ .r = 1.0, .g = 1.0, .b = 1.0, .color = 0xffffffff},
> +	{ .r = 1.0, .g = 0.0, .b = 0.0, .color = 0xffff0000},
> +};
> +
> +IGT_TEST_DESCRIPTION("Generic tests, working with any prime device");
> +
> +static bool has_prime_import(int fd)
> +{
> +	uint64_t value;
> +
> +	if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> +		return false;
> +
> +	return value & DRM_PRIME_CAP_IMPORT;
> +}
> +
> +static bool has_prime_export(int fd)
> +{
> +	uint64_t value;
> +
> +	if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> +		return false;
> +
> +	return value & DRM_PRIME_CAP_EXPORT;
> +}
> +
> +static void prepare_scratch(int exporter, struct vgem_bo *scratch,
> +			    drmModeModeInfo *mode, uint32_t color)
> +{
> +	int i;
> +	uint32_t *ptr;
> +
> +	scratch->width = mode->hdisplay;
> +	scratch->height = mode->vdisplay;
> +	scratch->bpp = 32;
> +	vgem_create(exporter, scratch);
> +
> +	ptr = vgem_mmap(exporter, scratch, PROT_WRITE);
> +	for (i = 0; i < scratch->size / sizeof(*ptr); ++i)
> +		ptr[i] = color;
> +
> +	munmap(ptr, scratch->size);
> +}
> +
> +static void prepare_fb(int importer, struct vgem_bo *scratch, struct igt_fb *fb)
> +{
> +	enum igt_color_encoding color_encoding = IGT_COLOR_YCBCR_BT709;
> +	enum igt_color_range color_range = IGT_COLOR_YCBCR_LIMITED_RANGE;
> +
> +	igt_init_fb(fb, importer, scratch->width, scratch->height,
> +		    DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> +		    color_encoding, color_range);
> +}
> +
> +static void import_fb(int importer, struct igt_fb *fb,
> +		      int dmabuf_fd, uint32_t pitch)
> +{
> +	uint32_t offsets[4], pitches[4], handles[4];
> +	int ret;
> +
> +	fb->gem_handle = prime_fd_to_handle(importer, dmabuf_fd);
> +
> +	handles[0] = fb->gem_handle;
> +	pitches[0] = pitch;
> +	offsets[0] = 0;
> +
> +	ret = drmModeAddFB2(importer, fb->width, fb->height,
> +			    DRM_FORMAT_XRGB8888,
> +			    handles, pitches, offsets,
> +			    &fb->fb_id, 0);
> +	igt_assert(ret == 0);
> +}
> +
> +static void set_fb(struct igt_fb *fb,
> +		   igt_display_t *display,
> +		   igt_output_t *output)
> +{
> +	igt_plane_t *primary;
> +	int ret;
> +
> +	primary = igt_output_get_plane(output, 0);
> +	igt_plane_set_fb(primary, fb);
> +	ret = igt_display_commit(display);
> +
> +	igt_assert(ret == 0);
> +}
> +
> +static void test_crc(int exporter, int importer)
> +{
> +	enum pipe pipe = PIPE_A;
> +	igt_display_t display;
> +	igt_output_t *output;
> +	igt_pipe_crc_t *pipe_crc;
> +	struct igt_fb fb;
> +	int dmabuf_fd;
> +	struct vgem_bo scratch; /* despite the name, it suits for any
> +				   gem-compatible device */
> +	int i, j;
> +	drmModeModeInfo *mode;
> +
> +	bool crc_equal;
> +
> +	igt_display_require(&display, importer);
> +	igt_skip_on(pipe >= display.n_pipes);
> +	output = igt_get_single_output_for_pipe(&display, pipe);
> +	igt_require_f(output, "No connector found for pipe %s\n",
> +		      kmstest_pipe_name(pipe));
> +
> +	igt_display_reset(&display);
> +	igt_output_set_pipe(output, pipe);
> +
> +	mode = igt_output_get_mode(output);
> +	pipe_crc = igt_pipe_crc_new(importer, pipe, INTEL_PIPE_CRC_SOURCE_AUTO);
> +
> +	for (i = 0; i < ARRAY_SIZE(colors); i++) {
> +		prepare_scratch(exporter, &scratch, mode, colors[i].color);
> +		dmabuf_fd = prime_handle_to_fd(exporter, scratch.handle);
> +		gem_close(exporter, scratch.handle);
> +
> +		prepare_fb(importer, &scratch, &fb);
> +		import_fb(importer, &fb, dmabuf_fd, scratch.pitch);
> +		close(dmabuf_fd);
> +
> +		set_fb(&fb, &display, output);
> +		igt_pipe_crc_collect_crc(pipe_crc, &colors[i].prime_crc);
> +		colors[i].prime_str = igt_crc_to_string(&colors[i].prime_crc);
> +		igt_debug("Prime CRC for %#08x is %s\n",
> +			  colors[i].color, colors[i].prime_str);
> +		igt_remove_fb(importer, &fb);
> +
> +		igt_create_color_fb(importer,
> +				    mode->hdisplay, mode->vdisplay,
> +				    DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> +				    colors[i].r, colors[i].g, colors[i].b,
> +				    &fb);
> +
> +		set_fb(&fb, &display, output);
> +		igt_pipe_crc_collect_crc(pipe_crc, &colors[i].direct_crc);
> +		colors[i].direct_str = igt_crc_to_string(&colors[i].direct_crc);
> +		igt_debug("Direct CRC for %#08x is %s\n",
> +			  colors[i].color, colors[i].direct_str);
> +		igt_remove_fb(importer, &fb);
> +	}
> +	igt_pipe_crc_free(pipe_crc);
> +
> +	igt_debug("CRC table:\n");
> +	igt_debug("Color\t\tPrime\t\tDirect\n");
> +	for (i = 0; i < ARRAY_SIZE(colors); i++) {
> +		igt_debug("%#08x\t%.8s\t%.8s\n", colors[i].color,
> +			  colors[i].prime_str, colors[i].direct_str);
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(colors); i++) {
> +		for (j = 0; j < ARRAY_SIZE(colors); j++) {
> +			if (i == j) {
> +				igt_assert_crc_equal(&colors[i].prime_crc,
> +						     &colors[j].direct_crc);
> +			} else {
> +				crc_equal = igt_check_crc_equal(&colors[i].prime_crc,
> +								&colors[j].direct_crc);
> +				igt_assert_f(!crc_equal, "CRC should be different");
> +			}
> +		}
> +	}
> +}
> +
> +static void run_test_crc(int export_chipset, int import_chipset)
> +{
> +	int importer = -1;
> +	int exporter = -1;
> +
> +	exporter = drm_open_driver(export_chipset);
> +	importer = drm_open_driver_master(import_chipset);
> +
> +	igt_require(has_prime_export(exporter));
> +	igt_require(has_prime_import(importer));
> +	igt_require_pipe_crc(importer);
> +
> +	test_crc(exporter, importer);
> +	close(importer);
> +	close(exporter);
> +}
> +
> +igt_main
> +{
> +	igt_fixture {
> +		kmstest_set_vt_graphics_mode();
> +	}
> +	igt_subtest("crc-vgem-vkms")
> +		run_test_crc(DRIVER_VGEM, DRIVER_VKMS);
> +	igt_subtest("crc-i915-vkms")
> +		run_test_crc(DRIVER_INTEL, DRIVER_VKMS);
> +	igt_subtest("crc-vgem-i915")
> +		run_test_crc(DRIVER_VGEM, DRIVER_INTEL);
> +	igt_subtest("crc-amd-i915")
> +		run_test_crc(DRIVER_AMDGPU, DRIVER_INTEL);
> +	igt_subtest("crc-i915-amd")
> +		run_test_crc(DRIVER_INTEL, DRIVER_AMDGPU);

This isn't any more generic than what we have e.g. in prime_vgem already.
The way to do a generic kms testcase is to use DRIVER_ANY (yes some of the
earlier conversion unfortunately used stuff like DRIVER_INTEL |
DRIVER_AMDGPU). With that you also don't need your library prep patch,
because no need for a DRIVER_VKMS.

Imo best to start out by converting prime_vgem.c for this, and adding for
the i915 specific tests (but _only_ for those) a specific "is this i915"
check, so we skip those tests. And replace DRIVER_INTEL by DRIVER_ANY.
Well, anything except vgem :-)

Also the crc testcases should be 100% generic.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH 2/2] tests/prime_generic: add vendor-agnostic prime tests

[Vasilev, Oleg]

[-- Attachment #1.1: Type: text/plain, Size: 11579 bytes --]

On Wed, 2019-07-10 at 18:35 +0200, Daniel Vetter wrote:
> On Thu, Jul 04, 2019 at 12:56:04PM +0300, Oleg Vasilev wrote:
> > Currently, we have different sets of prime tests:
> >  - vgem+i915
> >  - amdgpu+i915
> >  - nv+i915
> > 
> > Those tests use vendor-specific ioctls, therefore, not
> > interchangeable.
> > The idea is to create a set of tests which are expected to work on
> > any
> > prime-compatible driver. It can be run with any combination of
> > exporter+importer, where
> > 
> > The first test is simple:
> > 1. Exporter creates a framebuffer and fills it with a plain color
> > 2. Fb is transferred to the importer
> > 3. Importer modesets and computes pipe CRC
> > 4. Importer draws the same color through cairo and compares CRC
> > 
> > The initial motivation comes from the need to test prime support in
> > vkms.
> > 
> > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> > ---
> >  tests/meson.build     |   1 +
> >  tests/prime_generic.c | 238
> > ++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 239 insertions(+)
> >  create mode 100644 tests/prime_generic.c
> > 
> > diff --git a/tests/meson.build b/tests/meson.build
> > index 34a74025..1c938e95 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -76,6 +76,7 @@ test_progs = [
> >  	'prime_self_import',
> >  	'prime_udl',
> >  	'prime_vgem',
> > +	'prime_generic',
> >  	'syncobj_basic',
> >  	'syncobj_wait',
> >  	'template',
> > diff --git a/tests/prime_generic.c b/tests/prime_generic.c
> > new file mode 100644
> > index 00000000..65e48763
> > --- /dev/null
> > +++ b/tests/prime_generic.c
> > @@ -0,0 +1,238 @@
> > +/*
> > + * Copyright © 2019 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.
> > + */
> > +
> > +#include "igt.h"
> > +#include "igt_vgem.h"
> > +
> > +#include <sys/ioctl.h>
> > +#include <sys/poll.h>
> > +#include <time.h>
> > +
> > +static struct {
> > +	double r, g, b;
> > +	uint32_t color;
> > +	igt_crc_t prime_crc;
> > +	igt_crc_t direct_crc;
> > +	char *prime_str;
> > +	char *direct_str;
> > +} colors[3] = {
> > +	{ .r = 0.0, .g = 0.0, .b = 0.0, .color = 0xff000000},
> > +	{ .r = 1.0, .g = 1.0, .b = 1.0, .color = 0xffffffff},
> > +	{ .r = 1.0, .g = 0.0, .b = 0.0, .color = 0xffff0000},
> > +};
> > +
> > +IGT_TEST_DESCRIPTION("Generic tests, working with any prime
> > device");
> > +
> > +static bool has_prime_import(int fd)
> > +{
> > +	uint64_t value;
> > +
> > +	if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> > +		return false;
> > +
> > +	return value & DRM_PRIME_CAP_IMPORT;
> > +}
> > +
> > +static bool has_prime_export(int fd)
> > +{
> > +	uint64_t value;
> > +
> > +	if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> > +		return false;
> > +
> > +	return value & DRM_PRIME_CAP_EXPORT;
> > +}
> > +
> > +static void prepare_scratch(int exporter, struct vgem_bo *scratch,
> > +			    drmModeModeInfo *mode, uint32_t color)
> > +{
> > +	int i;
> > +	uint32_t *ptr;
> > +
> > +	scratch->width = mode->hdisplay;
> > +	scratch->height = mode->vdisplay;
> > +	scratch->bpp = 32;
> > +	vgem_create(exporter, scratch);
> > +
> > +	ptr = vgem_mmap(exporter, scratch, PROT_WRITE);
> > +	for (i = 0; i < scratch->size / sizeof(*ptr); ++i)
> > +		ptr[i] = color;
> > +
> > +	munmap(ptr, scratch->size);
> > +}
> > +
> > +static void prepare_fb(int importer, struct vgem_bo *scratch,
> > struct igt_fb *fb)
> > +{
> > +	enum igt_color_encoding color_encoding = IGT_COLOR_YCBCR_BT709;
> > +	enum igt_color_range color_range =
> > IGT_COLOR_YCBCR_LIMITED_RANGE;
> > +
> > +	igt_init_fb(fb, importer, scratch->width, scratch->height,
> > +		    DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> > +		    color_encoding, color_range);
> > +}
> > +
> > +static void import_fb(int importer, struct igt_fb *fb,
> > +		      int dmabuf_fd, uint32_t pitch)
> > +{
> > +	uint32_t offsets[4], pitches[4], handles[4];
> > +	int ret;
> > +
> > +	fb->gem_handle = prime_fd_to_handle(importer, dmabuf_fd);
> > +
> > +	handles[0] = fb->gem_handle;
> > +	pitches[0] = pitch;
> > +	offsets[0] = 0;
> > +
> > +	ret = drmModeAddFB2(importer, fb->width, fb->height,
> > +			    DRM_FORMAT_XRGB8888,
> > +			    handles, pitches, offsets,
> > +			    &fb->fb_id, 0);
> > +	igt_assert(ret == 0);
> > +}
> > +
> > +static void set_fb(struct igt_fb *fb,
> > +		   igt_display_t *display,
> > +		   igt_output_t *output)
> > +{
> > +	igt_plane_t *primary;
> > +	int ret;
> > +
> > +	primary = igt_output_get_plane(output, 0);
> > +	igt_plane_set_fb(primary, fb);
> > +	ret = igt_display_commit(display);
> > +
> > +	igt_assert(ret == 0);
> > +}
> > +
> > +static void test_crc(int exporter, int importer)
> > +{
> > +	enum pipe pipe = PIPE_A;
> > +	igt_display_t display;
> > +	igt_output_t *output;
> > +	igt_pipe_crc_t *pipe_crc;
> > +	struct igt_fb fb;
> > +	int dmabuf_fd;
> > +	struct vgem_bo scratch; /* despite the name, it suits for any
> > +				   gem-compatible device */
> > +	int i, j;
> > +	drmModeModeInfo *mode;
> > +
> > +	bool crc_equal;
> > +
> > +	igt_display_require(&display, importer);
> > +	igt_skip_on(pipe >= display.n_pipes);
> > +	output = igt_get_single_output_for_pipe(&display, pipe);
> > +	igt_require_f(output, "No connector found for pipe %s\n",
> > +		      kmstest_pipe_name(pipe));
> > +
> > +	igt_display_reset(&display);
> > +	igt_output_set_pipe(output, pipe);
> > +
> > +	mode = igt_output_get_mode(output);
> > +	pipe_crc = igt_pipe_crc_new(importer, pipe,
> > INTEL_PIPE_CRC_SOURCE_AUTO);
> > +
> > +	for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > +		prepare_scratch(exporter, &scratch, mode,
> > colors[i].color);
> > +		dmabuf_fd = prime_handle_to_fd(exporter,
> > scratch.handle);
> > +		gem_close(exporter, scratch.handle);
> > +
> > +		prepare_fb(importer, &scratch, &fb);
> > +		import_fb(importer, &fb, dmabuf_fd, scratch.pitch);
> > +		close(dmabuf_fd);
> > +
> > +		set_fb(&fb, &display, output);
> > +		igt_pipe_crc_collect_crc(pipe_crc,
> > &colors[i].prime_crc);
> > +		colors[i].prime_str =
> > igt_crc_to_string(&colors[i].prime_crc);
> > +		igt_debug("Prime CRC for %#08x is %s\n",
> > +			  colors[i].color, colors[i].prime_str);
> > +		igt_remove_fb(importer, &fb);
> > +
> > +		igt_create_color_fb(importer,
> > +				    mode->hdisplay, mode->vdisplay,
> > +				    DRM_FORMAT_XRGB8888,
> > LOCAL_DRM_FORMAT_MOD_NONE,
> > +				    colors[i].r, colors[i].g,
> > colors[i].b,
> > +				    &fb);
> > +
> > +		set_fb(&fb, &display, output);
> > +		igt_pipe_crc_collect_crc(pipe_crc,
> > &colors[i].direct_crc);
> > +		colors[i].direct_str =
> > igt_crc_to_string(&colors[i].direct_crc);
> > +		igt_debug("Direct CRC for %#08x is %s\n",
> > +			  colors[i].color, colors[i].direct_str);
> > +		igt_remove_fb(importer, &fb);
> > +	}
> > +	igt_pipe_crc_free(pipe_crc);
> > +
> > +	igt_debug("CRC table:\n");
> > +	igt_debug("Color\t\tPrime\t\tDirect\n");
> > +	for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > +		igt_debug("%#08x\t%.8s\t%.8s\n", colors[i].color,
> > +			  colors[i].prime_str, colors[i].direct_str);
> > +	}
> > +
> > +	for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > +		for (j = 0; j < ARRAY_SIZE(colors); j++) {
> > +			if (i == j) {
> > +				igt_assert_crc_equal(&colors[i].prime_c
> > rc,
> > +						     &colors[j].direct_
> > crc);
> > +			} else {
> > +				crc_equal =
> > igt_check_crc_equal(&colors[i].prime_crc,
> > +								&colors
> > [j].direct_crc);
> > +				igt_assert_f(!crc_equal, "CRC should be
> > different");
> > +			}
> > +		}
> > +	}
> > +}
> > +
> > +static void run_test_crc(int export_chipset, int import_chipset)
> > +{
> > +	int importer = -1;
> > +	int exporter = -1;
> > +
> > +	exporter = drm_open_driver(export_chipset);
> > +	importer = drm_open_driver_master(import_chipset);
> > +
> > +	igt_require(has_prime_export(exporter));
> > +	igt_require(has_prime_import(importer));
> > +	igt_require_pipe_crc(importer);
> > +
> > +	test_crc(exporter, importer);
> > +	close(importer);
> > +	close(exporter);
> > +}
> > +
> > +igt_main
> > +{
> > +	igt_fixture {
> > +		kmstest_set_vt_graphics_mode();
> > +	}
> > +	igt_subtest("crc-vgem-vkms")
> > +		run_test_crc(DRIVER_VGEM, DRIVER_VKMS);
> > +	igt_subtest("crc-i915-vkms")
> > +		run_test_crc(DRIVER_INTEL, DRIVER_VKMS);
> > +	igt_subtest("crc-vgem-i915")
> > +		run_test_crc(DRIVER_VGEM, DRIVER_INTEL);
> > +	igt_subtest("crc-amd-i915")
> > +		run_test_crc(DRIVER_AMDGPU, DRIVER_INTEL);
> > +	igt_subtest("crc-i915-amd")
> > +		run_test_crc(DRIVER_INTEL, DRIVER_AMDGPU);
> 
> This isn't any more generic than what we have e.g. in prime_vgem
> already.

Here non-generic part is only those 10 lines of subtests definitions.
The rest is common code.

I guess, this separate subtest definitions can document the device
pairs the test is expected to work + we can blacklist tests which we
don't have the hardware for.

> The way to do a generic kms testcase is to use DRIVER_ANY (yes some
> of the
> earlier conversion unfortunately used stuff like DRIVER_INTEL |
> DRIVER_AMDGPU). With that you also don't need your library prep
> patch,
> because no need for a DRIVER_VKMS.

But how can we open two different devices with DRIVER_ANY? If I put

fd = drm_open_driver(DRIVER_ANY)
fd2 = drm_open_driver(DRIVER_ANY)

I get the same device opened two times, do I?

> 
> Imo best to start out by converting prime_vgem.c for this, and adding
> for
> the i915 specific tests (but _only_ for those) a specific "is this
> i915"
> check, so we skip those tests. And replace DRIVER_INTEL by
> DRIVER_ANY.
> Well, anything except vgem :-)

If I am not mistaken, all tests in prime_vgem are intel-specific :D

Some ideas can be taken from it, but anyway, it probably needs a deep
rewrite.

And prime_vgem use only vgem as an exporter device. Here we can use any
device. Sure, some tests would eventually use vgem-specific features,
but some could remain more generic.

Oleg

> 
> Also the crc testcases should be 100% generic.
> -Daniel

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3261 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH 2/2] tests/prime_generic: add vendor-agnostic prime tests

[Daniel Vetter]

On Thu, Jul 11, 2019 at 11:09 AM Vasilev, Oleg <oleg.vasilev@intel.com> wrote:
>
> On Wed, 2019-07-10 at 18:35 +0200, Daniel Vetter wrote:
> > On Thu, Jul 04, 2019 at 12:56:04PM +0300, Oleg Vasilev wrote:
> > > Currently, we have different sets of prime tests:
> > >  - vgem+i915
> > >  - amdgpu+i915
> > >  - nv+i915
> > >
> > > Those tests use vendor-specific ioctls, therefore, not
> > > interchangeable.
> > > The idea is to create a set of tests which are expected to work on
> > > any
> > > prime-compatible driver. It can be run with any combination of
> > > exporter+importer, where
> > >
> > > The first test is simple:
> > > 1. Exporter creates a framebuffer and fills it with a plain color
> > > 2. Fb is transferred to the importer
> > > 3. Importer modesets and computes pipe CRC
> > > 4. Importer draws the same color through cairo and compares CRC
> > >
> > > The initial motivation comes from the need to test prime support in
> > > vkms.
> > >
> > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> > > ---
> > >  tests/meson.build     |   1 +
> > >  tests/prime_generic.c | 238
> > > ++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 239 insertions(+)
> > >  create mode 100644 tests/prime_generic.c
> > >
> > > diff --git a/tests/meson.build b/tests/meson.build
> > > index 34a74025..1c938e95 100644
> > > --- a/tests/meson.build
> > > +++ b/tests/meson.build
> > > @@ -76,6 +76,7 @@ test_progs = [
> > >     'prime_self_import',
> > >     'prime_udl',
> > >     'prime_vgem',
> > > +   'prime_generic',
> > >     'syncobj_basic',
> > >     'syncobj_wait',
> > >     'template',
> > > diff --git a/tests/prime_generic.c b/tests/prime_generic.c
> > > new file mode 100644
> > > index 00000000..65e48763
> > > --- /dev/null
> > > +++ b/tests/prime_generic.c
> > > @@ -0,0 +1,238 @@
> > > +/*
> > > + * Copyright © 2019 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.
> > > + */
> > > +
> > > +#include "igt.h"
> > > +#include "igt_vgem.h"
> > > +
> > > +#include <sys/ioctl.h>
> > > +#include <sys/poll.h>
> > > +#include <time.h>
> > > +
> > > +static struct {
> > > +   double r, g, b;
> > > +   uint32_t color;
> > > +   igt_crc_t prime_crc;
> > > +   igt_crc_t direct_crc;
> > > +   char *prime_str;
> > > +   char *direct_str;
> > > +} colors[3] = {
> > > +   { .r = 0.0, .g = 0.0, .b = 0.0, .color = 0xff000000},
> > > +   { .r = 1.0, .g = 1.0, .b = 1.0, .color = 0xffffffff},
> > > +   { .r = 1.0, .g = 0.0, .b = 0.0, .color = 0xffff0000},
> > > +};
> > > +
> > > +IGT_TEST_DESCRIPTION("Generic tests, working with any prime
> > > device");
> > > +
> > > +static bool has_prime_import(int fd)
> > > +{
> > > +   uint64_t value;
> > > +
> > > +   if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> > > +           return false;
> > > +
> > > +   return value & DRM_PRIME_CAP_IMPORT;
> > > +}
> > > +
> > > +static bool has_prime_export(int fd)
> > > +{
> > > +   uint64_t value;
> > > +
> > > +   if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> > > +           return false;
> > > +
> > > +   return value & DRM_PRIME_CAP_EXPORT;
> > > +}
> > > +
> > > +static void prepare_scratch(int exporter, struct vgem_bo *scratch,
> > > +                       drmModeModeInfo *mode, uint32_t color)
> > > +{
> > > +   int i;
> > > +   uint32_t *ptr;
> > > +
> > > +   scratch->width = mode->hdisplay;
> > > +   scratch->height = mode->vdisplay;
> > > +   scratch->bpp = 32;
> > > +   vgem_create(exporter, scratch);
> > > +
> > > +   ptr = vgem_mmap(exporter, scratch, PROT_WRITE);
> > > +   for (i = 0; i < scratch->size / sizeof(*ptr); ++i)
> > > +           ptr[i] = color;
> > > +
> > > +   munmap(ptr, scratch->size);
> > > +}
> > > +
> > > +static void prepare_fb(int importer, struct vgem_bo *scratch,
> > > struct igt_fb *fb)
> > > +{
> > > +   enum igt_color_encoding color_encoding = IGT_COLOR_YCBCR_BT709;
> > > +   enum igt_color_range color_range =
> > > IGT_COLOR_YCBCR_LIMITED_RANGE;
> > > +
> > > +   igt_init_fb(fb, importer, scratch->width, scratch->height,
> > > +               DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> > > +               color_encoding, color_range);
> > > +}
> > > +
> > > +static void import_fb(int importer, struct igt_fb *fb,
> > > +                 int dmabuf_fd, uint32_t pitch)
> > > +{
> > > +   uint32_t offsets[4], pitches[4], handles[4];
> > > +   int ret;
> > > +
> > > +   fb->gem_handle = prime_fd_to_handle(importer, dmabuf_fd);
> > > +
> > > +   handles[0] = fb->gem_handle;
> > > +   pitches[0] = pitch;
> > > +   offsets[0] = 0;
> > > +
> > > +   ret = drmModeAddFB2(importer, fb->width, fb->height,
> > > +                       DRM_FORMAT_XRGB8888,
> > > +                       handles, pitches, offsets,
> > > +                       &fb->fb_id, 0);
> > > +   igt_assert(ret == 0);
> > > +}
> > > +
> > > +static void set_fb(struct igt_fb *fb,
> > > +              igt_display_t *display,
> > > +              igt_output_t *output)
> > > +{
> > > +   igt_plane_t *primary;
> > > +   int ret;
> > > +
> > > +   primary = igt_output_get_plane(output, 0);
> > > +   igt_plane_set_fb(primary, fb);
> > > +   ret = igt_display_commit(display);
> > > +
> > > +   igt_assert(ret == 0);
> > > +}
> > > +
> > > +static void test_crc(int exporter, int importer)
> > > +{
> > > +   enum pipe pipe = PIPE_A;
> > > +   igt_display_t display;
> > > +   igt_output_t *output;
> > > +   igt_pipe_crc_t *pipe_crc;
> > > +   struct igt_fb fb;
> > > +   int dmabuf_fd;
> > > +   struct vgem_bo scratch; /* despite the name, it suits for any
> > > +                              gem-compatible device */
> > > +   int i, j;
> > > +   drmModeModeInfo *mode;
> > > +
> > > +   bool crc_equal;
> > > +
> > > +   igt_display_require(&display, importer);
> > > +   igt_skip_on(pipe >= display.n_pipes);
> > > +   output = igt_get_single_output_for_pipe(&display, pipe);
> > > +   igt_require_f(output, "No connector found for pipe %s\n",
> > > +                 kmstest_pipe_name(pipe));
> > > +
> > > +   igt_display_reset(&display);
> > > +   igt_output_set_pipe(output, pipe);
> > > +
> > > +   mode = igt_output_get_mode(output);
> > > +   pipe_crc = igt_pipe_crc_new(importer, pipe,
> > > INTEL_PIPE_CRC_SOURCE_AUTO);
> > > +
> > > +   for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > > +           prepare_scratch(exporter, &scratch, mode,
> > > colors[i].color);
> > > +           dmabuf_fd = prime_handle_to_fd(exporter,
> > > scratch.handle);
> > > +           gem_close(exporter, scratch.handle);
> > > +
> > > +           prepare_fb(importer, &scratch, &fb);
> > > +           import_fb(importer, &fb, dmabuf_fd, scratch.pitch);
> > > +           close(dmabuf_fd);
> > > +
> > > +           set_fb(&fb, &display, output);
> > > +           igt_pipe_crc_collect_crc(pipe_crc,
> > > &colors[i].prime_crc);
> > > +           colors[i].prime_str =
> > > igt_crc_to_string(&colors[i].prime_crc);
> > > +           igt_debug("Prime CRC for %#08x is %s\n",
> > > +                     colors[i].color, colors[i].prime_str);
> > > +           igt_remove_fb(importer, &fb);
> > > +
> > > +           igt_create_color_fb(importer,
> > > +                               mode->hdisplay, mode->vdisplay,
> > > +                               DRM_FORMAT_XRGB8888,
> > > LOCAL_DRM_FORMAT_MOD_NONE,
> > > +                               colors[i].r, colors[i].g,
> > > colors[i].b,
> > > +                               &fb);
> > > +
> > > +           set_fb(&fb, &display, output);
> > > +           igt_pipe_crc_collect_crc(pipe_crc,
> > > &colors[i].direct_crc);
> > > +           colors[i].direct_str =
> > > igt_crc_to_string(&colors[i].direct_crc);
> > > +           igt_debug("Direct CRC for %#08x is %s\n",
> > > +                     colors[i].color, colors[i].direct_str);
> > > +           igt_remove_fb(importer, &fb);
> > > +   }
> > > +   igt_pipe_crc_free(pipe_crc);
> > > +
> > > +   igt_debug("CRC table:\n");
> > > +   igt_debug("Color\t\tPrime\t\tDirect\n");
> > > +   for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > > +           igt_debug("%#08x\t%.8s\t%.8s\n", colors[i].color,
> > > +                     colors[i].prime_str, colors[i].direct_str);
> > > +   }
> > > +
> > > +   for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > > +           for (j = 0; j < ARRAY_SIZE(colors); j++) {
> > > +                   if (i == j) {
> > > +                           igt_assert_crc_equal(&colors[i].prime_c
> > > rc,
> > > +                                                &colors[j].direct_
> > > crc);
> > > +                   } else {
> > > +                           crc_equal =
> > > igt_check_crc_equal(&colors[i].prime_crc,
> > > +                                                           &colors
> > > [j].direct_crc);
> > > +                           igt_assert_f(!crc_equal, "CRC should be
> > > different");
> > > +                   }
> > > +           }
> > > +   }
> > > +}
> > > +
> > > +static void run_test_crc(int export_chipset, int import_chipset)
> > > +{
> > > +   int importer = -1;
> > > +   int exporter = -1;
> > > +
> > > +   exporter = drm_open_driver(export_chipset);
> > > +   importer = drm_open_driver_master(import_chipset);
> > > +
> > > +   igt_require(has_prime_export(exporter));
> > > +   igt_require(has_prime_import(importer));
> > > +   igt_require_pipe_crc(importer);
> > > +
> > > +   test_crc(exporter, importer);
> > > +   close(importer);
> > > +   close(exporter);
> > > +}
> > > +
> > > +igt_main
> > > +{
> > > +   igt_fixture {
> > > +           kmstest_set_vt_graphics_mode();
> > > +   }
> > > +   igt_subtest("crc-vgem-vkms")
> > > +           run_test_crc(DRIVER_VGEM, DRIVER_VKMS);
> > > +   igt_subtest("crc-i915-vkms")
> > > +           run_test_crc(DRIVER_INTEL, DRIVER_VKMS);
> > > +   igt_subtest("crc-vgem-i915")
> > > +           run_test_crc(DRIVER_VGEM, DRIVER_INTEL);
> > > +   igt_subtest("crc-amd-i915")
> > > +           run_test_crc(DRIVER_AMDGPU, DRIVER_INTEL);
> > > +   igt_subtest("crc-i915-amd")
> > > +           run_test_crc(DRIVER_INTEL, DRIVER_AMDGPU);
> >
> > This isn't any more generic than what we have e.g. in prime_vgem
> > already.
>
> Here non-generic part is only those 10 lines of subtests definitions.
> The rest is common code.
>
> I guess, this separate subtest definitions can document the device
> pairs the test is expected to work + we can blacklist tests which we
> don't have the hardware for.

But we already have tests for specific pairs, which can also test
specific stuff. In general we don't do tests with -gen9, gen11, -gen8
prefixes either. This here would be a first I'd say.

> > The way to do a generic kms testcase is to use DRIVER_ANY (yes some
> > of the
> > earlier conversion unfortunately used stuff like DRIVER_INTEL |
> > DRIVER_AMDGPU). With that you also don't need your library prep
> > patch,
> > because no need for a DRIVER_VKMS.
>
> But how can we open two different devices with DRIVER_ANY? If I put
>
> fd = drm_open_driver(DRIVER_ANY)
> fd2 = drm_open_driver(DRIVER_ANY)
>
> I get the same device opened two times, do I?

One of them would be DRIVER_VGEM. And I think we already make sure
that you don't accidentally get vgem for DRIVER_ANY. Otherwise I guess
we'd need a DRIVER_ANY_BUT_VGEM. Well really what we want is
DRIVER_ANY_KMS here, which would also exclude vgem. So if it doesn't
work already, small patch needed in lib/, but DRIVER_VKMS also needs
that. And I really don't want more DRIVER_FOO defines for a plain old
kms-only driver. kms is supposed to be a shared cross-driver standard,
if we can't even write shared testcases it's just epic fail :-)

> > Imo best to start out by converting prime_vgem.c for this, and adding
> > for
> > the i915 specific tests (but _only_ for those) a specific "is this
> > i915"
> > check, so we skip those tests. And replace DRIVER_INTEL by
> > DRIVER_ANY.
> > Well, anything except vgem :-)
>
> If I am not mistaken, all tests in prime_vgem are intel-specific :D
>
> Some ideas can be taken from it, but anyway, it probably needs a deep
> rewrite.
>
> And prime_vgem use only vgem as an exporter device. Here we can use any
> device. Sure, some tests would eventually use vgem-specific features,
> but some could remain more generic.

Maybe I wasn't all that clear then, I think what we want to test in a
generic prime test is vgem+DRIVER_ANY_KMS. Then drivers can test the
render side using DRIVER_FOO+vgem. This way you only need to have 1
generic kms test, plus N tests for all the rendering drivers. Instead
of M x N testcases like you're proposing here. vgem is just the
abstraction layer/mocking layer we use to separate these two worlds.
And of course for specific sharing you can still do that, e.g. to test
dma engine compatibility between amdgpu and i915, or nouveau and i915.
But those tests don't need to involve kms.

And yeah prime_vgem might need to be split into prime_i915_vgem and
prime_kms_vgem. But all the display related testcases should be
possible to extract into such a prime_kms_vgem framework.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH 2/2] tests/prime_generic: add vendor-agnostic prime tests

[Ser, Simon]

On Thu, 2019-07-11 at 14:36 +0200, Daniel Vetter wrote:
> On Thu, Jul 11, 2019 at 11:09 AM Vasilev, Oleg <oleg.vasilev@intel.com> wrote:
> > On Wed, 2019-07-10 at 18:35 +0200, Daniel Vetter wrote:
> > > On Thu, Jul 04, 2019 at 12:56:04PM +0300, Oleg Vasilev wrote:
> > > > Currently, we have different sets of prime tests:
> > > >  - vgem+i915
> > > >  - amdgpu+i915
> > > >  - nv+i915
> > > > 
> > > > Those tests use vendor-specific ioctls, therefore, not
> > > > interchangeable.
> > > > The idea is to create a set of tests which are expected to work on
> > > > any
> > > > prime-compatible driver. It can be run with any combination of
> > > > exporter+importer, where
> > > > 
> > > > The first test is simple:
> > > > 1. Exporter creates a framebuffer and fills it with a plain color
> > > > 2. Fb is transferred to the importer
> > > > 3. Importer modesets and computes pipe CRC
> > > > 4. Importer draws the same color through cairo and compares CRC
> > > > 
> > > > The initial motivation comes from the need to test prime support in
> > > > vkms.
> > > > 
> > > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> > > > ---
> > > >  tests/meson.build     |   1 +
> > > >  tests/prime_generic.c | 238
> > > > ++++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 239 insertions(+)
> > > >  create mode 100644 tests/prime_generic.c
> > > > 
> > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > index 34a74025..1c938e95 100644
> > > > --- a/tests/meson.build
> > > > +++ b/tests/meson.build
> > > > @@ -76,6 +76,7 @@ test_progs = [
> > > >     'prime_self_import',
> > > >     'prime_udl',
> > > >     'prime_vgem',
> > > > +   'prime_generic',
> > > >     'syncobj_basic',
> > > >     'syncobj_wait',
> > > >     'template',
> > > > diff --git a/tests/prime_generic.c b/tests/prime_generic.c
> > > > new file mode 100644
> > > > index 00000000..65e48763
> > > > --- /dev/null
> > > > +++ b/tests/prime_generic.c
> > > > @@ -0,0 +1,238 @@
> > > > +/*
> > > > + * Copyright © 2019 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.
> > > > + */
> > > > +
> > > > +#include "igt.h"
> > > > +#include "igt_vgem.h"
> > > > +
> > > > +#include <sys/ioctl.h>
> > > > +#include <sys/poll.h>
> > > > +#include <time.h>
> > > > +
> > > > +static struct {
> > > > +   double r, g, b;
> > > > +   uint32_t color;
> > > > +   igt_crc_t prime_crc;
> > > > +   igt_crc_t direct_crc;
> > > > +   char *prime_str;
> > > > +   char *direct_str;
> > > > +} colors[3] = {
> > > > +   { .r = 0.0, .g = 0.0, .b = 0.0, .color = 0xff000000},
> > > > +   { .r = 1.0, .g = 1.0, .b = 1.0, .color = 0xffffffff},
> > > > +   { .r = 1.0, .g = 0.0, .b = 0.0, .color = 0xffff0000},
> > > > +};
> > > > +
> > > > +IGT_TEST_DESCRIPTION("Generic tests, working with any prime
> > > > device");
> > > > +
> > > > +static bool has_prime_import(int fd)
> > > > +{
> > > > +   uint64_t value;
> > > > +
> > > > +   if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> > > > +           return false;
> > > > +
> > > > +   return value & DRM_PRIME_CAP_IMPORT;
> > > > +}
> > > > +
> > > > +static bool has_prime_export(int fd)
> > > > +{
> > > > +   uint64_t value;
> > > > +
> > > > +   if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> > > > +           return false;
> > > > +
> > > > +   return value & DRM_PRIME_CAP_EXPORT;
> > > > +}
> > > > +
> > > > +static void prepare_scratch(int exporter, struct vgem_bo *scratch,
> > > > +                       drmModeModeInfo *mode, uint32_t color)
> > > > +{
> > > > +   int i;
> > > > +   uint32_t *ptr;
> > > > +
> > > > +   scratch->width = mode->hdisplay;
> > > > +   scratch->height = mode->vdisplay;
> > > > +   scratch->bpp = 32;
> > > > +   vgem_create(exporter, scratch);
> > > > +
> > > > +   ptr = vgem_mmap(exporter, scratch, PROT_WRITE);
> > > > +   for (i = 0; i < scratch->size / sizeof(*ptr); ++i)
> > > > +           ptr[i] = color;
> > > > +
> > > > +   munmap(ptr, scratch->size);
> > > > +}
> > > > +
> > > > +static void prepare_fb(int importer, struct vgem_bo *scratch,
> > > > struct igt_fb *fb)
> > > > +{
> > > > +   enum igt_color_encoding color_encoding = IGT_COLOR_YCBCR_BT709;
> > > > +   enum igt_color_range color_range =
> > > > IGT_COLOR_YCBCR_LIMITED_RANGE;
> > > > +
> > > > +   igt_init_fb(fb, importer, scratch->width, scratch->height,
> > > > +               DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> > > > +               color_encoding, color_range);
> > > > +}
> > > > +
> > > > +static void import_fb(int importer, struct igt_fb *fb,
> > > > +                 int dmabuf_fd, uint32_t pitch)
> > > > +{
> > > > +   uint32_t offsets[4], pitches[4], handles[4];
> > > > +   int ret;
> > > > +
> > > > +   fb->gem_handle = prime_fd_to_handle(importer, dmabuf_fd);
> > > > +
> > > > +   handles[0] = fb->gem_handle;
> > > > +   pitches[0] = pitch;
> > > > +   offsets[0] = 0;
> > > > +
> > > > +   ret = drmModeAddFB2(importer, fb->width, fb->height,
> > > > +                       DRM_FORMAT_XRGB8888,
> > > > +                       handles, pitches, offsets,
> > > > +                       &fb->fb_id, 0);
> > > > +   igt_assert(ret == 0);
> > > > +}
> > > > +
> > > > +static void set_fb(struct igt_fb *fb,
> > > > +              igt_display_t *display,
> > > > +              igt_output_t *output)
> > > > +{
> > > > +   igt_plane_t *primary;
> > > > +   int ret;
> > > > +
> > > > +   primary = igt_output_get_plane(output, 0);
> > > > +   igt_plane_set_fb(primary, fb);
> > > > +   ret = igt_display_commit(display);
> > > > +
> > > > +   igt_assert(ret == 0);
> > > > +}
> > > > +
> > > > +static void test_crc(int exporter, int importer)
> > > > +{
> > > > +   enum pipe pipe = PIPE_A;
> > > > +   igt_display_t display;
> > > > +   igt_output_t *output;
> > > > +   igt_pipe_crc_t *pipe_crc;
> > > > +   struct igt_fb fb;
> > > > +   int dmabuf_fd;
> > > > +   struct vgem_bo scratch; /* despite the name, it suits for any
> > > > +                              gem-compatible device */
> > > > +   int i, j;
> > > > +   drmModeModeInfo *mode;
> > > > +
> > > > +   bool crc_equal;
> > > > +
> > > > +   igt_display_require(&display, importer);
> > > > +   igt_skip_on(pipe >= display.n_pipes);
> > > > +   output = igt_get_single_output_for_pipe(&display, pipe);
> > > > +   igt_require_f(output, "No connector found for pipe %s\n",
> > > > +                 kmstest_pipe_name(pipe));
> > > > +
> > > > +   igt_display_reset(&display);
> > > > +   igt_output_set_pipe(output, pipe);
> > > > +
> > > > +   mode = igt_output_get_mode(output);
> > > > +   pipe_crc = igt_pipe_crc_new(importer, pipe,
> > > > INTEL_PIPE_CRC_SOURCE_AUTO);
> > > > +
> > > > +   for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > > > +           prepare_scratch(exporter, &scratch, mode,
> > > > colors[i].color);
> > > > +           dmabuf_fd = prime_handle_to_fd(exporter,
> > > > scratch.handle);
> > > > +           gem_close(exporter, scratch.handle);
> > > > +
> > > > +           prepare_fb(importer, &scratch, &fb);
> > > > +           import_fb(importer, &fb, dmabuf_fd, scratch.pitch);
> > > > +           close(dmabuf_fd);
> > > > +
> > > > +           set_fb(&fb, &display, output);
> > > > +           igt_pipe_crc_collect_crc(pipe_crc,
> > > > &colors[i].prime_crc);
> > > > +           colors[i].prime_str =
> > > > igt_crc_to_string(&colors[i].prime_crc);
> > > > +           igt_debug("Prime CRC for %#08x is %s\n",
> > > > +                     colors[i].color, colors[i].prime_str);
> > > > +           igt_remove_fb(importer, &fb);
> > > > +
> > > > +           igt_create_color_fb(importer,
> > > > +                               mode->hdisplay, mode->vdisplay,
> > > > +                               DRM_FORMAT_XRGB8888,
> > > > LOCAL_DRM_FORMAT_MOD_NONE,
> > > > +                               colors[i].r, colors[i].g,
> > > > colors[i].b,
> > > > +                               &fb);
> > > > +
> > > > +           set_fb(&fb, &display, output);
> > > > +           igt_pipe_crc_collect_crc(pipe_crc,
> > > > &colors[i].direct_crc);
> > > > +           colors[i].direct_str =
> > > > igt_crc_to_string(&colors[i].direct_crc);
> > > > +           igt_debug("Direct CRC for %#08x is %s\n",
> > > > +                     colors[i].color, colors[i].direct_str);
> > > > +           igt_remove_fb(importer, &fb);
> > > > +   }
> > > > +   igt_pipe_crc_free(pipe_crc);
> > > > +
> > > > +   igt_debug("CRC table:\n");
> > > > +   igt_debug("Color\t\tPrime\t\tDirect\n");
> > > > +   for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > > > +           igt_debug("%#08x\t%.8s\t%.8s\n", colors[i].color,
> > > > +                     colors[i].prime_str, colors[i].direct_str);
> > > > +   }
> > > > +
> > > > +   for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > > > +           for (j = 0; j < ARRAY_SIZE(colors); j++) {
> > > > +                   if (i == j) {
> > > > +                           igt_assert_crc_equal(&colors[i].prime_c
> > > > rc,
> > > > +                                                &colors[j].direct_
> > > > crc);
> > > > +                   } else {
> > > > +                           crc_equal =
> > > > igt_check_crc_equal(&colors[i].prime_crc,
> > > > +                                                           &colors
> > > > [j].direct_crc);
> > > > +                           igt_assert_f(!crc_equal, "CRC should be
> > > > different");
> > > > +                   }
> > > > +           }
> > > > +   }
> > > > +}
> > > > +
> > > > +static void run_test_crc(int export_chipset, int import_chipset)
> > > > +{
> > > > +   int importer = -1;
> > > > +   int exporter = -1;
> > > > +
> > > > +   exporter = drm_open_driver(export_chipset);
> > > > +   importer = drm_open_driver_master(import_chipset);
> > > > +
> > > > +   igt_require(has_prime_export(exporter));
> > > > +   igt_require(has_prime_import(importer));
> > > > +   igt_require_pipe_crc(importer);
> > > > +
> > > > +   test_crc(exporter, importer);
> > > > +   close(importer);
> > > > +   close(exporter);
> > > > +}
> > > > +
> > > > +igt_main
> > > > +{
> > > > +   igt_fixture {
> > > > +           kmstest_set_vt_graphics_mode();
> > > > +   }
> > > > +   igt_subtest("crc-vgem-vkms")
> > > > +           run_test_crc(DRIVER_VGEM, DRIVER_VKMS);
> > > > +   igt_subtest("crc-i915-vkms")
> > > > +           run_test_crc(DRIVER_INTEL, DRIVER_VKMS);
> > > > +   igt_subtest("crc-vgem-i915")
> > > > +           run_test_crc(DRIVER_VGEM, DRIVER_INTEL);
> > > > +   igt_subtest("crc-amd-i915")
> > > > +           run_test_crc(DRIVER_AMDGPU, DRIVER_INTEL);
> > > > +   igt_subtest("crc-i915-amd")
> > > > +           run_test_crc(DRIVER_INTEL, DRIVER_AMDGPU);
> > > 
> > > This isn't any more generic than what we have e.g. in prime_vgem
> > > already.
> > 
> > Here non-generic part is only those 10 lines of subtests definitions.
> > The rest is common code.
> > 
> > I guess, this separate subtest definitions can document the device
> > pairs the test is expected to work + we can blacklist tests which we
> > don't have the hardware for.
> 
> But we already have tests for specific pairs, which can also test
> specific stuff. In general we don't do tests with -gen9, gen11, -gen8
> prefixes either. This here would be a first I'd say.
> 
> > > The way to do a generic kms testcase is to use DRIVER_ANY (yes some
> > > of the
> > > earlier conversion unfortunately used stuff like DRIVER_INTEL |
> > > DRIVER_AMDGPU). With that you also don't need your library prep
> > > patch,
> > > because no need for a DRIVER_VKMS.
> > 
> > But how can we open two different devices with DRIVER_ANY? If I put
> > 
> > fd = drm_open_driver(DRIVER_ANY)
> > fd2 = drm_open_driver(DRIVER_ANY)
> > 
> > I get the same device opened two times, do I?
> 
> One of them would be DRIVER_VGEM. And I think we already make sure
> that you don't accidentally get vgem for DRIVER_ANY. Otherwise I guess
> we'd need a DRIVER_ANY_BUT_VGEM. Well really what we want is
> DRIVER_ANY_KMS here, which would also exclude vgem. So if it doesn't
> work already, small patch needed in lib/, but DRIVER_VKMS also needs
> that. And I really don't want more DRIVER_FOO defines for a plain old
> kms-only driver. kms is supposed to be a shared cross-driver standard,
> if we can't even write shared testcases it's just epic fail :-)
> 
> > > Imo best to start out by converting prime_vgem.c for this, and adding
> > > for
> > > the i915 specific tests (but _only_ for those) a specific "is this
> > > i915"
> > > check, so we skip those tests. And replace DRIVER_INTEL by
> > > DRIVER_ANY.
> > > Well, anything except vgem :-)
> > 
> > If I am not mistaken, all tests in prime_vgem are intel-specific :D
> > 
> > Some ideas can be taken from it, but anyway, it probably needs a deep
> > rewrite.
> > 
> > And prime_vgem use only vgem as an exporter device. Here we can use any
> > device. Sure, some tests would eventually use vgem-specific features,
> > but some could remain more generic.
> 
> Maybe I wasn't all that clear then, I think what we want to test in a
> generic prime test is vgem+DRIVER_ANY_KMS. Then drivers can test the
> render side using DRIVER_FOO+vgem. This way you only need to have 1
> generic kms test, plus N tests for all the rendering drivers. Instead
> of M x N testcases like you're proposing here. vgem is just the
> abstraction layer/mocking layer we use to separate these two worlds.
> And of course for specific sharing you can still do that, e.g. to test
> dma engine compatibility between amdgpu and i915, or nouveau and i915.
> But those tests don't need to involve kms.
> 
> And yeah prime_vgem might need to be split into prime_i915_vgem and
> prime_kms_vgem. But all the display related testcases should be
> possible to extract into such a prime_kms_vgem framework.

One could also want to test drmPrimeHandleToFD and drmPrimeFDToHandle
between e.g. amdgpu and i915. I guess it's using a different code path
than gem.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH 2/2] tests/prime_generic: add vendor-agnostic prime tests

[Daniel Vetter]

On Thu, Jul 11, 2019 at 03:20:52PM +0000, Ser, Simon wrote:
> On Thu, 2019-07-11 at 14:36 +0200, Daniel Vetter wrote:
> > On Thu, Jul 11, 2019 at 11:09 AM Vasilev, Oleg <oleg.vasilev@intel.com> wrote:
> > > On Wed, 2019-07-10 at 18:35 +0200, Daniel Vetter wrote:
> > > > On Thu, Jul 04, 2019 at 12:56:04PM +0300, Oleg Vasilev wrote:
> > > > > Currently, we have different sets of prime tests:
> > > > >  - vgem+i915
> > > > >  - amdgpu+i915
> > > > >  - nv+i915
> > > > > 
> > > > > Those tests use vendor-specific ioctls, therefore, not
> > > > > interchangeable.
> > > > > The idea is to create a set of tests which are expected to work on
> > > > > any
> > > > > prime-compatible driver. It can be run with any combination of
> > > > > exporter+importer, where
> > > > > 
> > > > > The first test is simple:
> > > > > 1. Exporter creates a framebuffer and fills it with a plain color
> > > > > 2. Fb is transferred to the importer
> > > > > 3. Importer modesets and computes pipe CRC
> > > > > 4. Importer draws the same color through cairo and compares CRC
> > > > > 
> > > > > The initial motivation comes from the need to test prime support in
> > > > > vkms.
> > > > > 
> > > > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > > Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> > > > > ---
> > > > >  tests/meson.build     |   1 +
> > > > >  tests/prime_generic.c | 238
> > > > > ++++++++++++++++++++++++++++++++++++++++++
> > > > >  2 files changed, 239 insertions(+)
> > > > >  create mode 100644 tests/prime_generic.c
> > > > > 
> > > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > > index 34a74025..1c938e95 100644
> > > > > --- a/tests/meson.build
> > > > > +++ b/tests/meson.build
> > > > > @@ -76,6 +76,7 @@ test_progs = [
> > > > >     'prime_self_import',
> > > > >     'prime_udl',
> > > > >     'prime_vgem',
> > > > > +   'prime_generic',
> > > > >     'syncobj_basic',
> > > > >     'syncobj_wait',
> > > > >     'template',
> > > > > diff --git a/tests/prime_generic.c b/tests/prime_generic.c
> > > > > new file mode 100644
> > > > > index 00000000..65e48763
> > > > > --- /dev/null
> > > > > +++ b/tests/prime_generic.c
> > > > > @@ -0,0 +1,238 @@
> > > > > +/*
> > > > > + * Copyright © 2019 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.
> > > > > + */
> > > > > +
> > > > > +#include "igt.h"
> > > > > +#include "igt_vgem.h"
> > > > > +
> > > > > +#include <sys/ioctl.h>
> > > > > +#include <sys/poll.h>
> > > > > +#include <time.h>
> > > > > +
> > > > > +static struct {
> > > > > +   double r, g, b;
> > > > > +   uint32_t color;
> > > > > +   igt_crc_t prime_crc;
> > > > > +   igt_crc_t direct_crc;
> > > > > +   char *prime_str;
> > > > > +   char *direct_str;
> > > > > +} colors[3] = {
> > > > > +   { .r = 0.0, .g = 0.0, .b = 0.0, .color = 0xff000000},
> > > > > +   { .r = 1.0, .g = 1.0, .b = 1.0, .color = 0xffffffff},
> > > > > +   { .r = 1.0, .g = 0.0, .b = 0.0, .color = 0xffff0000},
> > > > > +};
> > > > > +
> > > > > +IGT_TEST_DESCRIPTION("Generic tests, working with any prime
> > > > > device");
> > > > > +
> > > > > +static bool has_prime_import(int fd)
> > > > > +{
> > > > > +   uint64_t value;
> > > > > +
> > > > > +   if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> > > > > +           return false;
> > > > > +
> > > > > +   return value & DRM_PRIME_CAP_IMPORT;
> > > > > +}
> > > > > +
> > > > > +static bool has_prime_export(int fd)
> > > > > +{
> > > > > +   uint64_t value;
> > > > > +
> > > > > +   if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> > > > > +           return false;
> > > > > +
> > > > > +   return value & DRM_PRIME_CAP_EXPORT;
> > > > > +}
> > > > > +
> > > > > +static void prepare_scratch(int exporter, struct vgem_bo *scratch,
> > > > > +                       drmModeModeInfo *mode, uint32_t color)
> > > > > +{
> > > > > +   int i;
> > > > > +   uint32_t *ptr;
> > > > > +
> > > > > +   scratch->width = mode->hdisplay;
> > > > > +   scratch->height = mode->vdisplay;
> > > > > +   scratch->bpp = 32;
> > > > > +   vgem_create(exporter, scratch);
> > > > > +
> > > > > +   ptr = vgem_mmap(exporter, scratch, PROT_WRITE);
> > > > > +   for (i = 0; i < scratch->size / sizeof(*ptr); ++i)
> > > > > +           ptr[i] = color;
> > > > > +
> > > > > +   munmap(ptr, scratch->size);
> > > > > +}
> > > > > +
> > > > > +static void prepare_fb(int importer, struct vgem_bo *scratch,
> > > > > struct igt_fb *fb)
> > > > > +{
> > > > > +   enum igt_color_encoding color_encoding = IGT_COLOR_YCBCR_BT709;
> > > > > +   enum igt_color_range color_range =
> > > > > IGT_COLOR_YCBCR_LIMITED_RANGE;
> > > > > +
> > > > > +   igt_init_fb(fb, importer, scratch->width, scratch->height,
> > > > > +               DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> > > > > +               color_encoding, color_range);
> > > > > +}
> > > > > +
> > > > > +static void import_fb(int importer, struct igt_fb *fb,
> > > > > +                 int dmabuf_fd, uint32_t pitch)
> > > > > +{
> > > > > +   uint32_t offsets[4], pitches[4], handles[4];
> > > > > +   int ret;
> > > > > +
> > > > > +   fb->gem_handle = prime_fd_to_handle(importer, dmabuf_fd);
> > > > > +
> > > > > +   handles[0] = fb->gem_handle;
> > > > > +   pitches[0] = pitch;
> > > > > +   offsets[0] = 0;
> > > > > +
> > > > > +   ret = drmModeAddFB2(importer, fb->width, fb->height,
> > > > > +                       DRM_FORMAT_XRGB8888,
> > > > > +                       handles, pitches, offsets,
> > > > > +                       &fb->fb_id, 0);
> > > > > +   igt_assert(ret == 0);
> > > > > +}
> > > > > +
> > > > > +static void set_fb(struct igt_fb *fb,
> > > > > +              igt_display_t *display,
> > > > > +              igt_output_t *output)
> > > > > +{
> > > > > +   igt_plane_t *primary;
> > > > > +   int ret;
> > > > > +
> > > > > +   primary = igt_output_get_plane(output, 0);
> > > > > +   igt_plane_set_fb(primary, fb);
> > > > > +   ret = igt_display_commit(display);
> > > > > +
> > > > > +   igt_assert(ret == 0);
> > > > > +}
> > > > > +
> > > > > +static void test_crc(int exporter, int importer)
> > > > > +{
> > > > > +   enum pipe pipe = PIPE_A;
> > > > > +   igt_display_t display;
> > > > > +   igt_output_t *output;
> > > > > +   igt_pipe_crc_t *pipe_crc;
> > > > > +   struct igt_fb fb;
> > > > > +   int dmabuf_fd;
> > > > > +   struct vgem_bo scratch; /* despite the name, it suits for any
> > > > > +                              gem-compatible device */
> > > > > +   int i, j;
> > > > > +   drmModeModeInfo *mode;
> > > > > +
> > > > > +   bool crc_equal;
> > > > > +
> > > > > +   igt_display_require(&display, importer);
> > > > > +   igt_skip_on(pipe >= display.n_pipes);
> > > > > +   output = igt_get_single_output_for_pipe(&display, pipe);
> > > > > +   igt_require_f(output, "No connector found for pipe %s\n",
> > > > > +                 kmstest_pipe_name(pipe));
> > > > > +
> > > > > +   igt_display_reset(&display);
> > > > > +   igt_output_set_pipe(output, pipe);
> > > > > +
> > > > > +   mode = igt_output_get_mode(output);
> > > > > +   pipe_crc = igt_pipe_crc_new(importer, pipe,
> > > > > INTEL_PIPE_CRC_SOURCE_AUTO);
> > > > > +
> > > > > +   for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > > > > +           prepare_scratch(exporter, &scratch, mode,
> > > > > colors[i].color);
> > > > > +           dmabuf_fd = prime_handle_to_fd(exporter,
> > > > > scratch.handle);
> > > > > +           gem_close(exporter, scratch.handle);
> > > > > +
> > > > > +           prepare_fb(importer, &scratch, &fb);
> > > > > +           import_fb(importer, &fb, dmabuf_fd, scratch.pitch);
> > > > > +           close(dmabuf_fd);
> > > > > +
> > > > > +           set_fb(&fb, &display, output);
> > > > > +           igt_pipe_crc_collect_crc(pipe_crc,
> > > > > &colors[i].prime_crc);
> > > > > +           colors[i].prime_str =
> > > > > igt_crc_to_string(&colors[i].prime_crc);
> > > > > +           igt_debug("Prime CRC for %#08x is %s\n",
> > > > > +                     colors[i].color, colors[i].prime_str);
> > > > > +           igt_remove_fb(importer, &fb);
> > > > > +
> > > > > +           igt_create_color_fb(importer,
> > > > > +                               mode->hdisplay, mode->vdisplay,
> > > > > +                               DRM_FORMAT_XRGB8888,
> > > > > LOCAL_DRM_FORMAT_MOD_NONE,
> > > > > +                               colors[i].r, colors[i].g,
> > > > > colors[i].b,
> > > > > +                               &fb);
> > > > > +
> > > > > +           set_fb(&fb, &display, output);
> > > > > +           igt_pipe_crc_collect_crc(pipe_crc,
> > > > > &colors[i].direct_crc);
> > > > > +           colors[i].direct_str =
> > > > > igt_crc_to_string(&colors[i].direct_crc);
> > > > > +           igt_debug("Direct CRC for %#08x is %s\n",
> > > > > +                     colors[i].color, colors[i].direct_str);
> > > > > +           igt_remove_fb(importer, &fb);
> > > > > +   }
> > > > > +   igt_pipe_crc_free(pipe_crc);
> > > > > +
> > > > > +   igt_debug("CRC table:\n");
> > > > > +   igt_debug("Color\t\tPrime\t\tDirect\n");
> > > > > +   for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > > > > +           igt_debug("%#08x\t%.8s\t%.8s\n", colors[i].color,
> > > > > +                     colors[i].prime_str, colors[i].direct_str);
> > > > > +   }
> > > > > +
> > > > > +   for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > > > > +           for (j = 0; j < ARRAY_SIZE(colors); j++) {
> > > > > +                   if (i == j) {
> > > > > +                           igt_assert_crc_equal(&colors[i].prime_c
> > > > > rc,
> > > > > +                                                &colors[j].direct_
> > > > > crc);
> > > > > +                   } else {
> > > > > +                           crc_equal =
> > > > > igt_check_crc_equal(&colors[i].prime_crc,
> > > > > +                                                           &colors
> > > > > [j].direct_crc);
> > > > > +                           igt_assert_f(!crc_equal, "CRC should be
> > > > > different");
> > > > > +                   }
> > > > > +           }
> > > > > +   }
> > > > > +}
> > > > > +
> > > > > +static void run_test_crc(int export_chipset, int import_chipset)
> > > > > +{
> > > > > +   int importer = -1;
> > > > > +   int exporter = -1;
> > > > > +
> > > > > +   exporter = drm_open_driver(export_chipset);
> > > > > +   importer = drm_open_driver_master(import_chipset);
> > > > > +
> > > > > +   igt_require(has_prime_export(exporter));
> > > > > +   igt_require(has_prime_import(importer));
> > > > > +   igt_require_pipe_crc(importer);
> > > > > +
> > > > > +   test_crc(exporter, importer);
> > > > > +   close(importer);
> > > > > +   close(exporter);
> > > > > +}
> > > > > +
> > > > > +igt_main
> > > > > +{
> > > > > +   igt_fixture {
> > > > > +           kmstest_set_vt_graphics_mode();
> > > > > +   }
> > > > > +   igt_subtest("crc-vgem-vkms")
> > > > > +           run_test_crc(DRIVER_VGEM, DRIVER_VKMS);
> > > > > +   igt_subtest("crc-i915-vkms")
> > > > > +           run_test_crc(DRIVER_INTEL, DRIVER_VKMS);
> > > > > +   igt_subtest("crc-vgem-i915")
> > > > > +           run_test_crc(DRIVER_VGEM, DRIVER_INTEL);
> > > > > +   igt_subtest("crc-amd-i915")
> > > > > +           run_test_crc(DRIVER_AMDGPU, DRIVER_INTEL);
> > > > > +   igt_subtest("crc-i915-amd")
> > > > > +           run_test_crc(DRIVER_INTEL, DRIVER_AMDGPU);
> > > > 
> > > > This isn't any more generic than what we have e.g. in prime_vgem
> > > > already.
> > > 
> > > Here non-generic part is only those 10 lines of subtests definitions.
> > > The rest is common code.
> > > 
> > > I guess, this separate subtest definitions can document the device
> > > pairs the test is expected to work + we can blacklist tests which we
> > > don't have the hardware for.
> > 
> > But we already have tests for specific pairs, which can also test
> > specific stuff. In general we don't do tests with -gen9, gen11, -gen8
> > prefixes either. This here would be a first I'd say.
> > 
> > > > The way to do a generic kms testcase is to use DRIVER_ANY (yes some
> > > > of the
> > > > earlier conversion unfortunately used stuff like DRIVER_INTEL |
> > > > DRIVER_AMDGPU). With that you also don't need your library prep
> > > > patch,
> > > > because no need for a DRIVER_VKMS.
> > > 
> > > But how can we open two different devices with DRIVER_ANY? If I put
> > > 
> > > fd = drm_open_driver(DRIVER_ANY)
> > > fd2 = drm_open_driver(DRIVER_ANY)
> > > 
> > > I get the same device opened two times, do I?
> > 
> > One of them would be DRIVER_VGEM. And I think we already make sure
> > that you don't accidentally get vgem for DRIVER_ANY. Otherwise I guess
> > we'd need a DRIVER_ANY_BUT_VGEM. Well really what we want is
> > DRIVER_ANY_KMS here, which would also exclude vgem. So if it doesn't
> > work already, small patch needed in lib/, but DRIVER_VKMS also needs
> > that. And I really don't want more DRIVER_FOO defines for a plain old
> > kms-only driver. kms is supposed to be a shared cross-driver standard,
> > if we can't even write shared testcases it's just epic fail :-)
> > 
> > > > Imo best to start out by converting prime_vgem.c for this, and adding
> > > > for
> > > > the i915 specific tests (but _only_ for those) a specific "is this
> > > > i915"
> > > > check, so we skip those tests. And replace DRIVER_INTEL by
> > > > DRIVER_ANY.
> > > > Well, anything except vgem :-)
> > > 
> > > If I am not mistaken, all tests in prime_vgem are intel-specific :D
> > > 
> > > Some ideas can be taken from it, but anyway, it probably needs a deep
> > > rewrite.
> > > 
> > > And prime_vgem use only vgem as an exporter device. Here we can use any
> > > device. Sure, some tests would eventually use vgem-specific features,
> > > but some could remain more generic.
> > 
> > Maybe I wasn't all that clear then, I think what we want to test in a
> > generic prime test is vgem+DRIVER_ANY_KMS. Then drivers can test the
> > render side using DRIVER_FOO+vgem. This way you only need to have 1
> > generic kms test, plus N tests for all the rendering drivers. Instead
> > of M x N testcases like you're proposing here. vgem is just the
> > abstraction layer/mocking layer we use to separate these two worlds.
> > And of course for specific sharing you can still do that, e.g. to test
> > dma engine compatibility between amdgpu and i915, or nouveau and i915.
> > But those tests don't need to involve kms.
> > 
> > And yeah prime_vgem might need to be split into prime_i915_vgem and
> > prime_kms_vgem. But all the display related testcases should be
> > possible to extract into such a prime_kms_vgem framework.
> 
> One could also want to test drmPrimeHandleToFD and drmPrimeFDToHandle
> between e.g. amdgpu and i915. I guess it's using a different code path
> than gem.

Yeah, but only if you also do some i915 or amdgpu specific command
submission. Otherwise if it's all cpu-only, then you wont hit any
additional path than what you can test with vgem already. Kinda my point
really, there's no room really for a "generic" i915+amdgpu prime test. As
soon as you look at that combo, it's all about specifics.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH 2/2] tests/prime_generic: add vendor-agnostic prime tests

[Vasilev, Oleg]

[-- Attachment #1.1: Type: text/plain, Size: 7370 bytes --]

On Thu, 2019-07-11 at 19:19 +0200, Daniel Vetter wrote:
> On Thu, Jul 11, 2019 at 03:20:52PM +0000, Ser, Simon wrote:
> > On Thu, 2019-07-11 at 14:36 +0200, Daniel Vetter wrote:
> > > On Thu, Jul 11, 2019 at 11:09 AM Vasilev, Oleg <
> > > oleg.vasilev@intel.com> wrote:
> > > > On Wed, 2019-07-10 at 18:35 +0200, Daniel Vetter wrote:
> > > > > On Thu, Jul 04, 2019 at 12:56:04PM +0300, Oleg Vasilev wrote:
> > > > > > Currently, we have different sets of prime tests:
> > > > > >  - vgem+i915
> > > > > >  - amdgpu+i915
> > > > > >  - nv+i915
> > > > > > 
> > > > > > Those tests use vendor-specific ioctls, therefore, not
> > > > > > interchangeable.
> > > > > > The idea is to create a set of tests which are expected to
> > > > > > work on
> > > > > > any
> > > > > > prime-compatible driver. It can be run with any combination
> > > > > > of
> > > > > > exporter+importer, where
> > > > > > 
> > > > > > The first test is simple:
> > > > > > 1. Exporter creates a framebuffer and fills it with a plain
> > > > > > color
> > > > > > 2. Fb is transferred to the importer
> > > > > > 3. Importer modesets and computes pipe CRC
> > > > > > 4. Importer draws the same color through cairo and compares
> > > > > > CRC
> > > > > > 
> > > > > > The initial motivation comes from the need to test prime
> > > > > > support in
> > > > > > vkms.
> > > > > > 
> > > > > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > > > Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> > > > > > ---
> > > > > >  tests/meson.build     |   1 +
> > > > > >  tests/prime_generic.c | 238
> > > > > > ++++++++++++++++++++++++++++++++++++++++++
> > > > > >  2 files changed, 239 insertions(+)
> > > > > >  create mode 100644 tests/prime_generic.c
> > > > > > 
> > > > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > > > index 34a74025..1c938e95 100644
> > > > > > --- a/tests/meson.build
> > > > > > +++ b/tests/meson.build
> > > > > > @@ -76,6 +76,7 @@ test_progs = [
> > > > > >     'prime_self_import',
> > > > > >     'prime_udl',
> > > > > >     'prime_vgem',
> > > > > > +   'prime_generic',
> > > > > >     'syncobj_basic',
> > > > > >     'syncobj_wait',
> > > > > >     'template',
> > > > > > +
> > > > > > +igt_main
> > > > > > +{
> > > > > > +   igt_fixture {
> > > > > > +           kmstest_set_vt_graphics_mode();
> > > > > > +   }
> > > > > > +   igt_subtest("crc-vgem-vkms")
> > > > > > +           run_test_crc(DRIVER_VGEM, DRIVER_VKMS);
> > > > > > +   igt_subtest("crc-i915-vkms")
> > > > > > +           run_test_crc(DRIVER_INTEL, DRIVER_VKMS);
> > > > > > +   igt_subtest("crc-vgem-i915")
> > > > > > +           run_test_crc(DRIVER_VGEM, DRIVER_INTEL);
> > > > > > +   igt_subtest("crc-amd-i915")
> > > > > > +           run_test_crc(DRIVER_AMDGPU, DRIVER_INTEL);
> > > > > > +   igt_subtest("crc-i915-amd")
> > > > > > +           run_test_crc(DRIVER_INTEL, DRIVER_AMDGPU);
> > > > > 
> > > > > This isn't any more generic than what we have e.g. in
> > > > > prime_vgem
> > > > > already.
> > > > 
> > > > Here non-generic part is only those 10 lines of subtests
> > > > definitions.
> > > > The rest is common code.
> > > > 
> > > > I guess, this separate subtest definitions can document the
> > > > device
> > > > pairs the test is expected to work + we can blacklist tests
> > > > which we
> > > > don't have the hardware for.
> > > 
> > > But we already have tests for specific pairs, which can also test
> > > specific stuff. In general we don't do tests with -gen9, gen11,
> > > -gen8
> > > prefixes either. This here would be a first I'd say.
> > > 
> > > > > The way to do a generic kms testcase is to use DRIVER_ANY
> > > > > (yes some
> > > > > of the
> > > > > earlier conversion unfortunately used stuff like DRIVER_INTEL
> > > > > |
> > > > > DRIVER_AMDGPU). With that you also don't need your library
> > > > > prep
> > > > > patch,
> > > > > because no need for a DRIVER_VKMS.
> > > > 
> > > > But how can we open two different devices with DRIVER_ANY? If I
> > > > put
> > > > 
> > > > fd = drm_open_driver(DRIVER_ANY)
> > > > fd2 = drm_open_driver(DRIVER_ANY)
> > > > 
> > > > I get the same device opened two times, do I?
> > > 
> > > One of them would be DRIVER_VGEM. And I think we already make
> > > sure
> > > that you don't accidentally get vgem for DRIVER_ANY. Otherwise I
> > > guess
> > > we'd need a DRIVER_ANY_BUT_VGEM. Well really what we want is
> > > DRIVER_ANY_KMS here, which would also exclude vgem. So if it
> > > doesn't
> > > work already, small patch needed in lib/, but DRIVER_VKMS also
> > > needs
> > > that. And I really don't want more DRIVER_FOO defines for a plain
> > > old
> > > kms-only driver. kms is supposed to be a shared cross-driver
> > > standard,
> > > if we can't even write shared testcases it's just epic fail :-)
> > > 
> > > > > Imo best to start out by converting prime_vgem.c for this,
> > > > > and adding
> > > > > for
> > > > > the i915 specific tests (but _only_ for those) a specific "is
> > > > > this
> > > > > i915"
> > > > > check, so we skip those tests. And replace DRIVER_INTEL by
> > > > > DRIVER_ANY.
> > > > > Well, anything except vgem :-)
> > > > 
> > > > If I am not mistaken, all tests in prime_vgem are intel-
> > > > specific :D
> > > > 
> > > > Some ideas can be taken from it, but anyway, it probably needs
> > > > a deep
> > > > rewrite.
> > > > 
> > > > And prime_vgem use only vgem as an exporter device. Here we can
> > > > use any
> > > > device. Sure, some tests would eventually use vgem-specific
> > > > features,
> > > > but some could remain more generic.
> > > 
> > > Maybe I wasn't all that clear then, I think what we want to test
> > > in a
> > > generic prime test is vgem+DRIVER_ANY_KMS. Then drivers can test
> > > the
> > > render side using DRIVER_FOO+vgem. This way you only need to have
> > > 1
> > > generic kms test, plus N tests for all the rendering drivers.
> > > Instead
> > > of M x N testcases like you're proposing here. vgem is just the
> > > abstraction layer/mocking layer we use to separate these two
> > > worlds.
> > > And of course for specific sharing you can still do that, e.g. to
> > > test
> > > dma engine compatibility between amdgpu and i915, or nouveau and
> > > i915.
> > > But those tests don't need to involve kms.
> > > 
> > > And yeah prime_vgem might need to be split into prime_i915_vgem
> > > and
> > > prime_kms_vgem. But all the display related testcases should be
> > > possible to extract into such a prime_kms_vgem framework.
> > 
> > One could also want to test drmPrimeHandleToFD and
> > drmPrimeFDToHandle
> > between e.g. amdgpu and i915. I guess it's using a different code
> > path
> > than gem.
> 
> Yeah, but only if you also do some i915 or amdgpu specific command
> submission. Otherwise if it's all cpu-only, then you wont hit any
> additional path than what you can test with vgem already. Kinda my
> point
> really, there's no room really for a "generic" i915+amdgpu prime
> test. As
> soon as you look at that combo, it's all about specifics.
> -Daniel

Ok, now I got the point, thanks for the explanation. 
I will send an updated patch.

Oleg

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3261 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH 2/2] tests/prime_generic: add vendor-agnostic prime tests

[Ser, Simon]

On Thu, 2019-07-11 at 19:19 +0200, Daniel Vetter wrote:
> On Thu, Jul 11, 2019 at 03:20:52PM +0000, Ser, Simon wrote:
> > On Thu, 2019-07-11 at 14:36 +0200, Daniel Vetter wrote:
> > > On Thu, Jul 11, 2019 at 11:09 AM Vasilev, Oleg <oleg.vasilev@intel.com> wrote:
> > > > On Wed, 2019-07-10 at 18:35 +0200, Daniel Vetter wrote:
> > > > > On Thu, Jul 04, 2019 at 12:56:04PM +0300, Oleg Vasilev wrote:
> > > > > > Currently, we have different sets of prime tests:
> > > > > >  - vgem+i915
> > > > > >  - amdgpu+i915
> > > > > >  - nv+i915
> > > > > > 
> > > > > > Those tests use vendor-specific ioctls, therefore, not
> > > > > > interchangeable.
> > > > > > The idea is to create a set of tests which are expected to work on
> > > > > > any
> > > > > > prime-compatible driver. It can be run with any combination of
> > > > > > exporter+importer, where
> > > > > > 
> > > > > > The first test is simple:
> > > > > > 1. Exporter creates a framebuffer and fills it with a plain color
> > > > > > 2. Fb is transferred to the importer
> > > > > > 3. Importer modesets and computes pipe CRC
> > > > > > 4. Importer draws the same color through cairo and compares CRC
> > > > > > 
> > > > > > The initial motivation comes from the need to test prime support in
> > > > > > vkms.
> > > > > > 
> > > > > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > > > Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> > > > > > ---
> > > > > >  tests/meson.build     |   1 +
> > > > > >  tests/prime_generic.c | 238
> > > > > > ++++++++++++++++++++++++++++++++++++++++++
> > > > > >  2 files changed, 239 insertions(+)
> > > > > >  create mode 100644 tests/prime_generic.c
> > > > > > 
> > > > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > > > index 34a74025..1c938e95 100644
> > > > > > --- a/tests/meson.build
> > > > > > +++ b/tests/meson.build
> > > > > > @@ -76,6 +76,7 @@ test_progs = [
> > > > > >     'prime_self_import',
> > > > > >     'prime_udl',
> > > > > >     'prime_vgem',
> > > > > > +   'prime_generic',
> > > > > >     'syncobj_basic',
> > > > > >     'syncobj_wait',
> > > > > >     'template',
> > > > > > diff --git a/tests/prime_generic.c b/tests/prime_generic.c
> > > > > > new file mode 100644
> > > > > > index 00000000..65e48763
> > > > > > --- /dev/null
> > > > > > +++ b/tests/prime_generic.c
> > > > > > @@ -0,0 +1,238 @@
> > > > > > +/*
> > > > > > + * Copyright © 2019 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.
> > > > > > + */
> > > > > > +
> > > > > > +#include "igt.h"
> > > > > > +#include "igt_vgem.h"
> > > > > > +
> > > > > > +#include <sys/ioctl.h>
> > > > > > +#include <sys/poll.h>
> > > > > > +#include <time.h>
> > > > > > +
> > > > > > +static struct {
> > > > > > +   double r, g, b;
> > > > > > +   uint32_t color;
> > > > > > +   igt_crc_t prime_crc;
> > > > > > +   igt_crc_t direct_crc;
> > > > > > +   char *prime_str;
> > > > > > +   char *direct_str;
> > > > > > +} colors[3] = {
> > > > > > +   { .r = 0.0, .g = 0.0, .b = 0.0, .color = 0xff000000},
> > > > > > +   { .r = 1.0, .g = 1.0, .b = 1.0, .color = 0xffffffff},
> > > > > > +   { .r = 1.0, .g = 0.0, .b = 0.0, .color = 0xffff0000},
> > > > > > +};
> > > > > > +
> > > > > > +IGT_TEST_DESCRIPTION("Generic tests, working with any prime
> > > > > > device");
> > > > > > +
> > > > > > +static bool has_prime_import(int fd)
> > > > > > +{
> > > > > > +   uint64_t value;
> > > > > > +
> > > > > > +   if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> > > > > > +           return false;
> > > > > > +
> > > > > > +   return value & DRM_PRIME_CAP_IMPORT;
> > > > > > +}
> > > > > > +
> > > > > > +static bool has_prime_export(int fd)
> > > > > > +{
> > > > > > +   uint64_t value;
> > > > > > +
> > > > > > +   if (drmGetCap(fd, DRM_CAP_PRIME, &value))
> > > > > > +           return false;
> > > > > > +
> > > > > > +   return value & DRM_PRIME_CAP_EXPORT;
> > > > > > +}
> > > > > > +
> > > > > > +static void prepare_scratch(int exporter, struct vgem_bo *scratch,
> > > > > > +                       drmModeModeInfo *mode, uint32_t color)
> > > > > > +{
> > > > > > +   int i;
> > > > > > +   uint32_t *ptr;
> > > > > > +
> > > > > > +   scratch->width = mode->hdisplay;
> > > > > > +   scratch->height = mode->vdisplay;
> > > > > > +   scratch->bpp = 32;
> > > > > > +   vgem_create(exporter, scratch);
> > > > > > +
> > > > > > +   ptr = vgem_mmap(exporter, scratch, PROT_WRITE);
> > > > > > +   for (i = 0; i < scratch->size / sizeof(*ptr); ++i)
> > > > > > +           ptr[i] = color;
> > > > > > +
> > > > > > +   munmap(ptr, scratch->size);
> > > > > > +}
> > > > > > +
> > > > > > +static void prepare_fb(int importer, struct vgem_bo *scratch,
> > > > > > struct igt_fb *fb)
> > > > > > +{
> > > > > > +   enum igt_color_encoding color_encoding = IGT_COLOR_YCBCR_BT709;
> > > > > > +   enum igt_color_range color_range =
> > > > > > IGT_COLOR_YCBCR_LIMITED_RANGE;
> > > > > > +
> > > > > > +   igt_init_fb(fb, importer, scratch->width, scratch->height,
> > > > > > +               DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,
> > > > > > +               color_encoding, color_range);
> > > > > > +}
> > > > > > +
> > > > > > +static void import_fb(int importer, struct igt_fb *fb,
> > > > > > +                 int dmabuf_fd, uint32_t pitch)
> > > > > > +{
> > > > > > +   uint32_t offsets[4], pitches[4], handles[4];
> > > > > > +   int ret;
> > > > > > +
> > > > > > +   fb->gem_handle = prime_fd_to_handle(importer, dmabuf_fd);
> > > > > > +
> > > > > > +   handles[0] = fb->gem_handle;
> > > > > > +   pitches[0] = pitch;
> > > > > > +   offsets[0] = 0;
> > > > > > +
> > > > > > +   ret = drmModeAddFB2(importer, fb->width, fb->height,
> > > > > > +                       DRM_FORMAT_XRGB8888,
> > > > > > +                       handles, pitches, offsets,
> > > > > > +                       &fb->fb_id, 0);
> > > > > > +   igt_assert(ret == 0);
> > > > > > +}
> > > > > > +
> > > > > > +static void set_fb(struct igt_fb *fb,
> > > > > > +              igt_display_t *display,
> > > > > > +              igt_output_t *output)
> > > > > > +{
> > > > > > +   igt_plane_t *primary;
> > > > > > +   int ret;
> > > > > > +
> > > > > > +   primary = igt_output_get_plane(output, 0);
> > > > > > +   igt_plane_set_fb(primary, fb);
> > > > > > +   ret = igt_display_commit(display);
> > > > > > +
> > > > > > +   igt_assert(ret == 0);
> > > > > > +}
> > > > > > +
> > > > > > +static void test_crc(int exporter, int importer)
> > > > > > +{
> > > > > > +   enum pipe pipe = PIPE_A;
> > > > > > +   igt_display_t display;
> > > > > > +   igt_output_t *output;
> > > > > > +   igt_pipe_crc_t *pipe_crc;
> > > > > > +   struct igt_fb fb;
> > > > > > +   int dmabuf_fd;
> > > > > > +   struct vgem_bo scratch; /* despite the name, it suits for any
> > > > > > +                              gem-compatible device */
> > > > > > +   int i, j;
> > > > > > +   drmModeModeInfo *mode;
> > > > > > +
> > > > > > +   bool crc_equal;
> > > > > > +
> > > > > > +   igt_display_require(&display, importer);
> > > > > > +   igt_skip_on(pipe >= display.n_pipes);
> > > > > > +   output = igt_get_single_output_for_pipe(&display, pipe);
> > > > > > +   igt_require_f(output, "No connector found for pipe %s\n",
> > > > > > +                 kmstest_pipe_name(pipe));
> > > > > > +
> > > > > > +   igt_display_reset(&display);
> > > > > > +   igt_output_set_pipe(output, pipe);
> > > > > > +
> > > > > > +   mode = igt_output_get_mode(output);
> > > > > > +   pipe_crc = igt_pipe_crc_new(importer, pipe,
> > > > > > INTEL_PIPE_CRC_SOURCE_AUTO);
> > > > > > +
> > > > > > +   for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > > > > > +           prepare_scratch(exporter, &scratch, mode,
> > > > > > colors[i].color);
> > > > > > +           dmabuf_fd = prime_handle_to_fd(exporter,
> > > > > > scratch.handle);
> > > > > > +           gem_close(exporter, scratch.handle);
> > > > > > +
> > > > > > +           prepare_fb(importer, &scratch, &fb);
> > > > > > +           import_fb(importer, &fb, dmabuf_fd, scratch.pitch);
> > > > > > +           close(dmabuf_fd);
> > > > > > +
> > > > > > +           set_fb(&fb, &display, output);
> > > > > > +           igt_pipe_crc_collect_crc(pipe_crc,
> > > > > > &colors[i].prime_crc);
> > > > > > +           colors[i].prime_str =
> > > > > > igt_crc_to_string(&colors[i].prime_crc);
> > > > > > +           igt_debug("Prime CRC for %#08x is %s\n",
> > > > > > +                     colors[i].color, colors[i].prime_str);
> > > > > > +           igt_remove_fb(importer, &fb);
> > > > > > +
> > > > > > +           igt_create_color_fb(importer,
> > > > > > +                               mode->hdisplay, mode->vdisplay,
> > > > > > +                               DRM_FORMAT_XRGB8888,
> > > > > > LOCAL_DRM_FORMAT_MOD_NONE,
> > > > > > +                               colors[i].r, colors[i].g,
> > > > > > colors[i].b,
> > > > > > +                               &fb);
> > > > > > +
> > > > > > +           set_fb(&fb, &display, output);
> > > > > > +           igt_pipe_crc_collect_crc(pipe_crc,
> > > > > > &colors[i].direct_crc);
> > > > > > +           colors[i].direct_str =
> > > > > > igt_crc_to_string(&colors[i].direct_crc);
> > > > > > +           igt_debug("Direct CRC for %#08x is %s\n",
> > > > > > +                     colors[i].color, colors[i].direct_str);
> > > > > > +           igt_remove_fb(importer, &fb);
> > > > > > +   }
> > > > > > +   igt_pipe_crc_free(pipe_crc);
> > > > > > +
> > > > > > +   igt_debug("CRC table:\n");
> > > > > > +   igt_debug("Color\t\tPrime\t\tDirect\n");
> > > > > > +   for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > > > > > +           igt_debug("%#08x\t%.8s\t%.8s\n", colors[i].color,
> > > > > > +                     colors[i].prime_str, colors[i].direct_str);
> > > > > > +   }
> > > > > > +
> > > > > > +   for (i = 0; i < ARRAY_SIZE(colors); i++) {
> > > > > > +           for (j = 0; j < ARRAY_SIZE(colors); j++) {
> > > > > > +                   if (i == j) {
> > > > > > +                           igt_assert_crc_equal(&colors[i].prime_c
> > > > > > rc,
> > > > > > +                                                &colors[j].direct_
> > > > > > crc);
> > > > > > +                   } else {
> > > > > > +                           crc_equal =
> > > > > > igt_check_crc_equal(&colors[i].prime_crc,
> > > > > > +                                                           &colors
> > > > > > [j].direct_crc);
> > > > > > +                           igt_assert_f(!crc_equal, "CRC should be
> > > > > > different");
> > > > > > +                   }
> > > > > > +           }
> > > > > > +   }
> > > > > > +}
> > > > > > +
> > > > > > +static void run_test_crc(int export_chipset, int import_chipset)
> > > > > > +{
> > > > > > +   int importer = -1;
> > > > > > +   int exporter = -1;
> > > > > > +
> > > > > > +   exporter = drm_open_driver(export_chipset);
> > > > > > +   importer = drm_open_driver_master(import_chipset);
> > > > > > +
> > > > > > +   igt_require(has_prime_export(exporter));
> > > > > > +   igt_require(has_prime_import(importer));
> > > > > > +   igt_require_pipe_crc(importer);
> > > > > > +
> > > > > > +   test_crc(exporter, importer);
> > > > > > +   close(importer);
> > > > > > +   close(exporter);
> > > > > > +}
> > > > > > +
> > > > > > +igt_main
> > > > > > +{
> > > > > > +   igt_fixture {
> > > > > > +           kmstest_set_vt_graphics_mode();
> > > > > > +   }
> > > > > > +   igt_subtest("crc-vgem-vkms")
> > > > > > +           run_test_crc(DRIVER_VGEM, DRIVER_VKMS);
> > > > > > +   igt_subtest("crc-i915-vkms")
> > > > > > +           run_test_crc(DRIVER_INTEL, DRIVER_VKMS);
> > > > > > +   igt_subtest("crc-vgem-i915")
> > > > > > +           run_test_crc(DRIVER_VGEM, DRIVER_INTEL);
> > > > > > +   igt_subtest("crc-amd-i915")
> > > > > > +           run_test_crc(DRIVER_AMDGPU, DRIVER_INTEL);
> > > > > > +   igt_subtest("crc-i915-amd")
> > > > > > +           run_test_crc(DRIVER_INTEL, DRIVER_AMDGPU);
> > > > > 
> > > > > This isn't any more generic than what we have e.g. in prime_vgem
> > > > > already.
> > > > 
> > > > Here non-generic part is only those 10 lines of subtests definitions.
> > > > The rest is common code.
> > > > 
> > > > I guess, this separate subtest definitions can document the device
> > > > pairs the test is expected to work + we can blacklist tests which we
> > > > don't have the hardware for.
> > > 
> > > But we already have tests for specific pairs, which can also test
> > > specific stuff. In general we don't do tests with -gen9, gen11, -gen8
> > > prefixes either. This here would be a first I'd say.
> > > 
> > > > > The way to do a generic kms testcase is to use DRIVER_ANY (yes some
> > > > > of the
> > > > > earlier conversion unfortunately used stuff like DRIVER_INTEL |
> > > > > DRIVER_AMDGPU). With that you also don't need your library prep
> > > > > patch,
> > > > > because no need for a DRIVER_VKMS.
> > > > 
> > > > But how can we open two different devices with DRIVER_ANY? If I put
> > > > 
> > > > fd = drm_open_driver(DRIVER_ANY)
> > > > fd2 = drm_open_driver(DRIVER_ANY)
> > > > 
> > > > I get the same device opened two times, do I?
> > > 
> > > One of them would be DRIVER_VGEM. And I think we already make sure
> > > that you don't accidentally get vgem for DRIVER_ANY. Otherwise I guess
> > > we'd need a DRIVER_ANY_BUT_VGEM. Well really what we want is
> > > DRIVER_ANY_KMS here, which would also exclude vgem. So if it doesn't
> > > work already, small patch needed in lib/, but DRIVER_VKMS also needs
> > > that. And I really don't want more DRIVER_FOO defines for a plain old
> > > kms-only driver. kms is supposed to be a shared cross-driver standard,
> > > if we can't even write shared testcases it's just epic fail :-)
> > > 
> > > > > Imo best to start out by converting prime_vgem.c for this, and adding
> > > > > for
> > > > > the i915 specific tests (but _only_ for those) a specific "is this
> > > > > i915"
> > > > > check, so we skip those tests. And replace DRIVER_INTEL by
> > > > > DRIVER_ANY.
> > > > > Well, anything except vgem :-)
> > > > 
> > > > If I am not mistaken, all tests in prime_vgem are intel-specific :D
> > > > 
> > > > Some ideas can be taken from it, but anyway, it probably needs a deep
> > > > rewrite.
> > > > 
> > > > And prime_vgem use only vgem as an exporter device. Here we can use any
> > > > device. Sure, some tests would eventually use vgem-specific features,
> > > > but some could remain more generic.
> > > 
> > > Maybe I wasn't all that clear then, I think what we want to test in a
> > > generic prime test is vgem+DRIVER_ANY_KMS. Then drivers can test the
> > > render side using DRIVER_FOO+vgem. This way you only need to have 1
> > > generic kms test, plus N tests for all the rendering drivers. Instead
> > > of M x N testcases like you're proposing here. vgem is just the
> > > abstraction layer/mocking layer we use to separate these two worlds.
> > > And of course for specific sharing you can still do that, e.g. to test
> > > dma engine compatibility between amdgpu and i915, or nouveau and i915.
> > > But those tests don't need to involve kms.
> > > 
> > > And yeah prime_vgem might need to be split into prime_i915_vgem and
> > > prime_kms_vgem. But all the display related testcases should be
> > > possible to extract into such a prime_kms_vgem framework.
> > 
> > One could also want to test drmPrimeHandleToFD and drmPrimeFDToHandle
> > between e.g. amdgpu and i915. I guess it's using a different code path
> > than gem.
> 
> Yeah, but only if you also do some i915 or amdgpu specific command
> submission. Otherwise if it's all cpu-only, then you wont hit any
> additional path than what you can test with vgem already. Kinda my point
> really, there's no room really for a "generic" i915+amdgpu prime test. As
> soon as you look at that combo, it's all about specifics.

I see, that makes sense. Thanks for taking the time to explain. :)
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH 1/2] lib: prepare helpers for generic prime test

[Ser, Simon]

On Thu, 2019-07-04 at 12:56 +0300, Oleg Vasilev wrote:
> - Add vkms to gpu providers list
> - Expose fb_init

Maybe this commit could be split into two?

> Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
> ---
>  lib/drmtest.c |  1 +
>  lib/drmtest.h |  1 +
>  lib/igt_fb.c  | 38 ++++++++++++++++++--------------------
>  lib/igt_fb.h  |  4 ++++
>  4 files changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 25f20353..81f4bf32 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -210,6 +210,7 @@ static const struct module {
>  	{ DRIVER_V3D, "v3d" },
>  	{ DRIVER_VC4, "vc4" },
>  	{ DRIVER_VGEM, "vgem" },
> +	{ DRIVER_VKMS, "vkms" },
>  	{ DRIVER_VIRTIO, "virtio_gpu", modprobe_virtio },
>  	{}
>  };
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 6c4c3899..d4043ff1 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -45,6 +45,7 @@
>  #define DRIVER_AMDGPU	(1 << 4)
>  #define DRIVER_V3D	(1 << 5)
>  #define DRIVER_PANFROST	(1 << 6)
> +#define DRIVER_VKMS	(1 << 7)
>  /*
>   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
>   * with vgem as well as a supported driver, you can end up with a
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 415a3d65..49c1dbe4 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -485,12 +485,10 @@ static int fb_num_planes(const struct igt_fb *fb)
>  		return format->num_planes;
>  }
>  
> -static void fb_init(struct igt_fb *fb,
> -		    int fd, int width, int height,
> -		    uint32_t drm_format,
> -		    uint64_t modifier,
> -		    enum igt_color_encoding color_encoding,
> -		    enum igt_color_range color_range)
> +void igt_init_fb(struct igt_fb *fb, int fd, int width, int height,
> +		 uint32_t drm_format, uint64_t modifier,
> +		 enum igt_color_encoding color_encoding,
> +		 enum igt_color_range color_range)
>  {
>  	const struct format_desc_struct *f = lookup_drm_format(drm_format);
>  
> @@ -627,8 +625,8 @@ void igt_calc_fb_size(int fd, int width, int height, uint32_t drm_format, uint64
>  {
>  	struct igt_fb fb;
>  
> -	fb_init(&fb, fd, width, height, drm_format, modifier,
> -		IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
> +	igt_init_fb(&fb, fd, width, height, drm_format, modifier,
> +		    IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
>  
>  	fb.size = calc_fb_size(&fb);
>  
> @@ -855,8 +853,8 @@ void igt_create_bo_for_fb(int fd, int width, int height,
>  			  uint32_t format, uint64_t modifier,
>  			  struct igt_fb *fb /* out */)
>  {
> -	fb_init(fb, fd, width, height, format, modifier,
> -		IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
> +	igt_init_fb(fb, fd, width, height, format, modifier,
> +		    IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
>  	create_bo_for_fb(fb);
>  }
>  
> @@ -885,8 +883,8 @@ int igt_create_bo_with_dimensions(int fd, int width, int height,
>  {
>  	struct igt_fb fb;
>  
> -	fb_init(&fb, fd, width, height, format, modifier,
> -		IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
> +	igt_init_fb(&fb, fd, width, height, format, modifier,
> +		    IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
>  
>  	for (int i = 0; i < fb.num_planes; i++)
>  		fb.strides[i] = stride;
> @@ -1440,8 +1438,8 @@ igt_create_fb_with_bo_size(int fd, int width, int height,
>  	enum igt_color_range color_range = IGT_COLOR_YCBCR_LIMITED_RANGE;
>  	uint32_t flags = 0;
>  
> -	fb_init(fb, fd, width, height, format, modifier,
> -		color_encoding, color_range);
> +	igt_init_fb(fb, fd, width, height, format, modifier,
> +		    color_encoding, color_range);
>  
>  	for (int i = 0; i < fb->num_planes; i++)
>  		fb->strides[i] = bo_stride;
> @@ -1971,9 +1969,9 @@ static void setup_linear_mapping(struct fb_blit_upload *blit)
>  	 * destination, tiling it at the same time.
>  	 */
>  
> -	fb_init(&linear->fb, fb->fd, fb->width, fb->height,
> -		fb->drm_format, LOCAL_DRM_FORMAT_MOD_NONE,
> -		fb->color_encoding, fb->color_range);
> +	igt_init_fb(&linear->fb, fb->fd, fb->width, fb->height,
> +		    fb->drm_format, LOCAL_DRM_FORMAT_MOD_NONE,
> +		    fb->color_encoding, fb->color_range);
>  
>  	create_bo_for_fb(&linear->fb);
>  
> @@ -2127,9 +2125,9 @@ static void *igt_fb_create_cairo_shadow_buffer(int fd,
>  
>  	igt_assert(shadow);
>  
> -	fb_init(shadow, fd, width, height,
> -		drm_format, LOCAL_DRM_FORMAT_MOD_NONE,
> -		IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
> +	igt_init_fb(shadow, fd, width, height,
> +		    drm_format, LOCAL_DRM_FORMAT_MOD_NONE,
> +		    IGT_COLOR_YCBCR_BT709, IGT_COLOR_YCBCR_LIMITED_RANGE);
>  
>  	shadow->strides[0] = ALIGN(width * (shadow->plane_bpp[0] / 8), 16);
>  	shadow->size = ALIGN((uint64_t)shadow->strides[0] * height,
> diff --git a/lib/igt_fb.h b/lib/igt_fb.h
> index be786911..3746fa1a 100644
> --- a/lib/igt_fb.h
> +++ b/lib/igt_fb.h
> @@ -117,6 +117,10 @@ void igt_get_fb_tile_size(int fd, uint64_t modifier, int fb_bpp,
>  			  unsigned *width_ret, unsigned *height_ret);
>  void igt_calc_fb_size(int fd, int width, int height, uint32_t format, uint64_t modifier,
>  		      uint64_t *size_ret, unsigned *stride_ret);
> +void igt_init_fb(struct igt_fb *fb, int fd, int width, int height,
> +		 uint32_t drm_format, uint64_t modifier,
> +		 enum igt_color_encoding color_encoding,
> +		 enum igt_color_range color_range);
>  unsigned int
>  igt_create_fb_with_bo_size(int fd, int width, int height,
>  			   uint32_t format, uint64_t modifier,
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ GitLab.Pipeline: warning for series starting with [1/2] lib: prepare helpers for generic prime test

[Patchwork]

== Series Details ==

Series: series starting with [1/2] lib: prepare helpers for generic prime test
URL   : https://patchwork.freedesktop.org/series/63216/
State : warning

== Summary ==

Pipeline status: FAILED.

See https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/46591 for more details.

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/46591
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [1/2] lib: prepare helpers for generic prime test

[Patchwork]

== Series Details ==

Series: series starting with [1/2] lib: prepare helpers for generic prime test
URL   : https://patchwork.freedesktop.org/series/63216/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6408 -> IGTPW_3240
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/63216/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_3240 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_mmap_gtt@basic-small-bo-tiledy:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/fi-icl-u3/igt@gem_mmap_gtt@basic-small-bo-tiledy.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/fi-icl-u3/igt@gem_mmap_gtt@basic-small-bo-tiledy.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [PASS][3] -> [DMESG-WARN][4] ([fdo#105541])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [PASS][5] -> [FAIL][6] ([fdo#109485])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  
#### Possible fixes ####

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-dsi:         [FAIL][7] ([fdo#103167]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/fi-icl-dsi/igt@kms_frontbuffer_tracking@basic.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/fi-icl-dsi/igt@kms_frontbuffer_tracking@basic.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#105541]: https://bugs.freedesktop.org/show_bug.cgi?id=105541
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045


Participating hosts (53 -> 47)
------------------------------

  Additional (2): fi-kbl-r fi-cml-u 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * IGT: IGT_5082 -> IGTPW_3240

  CI_DRM_6408: ee60427a5d560ee44370d88f70db1497df9df979 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3240: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/
  IGT_5082: f7c51e6fbf8da0784b64a1edaee5266aa9b9f829 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@prime_generic@crc-amd-i915
+igt@prime_generic@crc-i915-amd
+igt@prime_generic@crc-i915-vkms
+igt@prime_generic@crc-vgem-i915
+igt@prime_generic@crc-vgem-vkms

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ GitLab.Pipeline: warning for series starting with [1/2] lib: prepare helpers for generic prime test

[Patchwork]

== Series Details ==

Series: series starting with [1/2] lib: prepare helpers for generic prime test
URL   : https://patchwork.freedesktop.org/series/63216/
State : warning

== Summary ==

Pipeline status: FAILED.

See https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/46853 for more details.

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/pipelines/46853
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.IGT: failure for series starting with [1/2] lib: prepare helpers for generic prime test

[Patchwork]

== Series Details ==

Series: series starting with [1/2] lib: prepare helpers for generic prime test
URL   : https://patchwork.freedesktop.org/series/63216/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6408_full -> IGTPW_3240_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_3240_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_3240_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/63216/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_3240_full:

### IGT changes ###

#### Possible regressions ####

  * {igt@prime_generic@crc-i915-vkms} (NEW):
    - shard-iclb:         NOTRUN -> [SKIP][1] +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-iclb2/igt@prime_generic@crc-i915-vkms.html

  * igt@runner@aborted:
    - shard-kbl:          NOTRUN -> ([FAIL][2], [FAIL][3]) ([fdo#111063])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-kbl2/igt@runner@aborted.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-kbl3/igt@runner@aborted.html

  
New tests
---------

  New tests have been introduced between CI_DRM_6408_full and IGTPW_3240_full:

### New IGT tests (5) ###

  * igt@prime_generic@crc-amd-i915:
    - Statuses : 6 skip(s)
    - Exec time: [0.39, 0.74] s

  * igt@prime_generic@crc-i915-amd:
    - Statuses : 6 skip(s)
    - Exec time: [0.00, 0.61] s

  * igt@prime_generic@crc-i915-vkms:
    - Statuses : 6 skip(s)
    - Exec time: [0.00, 0.01] s

  * igt@prime_generic@crc-vgem-i915:
    - Statuses : 6 pass(s)
    - Exec time: [0.80, 1.20] s

  * igt@prime_generic@crc-vgem-vkms:
    - Statuses : 6 skip(s)
    - Exec time: [0.05] s

  

Known issues
------------

  Here are the changes found in IGTPW_3240_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_busy@close-race:
    - shard-snb:          [PASS][4] -> [DMESG-FAIL][5] ([fdo#111063])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-snb5/igt@gem_busy@close-race.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-snb2/igt@gem_busy@close-race.html
    - shard-kbl:          [PASS][6] -> [DMESG-FAIL][7] ([fdo#111063])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-kbl3/igt@gem_busy@close-race.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-kbl2/igt@gem_busy@close-race.html

  * igt@gem_ctx_isolation@vecs0-s3:
    - shard-kbl:          [PASS][8] -> [INCOMPLETE][9] ([fdo#103665])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-kbl4/igt@gem_ctx_isolation@vecs0-s3.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-kbl4/igt@gem_ctx_isolation@vecs0-s3.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][10] -> [SKIP][11] ([fdo#110854])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-iclb1/igt@gem_exec_balancer@smoke.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-iclb8/igt@gem_exec_balancer@smoke.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-glk:          [PASS][12] -> [DMESG-WARN][13] ([fdo#108686])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-glk8/igt@gem_tiled_swapping@non-threaded.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-glk5/igt@gem_tiled_swapping@non-threaded.html

  * igt@kms_color@pipe-a-ctm-0-25:
    - shard-hsw:          [PASS][14] -> [INCOMPLETE][15] ([fdo#103540])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-hsw7/igt@kms_color@pipe-a-ctm-0-25.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-hsw7/igt@kms_color@pipe-a-ctm-0-25.html

  * igt@kms_cursor_crc@pipe-c-cursor-128x128-sliding:
    - shard-kbl:          [PASS][16] -> [FAIL][17] ([fdo#103232])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-128x128-sliding.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-128x128-sliding.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-apl:          [PASS][18] -> [DMESG-WARN][19] ([fdo#108566]) +1 similar issue
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-apl8/igt@kms_flip@flip-vs-suspend.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-apl4/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
    - shard-iclb:         [PASS][20] -> [FAIL][21] ([fdo#103167]) +1 similar issue
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [PASS][22] -> [SKIP][23] ([fdo#109642] / [fdo#111068])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-iclb1/igt@kms_psr2_su@frontbuffer.html

  
#### Possible fixes ####

  * igt@gem_tiled_fence_blits@normal:
    - shard-apl:          [INCOMPLETE][24] ([fdo#103927]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-apl8/igt@gem_tiled_fence_blits@normal.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-apl8/igt@gem_tiled_fence_blits@normal.html
    - shard-iclb:         [INCOMPLETE][26] ([fdo#107713]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-iclb7/igt@gem_tiled_fence_blits@normal.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-iclb7/igt@gem_tiled_fence_blits@normal.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [DMESG-WARN][28] ([fdo#108566]) -> [PASS][29] +7 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-apl7/igt@i915_suspend@fence-restore-tiled2untiled.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-apl5/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
    - shard-glk:          [FAIL][30] ([fdo#104873]) -> [PASS][31]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-glk5/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-glk7/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-iclb:         [FAIL][32] ([fdo#103167]) -> [PASS][33] +6 similar issues
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [SKIP][34] ([fdo#109441]) -> [PASS][35] +2 similar issues
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6408/shard-iclb3/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#111063]: https://bugs.freedesktop.org/show_bug.cgi?id=111063
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068


Participating hosts (10 -> 6)
------------------------------

  Missing    (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005 


Build changes
-------------

  * IGT: IGT_5082 -> IGTPW_3240
  * Piglit: piglit_4509 -> None

  CI_DRM_6408: ee60427a5d560ee44370d88f70db1497df9df979 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3240: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/
  IGT_5082: f7c51e6fbf8da0784b64a1edaee5266aa9b9f829 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3240/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev