Add atomic DRM cursor hotspot test

PATCH Superseded Albert Esteve 3 participants 9 messages Aug 1, 2023 – Aug 2, 2023 Library code

lore

Share & tools

lore

Albert Esteve

author

2023-08-01 08:49 UTC #0 lore raw

v3 cover Add atomic DRM cursor hotspot test

Add test to verify the kernel patch at [1].

The test modifies the hotspot x and y properties and checks that they get properly updated after every display commit.

Note: the patch adds a DRM capability definition taken directly from the kernel patch. Once the definition lands on IGT after the drm.h is updated, the local definition shall be removed.

[1] - https://lists.freedesktop.org/archives/dri-devel/2023-July/414509.html

Albert Esteve (2):
igt_kms: add hotspot plane property
kms_cursor_legacy: modeset-atomic-cursor-hotspot

 lib/igt_kms.c             | 10 +++++
 lib/igt_kms.h             |  9 +++++
 tests/kms_cursor_legacy.c | 79 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+)

Signature · 1 line

2.40.0

Albert Esteve

author

2023-08-01 08:49 UTC #1 lore raw

PATCH v3 1/2 igt_kms: add hotspot plane property

Introduce LOCAL_DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT capability definition until patched drm.h uapi header is upstreamed and lands on IGT.

CURSOR_PLANE_HOTSPOT capability is enabled conditionally, and then tracked in the igt_display struct.

Add HOTSPOT_X and HOTSPOT_Y properties
to the atomic_plane_properties struct.

Signed-off-by Albert Esteve <[email protected]>

Acked-by Javier Martinez Canillas <[email protected]>

Acked-by Bhanuprakash Modem <[email protected]>

Reviewed-by Zbigniew Kempczyński <[email protected]>

---
 lib/igt_kms.c | 10 ++++++++++
 lib/igt_kms.h |  9 +++++++++
 2 files changed, 19 insertions(+)

lib/igt_kms.c +10 -0

index f2b0eed57..e0959ccff 100644
@@ -601,6 +601,8 @@ const char * const igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
 	[IGT_PLANE_CRTC_Y] = "CRTC_Y",
 	[IGT_PLANE_CRTC_W] = "CRTC_W",
 	[IGT_PLANE_CRTC_H] = "CRTC_H",
+	[IGT_PLANE_HOTSPOT_X] = "HOTSPOT_X",
+	[IGT_PLANE_HOTSPOT_Y] = "HOTSPOT_Y",
 	[IGT_PLANE_FB_ID] = "FB_ID",
 	[IGT_PLANE_CRTC_ID] = "CRTC_ID",
 	[IGT_PLANE_IN_FENCE_FD] = "IN_FENCE_FD",

@@ -2296,6 +2298,11 @@ static void igt_plane_reset(igt_plane_t *plane)
 	if (igt_plane_has_prop(plane, IGT_PLANE_SCALING_FILTER))
 		igt_plane_set_prop_enum(plane, IGT_PLANE_SCALING_FILTER, "Default");
 
+	if (igt_plane_has_prop(plane, IGT_PLANE_HOTSPOT_X))
+		igt_plane_set_prop_value(plane, IGT_PLANE_HOTSPOT_X, 0);
+	if (igt_plane_has_prop(plane, IGT_PLANE_HOTSPOT_Y))
+		igt_plane_set_prop_value(plane, IGT_PLANE_HOTSPOT_Y, 0);
+
 	igt_plane_clear_prop_changed(plane, IGT_PLANE_IN_FENCE_FD);
 	plane->values[IGT_PLANE_IN_FENCE_FD] = ~0ULL;
 	plane->gem_handle = 0;

@@ -2680,6 +2687,9 @@ void igt_display_require(igt_display_t *display, int drm_fd)
 	if (drmSetClientCap(drm_fd, DRM_CLIENT_CAP_ATOMIC, 1) == 0)
 		display->is_atomic = 1;
 
