[igt-dev] [PATCH i-g-t 1/9] tests/kms_addfb_basic: Check that only the owner can rmfb

[igt-dev] [PATCH i-g-t 1/9] tests/kms_addfb_basic: Check that only the owner can rmfb

[Daniel Vetter]

Just realized we don't seem to have any testcase for this. Fill this
gap asap!

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 tests/kms_addfb_basic.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
index 400241a92e81..491e9bf9ee34 100644
--- a/tests/kms_addfb_basic.c
+++ b/tests/kms_addfb_basic.c
@@ -667,6 +667,45 @@ static void prop_tests(int fd)
 
 }
 
+static void master_tests(int fd)
+{
+	struct drm_mode_fb_cmd2 f = {};
+
+	f.width = 1024;
+	f.height = 1024;
+	f.pixel_format = DRM_FORMAT_XRGB8888;
+	f.pitches[0] = 1024*4;
+
+	igt_fixture {
+		gem_bo = igt_create_bo_with_dimensions(fd, 1024, 1024,
+			DRM_FORMAT_XRGB8888, 0, 0, NULL, NULL, NULL);
+		igt_assert(gem_bo);
+
+		f.handles[0] = gem_bo;
+
+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == 0);
+	}
+
+	igt_subtest("master-rmfb") {
+		int master2_fd;
+
+		do_or_die(drmDropMaster(fd));
+
+		master2_fd = drm_open_driver_master(DRIVER_ANY);
+
+		igt_assert_eq(rmfb(master2_fd, f.fb_id), -ENOENT);
+
+		do_or_die(drmDropMaster(master2_fd));
+		close(master2_fd);
+
+		do_or_die(drmSetMaster(fd));
+	}
+
+	igt_fixture
+		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
+
+}
+
 static bool has_addfb2_iface(int fd)
 {
 	struct local_drm_mode_fb_cmd2 f = {};
@@ -713,6 +752,8 @@ igt_main
 
 	prop_tests(fd);
 
+	master_tests(fd);
+
 	igt_fixture
 		close(fd);
 }
-- 
2.14.4

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

[igt-dev] [PATCH i-g-t 2/9] tests:core_prop_blob: Drop local_ prefixes

[Daniel Vetter]

Been a while this landed in libdrm ...

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 tests/core_prop_blob.c | 55 ++++++++++++++++----------------------------------
 1 file changed, 17 insertions(+), 38 deletions(-)

diff --git a/tests/core_prop_blob.c b/tests/core_prop_blob.c
index a7e787be993d..0a2004c33040 100644
--- a/tests/core_prop_blob.c
+++ b/tests/core_prop_blob.c
@@ -33,27 +33,6 @@
 
 IGT_TEST_DESCRIPTION("Tests behaviour of mass-data 'blob' properties.");
 
-struct local_drm_mode_get_blob {
-	uint32_t blob_id;
-	uint32_t length;
-	uint64_t data;
-};
-struct local_drm_mode_create_blob {
-	uint64_t data;
-	uint32_t length;
-	uint32_t blob_id;
-};
-struct local_drm_mode_destroy_blob {
-	uint32_t blob_id;
-};
-
-#define LOCAL_DRM_IOCTL_MODE_GETPROPBLOB	DRM_IOWR(0xAC, \
-						struct local_drm_mode_get_blob)
-#define LOCAL_DRM_IOCTL_MODE_CREATEPROPBLOB	DRM_IOWR(0xBD, \
-						struct local_drm_mode_create_blob)
-#define LOCAL_DRM_IOCTL_MODE_DESTROYPROPBLOB	DRM_IOWR(0xBE, \
-						struct local_drm_mode_destroy_blob)
-
 static const struct drm_mode_modeinfo test_mode_valid = {
 	.clock = 1234,
 	.hdisplay = 640,
@@ -80,33 +59,33 @@ static const struct drm_mode_modeinfo test_mode_valid = {
 
 static void igt_require_propblob(int fd)
 {
-	struct local_drm_mode_create_blob c;
-	struct local_drm_mode_destroy_blob d;
+	struct drm_mode_create_blob c;
+	struct drm_mode_destroy_blob d;
 	uint32_t blob_data;
 	c.data = (uintptr_t) &blob_data;
 	c.length = sizeof(blob_data);
 
-	igt_require(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_CREATEPROPBLOB, &c) == 0);
+	igt_require(drmIoctl(fd, DRM_IOCTL_MODE_CREATEPROPBLOB, &c) == 0);
 	d.blob_id = c.blob_id;
-	igt_require(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_DESTROYPROPBLOB, &d) == 0);
+	igt_require(drmIoctl(fd, DRM_IOCTL_MODE_DESTROYPROPBLOB, &d) == 0);
 }
 
 static int
 validate_prop(int fd, uint32_t prop_id)
 {
-	struct local_drm_mode_get_blob get;
+	struct drm_mode_get_blob get;
 	struct drm_mode_modeinfo ret_mode;
 
 	get.blob_id = prop_id;
 	get.length = 0;
 	get.data = (uintptr_t) 0;
-	ioctl_or_ret_errno(fd, LOCAL_DRM_IOCTL_MODE_GETPROPBLOB, &get);
+	ioctl_or_ret_errno(fd, DRM_IOCTL_MODE_GETPROPBLOB, &get);
 
 	if (get.length != sizeof(test_mode_valid))
 		return ENOMEM;
 
 	get.data = (uintptr_t) &ret_mode;
-	ioctl_or_ret_errno(fd, LOCAL_DRM_IOCTL_MODE_GETPROPBLOB, &get);
+	ioctl_or_ret_errno(fd, DRM_IOCTL_MODE_GETPROPBLOB, &get);
 
 	if (memcmp(&ret_mode, &test_mode_valid, sizeof(test_mode_valid)) != 0)
 		return EINVAL;
@@ -117,12 +96,12 @@ validate_prop(int fd, uint32_t prop_id)
 static uint32_t
 create_prop(int fd)
 {
-	struct local_drm_mode_create_blob create;
+	struct drm_mode_create_blob create;
 
 	create.length = sizeof(test_mode_valid);
 	create.data = (uintptr_t) &test_mode_valid;
 
-	do_ioctl(fd, LOCAL_DRM_IOCTL_MODE_CREATEPROPBLOB, &create);
+	do_ioctl(fd, DRM_IOCTL_MODE_CREATEPROPBLOB, &create);
 	igt_assert_neq_u32(create.blob_id, 0);
 
 	return create.blob_id;
@@ -131,10 +110,10 @@ create_prop(int fd)
 static int
 destroy_prop(int fd, uint32_t prop_id)
 {
-	struct local_drm_mode_destroy_blob destroy;
+	struct drm_mode_destroy_blob destroy;
 
 	destroy.blob_id = prop_id;
-	ioctl_or_ret_errno(fd, LOCAL_DRM_IOCTL_MODE_DESTROYPROPBLOB, &destroy);
+	ioctl_or_ret_errno(fd, DRM_IOCTL_MODE_DESTROYPROPBLOB, &destroy);
 
 	return 0;
 }
@@ -142,8 +121,8 @@ destroy_prop(int fd, uint32_t prop_id)
 static void
 test_validate(int fd)
 {
-	struct local_drm_mode_create_blob create;
-	struct local_drm_mode_get_blob get;
+	struct drm_mode_create_blob create;
+	struct drm_mode_get_blob get;
 	char too_small[32];
 	uint32_t prop_id;
 
@@ -152,24 +131,24 @@ test_validate(int fd)
 	/* Outlandish size. */
 	create.length = 0x10000;
 	create.data = (uintptr_t) &too_small;
-	do_ioctl_err(fd, LOCAL_DRM_IOCTL_MODE_CREATEPROPBLOB, &create, EFAULT);
+	do_ioctl_err(fd, DRM_IOCTL_MODE_CREATEPROPBLOB, &create, EFAULT);
 
 	/* Outlandish address. */
 	create.length = sizeof(test_mode_valid);
 	create.data = (uintptr_t) 0xdeadbeee;
-	do_ioctl_err(fd, LOCAL_DRM_IOCTL_MODE_CREATEPROPBLOB, &create, EFAULT);
+	do_ioctl_err(fd, DRM_IOCTL_MODE_CREATEPROPBLOB, &create, EFAULT);
 
 	/* When we pass an incorrect size, the kernel should correct us. */
 	prop_id = create_prop(fd);
 	get.blob_id = prop_id;
 	get.length = sizeof(too_small);
 	get.data = (uintptr_t) too_small;
-	do_ioctl(fd, LOCAL_DRM_IOCTL_MODE_GETPROPBLOB, &get);
+	do_ioctl(fd, DRM_IOCTL_MODE_GETPROPBLOB, &get);
 	igt_assert_eq_u32(get.length, sizeof(test_mode_valid));
 
 	get.blob_id = prop_id;
 	get.data = (uintptr_t) 0xdeadbeee;
-	do_ioctl_err(fd, LOCAL_DRM_IOCTL_MODE_CREATEPROPBLOB, &create, EFAULT);
+	do_ioctl_err(fd, DRM_IOCTL_MODE_CREATEPROPBLOB, &create, EFAULT);
 }
 
 static void
-- 
2.14.4

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

[igt-dev] [PATCH i-g-t 3/9] tests: s/core_prop_blob/kms_prop_blob

[Daniel Vetter]

It's a kms test, name it accordingly. Also sort the build lists while
at it, one test got misplaced.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 tests/Makefile.sources                      | 4 ++--
 tests/intel-ci/fast-feedback.testlist       | 2 +-
 tests/{core_prop_blob.c => kms_prop_blob.c} | 0
 tests/meson.build                           | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)
 rename tests/{core_prop_blob.c => kms_prop_blob.c} (100%)

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 35c79592bfee..71ccf00af256 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -16,7 +16,6 @@ TESTS_progs = \
 	core_getclient \
 	core_getstats \
 	core_getversion \
-	core_prop_blob \
 	core_setmaster_vs_auth \
 	debugfs_test \
 	drm_import_export \
@@ -60,6 +59,7 @@ TESTS_progs = \
 	kms_plane_lowres \
 	kms_plane_multiple \
 	kms_plane_scaling \
+	kms_prop_blob \
 	kms_properties \
 	kms_psr \
 	kms_psr2_su \
@@ -67,13 +67,13 @@ TESTS_progs = \
 	kms_rmfb \
 	kms_rotation_crc \
 	kms_selftest \
+	kms_sequence \
 	kms_setmode \
 	kms_sysfs_edid_timing \
 	kms_tv_load_detect \
 	kms_universal_plane \
 	kms_vblank \
 	kms_vrr \
-	kms_sequence \
 	meta_test \
 	perf \
 	perf_pmu \
diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
index 2d22c2c1cc1e..9b71194670da 100644
--- a/tests/intel-ci/fast-feedback.testlist
+++ b/tests/intel-ci/fast-feedback.testlist
@@ -1,7 +1,6 @@
 # Keep alphabetically sorted by default
 
 igt@core_auth@basic-auth
-igt@core_prop_blob@basic
 igt@debugfs_test@read_all_entries
 igt@gem_basic@bad-close
 igt@gem_basic@create-close
@@ -185,6 +184,7 @@ igt@kms_chamelium@hdmi-crc-fast
 igt@kms_chamelium@vga-hpd-fast
 igt@kms_chamelium@vga-edid-read
 igt@kms_chamelium@common-hpd-after-suspend
+igt@kms_prop_blob@basic
 igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic
 igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy
 igt@kms_cursor_legacy@basic-flip-after-cursor-atomic
diff --git a/tests/core_prop_blob.c b/tests/kms_prop_blob.c
similarity index 100%
rename from tests/core_prop_blob.c
rename to tests/kms_prop_blob.c
diff --git a/tests/meson.build b/tests/meson.build
index f4bd4a109dbb..9015f809ed05 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -3,7 +3,6 @@ test_progs = [
 	'core_getclient',
 	'core_getstats',
 	'core_getversion',
-	'core_prop_blob',
 	'core_setmaster_vs_auth',
 	'debugfs_test',
 	'drm_import_export',
@@ -47,6 +46,7 @@ test_progs = [
 	'kms_plane_lowres',
 	'kms_plane_multiple',
 	'kms_plane_scaling',
+	'kms_prop_blob',
 	'kms_properties',
 	'kms_psr',
 	'kms_psr2_su',
@@ -54,8 +54,8 @@ test_progs = [
 	'kms_rmfb',
 	'kms_rotation_crc',
 	'kms_selftest',
-	'kms_setmode',
 	'kms_sequence',
+	'kms_setmode',
 	'kms_sysfs_edid_timing',
 	'kms_tv_load_detect',
 	'kms_universal_plane',
-- 
2.14.4

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

[igt-dev] [PATCH i-g-t 4/9] tests/kms_lease: test implicit primary plane usage in page_flip ioctl

[Daniel Vetter]

We need to make sure that legacy ioctls don't operate on a lease if
the lesse doesn't have access to the implicitly used primary plane.

Current kernels fail this.

Cc: Keith Packard <keithp@keithp.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 tests/kms_lease.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/tests/kms_lease.c b/tests/kms_lease.c
index 4823121e8cf4..6453c7beafd8 100644
--- a/tests/kms_lease.c
+++ b/tests/kms_lease.c
@@ -311,6 +311,67 @@ static void simple_lease(data_t *data)
 	terminate_lease(&lease);
 }
 
+static void page_flip_implicit_plane(data_t *data)
+{
+	uint32_t object_ids[3];
+	struct local_drm_mode_create_lease mcl;
+	drmModePlaneRes *plane_resources;
+	uint32_t wrong_plane_id = 0;
+	int i;
+
+	/* find a plane which isn't the primary one for us */
+	plane_resources = drmModeGetPlaneResources(data->master.fd);
+	for (i = 0; i < plane_resources->count_planes; i++) {
+		if (plane_resources->planes[i] != data->plane_id) {
+			wrong_plane_id = plane_resources->planes[i];
+			break;
+		}
+	}
+	drmModeFreePlaneResources(plane_resources);
+	igt_require(wrong_plane_id);
+
+	mcl.object_ids = (uint64_t) (uintptr_t) &object_ids[0];
+	mcl.object_count = 0;
+	mcl.flags = 0;
+
+	object_ids[mcl.object_count++] = data->connector_id;
+	object_ids[mcl.object_count++] = data->crtc_id;
+
+	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
+	do_or_die(create_lease(data->master.fd, &mcl));
+	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+
+	/* Set a mode on the leased output */
+	igt_assert_eq(0, prepare_crtc(&data->master, data->connector_id, data->crtc_id));
+
+	/* sanity check */
+	do_or_die(drmModePageFlip(data->master.fd, data->crtc_id,
+			      data->master.primary_fb.fb_id,
+			      0, NULL));
+	igt_wait_for_vblank_count(data->master.fd,
+				  crtc_id_to_pipe(&data->master.display, data->crtc_id),
+				  1);
+	do_or_die(drmModePageFlip(mcl.fd, data->crtc_id,
+			      data->master.primary_fb.fb_id,
+			      0, NULL));
+	close(mcl.fd);
+
+	object_ids[mcl.object_count++] = wrong_plane_id;
+	do_or_die(create_lease(data->master.fd, &mcl));
+
+	igt_wait_for_vblank_count(data->master.fd,
+				  crtc_id_to_pipe(&data->master.display, data->crtc_id),
+				  1);
+	igt_assert_eq(drmModePageFlip(mcl.fd, data->crtc_id,
+				      data->master.primary_fb.fb_id,
+				      0, NULL),
+		      -EACCES);
+	close(mcl.fd);
+
+	cleanup_crtc(&data->master,
+		     connector_id_to_output(&data->master.display, data->connector_id));
+}
+
 /* Test listing lessees */
 static void lessee_list(data_t *data)
 {
@@ -1024,6 +1085,7 @@ igt_main
 		{ "lease_invalid_connector", lease_invalid_connector },
 		{ "lease_invalid_crtc", lease_invalid_crtc },
 		{ "lease_invalid_plane", lease_invalid_plane },
+		{ "page_flip_implicit_plane", page_flip_implicit_plane },
 		{ }
 	}, *f;
 
-- 
2.14.4

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

[igt-dev] [PATCH i-g-t 5/9] tests/kms_lease: test implicit primary plane usage in setcrtc ioctl

[Daniel Vetter]

Again we need to make sure that a lease can't use planes it's not
allowed to through legacy ioctls. SetCrtc is a bit more tricky, since
we should still allow to shut down a CRTC, e.g. when we're allowed to
use other planes on that CRTC.

Current kernels fail this.

Cc: Keith Packard <keithp@keithp.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 tests/kms_lease.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/tests/kms_lease.c b/tests/kms_lease.c
index 6453c7beafd8..c11d37aeac7a 100644
--- a/tests/kms_lease.c
+++ b/tests/kms_lease.c
@@ -372,6 +372,65 @@ static void page_flip_implicit_plane(data_t *data)
 		     connector_id_to_output(&data->master.display, data->connector_id));
 }
 
+static void setcrtc_implicit_plane(data_t *data)
+{
+	uint32_t object_ids[3];
+	struct local_drm_mode_create_lease mcl;
+	drmModePlaneRes *plane_resources;
+	uint32_t wrong_plane_id = 0;
+	igt_output_t *output =
+		connector_id_to_output(&data->master.display,
+				       data->connector_id);
+	drmModeModeInfo *mode = igt_output_get_mode(output);
+	int i;
+
+	/* find a plane which isn't the primary one for us */
+	plane_resources = drmModeGetPlaneResources(data->master.fd);
+	for (i = 0; i < plane_resources->count_planes; i++) {
+		if (plane_resources->planes[i] != data->plane_id) {
+			wrong_plane_id = plane_resources->planes[i];
+			break;
+		}
+	}
+	drmModeFreePlaneResources(plane_resources);
+	igt_require(wrong_plane_id);
+
+	mcl.object_ids = (uint64_t) (uintptr_t) &object_ids[0];
+	mcl.object_count = 0;
+	mcl.flags = 0;
+
+	object_ids[mcl.object_count++] = data->connector_id;
+	object_ids[mcl.object_count++] = data->crtc_id;
+
+	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
+	do_or_die(create_lease(data->master.fd, &mcl));
+	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+
+	/* Set a mode on the leased output */
+	igt_assert_eq(0, prepare_crtc(&data->master, data->connector_id, data->crtc_id));
+
+	/* sanity check */
+	do_or_die(drmModeSetCrtc(data->master.fd, data->crtc_id, -1,
+				 0, 0, object_ids, 1, mode));
+	do_or_die(drmModeSetCrtc(mcl.fd, data->crtc_id, -1,
+				 0, 0, object_ids, 1, mode));
+	close(mcl.fd);
+
+	object_ids[mcl.object_count++] = wrong_plane_id;
+	do_or_die(create_lease(data->master.fd, &mcl));
+
+	igt_assert_eq(drmModeSetCrtc(mcl.fd, data->crtc_id, -1,
+				     0, 0, object_ids, 1, mode),
+		      -EACCES);
+	/* make sure we are allowed to turn the CRTC off */
+	do_or_die(drmModeSetCrtc(mcl.fd, data->crtc_id,
+				 0, 0, 0, NULL, 0, NULL));
+	close(mcl.fd);
+
+	cleanup_crtc(&data->master,
+		     connector_id_to_output(&data->master.display, data->connector_id));
+}
+
 /* Test listing lessees */
 static void lessee_list(data_t *data)
 {
@@ -1086,6 +1145,7 @@ igt_main
 		{ "lease_invalid_crtc", lease_invalid_crtc },
 		{ "lease_invalid_plane", lease_invalid_plane },
 		{ "page_flip_implicit_plane", page_flip_implicit_plane },
+		{ "setcrtc_implicit_plane", setcrtc_implicit_plane },
 		{ }
 	}, *f;
 
-- 
2.14.4

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

[igt-dev] [PATCH i-g-t 6/9] tests/kms_lease: test implicit cursor plane usage

[Daniel Vetter]

Figured I'll only test the SetCursor interface since in the kernel
it's all the same anyway, it's just libdrm that splits it up.

Current kernels fail this.

Cc: Keith Packard <keithp@keithp.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 tests/kms_lease.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/tests/kms_lease.c b/tests/kms_lease.c
index c11d37aeac7a..7930d0737ace 100644
--- a/tests/kms_lease.c
+++ b/tests/kms_lease.c
@@ -431,6 +431,42 @@ static void setcrtc_implicit_plane(data_t *data)
 		     connector_id_to_output(&data->master.display, data->connector_id));
 }
 
