[PATCH igt v2 0/3] Initial igt tests for drm/msm ioctls

[PATCH igt v2 0/3] Initial igt tests for drm/msm ioctls

[Rob Clark]

From: Rob Clark <robdclark@chromium.org>

Add an initial set of tests for the gpu SUBMIT ioctl.  There is
plenty more we can add, but need to start somewhere.

v2: add msm_device::gen and drop igt_msm_pipe_gen().. any test
    that needs to build even trivial cmdstream will need this
    so make it part of the util helper code

Rob Clark (3):
  drmtest: Add DRIVER_MSM support
  msm: Add helper library
  msm: Add submit ioctl tests

 lib/drmtest.c      |   3 +
 lib/drmtest.h      |   1 +
 lib/igt_msm.c      | 171 +++++++++++++++++++++++++++++++++++++++++
 lib/igt_msm.h      | 119 +++++++++++++++++++++++++++++
 lib/meson.build    |   1 +
 tests/meson.build  |   1 +
 tests/msm_submit.c | 186 +++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 482 insertions(+)
 create mode 100644 lib/igt_msm.c
 create mode 100644 lib/igt_msm.h
 create mode 100644 tests/msm_submit.c

-- 
2.31.1

[PATCH igt v2 1/3] drmtest: Add DRIVER_MSM support

[Rob Clark]