+	if (drmSetClientCap(drm_fd, LOCAL_DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT, 1) == 0)
+		display->has_virt_cursor_plane = 1;
+
 	plane_resources = drmModeGetPlaneResources(display->drm_fd);
 	igt_assert(plane_resources);

lib/igt_kms.h +9 -0

index 1b6988c17..91355c910 100644
@@ -41,6 +41,12 @@
 
 /* Low-level helpers with kmstest_ prefix */
 
+/**
+ * Clients which do set cursor hotspot and treat the cursor plane
+ * like a mouse cursor should set this property.
+ */
+#define LOCAL_DRM_CLIENT_CAP_CURSOR_PLANE_HOTSPOT	6
+
 /**
  * pipe:
  * @PIPE_NONE: Invalid pipe, used for disconnecting a output from a pipe.

@@ -318,6 +324,8 @@ enum igt_atomic_plane_properties {
        IGT_PLANE_ZPOS,
        IGT_PLANE_FB_DAMAGE_CLIPS,
        IGT_PLANE_SCALING_FILTER,
+       IGT_PLANE_HOTSPOT_X,
+       IGT_PLANE_HOTSPOT_Y,
        IGT_NUM_PLANE_PROPS
 };

@@ -448,6 +456,7 @@ struct igt_display {
 	igt_pipe_t *pipes;
 	bool has_cursor_plane;
 	bool is_atomic;
+	bool has_virt_cursor_plane;
 	bool first_commit;
 
 	uint64_t *modifiers;

Signature · 1 line

2.40.0

Albert Esteve

author

2023-08-01 08:49 UTC #2 lore raw

PATCH v3 2/2 kms_cursor_legacy: modeset-atomic-cursor-hotspot

In reply to Albert Esteve

Add a test for modesetting an atomic cursor plane hotspot property. Test added to verify the kernel patch at [1]. The test first checks if the plane is atomic and has the hotspot property. and if it does, it sets different random hot_x and hot_y values and checks that it is updated correctly after an atomic commit.

[1] https://lists.freedesktop.org/archives/dri-devel/2023-July/414509.html

Signed-off-by Albert Esteve <[email protected]>

Acked-by Javier Martinez Canillas <[email protected]>

Acked-by Zbigniew Kempczyński <[email protected]>

---
 tests/kms_cursor_legacy.c | 79 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

tests/kms_cursor_legacy.c +79 -0

index 1ebac9d31..87cfb9e64 100644
@@ -218,6 +218,12 @@ static	igt_plane_t
 	return cursor;
 }
 
+static void set_cursor_hotspot(igt_plane_t *cursor, int  hot_x, int hot_y)
+{
+	igt_output_set_prop_value(cursor, IGT_PLANE_HOTSPOT_X, hot_x);
+	igt_output_set_prop_value(cursor, IGT_PLANE_HOTSPOT_Y, hot_y);
+}
+
 static void populate_cursor_args(igt_display_t *display, enum pipe pipe,
 				 struct drm_mode_cursor *arg, struct igt_fb *fb)
 {

@@ -1607,6 +1613,68 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
 	put_ahnd(ahnd);
 }
 
+static void modeset_atomic_cursor_hotspot(igt_display_t *display)
+{
+	struct igt_fb cursor_fb;
+	igt_output_t *output;
+	enum pipe pipe;
+	igt_plane_t *cursor = NULL;
+	bool has_hotspot_prop;
+	uint64_t cursor_width, cursor_height;
+	uint32_t hot_x, hot_y, init_hot_x, init_hot_y;
+
+	igt_require(display->is_atomic);
+	igt_require(display->has_virt_cursor_plane);
+	pipe = find_connected_pipe(display, false, &output);
+	igt_require(output);
+
+	igt_info("Using pipe %s & %s\n",
+		 kmstest_pipe_name(pipe), igt_output_name(output));
+
+	cursor_width = cursor_height = 64;
+	igt_create_color_fb(display->drm_fd, cursor_width, cursor_height, DRM_FORMAT_ARGB8888,
+			    DRM_FORMAT_MOD_LINEAR, 1., 1., 1., &cursor_fb);
+
+	igt_display_commit2(display, COMMIT_ATOMIC);
+
+	cursor = set_cursor_on_pipe(display, pipe, &cursor_fb);
+
+	has_hotspot_prop = cursor->props[IGT_PLANE_HOTSPOT_X] ||
+		cursor->props[IGT_PLANE_HOTSPOT_Y];
+	igt_require_f(has_hotspot_prop, "Cursor plane lacks the hotspot property");
+
+	init_hot_x = igt_plane_get_prop(cursor, IGT_PLANE_HOTSPOT_X);
+	init_hot_y = igt_plane_get_prop(cursor, IGT_PLANE_HOTSPOT_Y);
+
+	/*
+	 * Change the hotspot coordinates randomly and check that the property
+	 * is updated accordingly.
+	 */
+	srand(time(NULL));
+	for (int i = 0; i < 20; i++) {
+		hot_x = rand() % cursor_width;
+		hot_y = rand() % cursor_height;
+		igt_debug("Update cursor hotspot: (%d, %d)\n", hot_x, hot_y);
+
+		/* Set cursor hotspot property values */
+		set_cursor_hotspot(cursor, hot_x, hot_y);
+
+		igt_display_commit2(display, COMMIT_ATOMIC);
+
+		/* After the commit, the cursor hotspot property values are updated */
+		igt_assert_eq(igt_plane_get_prop(cursor, IGT_PLANE_HOTSPOT_X), hot_x);
+		igt_assert_eq(igt_plane_get_prop(cursor, IGT_PLANE_HOTSPOT_Y), hot_y);
+	}
+
+	/* Clean-up */
+	set_cursor_hotspot(cursor, init_hot_x, init_hot_y);
+	igt_plane_set_fb(cursor, NULL);
+	igt_output_set_pipe(output, PIPE_NONE);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+
+	igt_remove_fb(display->drm_fd, &cursor_fb);
+}
+
 igt_main
 {
 	const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);