+static void cursor_implicit_plane(data_t *data)
+{
+	uint32_t object_ids[3];
+	struct local_drm_mode_create_lease mcl;
+
+	mcl.object_ids = (uint64_t) (uintptr_t) &object_ids[0];
+	mcl.object_count = 0;
+	mcl.flags = 0;
+
+	object_ids[mcl.object_count++] = data->connector_id;
+	object_ids[mcl.object_count++] = data->crtc_id;
+
+	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
+	do_or_die(create_lease(data->master.fd, &mcl));
+	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
+
+	/* Set a mode on the leased output */
+	igt_assert_eq(0, prepare_crtc(&data->master, data->connector_id, data->crtc_id));
+
+	/* sanity check */
+	do_or_die(drmModeSetCursor(data->master.fd, data->crtc_id, 0, 0, 0));
+	do_or_die(drmModeSetCursor(mcl.fd, data->crtc_id, 0, 0, 0));
+	close(mcl.fd);
+
+	/* primary plane is never the cursor */
+	object_ids[mcl.object_count++] = data->plane_id;
+	do_or_die(create_lease(data->master.fd, &mcl));
+
+	igt_assert_eq(drmModeSetCursor(mcl.fd, data->crtc_id, 0, 0, 0),
+		      -EACCES);
+	close(mcl.fd);
+
+	cleanup_crtc(&data->master,
+		     connector_id_to_output(&data->master.display, data->connector_id));
+}
+
 /* Test listing lessees */
 static void lessee_list(data_t *data)
 {
@@ -1146,6 +1182,7 @@ igt_main
 		{ "lease_invalid_plane", lease_invalid_plane },
 		{ "page_flip_implicit_plane", page_flip_implicit_plane },
 		{ "setcrtc_implicit_plane", setcrtc_implicit_plane },
+		{ "cursor_implicit_plane", cursor_implicit_plane },
 		{ }
 	}, *f;
 
-- 
2.14.4

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

[igt-dev] [PATCH i-g-t 7/9] tests/kms_lease: Adjust to kernel errno changes

[Daniel Vetter]

I dropped a superfluous check for negative object id (the kernel
did a cast to s32, despite that object ids are always unsigned).
Which changes the errno from EINVAL to ENOENT. Allow both.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 tests/kms_lease.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tests/kms_lease.c b/tests/kms_lease.c
index 7930d0737ace..7d7576cbb634 100644
--- a/tests/kms_lease.c
+++ b/tests/kms_lease.c
@@ -713,6 +713,10 @@ static void lease_again(data_t *data)
 	terminate_lease(&lease_b);
 }
 
+#define assert_unleased(ret) \
+	igt_assert_f((ret) == -EINVAL || (ret) == -ENOENT, \
+		     "wrong return code %i, %s\n", ret, \
+		     strerror(ret))
 /* Test leasing an invalid connector */
 static void lease_invalid_connector(data_t *data)
 {
@@ -725,7 +729,7 @@ static void lease_invalid_connector(data_t *data)
 	data->connector_id = 0xbaadf00d;
 	ret = make_lease(data, &lease);
 	data->connector_id = save_connector_id;
-	igt_assert_eq(ret, -EINVAL);
+	assert_unleased(ret);
 }
 
 /* Test leasing an invalid crtc */
@@ -740,7 +744,7 @@ static void lease_invalid_crtc(data_t *data)
 	data->crtc_id = 0xbaadf00d;
 	ret = make_lease(data, &lease);
 	data->crtc_id = save_crtc_id;
-	igt_assert_eq(ret, -EINVAL);
+	assert_unleased(ret);
 }
 
 static void lease_invalid_plane(data_t *data)
@@ -754,7 +758,7 @@ static void lease_invalid_plane(data_t *data)
 	data->plane_id = 0xbaadf00d;
 	ret = make_lease(data, &lease);
 	data->plane_id = save_plane_id;
-	igt_assert_eq(ret, -EINVAL);
+	assert_unleased(ret);
 }
 
 
-- 
2.14.4

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

[igt-dev] [PATCH i-g-t 8/9] tests/kms_lease: Handle new errno from idr/xa double insert

[Daniel Vetter]

The conversion from idr to xarray will change the errno for already
inserted object ids from ENOSPC to EBUSY. Allow both.

Cc: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 tests/kms_lease.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/tests/kms_lease.c b/tests/kms_lease.c
index 7d7576cbb634..b5db37c5d58a 100644
--- a/tests/kms_lease.c
+++ b/tests/kms_lease.c
@@ -797,12 +797,16 @@ static void run_test(data_t *data, void (*testfunc)(data_t *))
 		      "no valid crtc/connector combinations found\n");
 }
 
+#define assert_double_id_err(ret) \
+	igt_assert_f((ret) == -EBUSY || (ret) == -ENOSPC, \
+		     "wrong return code %i, %s\n", ret, \
+		     strerror(ret))
 static void invalid_create_leases(data_t *data)
 {
 	uint32_t object_ids[4];
 	struct local_drm_mode_create_lease mcl;
 	drmModeRes *resources;
-	int tmp_fd;
+	int tmp_fd, ret;
 
 	/* empty lease */
 	mcl.object_ids = 0;
@@ -881,7 +885,8 @@ static void invalid_create_leases(data_t *data)
 	object_ids[3] = object_ids[2];
 	mcl.object_count = 4;
 	/* Note: the ENOSPC is from idr double-insertion failing */
-	igt_assert_eq(create_lease(data->master.fd, &mcl), -ENOSPC);
+	ret = create_lease(data->master.fd, &mcl);
+	assert_double_id_err(ret);
 
 	/* no encoder leasing */
 	resources = drmModeGetResources(data->master.fd);
@@ -1094,7 +1099,7 @@ static void implicit_plane_lease(data_t *data)
 	uint32_t object_ids[3];
 	struct local_drm_mode_create_lease mcl;
 	struct local_drm_mode_get_lease mgl;
-
+	int ret;
 	uint32_t cursor_id = igt_pipe_get_plane_type(&data->master.display.pipes[0],
 						     DRM_PLANE_TYPE_CURSOR)->drm_plane->plane_id;
 
@@ -1127,11 +1132,13 @@ static void implicit_plane_lease(data_t *data)
 	/* check that implicit lease doesn't lead to confusion when
 	 * explicitly adding primary plane */
 	mcl.object_count = 3;
-	igt_assert_eq(create_lease(data->master.fd, &mcl), -ENOSPC);
+	ret = create_lease(data->master.fd, &mcl);
+	assert_double_id_err(ret);
 
 	/* same for the cursor */
 	object_ids[2] = cursor_id;
-	igt_assert_eq(create_lease(data->master.fd, &mcl), -ENOSPC);
+	ret = create_lease(data->master.fd, &mcl);
+	assert_double_id_err(ret);
 
 	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
 }
-- 
2.14.4

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

[igt-dev] [PATCH i-g-t 9/9] tests/kms_lease: Check crtc used in atomic ioctl

[Daniel Vetter]

We not only need to check that userspace is allowed to use the objects
it's changing, but also the objects it's using as property values. The
only ones relevant for leases are the CRTC_ID properties on connectors
and planes.

Current kernels fail this.

Cc: Keith Packard <keithp@keithp.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 tests/kms_lease.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/tests/kms_lease.c b/tests/kms_lease.c
index b5db37c5d58a..694a612f76a7 100644
--- a/tests/kms_lease.c
+++ b/tests/kms_lease.c
@@ -467,6 +467,94 @@ static void cursor_implicit_plane(data_t *data)
 		     connector_id_to_output(&data->master.display, data->connector_id));
 }
 