From: Rob Clark <robdclark@chromium.org>

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 lib/drmtest.c | 3 +++
 lib/drmtest.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index e1f9b115..29cb3f4c 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -179,6 +179,7 @@ static const struct module {
 } modules[] = {
 	{ DRIVER_AMDGPU, "amdgpu" },
 	{ DRIVER_INTEL, "i915", modprobe_i915 },
+	{ DRIVER_MSM, "msm" },
 	{ DRIVER_PANFROST, "panfrost" },
 	{ DRIVER_V3D, "v3d" },
 	{ DRIVER_VC4, "vc4" },
@@ -539,6 +540,8 @@ static const char *chipset_to_str(int chipset)
 		return "amdgpu";
 	case DRIVER_PANFROST:
 		return "panfrost";
+	case DRIVER_MSM:
+		return "msm";
 	case DRIVER_ANY:
 		return "any";
 	default:
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 7d17a0f9..a6eb60c3 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -50,6 +50,7 @@
 #define DRIVER_AMDGPU	(1 << 3)
 #define DRIVER_V3D	(1 << 4)
 #define DRIVER_PANFROST	(1 << 5)
+#define DRIVER_MSM	(1 << 6)
 
 /*
  * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
-- 
2.31.1

[PATCH igt v2 2/3] msm: Add helper library

[Rob Clark]

From: Rob Clark <robdclark@chromium.org>

Handle some of the boilerplate for tests.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 lib/igt_msm.c   | 171 ++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_msm.h   | 119 +++++++++++++++++++++++++++++++++
 lib/meson.build |   1 +
 3 files changed, 291 insertions(+)
 create mode 100644 lib/igt_msm.c
 create mode 100644 lib/igt_msm.h

diff --git a/lib/igt_msm.c b/lib/igt_msm.c
new file mode 100644
index 00000000..3bd0ee53
--- /dev/null
+++ b/lib/igt_msm.c
@@ -0,0 +1,171 @@
+/*
+ * Copyright © 2021 Google, Inc.
+ *
+ * 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 <assert.h>
+#include <string.h>
+#include <signal.h>
+#include <errno.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <fcntl.h>
+
+#include "drmtest.h"
+#include "igt_aux.h"
+#include "igt_core.h"
+#include "igt_msm.h"
+#include "ioctl_wrappers.h"
+
+/**
+ * SECTION:igt_msm
+ * @short_description: msm support library
+ * @title: msm
+ * @include: igt_msm.h
+ *
+ * This library provides various auxiliary helper functions for writing msm
+ * tests.
+ */
+
+static uint64_t
+get_param(struct msm_device *dev, uint32_t pipe, uint32_t param)
+{
+	struct drm_msm_param req = {
+			.pipe = pipe,
+			.param = param,
+	};
+
+	do_ioctl(dev->fd, DRM_IOCTL_MSM_GET_PARAM, &req);
+
+	return req.value;
+}
+
+struct msm_device *
+igt_msm_dev_open(void)
+{
+	struct msm_device *dev = calloc(1, sizeof(*dev));
+
+	dev->fd = drm_open_driver_render(DRIVER_MSM);
+	if (dev->fd < 0) {
+		free(dev);
+		return NULL;
+	}
+
+	dev->gen = (get_param(dev, MSM_PIPE_3D0, MSM_PARAM_CHIP_ID) >> 24) & 0xff;
+
+	return dev;
+}
+
+void
+igt_msm_dev_close(struct msm_device *dev)
+{
+	close(dev->fd);
+	free(dev);
+}
+
+struct msm_bo *
+igt_msm_bo_new(struct msm_device *dev, size_t size, uint32_t flags)
+{
+	struct msm_bo *bo = calloc(1, sizeof(*bo));
+
+	struct drm_msm_gem_new req = {
+			.size = size,
+			.flags = flags,
+	};
+
+	bo->dev = dev;
+	bo->size = size;
+
+	do_ioctl(dev->fd, DRM_IOCTL_MSM_GEM_NEW, &req);
+
+	bo->handle = req.handle;
+
+	return bo;
+}
+
+void
+igt_msm_bo_free(struct msm_bo *bo)
+{
+	if (bo->map)
+		munmap(bo->map, bo->size);
+	gem_close(bo->dev->fd, bo->handle);
+	free(bo);
+}
+
+void *
+igt_msm_bo_map(struct msm_bo *bo)
+{
+	if (!bo->map) {
+		struct drm_msm_gem_info req = {
+				.handle = bo->handle,
+				.info = MSM_INFO_GET_OFFSET,
+		};
+		void *ptr;
+
+		do_ioctl(bo->dev->fd, DRM_IOCTL_MSM_GEM_INFO, &req);
+
+		ptr = mmap(0, bo->size, PROT_READ | PROT_WRITE, MAP_SHARED,
+				bo->dev->fd, req.value);
+		if (ptr == MAP_FAILED)
+			return NULL;
+
+		bo->map = ptr;
+	}
+	return bo->map;
+}
+
+struct msm_pipe *
+igt_msm_pipe_open(struct msm_device *dev, uint32_t prio)
+{
+	struct msm_pipe *pipe = calloc(1, sizeof(*pipe));
+	struct drm_msm_submitqueue req = {
+			.flags = 0,
+			.prio = prio,
+	};
+
+	pipe->dev = dev;
+	pipe->pipe = MSM_PIPE_3D0;
+
+	/* Note that kerenels prior to v4.15 did not support submitqueues.
+	 * Mesa maintains support for older kernels, but I do not think
+	 * that IGT needs to.
+	 */
+	do_ioctl(dev->fd, DRM_IOCTL_MSM_SUBMITQUEUE_NEW, &req);
+
+	pipe->submitqueue_id = req.id;
+
+	return pipe;
+}
+
+void
+igt_msm_pipe_close(struct msm_pipe *pipe)
+{
+	do_ioctl(pipe->dev->fd, DRM_IOCTL_MSM_SUBMITQUEUE_CLOSE, &pipe->submitqueue_id);
+	free(pipe);
+}
+
+uint64_t
+igt_msm_pipe_get_param(struct msm_pipe *pipe, uint32_t param)
+{
+	return get_param(pipe->dev, pipe->pipe, param);
+}
diff --git a/lib/igt_msm.h b/lib/igt_msm.h
new file mode 100644
index 00000000..614c42ee
--- /dev/null
+++ b/lib/igt_msm.h
@@ -0,0 +1,119 @@
+/*
+ * Copyright © 2021 Google, Inc.
+ *
+ * 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.
+ */
+
+#ifndef IGT_MSM_H
+#define IGT_MSM_H
+
+#include "msm_drm.h"
+
+struct msm_device {
+	int fd;
+	unsigned gen;
+};
+
+struct msm_device *igt_msm_dev_open(void);
+void igt_msm_dev_close(struct msm_device *dev);
+
+struct msm_bo {
+	struct msm_device *dev;
+	int handle;
+	uint32_t size;
+	void *map;
+};
+
+struct msm_bo *igt_msm_bo_new(struct msm_device *dev, size_t size, uint32_t flags);
+void igt_msm_bo_free(struct msm_bo *bo);
+void *igt_msm_bo_map(struct msm_bo *bo);
+
+struct msm_pipe {
+	struct msm_device *dev;
+	uint32_t pipe;
+	uint32_t submitqueue_id;
+};
+
+struct msm_pipe *igt_msm_pipe_open(struct msm_device *dev, uint32_t prio);
+void igt_msm_pipe_close(struct msm_pipe *pipe);
+uint64_t igt_msm_pipe_get_param(struct msm_pipe *pipe, uint32_t param);
+
+/*
+ * Helpers for cmdstream building:
+ */
+
+enum adreno_pm4_packet_type {
+	CP_TYPE0_PKT = 0,
+	CP_TYPE1_PKT = 0x40000000,
+	CP_TYPE2_PKT = 0x80000000,
+	CP_TYPE3_PKT = 0xc0000000,
+	CP_TYPE4_PKT = 0x40000000,
+	CP_TYPE7_PKT = 0x70000000,
+};
+
+enum adreno_pm4_type3_packets {
+	CP_NOP = 16,
+};
+
+static inline unsigned
+pm4_odd_parity_bit(unsigned val)
+{
+	/* See: http://graphics.stanford.edu/~seander/bithacks.html#ParityParallel
+	 * note that we want odd parity so 0x6996 is inverted.
+	 */
+	val ^= val >> 16;
+	val ^= val >> 8;
+	val ^= val >> 4;
+	val &= 0xf;
+	return (~0x6996 >> val) & 1;
+}
+
+static inline uint32_t
+pm4_pkt0_hdr(uint16_t regindx, uint16_t cnt)
+{
+	return CP_TYPE0_PKT | ((cnt - 1) << 16) | (regindx & 0x7fff);
+}
+
+static inline uint32_t
+pm4_pkt3_hdr(uint8_t opcode, uint16_t cnt)
+{
+	return CP_TYPE3_PKT | ((cnt - 1) << 16) | ((opcode & 0xff) << 8);
+}
+
+static inline uint32_t
+pm4_pkt4_hdr(uint16_t regindx, uint16_t cnt)
+{
+	return CP_TYPE4_PKT | cnt | (pm4_odd_parity_bit(cnt) << 7) |
+			((regindx & 0x3ffff) << 8) |
+			((pm4_odd_parity_bit(regindx) << 27));
+}
+
+static inline uint32_t
+pm4_pkt7_hdr(uint8_t opcode, uint16_t cnt)
+{
+	return CP_TYPE7_PKT | cnt | (pm4_odd_parity_bit(cnt) << 15) |
+			((opcode & 0x7f) << 16) |
+			((pm4_odd_parity_bit(opcode) << 23));
+}
+
+#define U642VOID(x) ((void *)(uintptr_t)(x))
+#define VOID2U64(x) ((uint64_t)(uintptr_t)(x))
+
+#endif /* IGT_MSM_H */
diff --git a/lib/meson.build b/lib/meson.build
index 67d40512..c3080fc8 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -82,6 +82,7 @@ lib_sources = [
 	'igt_eld.c',
 	'igt_infoframe.c',
 	'veboxcopy_gen12.c',
+	'igt_msm.c',
 ]
 
 lib_deps = [
-- 
2.31.1

[PATCH igt v2 3/3] msm: Add submit ioctl tests

[Rob Clark]

From: Rob Clark <robdclark@chromium.org>

Add an initial set of tests for the submit ioctl.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 tests/meson.build  |   1 +
 tests/msm_submit.c | 186 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 187 insertions(+)
 create mode 100644 tests/msm_submit.c

diff --git a/tests/meson.build b/tests/meson.build
index 1bdfddbb..ff7c709a 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -107,6 +107,7 @@ test_progs = [
 	'vc4_wait_seqno',
 	'vgem_basic',
 	'vgem_slow',
+	'msm_submit',
 ]
 
 i915_progs = [
diff --git a/tests/msm_submit.c b/tests/msm_submit.c
new file mode 100644
index 00000000..da93c574
--- /dev/null
+++ b/tests/msm_submit.c
@@ -0,0 +1,186 @@
+/*
+ * Copyright © 2021 Google, Inc.
+ *
+ * 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_msm.h"
+
+igt_main
+{
+	struct msm_device *dev;
+	struct msm_pipe *pipe;
+	struct msm_bo *a, *b;
+
+	igt_fixture {
+		dev = igt_msm_dev_open();
+		pipe = igt_msm_pipe_open(dev, 0);
+		a = igt_msm_bo_new(dev, 0x1000, MSM_BO_WC);
+		b = igt_msm_bo_new(dev, 0x1000, MSM_BO_WC);
+	}
+
+	igt_subtest("empty-submit") {
+		struct drm_msm_gem_submit req = {
+				.flags   = pipe->pipe,
+				.queueid = pipe->submitqueue_id,
+		};
+		do_ioctl(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req);
+	}
+
+	igt_subtest("invalid-queue-submit") {
+		struct drm_msm_gem_submit req = {
+				.flags   = pipe->pipe,
+				.queueid = 0x1234,
+		};
+		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, ENOENT);
+	}
+
+	igt_subtest("invalid-flags-submit") {
+		struct drm_msm_gem_submit req = {
+				.flags   = 0x1234,
+				.queueid = pipe->submitqueue_id,
+		};
+		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
+	}
+
+	igt_subtest("invalid-in-fence-submit") {
+		struct drm_msm_gem_submit req = {
+				.flags   = pipe->pipe | MSM_SUBMIT_FENCE_FD_IN,
+				.queueid = pipe->submitqueue_id,
+				.fence_fd = dev->fd,  /* This is not a fence fd! */
+		};
+		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
+	}
+
+	igt_subtest("invalid-duplicate-bo-submit") {
+		struct drm_msm_gem_submit_bo bos[] = {
+			[0] = {
+				.handle     = a->handle,
+				.flags      = MSM_SUBMIT_BO_READ,
+			},
+			[1] = {
+				.handle     = b->handle,
+				.flags      = MSM_SUBMIT_BO_READ,
+			},
+			[2] = {
+				/* this is invalid.. there should not be two entries
+				 * for the same bo, instead a single entry w/ all
+				 * usage flags OR'd together should be used.  Kernel
+				 * should catch this, and return an error code after
+				 * cleaning up properly (not leaking any bo's)
+				 */
+				.handle     = a->handle,
+				.flags      = MSM_SUBMIT_BO_WRITE,
+			},
+		};
+		struct drm_msm_gem_submit req = {
+				.flags   = pipe->pipe,
+				.queueid = pipe->submitqueue_id,
+				.nr_bos  = ARRAY_SIZE(bos),
+				.bos     = VOID2U64(bos),
+		};
+		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
+	}
+
+	igt_subtest("invalid-cmd-idx-submit") {
+		struct drm_msm_gem_submit_cmd cmds[] = {
+			[0] = {
+				.type       = MSM_SUBMIT_CMD_BUF,
+				.submit_idx = 0,      /* bos[0] does not exist */
+				.size       = 4 * 4,  /* 4 dwords in cmdbuf */
+			},
+		};
+		struct drm_msm_gem_submit req = {
+				.flags   = pipe->pipe,
+				.queueid = pipe->submitqueue_id,
+				.nr_cmds    = ARRAY_SIZE(cmds),
+				.cmds       = VOID2U64(cmds),
+		};
+		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
+	}
+
+	igt_subtest("invalid-cmd-type-submit") {
+		struct drm_msm_gem_submit_bo bos[] = {
+			[0] = {
+				.handle     = a->handle,
+				.flags      = MSM_SUBMIT_BO_READ,
+			},
+		};
+		struct drm_msm_gem_submit_cmd cmds[] = {
+			[0] = {
+				.type       = 0x1234,
+				.submit_idx = 0,
+				.size       = 4 * 4,  /* 4 dwords in cmdbuf */
+			},
+		};
+		struct drm_msm_gem_submit req = {
+				.flags   = pipe->pipe,
+				.queueid = pipe->submitqueue_id,
+				.nr_cmds    = ARRAY_SIZE(cmds),
+				.cmds       = VOID2U64(cmds),
+				.nr_bos  = ARRAY_SIZE(bos),
+				.bos     = VOID2U64(bos),
+		};
+		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
+	}
+
+	igt_subtest("valid-submit") {
+		struct drm_msm_gem_submit_bo bos[] = {
+			[0] = {
+				.handle     = a->handle,
+				.flags      = MSM_SUBMIT_BO_READ,
+			},
+		};
+		struct drm_msm_gem_submit_cmd cmds[] = {
+			[0] = {
+				.type       = MSM_SUBMIT_CMD_BUF,
+				.submit_idx = 0,
+				.size       = 4 * 4,  /* 4 dwords in cmdbuf */
+			},
+		};
+		struct drm_msm_gem_submit req = {
+				.flags   = pipe->pipe,
+				.queueid = pipe->submitqueue_id,
+				.nr_cmds    = ARRAY_SIZE(cmds),
+				.cmds       = VOID2U64(cmds),
+				.nr_bos  = ARRAY_SIZE(bos),
+				.bos     = VOID2U64(bos),
+		};
+		uint32_t *cmdstream = igt_msm_bo_map(a);
+		if (dev->gen >= 5) {
+			*(cmdstream++) = pm4_pkt7_hdr(CP_NOP, 3);
+		} else {
+			*(cmdstream++) = pm4_pkt3_hdr(CP_NOP, 3);
+		}
+		*(cmdstream++) = 0;
+		*(cmdstream++) = 0;
+		*(cmdstream++) = 0;
+
+		do_ioctl(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req);
+	}
+
+	igt_fixture {
+		igt_msm_bo_free(a);
+		igt_msm_bo_free(b);
+		igt_msm_pipe_close(pipe);
+		igt_msm_dev_close(dev);
+	}
+}
-- 
2.31.1