@@ -1681,6 +1749,17 @@ igt_main
 			nonblocking_modeset_vs_cursor(&display, 16);
 	}
 
+	igt_describe("Test changes the cursor hotspot and checks that the "
+		      "property is updated accordignly");
+	igt_subtest_group {
+		igt_fixture
+			igt_display_require_output(&display);
+
+		igt_subtest("modeset-atomic-cursor-hotspot") {
+			modeset_atomic_cursor_hotspot(&display);
+		}
+	}
+
 	igt_describe("This test executes flips on both CRTCs "
 		     "while running cursor updates in parallel");
 	igt_subtest_group {

Signature · 1 line

2.40.0

Modem, Bhanuprakash

Reviewed-by on v3 2/2

2023-08-02 08:11 UTC #3 lore raw

Re: [igt-dev] [PATCH v3 2/2] kms_cursor_legacy: modeset-atomic-cursor-hotspot

Quoted 10 lines · Albert Esteve

Add a test for modesetting an atomic cursor plane hotspot property. Test added to verify the kernel patch at [1]. The test first checks if the plane is atomic and has the hotspot property. and if it does, it sets different random hot_x and hot_y values and checks that it is updated correctly after an atomic commit.
[1] https://lists.freedesktop.org/archives/dri-devel/2023-July/414509.html

Please keep maintain the history of the patch.

Example:

V2: - Changes on top of Initial version
V3: - Changes between V2 & Current version

You can update this info while merging the patch. With above comments addressed, this patch is

Reviewed-by Bhanuprakash Modem <[email protected]>

Quoted 111 lines

Signed-off-by Albert Esteve <[email protected]>

Acked-by Javier Martinez Canillas <[email protected]>

Acked-by Zbigniew Kempczyński <[email protected]>

---
  tests/kms_cursor_legacy.c | 79 +++++++++++++++++++++++++++++++++++++++
  1 file changed, 79 insertions(+)

tests/kms_cursor_legacy.c +79 -0

index 1ebac9d31..87cfb9e64 100644
@@ -218,6 +218,12 @@ static	igt_plane_t
  	return cursor;
  }
 