+static void atomic_implicit_crtc(data_t *data)
+{
+	uint32_t object_ids[3];
+	struct local_drm_mode_create_lease mcl;
+	drmModeRes *resources;
+	drmModeObjectPropertiesPtr props;
+	uint32_t wrong_crtc_id = 0;
+	uint32_t crtc_id_prop = 0;
+	drmModeAtomicReqPtr req = NULL;
+	int ret;
+
+	igt_require(data->master.display.is_atomic);
+
+	mcl.object_ids = (uint64_t) (uintptr_t) &object_ids[0];
+	mcl.object_count = 0;
+	mcl.flags = 0;
+
+	object_ids[mcl.object_count++] = data->connector_id;
+	object_ids[mcl.object_count++] = data->plane_id;
+
+	/* find a plane which isn't the primary one for us */
+	resources = drmModeGetResources(data->master.fd);
+	igt_assert(resources);
+	for (int i = 0; i < resources->count_crtcs; i++) {
+		if (resources->crtcs[i] != data->crtc_id) {
+			wrong_crtc_id = resources->crtcs[i];
+			break;
+		}
+	}
+	drmModeFreeResources(resources);
+	igt_require(wrong_crtc_id);
+	object_ids[mcl.object_count++] = wrong_crtc_id;
+
+	/* find the CRTC_ID prop, it's global */
+	props = drmModeObjectGetProperties(data->master.fd, data->plane_id,
+					   DRM_MODE_OBJECT_PLANE);
+	igt_assert(props);
+	for (int i = 0; i < props->count_props; i++) {
+		drmModePropertyPtr prop = drmModeGetProperty(data->master.fd,
+							     props->props[i]);
+		if (strcmp(prop->name, "CRTC_ID") == 0)
+			crtc_id_prop = props->props[i];
+
+		printf("prop name %s, prop id %u, prop id %u\n",
+		       prop->name, props->props[i], prop->prop_id);
+		drmModeFreeProperty(prop);
+		if (crtc_id_prop)
+			break;
+	}
+	drmModeFreeObjectProperties(props);
+	igt_assert(crtc_id_prop);
+
+	do_or_die(create_lease(data->master.fd, &mcl));
+	do_or_die(drmSetClientCap(mcl.fd, DRM_CLIENT_CAP_ATOMIC, 1));
+
+	/* check CRTC_ID property on the plane */
+	req = drmModeAtomicAlloc();
+	igt_assert(req);
+	ret = drmModeAtomicAddProperty(req, data->plane_id,
+				       crtc_id_prop, data->crtc_id);
+	igt_assert(ret >= 0);
+
+	/* sanity check */
+	ret = drmModeAtomicCommit(data->master.fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
+	igt_assert(ret == 0 || ret == -EINVAL);
+
+	ret = drmModeAtomicCommit(mcl.fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
+	igt_assert(ret == -EACCES);
+	drmModeAtomicFree(req);
+
+	/* check CRTC_ID property on the connector */
+	req = drmModeAtomicAlloc();
+	igt_assert(req);
+	ret = drmModeAtomicAddProperty(req, data->connector_id,
+				       crtc_id_prop, data->crtc_id);
+	igt_assert(ret >= 0);
+
+	/* sanity check */
+	ret = drmModeAtomicCommit(data->master.fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
+	igt_assert(ret == 0 || ret == -EINVAL);
+
+	ret = drmModeAtomicCommit(mcl.fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
+	igt_assert(ret == -EACCES);
+	drmModeAtomicFree(req);
+
+	close(mcl.fd);
+}
+
 /* Test listing lessees */
 static void lessee_list(data_t *data)
 {
@@ -1194,6 +1282,7 @@ igt_main
 		{ "page_flip_implicit_plane", page_flip_implicit_plane },
 		{ "setcrtc_implicit_plane", setcrtc_implicit_plane },
 		{ "cursor_implicit_plane", cursor_implicit_plane },
+		{ "atomic_implicit_crtc", atomic_implicit_crtc },
 		{ }
 	}, *f;
 
-- 
2.14.4

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

Re: [igt-dev] [PATCH i-g-t 1/9] tests/kms_addfb_basic: Check that only the owner can rmfb

[Ville Syrjälä]

On Thu, Feb 28, 2019 at 03:19:10PM +0100, Daniel Vetter wrote:
> Just realized we don't seem to have any testcase for this. Fill this
> gap asap!
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  tests/kms_addfb_basic.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
> index 400241a92e81..491e9bf9ee34 100644
> --- a/tests/kms_addfb_basic.c
> +++ b/tests/kms_addfb_basic.c
> @@ -667,6 +667,45 @@ static void prop_tests(int fd)
>  
>  }
>  
> +static void master_tests(int fd)
> +{
> +	struct drm_mode_fb_cmd2 f = {};
> +
> +	f.width = 1024;
> +	f.height = 1024;
> +	f.pixel_format = DRM_FORMAT_XRGB8888;
> +	f.pitches[0] = 1024*4;

Could use named intializers.

Anyways, looks reasonable to me.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
> +	igt_fixture {
> +		gem_bo = igt_create_bo_with_dimensions(fd, 1024, 1024,
> +			DRM_FORMAT_XRGB8888, 0, 0, NULL, NULL, NULL);
> +		igt_assert(gem_bo);
> +
> +		f.handles[0] = gem_bo;
> +
> +		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == 0);
> +	}
> +
> +	igt_subtest("master-rmfb") {
> +		int master2_fd;
> +
> +		do_or_die(drmDropMaster(fd));
> +
> +		master2_fd = drm_open_driver_master(DRIVER_ANY);
> +
> +		igt_assert_eq(rmfb(master2_fd, f.fb_id), -ENOENT);
> +
> +		do_or_die(drmDropMaster(master2_fd));
> +		close(master2_fd);
> +
> +		do_or_die(drmSetMaster(fd));
> +	}
> +
> +	igt_fixture
> +		igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_RMFB, &f.fb_id) == 0);
> +
> +}
> +
>  static bool has_addfb2_iface(int fd)
>  {
>  	struct local_drm_mode_fb_cmd2 f = {};
> @@ -713,6 +752,8 @@ igt_main
>  
>  	prop_tests(fd);
>  
> +	master_tests(fd);
> +
>  	igt_fixture
>  		close(fd);
>  }
> -- 
> 2.14.4
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/9] tests:core_prop_blob: Drop local_ prefixes

[Ville Syrjälä]