Re: [PATCH igt v2 2/3] msm: Add helper library

[Petri Latvala]

On Wed, Aug 25, 2021 at 04:31:38PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Handle some of the boilerplate for tests.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  lib/igt_msm.c   | 171 ++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_msm.h   | 119 +++++++++++++++++++++++++++++++++
>  lib/meson.build |   1 +
>  3 files changed, 291 insertions(+)
>  create mode 100644 lib/igt_msm.c
>  create mode 100644 lib/igt_msm.h
> 
> diff --git a/lib/igt_msm.c b/lib/igt_msm.c
> new file mode 100644
> index 00000000..3bd0ee53
> --- /dev/null
> +++ b/lib/igt_msm.c
> @@ -0,0 +1,171 @@
> +/*
> + * Copyright © 2021 Google, Inc.
> + *
> + * 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 <assert.h>
> +#include <string.h>
> +#include <signal.h>
> +#include <errno.h>
> +#include <sys/mman.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/ioctl.h>
> +#include <fcntl.h>
> +
> +#include "drmtest.h"
> +#include "igt_aux.h"
> +#include "igt_core.h"
> +#include "igt_msm.h"
> +#include "ioctl_wrappers.h"
> +
> +/**
> + * SECTION:igt_msm
> + * @short_description: msm support library
> + * @title: msm
> + * @include: igt_msm.h
> + *
> + * This library provides various auxiliary helper functions for writing msm
> + * tests.
> + */
> +
> +static uint64_t
> +get_param(struct msm_device *dev, uint32_t pipe, uint32_t param)
> +{
> +	struct drm_msm_param req = {
> +			.pipe = pipe,
> +			.param = param,
> +	};
> +
> +	do_ioctl(dev->fd, DRM_IOCTL_MSM_GET_PARAM, &req);
> +
> +	return req.value;
> +}
> +
> +struct msm_device *
> +igt_msm_dev_open(void)
> +{
> +	struct msm_device *dev = calloc(1, sizeof(*dev));
> +
> +	dev->fd = drm_open_driver_render(DRIVER_MSM);
> +	if (dev->fd < 0) {
> +		free(dev);
> +		return NULL;
> +	}

Note that drm_open_driver_render() cannot return < 0.


> +
> +	dev->gen = (get_param(dev, MSM_PIPE_3D0, MSM_PARAM_CHIP_ID) >> 24) & 0xff;
> +
> +	return dev;
> +}
> +
> +void
> +igt_msm_dev_close(struct msm_device *dev)
> +{
> +	close(dev->fd);
> +	free(dev);
> +}
> +
> +struct msm_bo *
> +igt_msm_bo_new(struct msm_device *dev, size_t size, uint32_t flags)
> +{
> +	struct msm_bo *bo = calloc(1, sizeof(*bo));
> +
> +	struct drm_msm_gem_new req = {
> +			.size = size,
> +			.flags = flags,
> +	};
> +
> +	bo->dev = dev;
> +	bo->size = size;
> +
> +	do_ioctl(dev->fd, DRM_IOCTL_MSM_GEM_NEW, &req);
> +
> +	bo->handle = req.handle;
> +
> +	return bo;
> +}
> +
> +void
> +igt_msm_bo_free(struct msm_bo *bo)
> +{
> +	if (bo->map)
> +		munmap(bo->map, bo->size);
> +	gem_close(bo->dev->fd, bo->handle);
> +	free(bo);
> +}
> +
> +void *
> +igt_msm_bo_map(struct msm_bo *bo)
> +{
> +	if (!bo->map) {
> +		struct drm_msm_gem_info req = {
> +				.handle = bo->handle,
> +				.info = MSM_INFO_GET_OFFSET,
> +		};
> +		void *ptr;
> +
> +		do_ioctl(bo->dev->fd, DRM_IOCTL_MSM_GEM_INFO, &req);
> +
> +		ptr = mmap(0, bo->size, PROT_READ | PROT_WRITE, MAP_SHARED,
> +				bo->dev->fd, req.value);
> +		if (ptr == MAP_FAILED)
> +			return NULL;
> +
> +		bo->map = ptr;
> +	}
> +	return bo->map;
> +}
> +
> +struct msm_pipe *
> +igt_msm_pipe_open(struct msm_device *dev, uint32_t prio)
> +{
> +	struct msm_pipe *pipe = calloc(1, sizeof(*pipe));
> +	struct drm_msm_submitqueue req = {
> +			.flags = 0,
> +			.prio = prio,
> +	};
> +
> +	pipe->dev = dev;
> +	pipe->pipe = MSM_PIPE_3D0;
> +
> +	/* Note that kerenels prior to v4.15 did not support submitqueues.
> +	 * Mesa maintains support for older kernels, but I do not think
> +	 * that IGT needs to.
> +	 */
> +	do_ioctl(dev->fd, DRM_IOCTL_MSM_SUBMITQUEUE_NEW, &req);

We try to maintain compatibility with older kernels to around "yay
back". If you want to be perfect, this part could produce a skip if
submitqueues don't exist, but most often such dancing is not worth the
trouble. Letting it fail "normally" on an old kernel is fine, the
error message received already points out which ioctl failed. You can
remove the uncertainty from this comment, in other words.

Also typo, kerenels -> kernels.


-- 
Petri Latvala

Re: [PATCH igt v2 3/3] msm: Add submit ioctl tests

[Petri Latvala]

On Wed, Aug 25, 2021 at 04:31:39PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Add an initial set of tests for the submit ioctl.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  tests/meson.build  |   1 +
>  tests/msm_submit.c | 186 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 187 insertions(+)
>  create mode 100644 tests/msm_submit.c
> 
> diff --git a/tests/meson.build b/tests/meson.build
> index 1bdfddbb..ff7c709a 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -107,6 +107,7 @@ test_progs = [
>  	'vc4_wait_seqno',
>  	'vgem_basic',
>  	'vgem_slow',
> +	'msm_submit',
>  ]
>  
>  i915_progs = [
> diff --git a/tests/msm_submit.c b/tests/msm_submit.c
> new file mode 100644
> index 00000000..da93c574
> --- /dev/null
> +++ b/tests/msm_submit.c
> @@ -0,0 +1,186 @@
> +/*
> + * Copyright © 2021 Google, Inc.
> + *
> + * 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_msm.h"
> +
> +igt_main
> +{
> +	struct msm_device *dev;
> +	struct msm_pipe *pipe;
> +	struct msm_bo *a, *b;
> +
> +	igt_fixture {
> +		dev = igt_msm_dev_open();

What I replied on 2/3 applies here: If opening the device fails,
igt_msm_dev_open() does not return and 'dev' is left uninitialized,
those other pointers likewise. Leading to...

> +		pipe = igt_msm_pipe_open(dev, 0);
> +		a = igt_msm_bo_new(dev, 0x1000, MSM_BO_WC);
> +		b = igt_msm_bo_new(dev, 0x1000, MSM_BO_WC);
> +	}
> +
> +	igt_subtest("empty-submit") {
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe,
> +				.queueid = pipe->submitqueue_id,
> +		};
> +		do_ioctl(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req);
> +	}
> +
> +	igt_subtest("invalid-queue-submit") {
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe,
> +				.queueid = 0x1234,
> +		};
> +		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, ENOENT);
> +	}
> +
> +	igt_subtest("invalid-flags-submit") {
> +		struct drm_msm_gem_submit req = {
> +				.flags   = 0x1234,
> +				.queueid = pipe->submitqueue_id,
> +		};
> +		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> +	}
> +
> +	igt_subtest("invalid-in-fence-submit") {
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe | MSM_SUBMIT_FENCE_FD_IN,
> +				.queueid = pipe->submitqueue_id,
> +				.fence_fd = dev->fd,  /* This is not a fence fd! */
> +		};
> +		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> +	}
> +
> +	igt_subtest("invalid-duplicate-bo-submit") {
> +		struct drm_msm_gem_submit_bo bos[] = {
> +			[0] = {
> +				.handle     = a->handle,
> +				.flags      = MSM_SUBMIT_BO_READ,
> +			},
> +			[1] = {
> +				.handle     = b->handle,
> +				.flags      = MSM_SUBMIT_BO_READ,
> +			},
> +			[2] = {
> +				/* this is invalid.. there should not be two entries
> +				 * for the same bo, instead a single entry w/ all
> +				 * usage flags OR'd together should be used.  Kernel
> +				 * should catch this, and return an error code after
> +				 * cleaning up properly (not leaking any bo's)
> +				 */
> +				.handle     = a->handle,
> +				.flags      = MSM_SUBMIT_BO_WRITE,
> +			},
> +		};
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe,
> +				.queueid = pipe->submitqueue_id,
> +				.nr_bos  = ARRAY_SIZE(bos),
> +				.bos     = VOID2U64(bos),
> +		};
> +		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> +	}
> +
> +	igt_subtest("invalid-cmd-idx-submit") {
> +		struct drm_msm_gem_submit_cmd cmds[] = {
> +			[0] = {
> +				.type       = MSM_SUBMIT_CMD_BUF,
> +				.submit_idx = 0,      /* bos[0] does not exist */
> +				.size       = 4 * 4,  /* 4 dwords in cmdbuf */
> +			},
> +		};
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe,
> +				.queueid = pipe->submitqueue_id,
> +				.nr_cmds    = ARRAY_SIZE(cmds),
> +				.cmds       = VOID2U64(cmds),
> +		};
> +		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> +	}
> +
> +	igt_subtest("invalid-cmd-type-submit") {
> +		struct drm_msm_gem_submit_bo bos[] = {
> +			[0] = {
> +				.handle     = a->handle,
> +				.flags      = MSM_SUBMIT_BO_READ,
> +			},
> +		};
> +		struct drm_msm_gem_submit_cmd cmds[] = {
> +			[0] = {
> +				.type       = 0x1234,
> +				.submit_idx = 0,
> +				.size       = 4 * 4,  /* 4 dwords in cmdbuf */
> +			},
> +		};
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe,
> +				.queueid = pipe->submitqueue_id,
> +				.nr_cmds    = ARRAY_SIZE(cmds),
> +				.cmds       = VOID2U64(cmds),
> +				.nr_bos  = ARRAY_SIZE(bos),
> +				.bos     = VOID2U64(bos),
> +		};
> +		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> +	}
> +
> +	igt_subtest("valid-submit") {
> +		struct drm_msm_gem_submit_bo bos[] = {
> +			[0] = {
> +				.handle     = a->handle,
> +				.flags      = MSM_SUBMIT_BO_READ,
> +			},
> +		};
> +		struct drm_msm_gem_submit_cmd cmds[] = {
> +			[0] = {
> +				.type       = MSM_SUBMIT_CMD_BUF,
> +				.submit_idx = 0,
> +				.size       = 4 * 4,  /* 4 dwords in cmdbuf */
> +			},
> +		};
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe,
> +				.queueid = pipe->submitqueue_id,
> +				.nr_cmds    = ARRAY_SIZE(cmds),
> +				.cmds       = VOID2U64(cmds),
> +				.nr_bos  = ARRAY_SIZE(bos),
> +				.bos     = VOID2U64(bos),
> +		};
> +		uint32_t *cmdstream = igt_msm_bo_map(a);
> +		if (dev->gen >= 5) {
> +			*(cmdstream++) = pm4_pkt7_hdr(CP_NOP, 3);
> +		} else {
> +			*(cmdstream++) = pm4_pkt3_hdr(CP_NOP, 3);
> +		}
> +		*(cmdstream++) = 0;
> +		*(cmdstream++) = 0;
> +		*(cmdstream++) = 0;
> +
> +		do_ioctl(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req);
> +	}
> +
> +	igt_fixture {
> +		igt_msm_bo_free(a);
> +		igt_msm_bo_free(b);
> +		igt_msm_pipe_close(pipe);
> +		igt_msm_dev_close(dev);

... crashes in here.


-- 
Petri Latvala

Re: [PATCH igt v2 3/3] msm: Add submit ioctl tests

[Rob Clark]

On Wed, Aug 25, 2021 at 10:28 PM Petri Latvala <petri.latvala@intel.com> wrote:
>
> On Wed, Aug 25, 2021 at 04:31:39PM -0700, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > Add an initial set of tests for the submit ioctl.
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >  tests/meson.build  |   1 +
> >  tests/msm_submit.c | 186 +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 187 insertions(+)
> >  create mode 100644 tests/msm_submit.c
> >
> > diff --git a/tests/meson.build b/tests/meson.build
> > index 1bdfddbb..ff7c709a 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -107,6 +107,7 @@ test_progs = [
> >       'vc4_wait_seqno',
> >       'vgem_basic',
> >       'vgem_slow',
> > +     'msm_submit',
> >  ]
> >
> >  i915_progs = [
> > diff --git a/tests/msm_submit.c b/tests/msm_submit.c
> > new file mode 100644
> > index 00000000..da93c574
> > --- /dev/null
> > +++ b/tests/msm_submit.c
> > @@ -0,0 +1,186 @@
> > +/*
> > + * Copyright © 2021 Google, Inc.
> > + *
> > + * 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_msm.h"
> > +
> > +igt_main
> > +{
> > +     struct msm_device *dev;
> > +     struct msm_pipe *pipe;
> > +     struct msm_bo *a, *b;
> > +
> > +     igt_fixture {
> > +             dev = igt_msm_dev_open();
>
> What I replied on 2/3 applies here: If opening the device fails,
> igt_msm_dev_open() does not return and 'dev' is left uninitialized,
> those other pointers likewise. Leading to...
>
> > +             pipe = igt_msm_pipe_open(dev, 0);
> > +             a = igt_msm_bo_new(dev, 0x1000, MSM_BO_WC);
> > +             b = igt_msm_bo_new(dev, 0x1000, MSM_BO_WC);
> > +     }
> > +
> > +     igt_subtest("empty-submit") {
> > +             struct drm_msm_gem_submit req = {
> > +                             .flags   = pipe->pipe,
> > +                             .queueid = pipe->submitqueue_id,
> > +             };
> > +             do_ioctl(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req);
> > +     }
> > +
> > +     igt_subtest("invalid-queue-submit") {
> > +             struct drm_msm_gem_submit req = {
> > +                             .flags   = pipe->pipe,
> > +                             .queueid = 0x1234,
> > +             };
> > +             do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, ENOENT);
> > +     }
> > +
> > +     igt_subtest("invalid-flags-submit") {
> > +             struct drm_msm_gem_submit req = {
> > +                             .flags   = 0x1234,
> > +                             .queueid = pipe->submitqueue_id,
> > +             };
> > +             do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> > +     }
> > +
> > +     igt_subtest("invalid-in-fence-submit") {
> > +             struct drm_msm_gem_submit req = {
> > +                             .flags   = pipe->pipe | MSM_SUBMIT_FENCE_FD_IN,
> > +                             .queueid = pipe->submitqueue_id,
> > +                             .fence_fd = dev->fd,  /* This is not a fence fd! */
> > +             };
> > +             do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> > +     }
> > +
> > +     igt_subtest("invalid-duplicate-bo-submit") {
> > +             struct drm_msm_gem_submit_bo bos[] = {
> > +                     [0] = {
> > +                             .handle     = a->handle,
> > +                             .flags      = MSM_SUBMIT_BO_READ,
> > +                     },
> > +                     [1] = {
> > +                             .handle     = b->handle,
> > +                             .flags      = MSM_SUBMIT_BO_READ,
> > +                     },
> > +                     [2] = {
> > +                             /* this is invalid.. there should not be two entries
> > +                              * for the same bo, instead a single entry w/ all
> > +                              * usage flags OR'd together should be used.  Kernel
> > +                              * should catch this, and return an error code after
> > +                              * cleaning up properly (not leaking any bo's)
> > +                              */
> > +                             .handle     = a->handle,
> > +                             .flags      = MSM_SUBMIT_BO_WRITE,
> > +                     },
> > +             };
> > +             struct drm_msm_gem_submit req = {
> > +                             .flags   = pipe->pipe,
> > +                             .queueid = pipe->submitqueue_id,
> > +                             .nr_bos  = ARRAY_SIZE(bos),
> > +                             .bos     = VOID2U64(bos),
> > +             };
> > +             do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> > +     }
> > +
> > +     igt_subtest("invalid-cmd-idx-submit") {
> > +             struct drm_msm_gem_submit_cmd cmds[] = {
> > +                     [0] = {
> > +                             .type       = MSM_SUBMIT_CMD_BUF,
> > +                             .submit_idx = 0,      /* bos[0] does not exist */
> > +                             .size       = 4 * 4,  /* 4 dwords in cmdbuf */
> > +                     },
> > +             };
> > +             struct drm_msm_gem_submit req = {
> > +                             .flags   = pipe->pipe,
> > +                             .queueid = pipe->submitqueue_id,
> > +                             .nr_cmds    = ARRAY_SIZE(cmds),
> > +                             .cmds       = VOID2U64(cmds),
> > +             };
> > +             do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> > +     }
> > +
> > +     igt_subtest("invalid-cmd-type-submit") {
> > +             struct drm_msm_gem_submit_bo bos[] = {
> > +                     [0] = {
> > +                             .handle     = a->handle,
> > +                             .flags      = MSM_SUBMIT_BO_READ,
> > +                     },
> > +             };
> > +             struct drm_msm_gem_submit_cmd cmds[] = {
> > +                     [0] = {
> > +                             .type       = 0x1234,
> > +                             .submit_idx = 0,
> > +                             .size       = 4 * 4,  /* 4 dwords in cmdbuf */
> > +                     },
> > +             };
> > +             struct drm_msm_gem_submit req = {
> > +                             .flags   = pipe->pipe,
> > +                             .queueid = pipe->submitqueue_id,
> > +                             .nr_cmds    = ARRAY_SIZE(cmds),
> > +                             .cmds       = VOID2U64(cmds),
> > +                             .nr_bos  = ARRAY_SIZE(bos),
> > +                             .bos     = VOID2U64(bos),
> > +             };
> > +             do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> > +     }
> > +
> > +     igt_subtest("valid-submit") {
> > +             struct drm_msm_gem_submit_bo bos[] = {
> > +                     [0] = {
> > +                             .handle     = a->handle,
> > +                             .flags      = MSM_SUBMIT_BO_READ,
> > +                     },
> > +             };
> > +             struct drm_msm_gem_submit_cmd cmds[] = {
> > +                     [0] = {
> > +                             .type       = MSM_SUBMIT_CMD_BUF,
> > +                             .submit_idx = 0,
> > +                             .size       = 4 * 4,  /* 4 dwords in cmdbuf */
> > +                     },
> > +             };
> > +             struct drm_msm_gem_submit req = {
> > +                             .flags   = pipe->pipe,
> > +                             .queueid = pipe->submitqueue_id,
> > +                             .nr_cmds    = ARRAY_SIZE(cmds),
> > +                             .cmds       = VOID2U64(cmds),
> > +                             .nr_bos  = ARRAY_SIZE(bos),
> > +                             .bos     = VOID2U64(bos),
> > +             };
> > +             uint32_t *cmdstream = igt_msm_bo_map(a);
> > +             if (dev->gen >= 5) {
> > +                     *(cmdstream++) = pm4_pkt7_hdr(CP_NOP, 3);
> > +             } else {
> > +                     *(cmdstream++) = pm4_pkt3_hdr(CP_NOP, 3);
> > +             }
> > +             *(cmdstream++) = 0;
> > +             *(cmdstream++) = 0;
> > +             *(cmdstream++) = 0;
> > +
> > +             do_ioctl(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req);
> > +     }
> > +
> > +     igt_fixture {
> > +             igt_msm_bo_free(a);
> > +             igt_msm_bo_free(b);
> > +             igt_msm_pipe_close(pipe);
> > +             igt_msm_dev_close(dev);
>
> ... crashes in here.
>

I did test this on intel as well, and it skips properly.. I think the
setjmp/longjmp magic just bails completely out so we never try to run
the cleanup?

BR,
-R

Re: [PATCH igt v2 3/3] msm: Add submit ioctl tests

[Petri Latvala]

On Thu, Aug 26, 2021 at 08:37:19AM -0700, Rob Clark wrote:
> On Wed, Aug 25, 2021 at 10:28 PM Petri Latvala <petri.latvala@intel.com> wrote:
> >
> > On Wed, Aug 25, 2021 at 04:31:39PM -0700, Rob Clark wrote:
> > > From: Rob Clark <robdclark@chromium.org>
> > >
> > > Add an initial set of tests for the submit ioctl.
> > >
> > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > ---
> > >  tests/meson.build  |   1 +
> > >  tests/msm_submit.c | 186 +++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 187 insertions(+)
> > >  create mode 100644 tests/msm_submit.c
> > >
> > > diff --git a/tests/meson.build b/tests/meson.build
> > > index 1bdfddbb..ff7c709a 100644
> > > --- a/tests/meson.build
> > > +++ b/tests/meson.build
> > > @@ -107,6 +107,7 @@ test_progs = [
> > >       'vc4_wait_seqno',
> > >       'vgem_basic',
> > >       'vgem_slow',
> > > +     'msm_submit',
> > >  ]
> > >
> > >  i915_progs = [
> > > diff --git a/tests/msm_submit.c b/tests/msm_submit.c
> > > new file mode 100644
> > > index 00000000..da93c574
> > > --- /dev/null
> > > +++ b/tests/msm_submit.c
> > > @@ -0,0 +1,186 @@
> > > +/*
> > > + * Copyright © 2021 Google, Inc.
> > > + *
> > > + * 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_msm.h"
> > > +
> > > +igt_main
> > > +{
> > > +     struct msm_device *dev;
> > > +     struct msm_pipe *pipe;
> > > +     struct msm_bo *a, *b;
> > > +
> > > +     igt_fixture {
> > > +             dev = igt_msm_dev_open();
> >
> > What I replied on 2/3 applies here: If opening the device fails,
> > igt_msm_dev_open() does not return and 'dev' is left uninitialized,
> > those other pointers likewise. Leading to...
> >
> > > +             pipe = igt_msm_pipe_open(dev, 0);
> > > +             a = igt_msm_bo_new(dev, 0x1000, MSM_BO_WC);
> > > +             b = igt_msm_bo_new(dev, 0x1000, MSM_BO_WC);
> > > +     }
> > > +
> > > +     igt_subtest("empty-submit") {
> > > +             struct drm_msm_gem_submit req = {
> > > +                             .flags   = pipe->pipe,
> > > +                             .queueid = pipe->submitqueue_id,
> > > +             };
> > > +             do_ioctl(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req);
> > > +     }
> > > +
> > > +     igt_subtest("invalid-queue-submit") {
> > > +             struct drm_msm_gem_submit req = {
> > > +                             .flags   = pipe->pipe,
> > > +                             .queueid = 0x1234,
> > > +             };
> > > +             do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, ENOENT);
> > > +     }
> > > +
> > > +     igt_subtest("invalid-flags-submit") {
> > > +             struct drm_msm_gem_submit req = {
> > > +                             .flags   = 0x1234,
> > > +                             .queueid = pipe->submitqueue_id,
> > > +             };
> > > +             do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> > > +     }
> > > +
> > > +     igt_subtest("invalid-in-fence-submit") {
> > > +             struct drm_msm_gem_submit req = {
> > > +                             .flags   = pipe->pipe | MSM_SUBMIT_FENCE_FD_IN,
> > > +                             .queueid = pipe->submitqueue_id,
> > > +                             .fence_fd = dev->fd,  /* This is not a fence fd! */
> > > +             };
> > > +             do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> > > +     }
> > > +
> > > +     igt_subtest("invalid-duplicate-bo-submit") {
> > > +             struct drm_msm_gem_submit_bo bos[] = {
> > > +                     [0] = {
> > > +                             .handle     = a->handle,
> > > +                             .flags      = MSM_SUBMIT_BO_READ,
> > > +                     },
> > > +                     [1] = {
> > > +                             .handle     = b->handle,
> > > +                             .flags      = MSM_SUBMIT_BO_READ,
> > > +                     },
> > > +                     [2] = {
> > > +                             /* this is invalid.. there should not be two entries
> > > +                              * for the same bo, instead a single entry w/ all
> > > +                              * usage flags OR'd together should be used.  Kernel
> > > +                              * should catch this, and return an error code after
> > > +                              * cleaning up properly (not leaking any bo's)
> > > +                              */
> > > +                             .handle     = a->handle,
> > > +                             .flags      = MSM_SUBMIT_BO_WRITE,
> > > +                     },
> > > +             };
> > > +             struct drm_msm_gem_submit req = {
> > > +                             .flags   = pipe->pipe,
> > > +                             .queueid = pipe->submitqueue_id,
> > > +                             .nr_bos  = ARRAY_SIZE(bos),
> > > +                             .bos     = VOID2U64(bos),
> > > +             };
> > > +             do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> > > +     }
> > > +
> > > +     igt_subtest("invalid-cmd-idx-submit") {
> > > +             struct drm_msm_gem_submit_cmd cmds[] = {
> > > +                     [0] = {
> > > +                             .type       = MSM_SUBMIT_CMD_BUF,
> > > +                             .submit_idx = 0,      /* bos[0] does not exist */
> > > +                             .size       = 4 * 4,  /* 4 dwords in cmdbuf */
> > > +                     },
> > > +             };
> > > +             struct drm_msm_gem_submit req = {
> > > +                             .flags   = pipe->pipe,
> > > +                             .queueid = pipe->submitqueue_id,
> > > +                             .nr_cmds    = ARRAY_SIZE(cmds),
> > > +                             .cmds       = VOID2U64(cmds),
> > > +             };
> > > +             do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> > > +     }
> > > +
> > > +     igt_subtest("invalid-cmd-type-submit") {
> > > +             struct drm_msm_gem_submit_bo bos[] = {
> > > +                     [0] = {
> > > +                             .handle     = a->handle,
> > > +                             .flags      = MSM_SUBMIT_BO_READ,
> > > +                     },
> > > +             };
> > > +             struct drm_msm_gem_submit_cmd cmds[] = {
> > > +                     [0] = {
> > > +                             .type       = 0x1234,
> > > +                             .submit_idx = 0,
> > > +                             .size       = 4 * 4,  /* 4 dwords in cmdbuf */
> > > +                     },
> > > +             };
> > > +             struct drm_msm_gem_submit req = {
> > > +                             .flags   = pipe->pipe,
> > > +                             .queueid = pipe->submitqueue_id,
> > > +                             .nr_cmds    = ARRAY_SIZE(cmds),
> > > +                             .cmds       = VOID2U64(cmds),
> > > +                             .nr_bos  = ARRAY_SIZE(bos),
> > > +                             .bos     = VOID2U64(bos),
> > > +             };
> > > +             do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> > > +     }
> > > +
> > > +     igt_subtest("valid-submit") {
> > > +             struct drm_msm_gem_submit_bo bos[] = {
> > > +                     [0] = {
> > > +                             .handle     = a->handle,
> > > +                             .flags      = MSM_SUBMIT_BO_READ,
> > > +                     },
> > > +             };
> > > +             struct drm_msm_gem_submit_cmd cmds[] = {
> > > +                     [0] = {
> > > +                             .type       = MSM_SUBMIT_CMD_BUF,
> > > +                             .submit_idx = 0,
> > > +                             .size       = 4 * 4,  /* 4 dwords in cmdbuf */
> > > +                     },
> > > +             };
> > > +             struct drm_msm_gem_submit req = {
> > > +                             .flags   = pipe->pipe,
> > > +                             .queueid = pipe->submitqueue_id,
> > > +                             .nr_cmds    = ARRAY_SIZE(cmds),
> > > +                             .cmds       = VOID2U64(cmds),
> > > +                             .nr_bos  = ARRAY_SIZE(bos),
> > > +                             .bos     = VOID2U64(bos),
> > > +             };
> > > +             uint32_t *cmdstream = igt_msm_bo_map(a);
> > > +             if (dev->gen >= 5) {
> > > +                     *(cmdstream++) = pm4_pkt7_hdr(CP_NOP, 3);
> > > +             } else {
> > > +                     *(cmdstream++) = pm4_pkt3_hdr(CP_NOP, 3);
> > > +             }
> > > +             *(cmdstream++) = 0;
> > > +             *(cmdstream++) = 0;
> > > +             *(cmdstream++) = 0;
> > > +
> > > +             do_ioctl(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req);
> > > +     }
> > > +
> > > +     igt_fixture {
> > > +             igt_msm_bo_free(a);
> > > +             igt_msm_bo_free(b);
> > > +             igt_msm_pipe_close(pipe);
> > > +             igt_msm_dev_close(dev);
> >
> > ... crashes in here.
> >
> 
> I did test this on intel as well, and it skips properly.. I think the
> setjmp/longjmp magic just bails completely out so we never try to run
> the cleanup?

Ah, indeed, skip_henceforth prevents entering the latter fixture.

It still looks scary, can I implore to add some defensive layers with
NULL-inits and handling of NULLs in the functions? It might not crash
today...


-- 
Petri Latvala

Re: [PATCH igt v2 1/3] drmtest: Add DRIVER_MSM support

[Petri Latvala]

On Wed, Aug 25, 2021 at 04:31:37PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>

Reviewed-by: Petri Latvala <petri.latvala@intel.com>


> ---
>  lib/drmtest.c | 3 +++
>  lib/drmtest.h | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index e1f9b115..29cb3f4c 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -179,6 +179,7 @@ static const struct module {
>  } modules[] = {
>  	{ DRIVER_AMDGPU, "amdgpu" },
>  	{ DRIVER_INTEL, "i915", modprobe_i915 },
> +	{ DRIVER_MSM, "msm" },
>  	{ DRIVER_PANFROST, "panfrost" },
>  	{ DRIVER_V3D, "v3d" },
>  	{ DRIVER_VC4, "vc4" },
> @@ -539,6 +540,8 @@ static const char *chipset_to_str(int chipset)
>  		return "amdgpu";
>  	case DRIVER_PANFROST:
>  		return "panfrost";
> +	case DRIVER_MSM:
> +		return "msm";
>  	case DRIVER_ANY:
>  		return "any";
>  	default:
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 7d17a0f9..a6eb60c3 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -50,6 +50,7 @@
>  #define DRIVER_AMDGPU	(1 << 3)
>  #define DRIVER_V3D	(1 << 4)
>  #define DRIVER_PANFROST	(1 << 5)
> +#define DRIVER_MSM	(1 << 6)
>  
>  /*
>   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
> -- 
> 2.31.1
> 

Re: [PATCH igt v2 2/3] msm: Add helper library

[Petri Latvala]

On Wed, Aug 25, 2021 at 04:31:38PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Handle some of the boilerplate for tests.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  lib/igt_msm.c   | 171 ++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_msm.h   | 119 +++++++++++++++++++++++++++++++++
>  lib/meson.build |   1 +
>  3 files changed, 291 insertions(+)
>  create mode 100644 lib/igt_msm.c
>  create mode 100644 lib/igt_msm.h
> 
> diff --git a/lib/igt_msm.c b/lib/igt_msm.c
> new file mode 100644
> index 00000000..3bd0ee53
> --- /dev/null
> +++ b/lib/igt_msm.c
> @@ -0,0 +1,171 @@
> +/*
> + * Copyright © 2021 Google, Inc.
> + *
> + * 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 <assert.h>
> +#include <string.h>
> +#include <signal.h>
> +#include <errno.h>
> +#include <sys/mman.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/ioctl.h>
> +#include <fcntl.h>
> +
> +#include "drmtest.h"
> +#include "igt_aux.h"
> +#include "igt_core.h"
> +#include "igt_msm.h"
> +#include "ioctl_wrappers.h"
> +
> +/**
> + * SECTION:igt_msm
> + * @short_description: msm support library
> + * @title: msm
> + * @include: igt_msm.h
> + *
> + * This library provides various auxiliary helper functions for writing msm
> + * tests.
> + */

You need to add

<xi:include href="xml/igt_msm.xml"/>

to docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml so this gets
included, and please add documentation for all nonstatic functions.


-- 
Petri Latvala


> +
> +static uint64_t
> +get_param(struct msm_device *dev, uint32_t pipe, uint32_t param)
> +{
> +	struct drm_msm_param req = {
> +			.pipe = pipe,
> +			.param = param,
> +	};
> +
> +	do_ioctl(dev->fd, DRM_IOCTL_MSM_GET_PARAM, &req);
> +
> +	return req.value;
> +}
> +
> +struct msm_device *
> +igt_msm_dev_open(void)
> +{
> +	struct msm_device *dev = calloc(1, sizeof(*dev));
> +
> +	dev->fd = drm_open_driver_render(DRIVER_MSM);
> +	if (dev->fd < 0) {
> +		free(dev);
> +		return NULL;
> +	}
> +
> +	dev->gen = (get_param(dev, MSM_PIPE_3D0, MSM_PARAM_CHIP_ID) >> 24) & 0xff;
> +
> +	return dev;
> +}
> +
> +void
> +igt_msm_dev_close(struct msm_device *dev)
> +{
> +	close(dev->fd);
> +	free(dev);
> +}
> +
> +struct msm_bo *
> +igt_msm_bo_new(struct msm_device *dev, size_t size, uint32_t flags)
> +{
> +	struct msm_bo *bo = calloc(1, sizeof(*bo));
> +
> +	struct drm_msm_gem_new req = {
> +			.size = size,
> +			.flags = flags,
> +	};
> +
> +	bo->dev = dev;
> +	bo->size = size;
> +
> +	do_ioctl(dev->fd, DRM_IOCTL_MSM_GEM_NEW, &req);
> +
> +	bo->handle = req.handle;
> +
> +	return bo;
> +}
> +
> +void
> +igt_msm_bo_free(struct msm_bo *bo)
> +{
> +	if (bo->map)
> +		munmap(bo->map, bo->size);
> +	gem_close(bo->dev->fd, bo->handle);
> +	free(bo);
> +}
> +
> +void *
> +igt_msm_bo_map(struct msm_bo *bo)
> +{
> +	if (!bo->map) {
> +		struct drm_msm_gem_info req = {
> +				.handle = bo->handle,
> +				.info = MSM_INFO_GET_OFFSET,
> +		};
> +		void *ptr;
> +
> +		do_ioctl(bo->dev->fd, DRM_IOCTL_MSM_GEM_INFO, &req);
> +
> +		ptr = mmap(0, bo->size, PROT_READ | PROT_WRITE, MAP_SHARED,
> +				bo->dev->fd, req.value);
> +		if (ptr == MAP_FAILED)
> +			return NULL;
> +
> +		bo->map = ptr;
> +	}
> +	return bo->map;
> +}
> +
> +struct msm_pipe *
> +igt_msm_pipe_open(struct msm_device *dev, uint32_t prio)
> +{
> +	struct msm_pipe *pipe = calloc(1, sizeof(*pipe));
> +	struct drm_msm_submitqueue req = {
> +			.flags = 0,
> +			.prio = prio,
> +	};
> +
> +	pipe->dev = dev;
> +	pipe->pipe = MSM_PIPE_3D0;
> +
> +	/* Note that kerenels prior to v4.15 did not support submitqueues.
> +	 * Mesa maintains support for older kernels, but I do not think
> +	 * that IGT needs to.
> +	 */
> +	do_ioctl(dev->fd, DRM_IOCTL_MSM_SUBMITQUEUE_NEW, &req);
> +
> +	pipe->submitqueue_id = req.id;
> +
> +	return pipe;
> +}
> +
> +void
> +igt_msm_pipe_close(struct msm_pipe *pipe)
> +{
> +	do_ioctl(pipe->dev->fd, DRM_IOCTL_MSM_SUBMITQUEUE_CLOSE, &pipe->submitqueue_id);
> +	free(pipe);
> +}
> +
> +uint64_t
> +igt_msm_pipe_get_param(struct msm_pipe *pipe, uint32_t param)
> +{
> +	return get_param(pipe->dev, pipe->pipe, param);
> +}
> diff --git a/lib/igt_msm.h b/lib/igt_msm.h
> new file mode 100644
> index 00000000..614c42ee
> --- /dev/null
> +++ b/lib/igt_msm.h
> @@ -0,0 +1,119 @@
> +/*
> + * Copyright © 2021 Google, Inc.
> + *
> + * 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.
> + */
> +
> +#ifndef IGT_MSM_H
> +#define IGT_MSM_H
> +
> +#include "msm_drm.h"
> +
> +struct msm_device {
> +	int fd;
> +	unsigned gen;
> +};
> +
> +struct msm_device *igt_msm_dev_open(void);
> +void igt_msm_dev_close(struct msm_device *dev);
> +
> +struct msm_bo {
> +	struct msm_device *dev;
> +	int handle;
> +	uint32_t size;
> +	void *map;
> +};
> +
> +struct msm_bo *igt_msm_bo_new(struct msm_device *dev, size_t size, uint32_t flags);
> +void igt_msm_bo_free(struct msm_bo *bo);
> +void *igt_msm_bo_map(struct msm_bo *bo);
> +
> +struct msm_pipe {
> +	struct msm_device *dev;
> +	uint32_t pipe;
> +	uint32_t submitqueue_id;
> +};
> +
> +struct msm_pipe *igt_msm_pipe_open(struct msm_device *dev, uint32_t prio);
> +void igt_msm_pipe_close(struct msm_pipe *pipe);
> +uint64_t igt_msm_pipe_get_param(struct msm_pipe *pipe, uint32_t param);
> +
> +/*
> + * Helpers for cmdstream building:
> + */
> +
> +enum adreno_pm4_packet_type {
> +	CP_TYPE0_PKT = 0,
> +	CP_TYPE1_PKT = 0x40000000,
> +	CP_TYPE2_PKT = 0x80000000,
> +	CP_TYPE3_PKT = 0xc0000000,
> +	CP_TYPE4_PKT = 0x40000000,
> +	CP_TYPE7_PKT = 0x70000000,
> +};
> +
> +enum adreno_pm4_type3_packets {
> +	CP_NOP = 16,
> +};
> +
> +static inline unsigned
> +pm4_odd_parity_bit(unsigned val)
> +{
> +	/* See: http://graphics.stanford.edu/~seander/bithacks.html#ParityParallel
> +	 * note that we want odd parity so 0x6996 is inverted.
> +	 */
> +	val ^= val >> 16;
> +	val ^= val >> 8;
> +	val ^= val >> 4;
> +	val &= 0xf;
> +	return (~0x6996 >> val) & 1;
> +}
> +
> +static inline uint32_t
> +pm4_pkt0_hdr(uint16_t regindx, uint16_t cnt)
> +{
> +	return CP_TYPE0_PKT | ((cnt - 1) << 16) | (regindx & 0x7fff);
> +}
> +
> +static inline uint32_t
> +pm4_pkt3_hdr(uint8_t opcode, uint16_t cnt)
> +{
> +	return CP_TYPE3_PKT | ((cnt - 1) << 16) | ((opcode & 0xff) << 8);
> +}
> +
> +static inline uint32_t
> +pm4_pkt4_hdr(uint16_t regindx, uint16_t cnt)
> +{
> +	return CP_TYPE4_PKT | cnt | (pm4_odd_parity_bit(cnt) << 7) |
> +			((regindx & 0x3ffff) << 8) |
> +			((pm4_odd_parity_bit(regindx) << 27));
> +}
> +
> +static inline uint32_t
> +pm4_pkt7_hdr(uint8_t opcode, uint16_t cnt)
> +{
> +	return CP_TYPE7_PKT | cnt | (pm4_odd_parity_bit(cnt) << 15) |
> +			((opcode & 0x7f) << 16) |
> +			((pm4_odd_parity_bit(opcode) << 23));
> +}
> +
> +#define U642VOID(x) ((void *)(uintptr_t)(x))
> +#define VOID2U64(x) ((uint64_t)(uintptr_t)(x))
> +
> +#endif /* IGT_MSM_H */
> diff --git a/lib/meson.build b/lib/meson.build
> index 67d40512..c3080fc8 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -82,6 +82,7 @@ lib_sources = [
>  	'igt_eld.c',
>  	'igt_infoframe.c',
>  	'veboxcopy_gen12.c',
> +	'igt_msm.c',
>  ]
>  
>  lib_deps = [
> -- 
> 2.31.1
> 

Re: [PATCH igt v2 3/3] msm: Add submit ioctl tests

[Petri Latvala]

On Wed, Aug 25, 2021 at 04:31:39PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Add an initial set of tests for the submit ioctl.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  tests/meson.build  |   1 +
>  tests/msm_submit.c | 186 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 187 insertions(+)
>  create mode 100644 tests/msm_submit.c
> 
> diff --git a/tests/meson.build b/tests/meson.build
> index 1bdfddbb..ff7c709a 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -107,6 +107,7 @@ test_progs = [
>  	'vc4_wait_seqno',
>  	'vgem_basic',
>  	'vgem_slow',
> +	'msm_submit',
>  ]
>  
>  i915_progs = [
> diff --git a/tests/msm_submit.c b/tests/msm_submit.c
> new file mode 100644
> index 00000000..da93c574
> --- /dev/null
> +++ b/tests/msm_submit.c
> @@ -0,0 +1,186 @@
> +/*
> + * Copyright © 2021 Google, Inc.
> + *
> + * 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_msm.h"
> +
> +igt_main
> +{
> +	struct msm_device *dev;
> +	struct msm_pipe *pipe;
> +	struct msm_bo *a, *b;
> +
> +	igt_fixture {
> +		dev = igt_msm_dev_open();
> +		pipe = igt_msm_pipe_open(dev, 0);
> +		a = igt_msm_bo_new(dev, 0x1000, MSM_BO_WC);
> +		b = igt_msm_bo_new(dev, 0x1000, MSM_BO_WC);
> +	}
> +
> +	igt_subtest("empty-submit") {
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe,
> +				.queueid = pipe->submitqueue_id,
> +		};
> +		do_ioctl(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req);
> +	}
> +
> +	igt_subtest("invalid-queue-submit") {
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe,
> +				.queueid = 0x1234,
> +		};
> +		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, ENOENT);
> +	}
> +
> +	igt_subtest("invalid-flags-submit") {
> +		struct drm_msm_gem_submit req = {
> +				.flags   = 0x1234,
> +				.queueid = pipe->submitqueue_id,
> +		};
> +		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> +	}
> +
> +	igt_subtest("invalid-in-fence-submit") {
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe | MSM_SUBMIT_FENCE_FD_IN,
> +				.queueid = pipe->submitqueue_id,
> +				.fence_fd = dev->fd,  /* This is not a fence fd! */
> +		};
> +		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> +	}
> +
> +	igt_subtest("invalid-duplicate-bo-submit") {
> +		struct drm_msm_gem_submit_bo bos[] = {
> +			[0] = {
> +				.handle     = a->handle,
> +				.flags      = MSM_SUBMIT_BO_READ,
> +			},
> +			[1] = {
> +				.handle     = b->handle,
> +				.flags      = MSM_SUBMIT_BO_READ,
> +			},
> +			[2] = {
> +				/* this is invalid.. there should not be two entries
> +				 * for the same bo, instead a single entry w/ all
> +				 * usage flags OR'd together should be used.  Kernel
> +				 * should catch this, and return an error code after
> +				 * cleaning up properly (not leaking any bo's)
> +				 */
> +				.handle     = a->handle,
> +				.flags      = MSM_SUBMIT_BO_WRITE,
> +			},
> +		};
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe,
> +				.queueid = pipe->submitqueue_id,
> +				.nr_bos  = ARRAY_SIZE(bos),
> +				.bos     = VOID2U64(bos),
> +		};
> +		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> +	}
> +
> +	igt_subtest("invalid-cmd-idx-submit") {
> +		struct drm_msm_gem_submit_cmd cmds[] = {
> +			[0] = {
> +				.type       = MSM_SUBMIT_CMD_BUF,
> +				.submit_idx = 0,      /* bos[0] does not exist */
> +				.size       = 4 * 4,  /* 4 dwords in cmdbuf */
> +			},
> +		};
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe,
> +				.queueid = pipe->submitqueue_id,
> +				.nr_cmds    = ARRAY_SIZE(cmds),
> +				.cmds       = VOID2U64(cmds),
> +		};
> +		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> +	}
> +
> +	igt_subtest("invalid-cmd-type-submit") {
> +		struct drm_msm_gem_submit_bo bos[] = {
> +			[0] = {
> +				.handle     = a->handle,
> +				.flags      = MSM_SUBMIT_BO_READ,
> +			},
> +		};
> +		struct drm_msm_gem_submit_cmd cmds[] = {
> +			[0] = {
> +				.type       = 0x1234,
> +				.submit_idx = 0,
> +				.size       = 4 * 4,  /* 4 dwords in cmdbuf */
> +			},
> +		};
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe,
> +				.queueid = pipe->submitqueue_id,
> +				.nr_cmds    = ARRAY_SIZE(cmds),
> +				.cmds       = VOID2U64(cmds),
> +				.nr_bos  = ARRAY_SIZE(bos),
> +				.bos     = VOID2U64(bos),
> +		};
> +		do_ioctl_err(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req, EINVAL);
> +	}
> +
> +	igt_subtest("valid-submit") {
> +		struct drm_msm_gem_submit_bo bos[] = {
> +			[0] = {
> +				.handle     = a->handle,
> +				.flags      = MSM_SUBMIT_BO_READ,
> +			},
> +		};
> +		struct drm_msm_gem_submit_cmd cmds[] = {
> +			[0] = {
> +				.type       = MSM_SUBMIT_CMD_BUF,
> +				.submit_idx = 0,
> +				.size       = 4 * 4,  /* 4 dwords in cmdbuf */
> +			},
> +		};
> +		struct drm_msm_gem_submit req = {
> +				.flags   = pipe->pipe,
> +				.queueid = pipe->submitqueue_id,
> +				.nr_cmds    = ARRAY_SIZE(cmds),
> +				.cmds       = VOID2U64(cmds),
> +				.nr_bos  = ARRAY_SIZE(bos),
> +				.bos     = VOID2U64(bos),
> +		};
> +		uint32_t *cmdstream = igt_msm_bo_map(a);
> +		if (dev->gen >= 5) {
> +			*(cmdstream++) = pm4_pkt7_hdr(CP_NOP, 3);
> +		} else {
> +			*(cmdstream++) = pm4_pkt3_hdr(CP_NOP, 3);
> +		}
> +		*(cmdstream++) = 0;
> +		*(cmdstream++) = 0;
> +		*(cmdstream++) = 0;
> +
> +		do_ioctl(dev->fd, DRM_IOCTL_MSM_GEM_SUBMIT, &req);
> +	}

Add igt_describe()s for all subtests.


-- 
Petri Latvala



> +
> +	igt_fixture {
> +		igt_msm_bo_free(a);
> +		igt_msm_bo_free(b);
> +		igt_msm_pipe_close(pipe);
> +		igt_msm_dev_close(dev);
> +	}
> +}
> -- 
> 2.31.1
>