+static void set_cursor_hotspot(igt_plane_t *cursor, int  hot_x, int hot_y)
+{
+	igt_output_set_prop_value(cursor, IGT_PLANE_HOTSPOT_X, hot_x);
+	igt_output_set_prop_value(cursor, IGT_PLANE_HOTSPOT_Y, hot_y);
+}
+
  static void populate_cursor_args(igt_display_t *display, enum pipe pipe,
  				 struct drm_mode_cursor *arg, struct igt_fb *fb)
  {

@@ -1607,6 +1613,68 @@ static void flip_vs_cursor_busy_crc(igt_display_t *display, bool atomic)
  	put_ahnd(ahnd);
  }
 
+static void modeset_atomic_cursor_hotspot(igt_display_t *display)
+{
+	struct igt_fb cursor_fb;
+	igt_output_t *output;
+	enum pipe pipe;
+	igt_plane_t *cursor = NULL;
+	bool has_hotspot_prop;
+	uint64_t cursor_width, cursor_height;
+	uint32_t hot_x, hot_y, init_hot_x, init_hot_y;
+
+	igt_require(display->is_atomic);
+	igt_require(display->has_virt_cursor_plane);
+	pipe = find_connected_pipe(display, false, &output);
+	igt_require(output);
+
+	igt_info("Using pipe %s & %s\n",
+		 kmstest_pipe_name(pipe), igt_output_name(output));
+
+	cursor_width = cursor_height = 64;
+	igt_create_color_fb(display->drm_fd, cursor_width, cursor_height, DRM_FORMAT_ARGB8888,
+			    DRM_FORMAT_MOD_LINEAR, 1., 1., 1., &cursor_fb);
+
+	igt_display_commit2(display, COMMIT_ATOMIC);
+
+	cursor = set_cursor_on_pipe(display, pipe, &cursor_fb);
+
+	has_hotspot_prop = cursor->props[IGT_PLANE_HOTSPOT_X] ||
+		cursor->props[IGT_PLANE_HOTSPOT_Y];
+	igt_require_f(has_hotspot_prop, "Cursor plane lacks the hotspot property");
+
+	init_hot_x = igt_plane_get_prop(cursor, IGT_PLANE_HOTSPOT_X);
+	init_hot_y = igt_plane_get_prop(cursor, IGT_PLANE_HOTSPOT_Y);
+
+	/*
+	 * Change the hotspot coordinates randomly and check that the property
+	 * is updated accordingly.
+	 */
+	srand(time(NULL));
+	for (int i = 0; i < 20; i++) {
+		hot_x = rand() % cursor_width;
+		hot_y = rand() % cursor_height;
+		igt_debug("Update cursor hotspot: (%d, %d)\n", hot_x, hot_y);
+
+		/* Set cursor hotspot property values */
+		set_cursor_hotspot(cursor, hot_x, hot_y);
+
+		igt_display_commit2(display, COMMIT_ATOMIC);
+
+		/* After the commit, the cursor hotspot property values are updated */
+		igt_assert_eq(igt_plane_get_prop(cursor, IGT_PLANE_HOTSPOT_X), hot_x);
+		igt_assert_eq(igt_plane_get_prop(cursor, IGT_PLANE_HOTSPOT_Y), hot_y);
+	}
+
+	/* Clean-up */
+	set_cursor_hotspot(cursor, init_hot_x, init_hot_y);
+	igt_plane_set_fb(cursor, NULL);
+	igt_output_set_pipe(output, PIPE_NONE);
+	igt_display_commit2(display, COMMIT_ATOMIC);
+
+	igt_remove_fb(display->drm_fd, &cursor_fb);
+}
+
  igt_main
  {
  	const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);

@@ -1681,6 +1749,17 @@ igt_main
  			nonblocking_modeset_vs_cursor(&display, 16);
  	}
 