On Thu, Feb 28, 2019 at 03:19:11PM +0100, Daniel Vetter wrote:
> Been a while this landed in libdrm ...
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  tests/core_prop_blob.c | 55 ++++++++++++++++----------------------------------
>  1 file changed, 17 insertions(+), 38 deletions(-)
> 
> diff --git a/tests/core_prop_blob.c b/tests/core_prop_blob.c
> index a7e787be993d..0a2004c33040 100644
> --- a/tests/core_prop_blob.c
> +++ b/tests/core_prop_blob.c
> @@ -33,27 +33,6 @@
>  
>  IGT_TEST_DESCRIPTION("Tests behaviour of mass-data 'blob' properties.");
>  
> -struct local_drm_mode_get_blob {
> -	uint32_t blob_id;
> -	uint32_t length;
> -	uint64_t data;
> -};
> -struct local_drm_mode_create_blob {
> -	uint64_t data;
> -	uint32_t length;
> -	uint32_t blob_id;
> -};
> -struct local_drm_mode_destroy_blob {
> -	uint32_t blob_id;
> -};
> -
> -#define LOCAL_DRM_IOCTL_MODE_GETPROPBLOB	DRM_IOWR(0xAC, \
> -						struct local_drm_mode_get_blob)
> -#define LOCAL_DRM_IOCTL_MODE_CREATEPROPBLOB	DRM_IOWR(0xBD, \
> -						struct local_drm_mode_create_blob)
> -#define LOCAL_DRM_IOCTL_MODE_DESTROYPROPBLOB	DRM_IOWR(0xBE, \
> -						struct local_drm_mode_destroy_blob)
> -
>  static const struct drm_mode_modeinfo test_mode_valid = {
>  	.clock = 1234,
>  	.hdisplay = 640,
> @@ -80,33 +59,33 @@ static const struct drm_mode_modeinfo test_mode_valid = {
>  
>  static void igt_require_propblob(int fd)
>  {
> -	struct local_drm_mode_create_blob c;
> -	struct local_drm_mode_destroy_blob d;
> +	struct drm_mode_create_blob c;
> +	struct drm_mode_destroy_blob d;
>  	uint32_t blob_data;
>  	c.data = (uintptr_t) &blob_data;
>  	c.length = sizeof(blob_data);
>  
> -	igt_require(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_CREATEPROPBLOB, &c) == 0);
> +	igt_require(drmIoctl(fd, DRM_IOCTL_MODE_CREATEPROPBLOB, &c) == 0);
>  	d.blob_id = c.blob_id;
> -	igt_require(drmIoctl(fd, LOCAL_DRM_IOCTL_MODE_DESTROYPROPBLOB, &d) == 0);
> +	igt_require(drmIoctl(fd, DRM_IOCTL_MODE_DESTROYPROPBLOB, &d) == 0);
>  }
>  
>  static int
>  validate_prop(int fd, uint32_t prop_id)
>  {
> -	struct local_drm_mode_get_blob get;
> +	struct drm_mode_get_blob get;
>  	struct drm_mode_modeinfo ret_mode;
>  
>  	get.blob_id = prop_id;
>  	get.length = 0;
>  	get.data = (uintptr_t) 0;
> -	ioctl_or_ret_errno(fd, LOCAL_DRM_IOCTL_MODE_GETPROPBLOB, &get);
> +	ioctl_or_ret_errno(fd, DRM_IOCTL_MODE_GETPROPBLOB, &get);
>  
>  	if (get.length != sizeof(test_mode_valid))
>  		return ENOMEM;
>  
>  	get.data = (uintptr_t) &ret_mode;
> -	ioctl_or_ret_errno(fd, LOCAL_DRM_IOCTL_MODE_GETPROPBLOB, &get);
> +	ioctl_or_ret_errno(fd, DRM_IOCTL_MODE_GETPROPBLOB, &get);
>  
>  	if (memcmp(&ret_mode, &test_mode_valid, sizeof(test_mode_valid)) != 0)
>  		return EINVAL;
> @@ -117,12 +96,12 @@ validate_prop(int fd, uint32_t prop_id)
>  static uint32_t
>  create_prop(int fd)
>  {
> -	struct local_drm_mode_create_blob create;
> +	struct drm_mode_create_blob create;
>  
>  	create.length = sizeof(test_mode_valid);
>  	create.data = (uintptr_t) &test_mode_valid;
>  
> -	do_ioctl(fd, LOCAL_DRM_IOCTL_MODE_CREATEPROPBLOB, &create);
> +	do_ioctl(fd, DRM_IOCTL_MODE_CREATEPROPBLOB, &create);
>  	igt_assert_neq_u32(create.blob_id, 0);
>  
>  	return create.blob_id;
> @@ -131,10 +110,10 @@ create_prop(int fd)
>  static int
>  destroy_prop(int fd, uint32_t prop_id)
>  {
> -	struct local_drm_mode_destroy_blob destroy;
> +	struct drm_mode_destroy_blob destroy;
>  
>  	destroy.blob_id = prop_id;
> -	ioctl_or_ret_errno(fd, LOCAL_DRM_IOCTL_MODE_DESTROYPROPBLOB, &destroy);
> +	ioctl_or_ret_errno(fd, DRM_IOCTL_MODE_DESTROYPROPBLOB, &destroy);
>  
>  	return 0;
>  }
> @@ -142,8 +121,8 @@ destroy_prop(int fd, uint32_t prop_id)
>  static void
>  test_validate(int fd)
>  {
> -	struct local_drm_mode_create_blob create;
> -	struct local_drm_mode_get_blob get;
> +	struct drm_mode_create_blob create;
> +	struct drm_mode_get_blob get;
>  	char too_small[32];
>  	uint32_t prop_id;
>  
> @@ -152,24 +131,24 @@ test_validate(int fd)
>  	/* Outlandish size. */
>  	create.length = 0x10000;
>  	create.data = (uintptr_t) &too_small;
> -	do_ioctl_err(fd, LOCAL_DRM_IOCTL_MODE_CREATEPROPBLOB, &create, EFAULT);
> +	do_ioctl_err(fd, DRM_IOCTL_MODE_CREATEPROPBLOB, &create, EFAULT);
>  
>  	/* Outlandish address. */
>  	create.length = sizeof(test_mode_valid);
>  	create.data = (uintptr_t) 0xdeadbeee;
> -	do_ioctl_err(fd, LOCAL_DRM_IOCTL_MODE_CREATEPROPBLOB, &create, EFAULT);
> +	do_ioctl_err(fd, DRM_IOCTL_MODE_CREATEPROPBLOB, &create, EFAULT);
>  
>  	/* When we pass an incorrect size, the kernel should correct us. */
>  	prop_id = create_prop(fd);
>  	get.blob_id = prop_id;
>  	get.length = sizeof(too_small);
>  	get.data = (uintptr_t) too_small;
> -	do_ioctl(fd, LOCAL_DRM_IOCTL_MODE_GETPROPBLOB, &get);
> +	do_ioctl(fd, DRM_IOCTL_MODE_GETPROPBLOB, &get);
>  	igt_assert_eq_u32(get.length, sizeof(test_mode_valid));
>  
>  	get.blob_id = prop_id;
>  	get.data = (uintptr_t) 0xdeadbeee;
> -	do_ioctl_err(fd, LOCAL_DRM_IOCTL_MODE_CREATEPROPBLOB, &create, EFAULT);
> +	do_ioctl_err(fd, DRM_IOCTL_MODE_CREATEPROPBLOB, &create, EFAULT);
>  }
>  
>  static void
> -- 
> 2.14.4
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/9] tests: s/core_prop_blob/kms_prop_blob

[Ville Syrjälä]

On Thu, Feb 28, 2019 at 03:19:12PM +0100, Daniel Vetter wrote:
> It's a kms test, name it accordingly. Also sort the build lists while
> at it, one test got misplaced.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  tests/Makefile.sources                      | 4 ++--
>  tests/intel-ci/fast-feedback.testlist       | 2 +-
>  tests/{core_prop_blob.c => kms_prop_blob.c} | 0
>  tests/meson.build                           | 4 ++--
>  4 files changed, 5 insertions(+), 5 deletions(-)
>  rename tests/{core_prop_blob.c => kms_prop_blob.c} (100%)
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 35c79592bfee..71ccf00af256 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -16,7 +16,6 @@ TESTS_progs = \
>  	core_getclient \
>  	core_getstats \
>  	core_getversion \
> -	core_prop_blob \
>  	core_setmaster_vs_auth \
>  	debugfs_test \
>  	drm_import_export \
> @@ -60,6 +59,7 @@ TESTS_progs = \
>  	kms_plane_lowres \
>  	kms_plane_multiple \
>  	kms_plane_scaling \
> +	kms_prop_blob \
>  	kms_properties \
>  	kms_psr \
>  	kms_psr2_su \
> @@ -67,13 +67,13 @@ TESTS_progs = \
>  	kms_rmfb \
>  	kms_rotation_crc \
>  	kms_selftest \
> +	kms_sequence \
>  	kms_setmode \
>  	kms_sysfs_edid_timing \
>  	kms_tv_load_detect \
>  	kms_universal_plane \
>  	kms_vblank \
>  	kms_vrr \
> -	kms_sequence \
>  	meta_test \
>  	perf \
>  	perf_pmu \
> diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> index 2d22c2c1cc1e..9b71194670da 100644
> --- a/tests/intel-ci/fast-feedback.testlist
> +++ b/tests/intel-ci/fast-feedback.testlist
> @@ -1,7 +1,6 @@
>  # Keep alphabetically sorted by default
>  
>  igt@core_auth@basic-auth
> -igt@core_prop_blob@basic
>  igt@debugfs_test@read_all_entries
>  igt@gem_basic@bad-close
>  igt@gem_basic@create-close
> @@ -185,6 +184,7 @@ igt@kms_chamelium@hdmi-crc-fast
>  igt@kms_chamelium@vga-hpd-fast
>  igt@kms_chamelium@vga-edid-read
>  igt@kms_chamelium@common-hpd-after-suspend
> +igt@kms_prop_blob@basic
>  igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic
>  igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy
>  igt@kms_cursor_legacy@basic-flip-after-cursor-atomic
> diff --git a/tests/core_prop_blob.c b/tests/kms_prop_blob.c
> similarity index 100%
> rename from tests/core_prop_blob.c
> rename to tests/kms_prop_blob.c
> diff --git a/tests/meson.build b/tests/meson.build
> index f4bd4a109dbb..9015f809ed05 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -3,7 +3,6 @@ test_progs = [
>  	'core_getclient',
>  	'core_getstats',
>  	'core_getversion',
> -	'core_prop_blob',
>  	'core_setmaster_vs_auth',
>  	'debugfs_test',
>  	'drm_import_export',
> @@ -47,6 +46,7 @@ test_progs = [
>  	'kms_plane_lowres',
>  	'kms_plane_multiple',
>  	'kms_plane_scaling',
> +	'kms_prop_blob',
>  	'kms_properties',
>  	'kms_psr',
>  	'kms_psr2_su',
> @@ -54,8 +54,8 @@ test_progs = [
>  	'kms_rmfb',
>  	'kms_rotation_crc',
>  	'kms_selftest',
> -	'kms_setmode',
>  	'kms_sequence',
> +	'kms_setmode',
>  	'kms_sysfs_edid_timing',
>  	'kms_tv_load_detect',
>  	'kms_universal_plane',
> -- 
> 2.14.4
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/9] tests/kms_addfb_basic: Check that only the owner can rmfb

[Chris Wilson]

Quoting Daniel Vetter (2019-02-28 14:19:10)
> Just realized we don't seem to have any testcase for this. Fill this
> gap asap!
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  tests/kms_addfb_basic.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/tests/kms_addfb_basic.c b/tests/kms_addfb_basic.c
> index 400241a92e81..491e9bf9ee34 100644
> --- a/tests/kms_addfb_basic.c
> +++ b/tests/kms_addfb_basic.c
> @@ -667,6 +667,45 @@ static void prop_tests(int fd)
>  
>  }
>  
> +static void master_tests(int fd)
> +{
> +       struct drm_mode_fb_cmd2 f = {};
> +
> +       f.width = 1024;
> +       f.height = 1024;
> +       f.pixel_format = DRM_FORMAT_XRGB8888;
> +       f.pitches[0] = 1024*4;
> +
> +       igt_fixture {
> +               gem_bo = igt_create_bo_with_dimensions(fd, 1024, 1024,
> +                       DRM_FORMAT_XRGB8888, 0, 0, NULL, NULL, NULL);
> +               igt_assert(gem_bo);
> +
> +               f.handles[0] = gem_bo;
> +
> +               igt_assert(drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f) == 0);
> +       }
> +
> +       igt_subtest("master-rmfb") {
> +               int master2_fd;
> +
> +               do_or_die(drmDropMaster(fd));

igt_device_drop_master(fd)

> +               master2_fd = drm_open_driver_master(DRIVER_ANY);
> +
> +               igt_assert_eq(rmfb(master2_fd, f.fb_id), -ENOENT);
> +
> +               do_or_die(drmDropMaster(master2_fd));
> +               close(master2_fd);
> +
> +               do_or_die(drmSetMaster(fd));

igt_device_set_master(fd)

If you want some with pretty printing?
-Chris
_______________________________________________
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 [i-g-t,1/9] tests/kms_addfb_basic: Check that only the owner can rmfb

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/9] tests/kms_addfb_basic: Check that only the owner can rmfb
URL   : https://patchwork.freedesktop.org/series/57346/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5674 -> IGTPW_2536
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

New tests
---------

  New tests have been introduced between CI_DRM_5674 and IGTPW_2536:

### New IGT tests (1) ###

  * igt@kms_prop_blob@basic:
    - Statuses : 39 pass(s)
    - Exec time: [0.0] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-hsw-4770:        PASS -> SKIP [fdo#109271] +3
    - fi-bsw-n3050:       PASS -> SKIP [fdo#109271]
    - fi-bsw-kefka:       PASS -> SKIP [fdo#109271]

  * igt@i915_pm_rpm@basic-rte:
    - fi-bsw-n3050:       PASS -> FAIL [fdo#108800]
    - fi-bsw-kefka:       PASS -> FAIL [fdo#108800]

  * igt@i915_selftest@live_evict:
    - fi-bsw-kefka:       PASS -> DMESG-WARN [fdo#107709]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182] +1

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-hsw-peppy:       NOTRUN -> SKIP [fdo#109271] +46

  * igt@kms_flip@basic-flip-vs-wf_vblank:
    - fi-bsw-n3050:       PASS -> FAIL [fdo#100368]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       NOTRUN -> DMESG-FAIL [fdo#102614] / [fdo#107814]

  * igt@runner@aborted:
    - fi-bsw-kefka:       NOTRUN -> FAIL [fdo#107709]
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720] / [fdo#109799]

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

  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#107814]: https://bugs.freedesktop.org/show_bug.cgi?id=107814
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109294]: https://bugs.freedesktop.org/show_bug.cgi?id=109294
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#109799]: https://bugs.freedesktop.org/show_bug.cgi?id=109799


Participating hosts (44 -> 41)
------------------------------

  Additional (2): fi-icl-y fi-hsw-peppy 
  Missing    (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


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

    * IGT: IGT_4863 -> IGTPW_2536

  CI_DRM_5674: 71bb3bfb61fb58f93f8b09e6ad576a403cd7752c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2536: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2536/
  IGT_4863: 0f0db14e7f4ec41251ca156d7cb5b8d531a38006 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_addfb_basic@master-rmfb
+igt@kms_lease@atomic_implicit_crtc
+igt@kms_lease@cursor_implicit_plane
+igt@kms_lease@page_flip_implicit_plane
+igt@kms_lease@setcrtc_implicit_plane
+igt@kms_prop_blob@basic
+igt@kms_prop_blob@blob-multiple
+igt@kms_prop_blob@blob-prop-core
+igt@kms_prop_blob@blob-prop-lifetime
+igt@kms_prop_blob@blob-prop-validate
+igt@kms_prop_blob@invalid-get-prop
+igt@kms_prop_blob@invalid-get-prop-any
+igt@kms_prop_blob@invalid-set-prop
+igt@kms_prop_blob@invalid-set-prop-any
-igt@core_prop_blob@basic
-igt@core_prop_blob@blob-multiple
-igt@core_prop_blob@blob-prop-core
-igt@core_prop_blob@blob-prop-lifetime
-igt@core_prop_blob@blob-prop-validate
-igt@core_prop_blob@invalid-get-prop
-igt@core_prop_blob@invalid-get-prop-any
-igt@core_prop_blob@invalid-set-prop
-igt@core_prop_blob@invalid-set-prop-any

== Logs ==

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

[igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/9] tests/kms_addfb_basic: Check that only the owner can rmfb

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/9] tests/kms_addfb_basic: Check that only the owner can rmfb
URL   : https://patchwork.freedesktop.org/series/57346/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5674_full -> IGTPW_2536_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Possible regressions ####

  * {igt@kms_lease@atomic_implicit_crtc} (NEW):
    - shard-kbl:          NOTRUN -> FAIL +3

  * {igt@kms_lease@cursor_implicit_plane} (NEW):
    - shard-hsw:          NOTRUN -> FAIL +3
    - shard-snb:          NOTRUN -> FAIL +3

  * {igt@kms_lease@page_flip_implicit_plane} (NEW):
    - shard-apl:          NOTRUN -> FAIL +3
    - shard-glk:          NOTRUN -> FAIL +3

  
New tests
---------

  New tests have been introduced between CI_DRM_5674_full and IGTPW_2536_full:

### New IGT tests (14) ###

  * igt@kms_addfb_basic@master-rmfb:
    - Statuses : 5 pass(s)
    - Exec time: [0.0, 0.00] s

  * igt@kms_lease@atomic_implicit_crtc:
    - Statuses : 5 fail(s)
    - Exec time: [0.01, 0.15] s

  * igt@kms_lease@cursor_implicit_plane:
    - Statuses : 5 fail(s)
    - Exec time: [0.05, 0.27] s

  * igt@kms_lease@page_flip_implicit_plane:
    - Statuses : 5 fail(s)
    - Exec time: [0.12, 0.27] s

  * igt@kms_lease@setcrtc_implicit_plane:
    - Statuses : 5 fail(s)
    - Exec time: [0.17, 0.82] s

  * igt@kms_prop_blob@basic:
    - Statuses : 5 pass(s)
    - Exec time: [0.0] s

  * igt@kms_prop_blob@blob-multiple:
    - Statuses : 5 pass(s)
    - Exec time: [0.0, 0.00] s

  * igt@kms_prop_blob@blob-prop-core:
    - Statuses : 5 pass(s)
    - Exec time: [0.0] s

  * igt@kms_prop_blob@blob-prop-lifetime:
    - Statuses : 5 pass(s)
    - Exec time: [0.0] s

  * igt@kms_prop_blob@blob-prop-validate:
    - Statuses : 5 pass(s)
    - Exec time: [0.0] s

  * igt@kms_prop_blob@invalid-get-prop:
    - Statuses : 5 pass(s)
    - Exec time: [0.0] s

  * igt@kms_prop_blob@invalid-get-prop-any:
    - Statuses : 5 pass(s)
    - Exec time: [0.0] s

  * igt@kms_prop_blob@invalid-set-prop:
    - Statuses : 5 pass(s)
    - Exec time: [0.0] s

  * igt@kms_prop_blob@invalid-set-prop-any:
    - Statuses : 5 pass(s)
    - Exec time: [0.0] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_userptr_blits@unsync-unmap-cycles:
    - shard-snb:          PASS -> INCOMPLETE [fdo#105411]

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          PASS -> SKIP [fdo#109271]

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
    - shard-apl:          PASS -> FAIL [fdo#109660]

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
    - shard-kbl:          PASS -> FAIL [fdo#109660]

  * igt@kms_busy@basic-flip-d:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +12

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_cursor_crc@cursor-256x256-onscreen:
    - shard-kbl:          PASS -> FAIL [fdo#103232] +2
    - shard-glk:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +6

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-apl:          PASS -> FAIL [fdo#103060]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
    - shard-glk:          PASS -> FAIL [fdo#103167] +6

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-apl:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
    - shard-kbl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-fullscreen:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +17

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-msflip-blt:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +13

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-kbl:          PASS -> FAIL [fdo#103166] +2

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
    - shard-apl:          PASS -> FAIL [fdo#103166] +3

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-glk:          PASS -> DMESG-FAIL [fdo#105763] / [fdo#106538]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]

  * igt@kms_universal_plane@cursor-fb-leak-pipe-d:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_universal_plane@universal-plane-pipe-c-functional:
    - shard-glk:          PASS -> FAIL [fdo#103166] +3

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-apl:          PASS -> FAIL [fdo#104894] +2

  * igt@perf_pmu@busy-check-all-vecs0:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +115

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-suspend:
    - shard-snb:          FAIL [fdo#103375] -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-hsw:          DMESG-WARN [fdo#107956] -> PASS +1

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-kbl:          DMESG-WARN [fdo#107956] -> PASS +2
    - shard-snb:          DMESG-WARN [fdo#107956] -> PASS +1

  * igt@kms_chv_cursor_fail@pipe-c-128x128-top-edge:
    - shard-hsw:          DMESG-WARN [fdo#102614] -> PASS +1

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-apl:          FAIL [fdo#104782] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-offscreen:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +3
    - shard-kbl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-alpha-opaque:
    - shard-apl:          FAIL [fdo#109350] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-kbl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-apl:          FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-glk:          FAIL [fdo#103167] -> PASS +5

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-glk:          FAIL [fdo#108948] -> PASS
    - shard-apl:          FAIL [fdo#108948] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_setmode@basic:
    - shard-hsw:          FAIL [fdo#99912] -> PASS

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-snb:          INCOMPLETE [fdo#105411] -> PASS

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

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (6 -> 5)
------------------------------

  Missing    (1): shard-skl 


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

    * IGT: IGT_4863 -> IGTPW_2536
    * Piglit: piglit_4509 -> None

  CI_DRM_5674: 71bb3bfb61fb58f93f8b09e6ad576a403cd7752c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2536: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2536/
  IGT_4863: 0f0db14e7f4ec41251ca156d7cb5b8d531a38006 @ 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_2536/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/9] tests/kms_lease: test implicit primary plane usage in page_flip ioctl

[Boris Brezillon]

On Thu, 28 Feb 2019 15:19:13 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> We need to make sure that legacy ioctls don't operate on a lease if
> the lesse doesn't have access to the implicitly used primary plane.
> 
> Current kernels fail this.
> 
> Cc: Keith Packard <keithp@keithp.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  tests/kms_lease.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/tests/kms_lease.c b/tests/kms_lease.c
> index 4823121e8cf4..6453c7beafd8 100644
> --- a/tests/kms_lease.c
> +++ b/tests/kms_lease.c
> @@ -311,6 +311,67 @@ static void simple_lease(data_t *data)
>  	terminate_lease(&lease);
>  }
>  
> +static void page_flip_implicit_plane(data_t *data)
> +{
> +	uint32_t object_ids[3];
> +	struct local_drm_mode_create_lease mcl;
> +	drmModePlaneRes *plane_resources;
> +	uint32_t wrong_plane_id = 0;
> +	int i;
> +
> +	/* find a plane which isn't the primary one for us */
> +	plane_resources = drmModeGetPlaneResources(data->master.fd);
> +	for (i = 0; i < plane_resources->count_planes; i++) {
> +		if (plane_resources->planes[i] != data->plane_id) {
> +			wrong_plane_id = plane_resources->planes[i];
> +			break;
> +		}
> +	}
> +	drmModeFreePlaneResources(plane_resources);
> +	igt_require(wrong_plane_id);
> +
> +	mcl.object_ids = (uint64_t) (uintptr_t) &object_ids[0];
> +	mcl.object_count = 0;
> +	mcl.flags = 0;
> +
> +	object_ids[mcl.object_count++] = data->connector_id;
> +	object_ids[mcl.object_count++] = data->crtc_id;
> +
> +	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
> +	do_or_die(create_lease(data->master.fd, &mcl));
> +	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> +
> +	/* Set a mode on the leased output */
> +	igt_assert_eq(0, prepare_crtc(&data->master, data->connector_id, data->crtc_id));
> +
> +	/* sanity check */
> +	do_or_die(drmModePageFlip(data->master.fd, data->crtc_id,
> +			      data->master.primary_fb.fb_id,
> +			      0, NULL));
> +	igt_wait_for_vblank_count(data->master.fd,
> +				  crtc_id_to_pipe(&data->master.display, data->crtc_id),
> +				  1);
> +	do_or_die(drmModePageFlip(mcl.fd, data->crtc_id,
> +			      data->master.primary_fb.fb_id,
> +			      0, NULL));
> +	close(mcl.fd);
> +
> +	object_ids[mcl.object_count++] = wrong_plane_id;
> +	do_or_die(create_lease(data->master.fd, &mcl));
> +
> +	igt_wait_for_vblank_count(data->master.fd,
> +				  crtc_id_to_pipe(&data->master.display, data->crtc_id),
> +				  1);
> +	igt_assert_eq(drmModePageFlip(mcl.fd, data->crtc_id,
> +				      data->master.primary_fb.fb_id,
> +				      0, NULL),
> +		      -EACCES);
> +	close(mcl.fd);
> +
> +	cleanup_crtc(&data->master,
> +		     connector_id_to_output(&data->master.display, data->connector_id));
> +}
> +
>  /* Test listing lessees */
>  static void lessee_list(data_t *data)
>  {
> @@ -1024,6 +1085,7 @@ igt_main
>  		{ "lease_invalid_connector", lease_invalid_connector },
>  		{ "lease_invalid_crtc", lease_invalid_crtc },
>  		{ "lease_invalid_plane", lease_invalid_plane },
> +		{ "page_flip_implicit_plane", page_flip_implicit_plane },
>  		{ }
>  	}, *f;
>  

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

Re: [igt-dev] [PATCH i-g-t 5/9] tests/kms_lease: test implicit primary plane usage in setcrtc ioctl

[Boris Brezillon]

On Thu, 28 Feb 2019 15:19:14 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Again we need to make sure that a lease can't use planes it's not
> allowed to through legacy ioctls. SetCrtc is a bit more tricky, since

	  ^ "to use through"?

> we should still allow to shut down a CRTC, e.g. when we're allowed to
> use other planes on that CRTC.
> 
> Current kernels fail this.
> 
> Cc: Keith Packard <keithp@keithp.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  tests/kms_lease.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/tests/kms_lease.c b/tests/kms_lease.c
> index 6453c7beafd8..c11d37aeac7a 100644
> --- a/tests/kms_lease.c
> +++ b/tests/kms_lease.c
> @@ -372,6 +372,65 @@ static void page_flip_implicit_plane(data_t *data)
>  		     connector_id_to_output(&data->master.display, data->connector_id));
>  }
>  
> +static void setcrtc_implicit_plane(data_t *data)
> +{
> +	uint32_t object_ids[3];
> +	struct local_drm_mode_create_lease mcl;
> +	drmModePlaneRes *plane_resources;
> +	uint32_t wrong_plane_id = 0;
> +	igt_output_t *output =
> +		connector_id_to_output(&data->master.display,
> +				       data->connector_id);
> +	drmModeModeInfo *mode = igt_output_get_mode(output);
> +	int i;
> +
> +	/* find a plane which isn't the primary one for us */
> +	plane_resources = drmModeGetPlaneResources(data->master.fd);
> +	for (i = 0; i < plane_resources->count_planes; i++) {
> +		if (plane_resources->planes[i] != data->plane_id) {
> +			wrong_plane_id = plane_resources->planes[i];
> +			break;
> +		}
> +	}
> +	drmModeFreePlaneResources(plane_resources);
> +	igt_require(wrong_plane_id);

Maybe we could add an helper to retrieve wrong_plane_id instead of
duplicating the code.

> +
> +	mcl.object_ids = (uint64_t) (uintptr_t) &object_ids[0];
> +	mcl.object_count = 0;
> +	mcl.flags = 0;
> +
> +	object_ids[mcl.object_count++] = data->connector_id;
> +	object_ids[mcl.object_count++] = data->crtc_id;
> +
> +	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
> +	do_or_die(create_lease(data->master.fd, &mcl));
> +	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> +
> +	/* Set a mode on the leased output */
> +	igt_assert_eq(0, prepare_crtc(&data->master, data->connector_id, data->crtc_id));

Same goes for those create-lease+prepare-CRTC steps.

> +
> +	/* sanity check */
> +	do_or_die(drmModeSetCrtc(data->master.fd, data->crtc_id, -1,
> +				 0, 0, object_ids, 1, mode));
> +	do_or_die(drmModeSetCrtc(mcl.fd, data->crtc_id, -1,
> +				 0, 0, object_ids, 1, mode));
> +	close(mcl.fd);
> +
> +	object_ids[mcl.object_count++] = wrong_plane_id;
> +	do_or_die(create_lease(data->master.fd, &mcl));
> +
> +	igt_assert_eq(drmModeSetCrtc(mcl.fd, data->crtc_id, -1,
> +				     0, 0, object_ids, 1, mode),
> +		      -EACCES);
> +	/* make sure we are allowed to turn the CRTC off */
> +	do_or_die(drmModeSetCrtc(mcl.fd, data->crtc_id,
> +				 0, 0, 0, NULL, 0, NULL));

Do we actually test the case where the lessor has setup the CRTC with a
plane that's not accessible by the lesse. I think that's what you were
trying to test here, but I don't see where it's done in the code.
Shouldn't we have something like:

	do_or_die(drmModeSetCrtc(data->master.fd, data->crtc_id, -1,
				 0, 0, object_ids, 1, mode));
	igt_assert_eq(0, drmModeSetCrtc(mcl.fd, data->crtc_id, 0, 0, 0,
					NULL, 0, NULL));


Looks good otherwise.

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> +	close(mcl.fd);
> +
> +	cleanup_crtc(&data->master,
> +		     connector_id_to_output(&data->master.display, data->connector_id));
> +}
> +
>  /* Test listing lessees */
>  static void lessee_list(data_t *data)
>  {
> @@ -1086,6 +1145,7 @@ igt_main
>  		{ "lease_invalid_crtc", lease_invalid_crtc },
>  		{ "lease_invalid_plane", lease_invalid_plane },
>  		{ "page_flip_implicit_plane", page_flip_implicit_plane },
> +		{ "setcrtc_implicit_plane", setcrtc_implicit_plane },
>  		{ }
>  	}, *f;
>  

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

Re: [igt-dev] [PATCH i-g-t 6/9] tests/kms_lease: test implicit cursor plane usage

[Boris Brezillon]

On Thu, 28 Feb 2019 15:19:15 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Figured I'll only test the SetCursor interface since in the kernel
> it's all the same anyway, it's just libdrm that splits it up.
> 
> Current kernels fail this.
> 
> Cc: Keith Packard <keithp@keithp.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  tests/kms_lease.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/tests/kms_lease.c b/tests/kms_lease.c
> index c11d37aeac7a..7930d0737ace 100644
> --- a/tests/kms_lease.c
> +++ b/tests/kms_lease.c
> @@ -431,6 +431,42 @@ static void setcrtc_implicit_plane(data_t *data)
>  		     connector_id_to_output(&data->master.display, data->connector_id));
>  }
>  
> +static void cursor_implicit_plane(data_t *data)
> +{
> +	uint32_t object_ids[3];
> +	struct local_drm_mode_create_lease mcl;
> +
> +	mcl.object_ids = (uint64_t) (uintptr_t) &object_ids[0];
> +	mcl.object_count = 0;
> +	mcl.flags = 0;
> +
> +	object_ids[mcl.object_count++] = data->connector_id;
> +	object_ids[mcl.object_count++] = data->crtc_id;
> +
> +	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
> +	do_or_die(create_lease(data->master.fd, &mcl));
> +	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> +
> +	/* Set a mode on the leased output */
> +	igt_assert_eq(0, prepare_crtc(&data->master, data->connector_id, data->crtc_id));
> +
> +	/* sanity check */
> +	do_or_die(drmModeSetCursor(data->master.fd, data->crtc_id, 0, 0, 0));
> +	do_or_die(drmModeSetCursor(mcl.fd, data->crtc_id, 0, 0, 0));
> +	close(mcl.fd);
> +
> +	/* primary plane is never the cursor */
> +	object_ids[mcl.object_count++] = data->plane_id;
> +	do_or_die(create_lease(data->master.fd, &mcl));
> +
> +	igt_assert_eq(drmModeSetCursor(mcl.fd, data->crtc_id, 0, 0, 0),
> +		      -EACCES);
> +	close(mcl.fd);
> +
> +	cleanup_crtc(&data->master,
> +		     connector_id_to_output(&data->master.display, data->connector_id));
> +}
> +
>  /* Test listing lessees */
>  static void lessee_list(data_t *data)
>  {
> @@ -1146,6 +1182,7 @@ igt_main
>  		{ "lease_invalid_plane", lease_invalid_plane },
>  		{ "page_flip_implicit_plane", page_flip_implicit_plane },
>  		{ "setcrtc_implicit_plane", setcrtc_implicit_plane },
> +		{ "cursor_implicit_plane", cursor_implicit_plane },
>  		{ }
>  	}, *f;
>  

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

Re: [igt-dev] [PATCH i-g-t 7/9] tests/kms_lease: Adjust to kernel errno changes

[Boris Brezillon]

On Thu, 28 Feb 2019 15:19:16 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> I dropped a superfluous check for negative object id (the kernel
> did a cast to s32, despite that object ids are always unsigned).
> Which changes the errno from EINVAL to ENOENT. Allow both.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  tests/kms_lease.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/kms_lease.c b/tests/kms_lease.c
> index 7930d0737ace..7d7576cbb634 100644
> --- a/tests/kms_lease.c
> +++ b/tests/kms_lease.c
> @@ -713,6 +713,10 @@ static void lease_again(data_t *data)
>  	terminate_lease(&lease_b);
>  }
>  
> +#define assert_unleased(ret) \
> +	igt_assert_f((ret) == -EINVAL || (ret) == -ENOENT, \
> +		     "wrong return code %i, %s\n", ret, \
> +		     strerror(ret))
>  /* Test leasing an invalid connector */
>  static void lease_invalid_connector(data_t *data)
>  {
> @@ -725,7 +729,7 @@ static void lease_invalid_connector(data_t *data)
>  	data->connector_id = 0xbaadf00d;
>  	ret = make_lease(data, &lease);
>  	data->connector_id = save_connector_id;
> -	igt_assert_eq(ret, -EINVAL);
> +	assert_unleased(ret);
>  }
>  
>  /* Test leasing an invalid crtc */
> @@ -740,7 +744,7 @@ static void lease_invalid_crtc(data_t *data)
>  	data->crtc_id = 0xbaadf00d;
>  	ret = make_lease(data, &lease);
>  	data->crtc_id = save_crtc_id;
> -	igt_assert_eq(ret, -EINVAL);
> +	assert_unleased(ret);
>  }
>  
>  static void lease_invalid_plane(data_t *data)
> @@ -754,7 +758,7 @@ static void lease_invalid_plane(data_t *data)
>  	data->plane_id = 0xbaadf00d;
>  	ret = make_lease(data, &lease);
>  	data->plane_id = save_plane_id;
> -	igt_assert_eq(ret, -EINVAL);
> +	assert_unleased(ret);
>  }
>  
>  

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

Re: [igt-dev] [PATCH i-g-t 8/9] tests/kms_lease: Handle new errno from idr/xa double insert

[Boris Brezillon]

On Thu, 28 Feb 2019 15:19:17 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> The conversion from idr to xarray will change the errno for already
> inserted object ids from ENOSPC to EBUSY. Allow both.
> 
> Cc: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  tests/kms_lease.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/kms_lease.c b/tests/kms_lease.c
> index 7d7576cbb634..b5db37c5d58a 100644
> --- a/tests/kms_lease.c
> +++ b/tests/kms_lease.c
> @@ -797,12 +797,16 @@ static void run_test(data_t *data, void (*testfunc)(data_t *))
>  		      "no valid crtc/connector combinations found\n");
>  }
>  
> +#define assert_double_id_err(ret) \
> +	igt_assert_f((ret) == -EBUSY || (ret) == -ENOSPC, \
> +		     "wrong return code %i, %s\n", ret, \
> +		     strerror(ret))
>  static void invalid_create_leases(data_t *data)
>  {
>  	uint32_t object_ids[4];
>  	struct local_drm_mode_create_lease mcl;
>  	drmModeRes *resources;
> -	int tmp_fd;
> +	int tmp_fd, ret;
>  
>  	/* empty lease */
>  	mcl.object_ids = 0;
> @@ -881,7 +885,8 @@ static void invalid_create_leases(data_t *data)
>  	object_ids[3] = object_ids[2];
>  	mcl.object_count = 4;
>  	/* Note: the ENOSPC is from idr double-insertion failing */
> -	igt_assert_eq(create_lease(data->master.fd, &mcl), -ENOSPC);
> +	ret = create_lease(data->master.fd, &mcl);
> +	assert_double_id_err(ret);
>  
>  	/* no encoder leasing */
>  	resources = drmModeGetResources(data->master.fd);
> @@ -1094,7 +1099,7 @@ static void implicit_plane_lease(data_t *data)
>  	uint32_t object_ids[3];
>  	struct local_drm_mode_create_lease mcl;
>  	struct local_drm_mode_get_lease mgl;
> -
> +	int ret;
>  	uint32_t cursor_id = igt_pipe_get_plane_type(&data->master.display.pipes[0],
>  						     DRM_PLANE_TYPE_CURSOR)->drm_plane->plane_id;
>  
> @@ -1127,11 +1132,13 @@ static void implicit_plane_lease(data_t *data)
>  	/* check that implicit lease doesn't lead to confusion when
>  	 * explicitly adding primary plane */
>  	mcl.object_count = 3;
> -	igt_assert_eq(create_lease(data->master.fd, &mcl), -ENOSPC);
> +	ret = create_lease(data->master.fd, &mcl);
> +	assert_double_id_err(ret);
>  
>  	/* same for the cursor */
>  	object_ids[2] = cursor_id;
> -	igt_assert_eq(create_lease(data->master.fd, &mcl), -ENOSPC);
> +	ret = create_lease(data->master.fd, &mcl);
> +	assert_double_id_err(ret);
>  
>  	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
>  }

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

Re: [igt-dev] [PATCH i-g-t 9/9] tests/kms_lease: Check crtc used in atomic ioctl

[Boris Brezillon]

On Thu, 28 Feb 2019 15:19:18 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> We not only need to check that userspace is allowed to use the objects
> it's changing, but also the objects it's using as property values. The
> only ones relevant for leases are the CRTC_ID properties on connectors
> and planes.
> 
> Current kernels fail this.
> 
> Cc: Keith Packard <keithp@keithp.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  tests/kms_lease.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 89 insertions(+)
> 
> diff --git a/tests/kms_lease.c b/tests/kms_lease.c
> index b5db37c5d58a..694a612f76a7 100644
> --- a/tests/kms_lease.c
> +++ b/tests/kms_lease.c
> @@ -467,6 +467,94 @@ static void cursor_implicit_plane(data_t *data)
>  		     connector_id_to_output(&data->master.display, data->connector_id));
>  }
>  
> +static void atomic_implicit_crtc(data_t *data)
> +{
> +	uint32_t object_ids[3];
> +	struct local_drm_mode_create_lease mcl;
> +	drmModeRes *resources;
> +	drmModeObjectPropertiesPtr props;
> +	uint32_t wrong_crtc_id = 0;
> +	uint32_t crtc_id_prop = 0;
> +	drmModeAtomicReqPtr req = NULL;
> +	int ret;
> +
> +	igt_require(data->master.display.is_atomic);
> +
> +	mcl.object_ids = (uint64_t) (uintptr_t) &object_ids[0];
> +	mcl.object_count = 0;
> +	mcl.flags = 0;
> +
> +	object_ids[mcl.object_count++] = data->connector_id;
> +	object_ids[mcl.object_count++] = data->plane_id;
> +
> +	/* find a plane which isn't the primary one for us */
> +	resources = drmModeGetResources(data->master.fd);
> +	igt_assert(resources);
> +	for (int i = 0; i < resources->count_crtcs; i++) {
> +		if (resources->crtcs[i] != data->crtc_id) {
> +			wrong_crtc_id = resources->crtcs[i];
> +			break;
> +		}
> +	}
> +	drmModeFreeResources(resources);
> +	igt_require(wrong_crtc_id);

Same logic duplicated here again.

> +	object_ids[mcl.object_count++] = wrong_crtc_id;
> +
> +	/* find the CRTC_ID prop, it's global */
> +	props = drmModeObjectGetProperties(data->master.fd, data->plane_id,
> +					   DRM_MODE_OBJECT_PLANE);
> +	igt_assert(props);
> +	for (int i = 0; i < props->count_props; i++) {
> +		drmModePropertyPtr prop = drmModeGetProperty(data->master.fd,
> +							     props->props[i]);
> +		if (strcmp(prop->name, "CRTC_ID") == 0)
> +			crtc_id_prop = props->props[i];
> +
> +		printf("prop name %s, prop id %u, prop id %u\n",
> +		       prop->name, props->props[i], prop->prop_id);
> +		drmModeFreeProperty(prop);
> +		if (crtc_id_prop)
> +			break;
> +	}
> +	drmModeFreeObjectProperties(props);
> +	igt_assert(crtc_id_prop);
> +
> +	do_or_die(create_lease(data->master.fd, &mcl));
> +	do_or_die(drmSetClientCap(mcl.fd, DRM_CLIENT_CAP_ATOMIC, 1));
> +
> +	/* check CRTC_ID property on the plane */
> +	req = drmModeAtomicAlloc();
> +	igt_assert(req);
> +	ret = drmModeAtomicAddProperty(req, data->plane_id,
> +				       crtc_id_prop, data->crtc_id);
> +	igt_assert(ret >= 0);
> +
> +	/* sanity check */
> +	ret = drmModeAtomicCommit(data->master.fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> +	igt_assert(ret == 0 || ret == -EINVAL);

Maybe it's worth adding a comment explaining why this call can return
EINVAL.

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> +
> +	ret = drmModeAtomicCommit(mcl.fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> +	igt_assert(ret == -EACCES);
> +	drmModeAtomicFree(req);
> +
> +	/* check CRTC_ID property on the connector */
> +	req = drmModeAtomicAlloc();
> +	igt_assert(req);
> +	ret = drmModeAtomicAddProperty(req, data->connector_id,
> +				       crtc_id_prop, data->crtc_id);
> +	igt_assert(ret >= 0);
> +
> +	/* sanity check */
> +	ret = drmModeAtomicCommit(data->master.fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> +	igt_assert(ret == 0 || ret == -EINVAL);
> +
> +	ret = drmModeAtomicCommit(mcl.fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> +	igt_assert(ret == -EACCES);
> +	drmModeAtomicFree(req);
> +
> +	close(mcl.fd);
> +}
> +
>  /* Test listing lessees */
>  static void lessee_list(data_t *data)
>  {
> @@ -1194,6 +1282,7 @@ igt_main
>  		{ "page_flip_implicit_plane", page_flip_implicit_plane },
>  		{ "setcrtc_implicit_plane", setcrtc_implicit_plane },
>  		{ "cursor_implicit_plane", cursor_implicit_plane },
> +		{ "atomic_implicit_crtc", atomic_implicit_crtc },
>  		{ }
>  	}, *f;
>  

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

Re: [igt-dev] [PATCH i-g-t 5/9] tests/kms_lease: test implicit primary plane usage in setcrtc ioctl

[Daniel Vetter]

On Wed, Mar 27, 2019 at 09:26:23AM +0100, Boris Brezillon wrote:
> On Thu, 28 Feb 2019 15:19:14 +0100
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> 
> > Again we need to make sure that a lease can't use planes it's not
> > allowed to through legacy ioctls. SetCrtc is a bit more tricky, since
> 
> 	  ^ "to use through"?

Yup.

> 
> > we should still allow to shut down a CRTC, e.g. when we're allowed to
> > use other planes on that CRTC.
> > 
> > Current kernels fail this.
> > 
> > Cc: Keith Packard <keithp@keithp.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  tests/kms_lease.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 60 insertions(+)
> > 
> > diff --git a/tests/kms_lease.c b/tests/kms_lease.c
> > index 6453c7beafd8..c11d37aeac7a 100644
> > --- a/tests/kms_lease.c
> > +++ b/tests/kms_lease.c
> > @@ -372,6 +372,65 @@ static void page_flip_implicit_plane(data_t *data)
> >  		     connector_id_to_output(&data->master.display, data->connector_id));
> >  }
> >  
> > +static void setcrtc_implicit_plane(data_t *data)
> > +{
> > +	uint32_t object_ids[3];
> > +	struct local_drm_mode_create_lease mcl;
> > +	drmModePlaneRes *plane_resources;
> > +	uint32_t wrong_plane_id = 0;
> > +	igt_output_t *output =
> > +		connector_id_to_output(&data->master.display,
> > +				       data->connector_id);
> > +	drmModeModeInfo *mode = igt_output_get_mode(output);
> > +	int i;
> > +
> > +	/* find a plane which isn't the primary one for us */
> > +	plane_resources = drmModeGetPlaneResources(data->master.fd);
> > +	for (i = 0; i < plane_resources->count_planes; i++) {
> > +		if (plane_resources->planes[i] != data->plane_id) {
> > +			wrong_plane_id = plane_resources->planes[i];
> > +			break;
> > +		}
> > +	}
> > +	drmModeFreePlaneResources(plane_resources);
> > +	igt_require(wrong_plane_id);
> 
> Maybe we could add an helper to retrieve wrong_plane_id instead of
> duplicating the code.

Hm, where is it duplicated? The other very similar loop looks for the
worng connector, and there's a few other loops to look for the right
plane/connector. I think they're all different.
> 
> > +
> > +	mcl.object_ids = (uint64_t) (uintptr_t) &object_ids[0];
> > +	mcl.object_count = 0;
> > +	mcl.flags = 0;
> > +
> > +	object_ids[mcl.object_count++] = data->connector_id;
> > +	object_ids[mcl.object_count++] = data->crtc_id;
> > +
> > +	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
> > +	do_or_die(create_lease(data->master.fd, &mcl));
> > +	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> > +
> > +	/* Set a mode on the leased output */
> > +	igt_assert_eq(0, prepare_crtc(&data->master, data->connector_id, data->crtc_id));
> 
> Same goes for those create-lease+prepare-CRTC steps.

There's a helper for the common case, but for these "wrong" leases there's
always subtle differences. A helper would be be about as much code as just
open-coding it. Maybe we could move the UNIVERSAL_PLANES as a mode flag
into create_lease, but given how often I screamed at my tests for not
working because I got that one wrong, I think it's better to be explicit.
> 
> > +
> > +	/* sanity check */
> > +	do_or_die(drmModeSetCrtc(data->master.fd, data->crtc_id, -1,
> > +				 0, 0, object_ids, 1, mode));
> > +	do_or_die(drmModeSetCrtc(mcl.fd, data->crtc_id, -1,
> > +				 0, 0, object_ids, 1, mode));
> > +	close(mcl.fd);
> > +
> > +	object_ids[mcl.object_count++] = wrong_plane_id;
> > +	do_or_die(create_lease(data->master.fd, &mcl));
> > +
> > +	igt_assert_eq(drmModeSetCrtc(mcl.fd, data->crtc_id, -1,
> > +				     0, 0, object_ids, 1, mode),
> > +		      -EACCES);
> > +	/* make sure we are allowed to turn the CRTC off */
> > +	do_or_die(drmModeSetCrtc(mcl.fd, data->crtc_id,
> > +				 0, 0, 0, NULL, 0, NULL));
> 
> Do we actually test the case where the lessor has setup the CRTC with a
> plane that's not accessible by the lesse. I think that's what you were
> trying to test here, but I don't see where it's done in the code.
> Shouldn't we have something like:
> 
> 	do_or_die(drmModeSetCrtc(data->master.fd, data->crtc_id, -1,
> 				 0, 0, object_ids, 1, mode));

Sanity check above does exactly that.

> 	igt_assert_eq(0, drmModeSetCrtc(mcl.fd, data->crtc_id, 0, 0, 0,
> 					NULL, 0, NULL));

That's the line right above wrapped in the do_or_die.

> Looks good otherwise.

Cleared up with my comments above and r-b for the entire thing (with the
commit message fixed)?
-Daniel

> 
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> 
> > +	close(mcl.fd);
> > +
> > +	cleanup_crtc(&data->master,
> > +		     connector_id_to_output(&data->master.display, data->connector_id));
> > +}
> > +
> >  /* Test listing lessees */
> >  static void lessee_list(data_t *data)
> >  {
> > @@ -1086,6 +1145,7 @@ igt_main
> >  		{ "lease_invalid_crtc", lease_invalid_crtc },
> >  		{ "lease_invalid_plane", lease_invalid_plane },
> >  		{ "page_flip_implicit_plane", page_flip_implicit_plane },
> > +		{ "setcrtc_implicit_plane", setcrtc_implicit_plane },
> >  		{ }
> >  	}, *f;
> >  
> 

-- 
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 i-g-t 9/9] tests/kms_lease: Check crtc used in atomic ioctl

[Daniel Vetter]

On Wed, Mar 27, 2019 at 09:38:09AM +0100, Boris Brezillon wrote:
> On Thu, 28 Feb 2019 15:19:18 +0100
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> 
> > We not only need to check that userspace is allowed to use the objects
> > it's changing, but also the objects it's using as property values. The
> > only ones relevant for leases are the CRTC_ID properties on connectors
> > and planes.
> > 
> > Current kernels fail this.
> > 
> > Cc: Keith Packard <keithp@keithp.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  tests/kms_lease.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 89 insertions(+)
> > 
> > diff --git a/tests/kms_lease.c b/tests/kms_lease.c
> > index b5db37c5d58a..694a612f76a7 100644
> > --- a/tests/kms_lease.c
> > +++ b/tests/kms_lease.c
> > @@ -467,6 +467,94 @@ static void cursor_implicit_plane(data_t *data)
> >  		     connector_id_to_output(&data->master.display, data->connector_id));
> >  }
> >  
> > +static void atomic_implicit_crtc(data_t *data)
> > +{
> > +	uint32_t object_ids[3];
> > +	struct local_drm_mode_create_lease mcl;
> > +	drmModeRes *resources;
> > +	drmModeObjectPropertiesPtr props;
> > +	uint32_t wrong_crtc_id = 0;
> > +	uint32_t crtc_id_prop = 0;
> > +	drmModeAtomicReqPtr req = NULL;
> > +	int ret;
> > +
> > +	igt_require(data->master.display.is_atomic);
> > +
> > +	mcl.object_ids = (uint64_t) (uintptr_t) &object_ids[0];
> > +	mcl.object_count = 0;
> > +	mcl.flags = 0;
> > +
> > +	object_ids[mcl.object_count++] = data->connector_id;
> > +	object_ids[mcl.object_count++] = data->plane_id;
> > +
> > +	/* find a plane which isn't the primary one for us */
> > +	resources = drmModeGetResources(data->master.fd);
> > +	igt_assert(resources);
> > +	for (int i = 0; i < resources->count_crtcs; i++) {
> > +		if (resources->crtcs[i] != data->crtc_id) {
> > +			wrong_crtc_id = resources->crtcs[i];
> > +			break;
> > +		}
> > +	}
> > +	drmModeFreeResources(resources);
> > +	igt_require(wrong_crtc_id);
> 
> Same logic duplicated here again.

Now we look for a wrong crtc. The other loops look for a wrong plane or
wrong connector.

> 
> > +	object_ids[mcl.object_count++] = wrong_crtc_id;
> > +
> > +	/* find the CRTC_ID prop, it's global */
> > +	props = drmModeObjectGetProperties(data->master.fd, data->plane_id,
> > +					   DRM_MODE_OBJECT_PLANE);
> > +	igt_assert(props);
> > +	for (int i = 0; i < props->count_props; i++) {
> > +		drmModePropertyPtr prop = drmModeGetProperty(data->master.fd,
> > +							     props->props[i]);
> > +		if (strcmp(prop->name, "CRTC_ID") == 0)
> > +			crtc_id_prop = props->props[i];
> > +
> > +		printf("prop name %s, prop id %u, prop id %u\n",
> > +		       prop->name, props->props[i], prop->prop_id);
> > +		drmModeFreeProperty(prop);
> > +		if (crtc_id_prop)
> > +			break;
> > +	}
> > +	drmModeFreeObjectProperties(props);
> > +	igt_assert(crtc_id_prop);
> > +
> > +	do_or_die(create_lease(data->master.fd, &mcl));
> > +	do_or_die(drmSetClientCap(mcl.fd, DRM_CLIENT_CAP_ATOMIC, 1));
> > +
> > +	/* check CRTC_ID property on the plane */
> > +	req = drmModeAtomicAlloc();
> > +	igt_assert(req);
> > +	ret = drmModeAtomicAddProperty(req, data->plane_id,
> > +				       crtc_id_prop, data->crtc_id);
> > +	igt_assert(ret >= 0);
> > +
> > +	/* sanity check */
> > +	ret = drmModeAtomicCommit(data->master.fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> > +	igt_assert(ret == 0 || ret == -EINVAL);
> 
> Maybe it's worth adding a comment explaining why this call can return
> EINVAL.

Hm yeah makes sense. Something like this?

	/* We don't know whether this is a valid configuration for the
	 * driver we're testing, so allow both success and "invalid
	 * config" errno codes */
> 
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

Thanks, Daniel

> 
> > +
> > +	ret = drmModeAtomicCommit(mcl.fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> > +	igt_assert(ret == -EACCES);
> > +	drmModeAtomicFree(req);
> > +
> > +	/* check CRTC_ID property on the connector */
> > +	req = drmModeAtomicAlloc();
> > +	igt_assert(req);
> > +	ret = drmModeAtomicAddProperty(req, data->connector_id,
> > +				       crtc_id_prop, data->crtc_id);
> > +	igt_assert(ret >= 0);
> > +
> > +	/* sanity check */
> > +	ret = drmModeAtomicCommit(data->master.fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> > +	igt_assert(ret == 0 || ret == -EINVAL);
> > +
> > +	ret = drmModeAtomicCommit(mcl.fd, req, DRM_MODE_ATOMIC_TEST_ONLY, NULL);
> > +	igt_assert(ret == -EACCES);
> > +	drmModeAtomicFree(req);
> > +
> > +	close(mcl.fd);
> > +}
> > +
> >  /* Test listing lessees */
> >  static void lessee_list(data_t *data)
> >  {
> > @@ -1194,6 +1282,7 @@ igt_main
> >  		{ "page_flip_implicit_plane", page_flip_implicit_plane },
> >  		{ "setcrtc_implicit_plane", setcrtc_implicit_plane },
> >  		{ "cursor_implicit_plane", cursor_implicit_plane },
> > +		{ "atomic_implicit_crtc", atomic_implicit_crtc },
> >  		{ }
> >  	}, *f;
> >  
> 

-- 
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 i-g-t 5/9] tests/kms_lease: test implicit primary plane usage in setcrtc ioctl

[Boris Brezillon]

On Wed, 27 Mar 2019 10:21:15 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, Mar 27, 2019 at 09:26:23AM +0100, Boris Brezillon wrote:
> > On Thu, 28 Feb 2019 15:19:14 +0100
> > Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >   
> > > Again we need to make sure that a lease can't use planes it's not
> > > allowed to through legacy ioctls. SetCrtc is a bit more tricky, since  
> > 
> > 	  ^ "to use through"?  
> 
> Yup.
> 
> >   
> > > we should still allow to shut down a CRTC, e.g. when we're allowed to
> > > use other planes on that CRTC.
> > > 
> > > Current kernels fail this.
> > > 
> > > Cc: Keith Packard <keithp@keithp.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > ---
> > >  tests/kms_lease.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 60 insertions(+)
> > > 
> > > diff --git a/tests/kms_lease.c b/tests/kms_lease.c
> > > index 6453c7beafd8..c11d37aeac7a 100644
> > > --- a/tests/kms_lease.c
> > > +++ b/tests/kms_lease.c
> > > @@ -372,6 +372,65 @@ static void page_flip_implicit_plane(data_t *data)
> > >  		     connector_id_to_output(&data->master.display, data->connector_id));
> > >  }
> > >  
> > > +static void setcrtc_implicit_plane(data_t *data)
> > > +{
> > > +	uint32_t object_ids[3];
> > > +	struct local_drm_mode_create_lease mcl;
> > > +	drmModePlaneRes *plane_resources;
> > > +	uint32_t wrong_plane_id = 0;
> > > +	igt_output_t *output =
> > > +		connector_id_to_output(&data->master.display,
> > > +				       data->connector_id);
> > > +	drmModeModeInfo *mode = igt_output_get_mode(output);
> > > +	int i;
> > > +
> > > +	/* find a plane which isn't the primary one for us */
> > > +	plane_resources = drmModeGetPlaneResources(data->master.fd);
> > > +	for (i = 0; i < plane_resources->count_planes; i++) {
> > > +		if (plane_resources->planes[i] != data->plane_id) {
> > > +			wrong_plane_id = plane_resources->planes[i];
> > > +			break;
> > > +		}
> > > +	}
> > > +	drmModeFreePlaneResources(plane_resources);
> > > +	igt_require(wrong_plane_id);  
> > 
> > Maybe we could add an helper to retrieve wrong_plane_id instead of
> > duplicating the code.  
> 
> Hm, where is it duplicated? The other very similar loop looks for the
> worng connector, and there's a few other loops to look for the right
> plane/connector. I think they're all different.
> >   
> > > +
> > > +	mcl.object_ids = (uint64_t) (uintptr_t) &object_ids[0];
> > > +	mcl.object_count = 0;
> > > +	mcl.flags = 0;
> > > +
> > > +	object_ids[mcl.object_count++] = data->connector_id;
> > > +	object_ids[mcl.object_count++] = data->crtc_id;
> > > +
> > > +	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
> > > +	do_or_die(create_lease(data->master.fd, &mcl));
> > > +	drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> > > +
> > > +	/* Set a mode on the leased output */
> > > +	igt_assert_eq(0, prepare_crtc(&data->master, data->connector_id, data->crtc_id));  
> > 
> > Same goes for those create-lease+prepare-CRTC steps.  
> 
> There's a helper for the common case, but for these "wrong" leases there's
> always subtle differences. A helper would be be about as much code as just
> open-coding it. Maybe we could move the UNIVERSAL_PLANES as a mode flag
> into create_lease, but given how often I screamed at my tests for not
> working because I got that one wrong, I think it's better to be explicit.
> >   
> > > +
> > > +	/* sanity check */
> > > +	do_or_die(drmModeSetCrtc(data->master.fd, data->crtc_id, -1,
> > > +				 0, 0, object_ids, 1, mode));
> > > +	do_or_die(drmModeSetCrtc(mcl.fd, data->crtc_id, -1,
> > > +				 0, 0, object_ids, 1, mode));
> > > +	close(mcl.fd);
> > > +
> > > +	object_ids[mcl.object_count++] = wrong_plane_id;
> > > +	do_or_die(create_lease(data->master.fd, &mcl));
> > > +
> > > +	igt_assert_eq(drmModeSetCrtc(mcl.fd, data->crtc_id, -1,
> > > +				     0, 0, object_ids, 1, mode),
> > > +		      -EACCES);
> > > +	/* make sure we are allowed to turn the CRTC off */
> > > +	do_or_die(drmModeSetCrtc(mcl.fd, data->crtc_id,
> > > +				 0, 0, 0, NULL, 0, NULL));  
> > 
> > Do we actually test the case where the lessor has setup the CRTC with a
> > plane that's not accessible by the lesse. I think that's what you were
> > trying to test here, but I don't see where it's done in the code.
> > Shouldn't we have something like:
> > 
> > 	do_or_die(drmModeSetCrtc(data->master.fd, data->crtc_id, -1,
> > 				 0, 0, object_ids, 1, mode));  
> 
> Sanity check above does exactly that.

Yes, but wrong_plane_id is not part of the object list in your sanity
check, is it?

> 
> > 	igt_assert_eq(0, drmModeSetCrtc(mcl.fd, data->crtc_id, 0, 0, 0,
> > 					NULL, 0, NULL));  
> 
> That's the line right above wrapped in the do_or_die.

Are you sure wrong_plane_id was enabled when you call drmModeSetCrtc()
to disable the CRTC? If it wasn't, that means you're not really testing
the 'disable CRTC even if other planes I don't own are still attached to
it'. But I'm probably missing something.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 5/9] tests/kms_lease: test implicit primary plane usage in setcrtc ioctl

[Daniel Vetter]

On Wed, Mar 27, 2019 at 11:10 AM Boris Brezillon
<boris.brezillon@collabora.com> wrote:
>
> On Wed, 27 Mar 2019 10:21:15 +0100
> Daniel Vetter <daniel@ffwll.ch> wrote:
>
> > On Wed, Mar 27, 2019 at 09:26:23AM +0100, Boris Brezillon wrote:
> > > On Thu, 28 Feb 2019 15:19:14 +0100
> > > Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > >
> > > > Again we need to make sure that a lease can't use planes it's not
> > > > allowed to through legacy ioctls. SetCrtc is a bit more tricky, since
> > >
> > >       ^ "to use through"?
> >
> > Yup.
> >
> > >
> > > > we should still allow to shut down a CRTC, e.g. when we're allowed to
> > > > use other planes on that CRTC.
> > > >
> > > > Current kernels fail this.
> > > >
> > > > Cc: Keith Packard <keithp@keithp.com>
> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > ---
> > > >  tests/kms_lease.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 60 insertions(+)
> > > >
> > > > diff --git a/tests/kms_lease.c b/tests/kms_lease.c
> > > > index 6453c7beafd8..c11d37aeac7a 100644
> > > > --- a/tests/kms_lease.c
> > > > +++ b/tests/kms_lease.c
> > > > @@ -372,6 +372,65 @@ static void page_flip_implicit_plane(data_t *data)
> > > >                connector_id_to_output(&data->master.display, data->connector_id));
> > > >  }
> > > >
> > > > +static void setcrtc_implicit_plane(data_t *data)
> > > > +{
> > > > + uint32_t object_ids[3];
> > > > + struct local_drm_mode_create_lease mcl;
> > > > + drmModePlaneRes *plane_resources;
> > > > + uint32_t wrong_plane_id = 0;
> > > > + igt_output_t *output =
> > > > +         connector_id_to_output(&data->master.display,
> > > > +                                data->connector_id);
> > > > + drmModeModeInfo *mode = igt_output_get_mode(output);
> > > > + int i;
> > > > +
> > > > + /* find a plane which isn't the primary one for us */
> > > > + plane_resources = drmModeGetPlaneResources(data->master.fd);
> > > > + for (i = 0; i < plane_resources->count_planes; i++) {
> > > > +         if (plane_resources->planes[i] != data->plane_id) {
> > > > +                 wrong_plane_id = plane_resources->planes[i];
> > > > +                 break;
> > > > +         }
> > > > + }
> > > > + drmModeFreePlaneResources(plane_resources);
> > > > + igt_require(wrong_plane_id);
> > >
> > > Maybe we could add an helper to retrieve wrong_plane_id instead of
> > > duplicating the code.
> >
> > Hm, where is it duplicated? The other very similar loop looks for the
> > worng connector, and there's a few other loops to look for the right
> > plane/connector. I think they're all different.
> > >
> > > > +
> > > > + mcl.object_ids = (uint64_t) (uintptr_t) &object_ids[0];
> > > > + mcl.object_count = 0;
> > > > + mcl.flags = 0;
> > > > +
> > > > + object_ids[mcl.object_count++] = data->connector_id;
> > > > + object_ids[mcl.object_count++] = data->crtc_id;
> > > > +
> > > > + drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
> > > > + do_or_die(create_lease(data->master.fd, &mcl));
> > > > + drmSetClientCap(data->master.fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> > > > +
> > > > + /* Set a mode on the leased output */
> > > > + igt_assert_eq(0, prepare_crtc(&data->master, data->connector_id, data->crtc_id));
> > >
> > > Same goes for those create-lease+prepare-CRTC steps.
> >
> > There's a helper for the common case, but for these "wrong" leases there's
> > always subtle differences. A helper would be be about as much code as just
> > open-coding it. Maybe we could move the UNIVERSAL_PLANES as a mode flag
> > into create_lease, but given how often I screamed at my tests for not
> > working because I got that one wrong, I think it's better to be explicit.
> > >
> > > > +
> > > > + /* sanity check */
> > > > + do_or_die(drmModeSetCrtc(data->master.fd, data->crtc_id, -1,
> > > > +                          0, 0, object_ids, 1, mode));
> > > > + do_or_die(drmModeSetCrtc(mcl.fd, data->crtc_id, -1,
> > > > +                          0, 0, object_ids, 1, mode));
> > > > + close(mcl.fd);
> > > > +
> > > > + object_ids[mcl.object_count++] = wrong_plane_id;
> > > > + do_or_die(create_lease(data->master.fd, &mcl));
> > > > +
> > > > + igt_assert_eq(drmModeSetCrtc(mcl.fd, data->crtc_id, -1,
> > > > +                              0, 0, object_ids, 1, mode),
> > > > +               -EACCES);
> > > > + /* make sure we are allowed to turn the CRTC off */
> > > > + do_or_die(drmModeSetCrtc(mcl.fd, data->crtc_id,
> > > > +                          0, 0, 0, NULL, 0, NULL));
> > >
> > > Do we actually test the case where the lessor has setup the CRTC with a
> > > plane that's not accessible by the lesse. I think that's what you were
> > > trying to test here, but I don't see where it's done in the code.
> > > Shouldn't we have something like:
> > >
> > >     do_or_die(drmModeSetCrtc(data->master.fd, data->crtc_id, -1,
> > >                              0, 0, object_ids, 1, mode));
> >
> > Sanity check above does exactly that.
>
> Yes, but wrong_plane_id is not part of the object list in your sanity
> check, is it?
>
> >
> > >     igt_assert_eq(0, drmModeSetCrtc(mcl.fd, data->crtc_id, 0, 0, 0,
> > >                                     NULL, 0, NULL));
> >
> > That's the line right above wrapped in the do_or_die.
>
> Are you sure wrong_plane_id was enabled when you call drmModeSetCrtc()
> to disable the CRTC? If it wasn't, that means you're not really testing
> the 'disable CRTC even if other planes I don't own are still attached to
> it'. But I'm probably missing something.

Ok, I guess what the test does isn't clear:
- a lease allows you to access the objects in your lease, but not any others
- a bunch of legacy ioctl don't just access the crtc that you pass in
explicitly, but also some implicit stuff
- we want to make sure all legacy ioctls also validate for these
implicit objects

For that we need to construct:
- a valid lease, which means it needs to contain at least a plane,
crtc and connector
- but the lease must _not_ contain the primary plane of the crtc we've picked
Hence the dance with wrong_plane_id. That's the only use of
wrong_plane_id, we don't care at all what that plane does nor where it
is attached to, all we need is a plane which isn't the one we care
about, to be able to construct the lease.

Then the actual test verifies:
- You can't enable the crtc (since that also enables the primary plane
and changes it's fb, which isn't in our lease)
- You can still disable the crtc (since that just changes crtc state,
leaving the planes as-is)
- plus a sanity check: we verify both of these work if we use the master fd

Neither of these tests care what wrong_plane_id does, as long as it's
not the primary plane. And becasuse all the access checks must happen
before we touch the hw there's also no need to first enable the crtc
to be able to shut id down. If it's off already the ioctl is a no-op,
but still succeeds/fails in the access checks.
-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] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/9] tests/kms_addfb_basic: Check that only the owner can rmfb

[Daniel Vetter]

On Thu, Feb 28, 2019 at 06:05:54PM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [i-g-t,1/9] tests/kms_addfb_basic: Check that only the owner can rmfb
> URL   : https://patchwork.freedesktop.org/series/57346/
> State : success
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_5674_full -> IGTPW_2536_full
> ====================================================
> 
> Summary
> -------
> 
>   **SUCCESS**
> 
>   No regressions found.
> 
>   External URL: https://patchwork.freedesktop.org/api/1.0/series/57346/revisions/1/mbox/

Remaining patches now merged.
-Daniel

> 
> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in IGTPW_2536_full:
> 
> ### IGT changes ###
> 
> #### Possible regressions ####
> 
>   * {igt@kms_lease@atomic_implicit_crtc} (NEW):
>     - shard-kbl:          NOTRUN -> FAIL +3
> 
>   * {igt@kms_lease@cursor_implicit_plane} (NEW):
>     - shard-hsw:          NOTRUN -> FAIL +3
>     - shard-snb:          NOTRUN -> FAIL +3
> 
>   * {igt@kms_lease@page_flip_implicit_plane} (NEW):
>     - shard-apl:          NOTRUN -> FAIL +3
>     - shard-glk:          NOTRUN -> FAIL +3
> 
>   
> New tests
> ---------
> 
>   New tests have been introduced between CI_DRM_5674_full and IGTPW_2536_full:
> 
> ### New IGT tests (14) ###
> 
>   * igt@kms_addfb_basic@master-rmfb:
>     - Statuses : 5 pass(s)
>     - Exec time: [0.0, 0.00] s
> 
>   * igt@kms_lease@atomic_implicit_crtc:
>     - Statuses : 5 fail(s)
>     - Exec time: [0.01, 0.15] s
> 
>   * igt@kms_lease@cursor_implicit_plane:
>     - Statuses : 5 fail(s)
>     - Exec time: [0.05, 0.27] s
> 
>   * igt@kms_lease@page_flip_implicit_plane:
>     - Statuses : 5 fail(s)
>     - Exec time: [0.12, 0.27] s
> 
>   * igt@kms_lease@setcrtc_implicit_plane:
>     - Statuses : 5 fail(s)
>     - Exec time: [0.17, 0.82] s
> 
>   * igt@kms_prop_blob@basic:
>     - Statuses : 5 pass(s)
>     - Exec time: [0.0] s
> 
>   * igt@kms_prop_blob@blob-multiple:
>     - Statuses : 5 pass(s)
>     - Exec time: [0.0, 0.00] s
> 
>   * igt@kms_prop_blob@blob-prop-core:
>     - Statuses : 5 pass(s)
>     - Exec time: [0.0] s
> 
>   * igt@kms_prop_blob@blob-prop-lifetime:
>     - Statuses : 5 pass(s)
>     - Exec time: [0.0] s
> 
>   * igt@kms_prop_blob@blob-prop-validate:
>     - Statuses : 5 pass(s)
>     - Exec time: [0.0] s
> 
>   * igt@kms_prop_blob@invalid-get-prop:
>     - Statuses : 5 pass(s)
>     - Exec time: [0.0] s
> 
>   * igt@kms_prop_blob@invalid-get-prop-any:
>     - Statuses : 5 pass(s)
>     - Exec time: [0.0] s
> 
>   * igt@kms_prop_blob@invalid-set-prop:
>     - Statuses : 5 pass(s)
>     - Exec time: [0.0] s
> 
>   * igt@kms_prop_blob@invalid-set-prop-any:
>     - Statuses : 5 pass(s)
>     - Exec time: [0.0] s
> 
>   
> 
> Known issues
> ------------
> 
>   Here are the changes found in IGTPW_2536_full that come from known issues:
> 
> ### IGT changes ###
> 
> #### Issues hit ####
> 
>   * igt@gem_userptr_blits@unsync-unmap-cycles:
>     - shard-snb:          PASS -> INCOMPLETE [fdo#105411]
> 
>   * igt@i915_pm_rc6_residency@rc6-accuracy:
>     - shard-kbl:          PASS -> SKIP [fdo#109271]
> 
>   * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
>     - shard-apl:          PASS -> FAIL [fdo#109660]
> 
>   * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
>     - shard-kbl:          PASS -> FAIL [fdo#109660]
> 
>   * igt@kms_busy@basic-flip-d:
>     - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +12
> 
>   * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
>     - shard-kbl:          PASS -> DMESG-WARN [fdo#107956]
> 
>   * igt@kms_cursor_crc@cursor-256x256-onscreen:
>     - shard-kbl:          PASS -> FAIL [fdo#103232] +2
>     - shard-glk:          NOTRUN -> FAIL [fdo#103232]
> 
>   * igt@kms_cursor_crc@cursor-64x21-sliding:
>     - shard-apl:          PASS -> FAIL [fdo#103232] +2
> 
>   * igt@kms_dp_dsc@basic-dsc-enable-edp:
>     - shard-glk:          NOTRUN -> SKIP [fdo#109271] +6
> 
>   * igt@kms_flip@modeset-vs-vblank-race:
>     - shard-apl:          PASS -> FAIL [fdo#103060]
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
>     - shard-glk:          PASS -> FAIL [fdo#103167] +6
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
>     - shard-apl:          PASS -> FAIL [fdo#103167] +1
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
>     - shard-kbl:          PASS -> FAIL [fdo#103167]
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-fullscreen:
>     - shard-apl:          NOTRUN -> SKIP [fdo#109271] +17
> 
>   * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-msflip-blt:
>     - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +13
> 
>   * igt@kms_plane@pixel-format-pipe-b-planes:
>     - shard-kbl:          PASS -> FAIL [fdo#103166] +2
> 
>   * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
>     - shard-apl:          PASS -> FAIL [fdo#103166] +3
> 
>   * igt@kms_rotation_crc@multiplane-rotation:
>     - shard-glk:          PASS -> DMESG-FAIL [fdo#105763] / [fdo#106538]
> 
>   * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
>     - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]
> 
>   * igt@kms_universal_plane@cursor-fb-leak-pipe-d:
>     - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
> 
>   * igt@kms_universal_plane@universal-plane-pipe-c-functional:
>     - shard-glk:          PASS -> FAIL [fdo#103166] +3
> 
>   * igt@kms_vblank@pipe-a-ts-continuation-suspend:
>     - shard-apl:          PASS -> FAIL [fdo#104894] +2
> 
>   * igt@perf_pmu@busy-check-all-vecs0:
>     - shard-snb:          NOTRUN -> SKIP [fdo#109271] +115
> 
>   
> #### Possible fixes ####
> 
>   * igt@gem_eio@in-flight-suspend:
>     - shard-snb:          FAIL [fdo#103375] -> PASS
> 
>   * igt@kms_busy@extended-modeset-hang-newfb-render-b:
>     - shard-hsw:          DMESG-WARN [fdo#107956] -> PASS +1
> 
>   * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
>     - shard-kbl:          DMESG-WARN [fdo#107956] -> PASS +2
>     - shard-snb:          DMESG-WARN [fdo#107956] -> PASS +1
> 
>   * igt@kms_chv_cursor_fail@pipe-c-128x128-top-edge:
>     - shard-hsw:          DMESG-WARN [fdo#102614] -> PASS +1
> 
>   * igt@kms_color@pipe-b-legacy-gamma:
>     - shard-apl:          FAIL [fdo#104782] -> PASS
> 
>   * igt@kms_cursor_crc@cursor-128x128-offscreen:
>     - shard-apl:          INCOMPLETE [fdo#103927] -> PASS
> 
>   * igt@kms_cursor_crc@cursor-64x21-random:
>     - shard-apl:          FAIL [fdo#103232] -> PASS +3
>     - shard-kbl:          FAIL [fdo#103232] -> PASS
> 
>   * igt@kms_cursor_crc@cursor-alpha-opaque:
>     - shard-apl:          FAIL [fdo#109350] -> PASS
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
>     - shard-kbl:          FAIL [fdo#103167] -> PASS
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
>     - shard-apl:          FAIL [fdo#103167] -> PASS +2
> 
>   * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
>     - shard-glk:          FAIL [fdo#103167] -> PASS +5
> 
>   * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
>     - shard-glk:          FAIL [fdo#108948] -> PASS
>     - shard-apl:          FAIL [fdo#108948] -> PASS
> 
>   * igt@kms_plane@plane-position-covered-pipe-c-planes:
>     - shard-apl:          FAIL [fdo#103166] -> PASS +1
> 
>   * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
>     - shard-glk:          FAIL [fdo#103166] -> PASS +1
> 
>   * igt@kms_rotation_crc@multiplane-rotation:
>     - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS
> 
>   * igt@kms_setmode@basic:
>     - shard-hsw:          FAIL [fdo#99912] -> PASS
> 
>   * igt@kms_vblank@pipe-b-ts-continuation-suspend:
>     - shard-snb:          INCOMPLETE [fdo#105411] -> PASS
> 
>   
>   {name}: This element is suppressed. This means it is ignored when computing
>           the status of the difference (SUCCESS, WARNING, or FAILURE).
> 
>   [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
>   [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
>   [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
>   [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
>   [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
>   [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
>   [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
>   [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
>   [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
>   [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
>   [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
>   [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
>   [fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
>   [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
>   [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
>   [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
>   [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
>   [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
>   [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660
>   [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
> 
> 
> Participating hosts (6 -> 5)
> ------------------------------
> 
>   Missing    (1): shard-skl 
> 
> 
> Build changes
> -------------
> 
>     * IGT: IGT_4863 -> IGTPW_2536
>     * Piglit: piglit_4509 -> None
> 
>   CI_DRM_5674: 71bb3bfb61fb58f93f8b09e6ad576a403cd7752c @ git://anongit.freedesktop.org/gfx-ci/linux
>   IGTPW_2536: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2536/
>   IGT_4863: 0f0db14e7f4ec41251ca156d7cb5b8d531a38006 @ 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_2536/

-- 
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