+	igt_describe("Test changes the cursor hotspot and checks that the "
+		      "property is updated accordignly");
+	igt_subtest_group {
+		igt_fixture
+			igt_display_require_output(&display);
+
+		igt_subtest("modeset-atomic-cursor-hotspot") {
+			modeset_atomic_cursor_hotspot(&display);
+		}
+	}
+
  	igt_describe("This test executes flips on both CRTCs "
  		     "while running cursor updates in parallel");
  	igt_subtest_group {

Albert Esteve

author

2023-08-02 09:21 UTC #4 lore raw

Re: [igt-dev] [PATCH v3 2/2] kms_cursor_legacy: modeset-atomic-cursor-hotspot

Quoted 19 lines · Bhanuprakash

Quoted 9 lines · Albert Esteve
Add a test for modesetting an atomic cursor plane hotspot property. Test added to verify the kernel patch at [1]. The test first checks if the plane is atomic and has the hotspot property. and if it does, it sets different random hot_x and hot_y values and checks that it is updated correctly after an atomic commit.
[1]
https://lists.freedesktop.org/archives/dri-devel/2023-July/414509.html
Quoted 1 line
Please keep maintain the history of the patch.
Example:

Quoted 8 lines

V2: - Changes on top of Initial version
V3: - Changes between V2 & Current version
You can update this info while merging the patch. With above comments addressed, this patch is
Reviewed-by Bhanuprakash Modem <[email protected]>

Thanks.

I should've tracked the history in the cover letters, my bad.

Do you mean to track it in the commit message?
Also, what do you mean I can update it while merging? I do not have permissions to merge. Should I push a new revision?

BR,
Albert.

Quoted 125 lines

Quoted 13 lines

Signed-off-by Albert Esteve <[email protected]>
Acked-by Javier Martinez Canillas <[email protected]>
Acked-by Zbigniew Kempczyński <[email protected]>
---
  tests/kms_cursor_legacy.c | 79 +++++++++++++++++++++++++++++++++++++++
  1 file changed, 79 insertions(+)
tests/kms_cursor_legacy.c +0 -0
index 1ebac9d31..87cfb9e64 100644

tests/kms_cursor_legacy.c static igt_plane_t · line 221

Quoted 4 lines

tests/kms_cursor_legacy.c +1 -0

@@ -218,6 +218,12 @@ static   igt_plane_t
      return cursor;
  }
 
+static void set_cursor_hotspot(igt_plane_t *cursor, int  hot_x, int

hot_y)

tests/kms_cursor_legacy.c static igt_plane_t

Quoted 6 lines

tests/kms_cursor_legacy.c +5 -0

+{
+     igt_output_set_prop_value(cursor, IGT_PLANE_HOTSPOT_X, hot_x);
+     igt_output_set_prop_value(cursor, IGT_PLANE_HOTSPOT_Y, hot_y);
+}
+
  static void populate_cursor_args(igt_display_t *display, enum pipe

pipe,

tests/kms_cursor_legacy.c static igt_plane_t · line 228

Quoted 1 line

tests/kms_cursor_legacy.c +0 -0

                               struct drm_mode_cursor *arg, struct

igt_fb *fb)

tests/kms_cursor_legacy.c static void flip_vs_cursor_busy_crc(igt_display_t

Quoted 2 lines

tests/kms_cursor_legacy.c +0 -0

@@ -1607,6 +1613,68 @@ static void flip_vs_cursor_busy_crc(igt_display_t

*display, bool atomic)

Quoted 13 lines

tests/kms_cursor_legacy.c +10 -0

      put_ahnd(ahnd);
  }
 
+static void modeset_atomic_cursor_hotspot(igt_display_t *display)
+{
+     struct igt_fb cursor_fb;
+     igt_output_t *output;
+     enum pipe pipe;
+     igt_plane_t *cursor = NULL;
+     bool has_hotspot_prop;
+     uint64_t cursor_width, cursor_height;
+     uint32_t hot_x, hot_y, init_hot_x, init_hot_y;
+