intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [RFC][PATCH v2 0/3] drm: Add plane SIZE_HINTS property
@ 2023-03-21 14:36 Ville Syrjala
  2023-03-21 14:36 ` [Intel-gfx] [RFC][PATCH v2 1/3] drm: Introduce " Ville Syrjala
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Ville Syrjala @ 2023-03-21 14:36 UTC (permalink / raw)
  To: dri-devel
  Cc: Simon Ser, intel-gfx, Pekka Paalanen, Jonas Ådahl, Daniel Stone

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I was pondering how I'd be able to do non-square cursor
sizes without have a massive list of them in the SIZE_HINTS
blob.

So I came up with this idea of having a 2D bitmap in
there to indicate support for (mostly) POT non-square
sizes..

What does everyone think? Is this just getting too
complicated and should we just go with the original
"a list of suppored sizes" approach?

Cc: Simon Ser <contact@emersion.fr>
Cc: Jonas Ådahl <jadahl@redhat.com>
Cc: Daniel Stone <daniel@fooishbar.org>
Cc: Pekka Paalanen <pekka.paalanen@collabora.com>

Ville Syrjälä (3):
  drm: Introduce plane SIZE_HINTS property
  drm/i915: Adjust cursor_size_ok() func calling convention
  drm/i915: Add SIZE_HINTS property for cursors

 drivers/gpu/drm/drm_mode_config.c           |  7 ++
 drivers/gpu/drm/drm_plane.c                 | 96 +++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_cursor.c | 96 ++++++++++++++-------
 include/drm/drm_mode_config.h               |  5 ++
 include/drm/drm_plane.h                     |  6 ++
 include/uapi/drm/drm_mode.h                 | 29 +++++++
 6 files changed, 208 insertions(+), 31 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Intel-gfx] [RFC][PATCH v2 1/3] drm: Introduce plane SIZE_HINTS property
  2023-03-21 14:36 [Intel-gfx] [RFC][PATCH v2 0/3] drm: Add plane SIZE_HINTS property Ville Syrjala
@ 2023-03-21 14:36 ` Ville Syrjala
  2023-03-30 10:29   ` Pekka Paalanen
  2023-03-21 14:36 ` [Intel-gfx] [RFC][PATCH v2 2/3] drm/i915: Adjust cursor_size_ok() func calling convention Ville Syrjala
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjala @ 2023-03-21 14:36 UTC (permalink / raw)
  To: dri-devel
  Cc: Simon Ser, intel-gfx, Pekka Paalanen, Jonas Ådahl, Daniel Stone

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add a new immutable plane property by which a plane can advertise
a handful of recommended plane sizes. This would be mostly exposed
by cursor planes as a slightly more capable replacement for
the DRM_CAP_CURSOR_WIDTH/HEIGHT caps, which can only declare
a one size fits all limit for the whole device.

Currently eg. amdgpu/i915/nouveau just advertize the max cursor
size via the cursor size caps. But always using the max sized
cursor can waste a surprising amount of power, so a better
stragey is desirable.

Most other drivers don't specify any cursor size at all, in
which case the ioctl code just claims that 64x64 is a great
choice. Whether that is actually true is debatable.

A poll of various compositor developers informs us that
blindly probing with setcursor/atomic ioctl to determine
suitable cursor sizes is not acceptable, thus the
introduction of the new property to supplant the cursor
size caps. The compositor will now be free to select a
more optimal cursor size from the short list of options.

The blob contains explicit min and max plane sizes, as
well as a 2D bitmap representing all the square and non-square
power of two sizes between the min and max.

Note that the reported sizes (either via the property or the
caps) make no claims about things such as plane scaling. So
these things should only really be consulted for simple
"cursor like" use cases.

v2: Try to add some docs
v3: Specify that value 0 is reserved for future use (basic idea from Jonas)
    Drop the note about typical hardware (Pekka)
v4: Total redesign to include the 2D bitmap

Cc: Simon Ser <contact@emersion.fr>
Cc: Jonas Ådahl <jadahl@redhat.com>
Cc: Daniel Stone <daniel@fooishbar.org>
Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_mode_config.c |  7 +++
 drivers/gpu/drm/drm_plane.c       | 96 +++++++++++++++++++++++++++++++
 include/drm/drm_mode_config.h     |  5 ++
 include/drm/drm_plane.h           |  6 ++
 include/uapi/drm/drm_mode.h       | 29 ++++++++++
 5 files changed, 143 insertions(+)

diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index 87eb591fe9b5..21860f94a18c 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -374,6 +374,13 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.modifiers_property = prop;
 
+	prop = drm_property_create(dev,
+				   DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB,
+				   "SIZE_HINTS", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.size_hints_property = prop;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 24e7998d1731..fb9cee504767 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -140,6 +140,27 @@
  *     DRM_FORMAT_MOD_LINEAR. Before linux kernel release v5.1 there have been
  *     various bugs in this area with inconsistencies between the capability
  *     flag and per-plane properties.
+ *
+ * SIZE_HINTS:
+ *     Blob property which contains the set of recommended plane size
+ *     which can used for simple "cursor like" use cases (eg. no scaling).
+ *     Using these hints frees userspace from extensive probing of
+ *     supported plane sizes through atomic/setcursor ioctls.
+ *
+ *     For optimal usage userspace should pick the smallest size
+ *     that satisfies its own requirements.
+ *
+ *     The blob contains explicit min/max sizes, as well as a 2D bitmap
+ *     representign all square and non-square POT sizes between.
+ *
+ *     Drivers should only attach this property to planes that
+ *     support a very limited set of sizes.
+ *
+ *     Note that property value 0 (ie. no blob) is reserved for potential
+ *     future use. Current userspace is expected to ignore the property
+ *     if the value is 0, and fall back to some other means (eg.
+ *     &DRM_CAP_CURSOR_WIDTH and &DRM_CAP_CURSOR_HEIGHT) to determine
+ *     the appropriate plane size to use.
  */
 
 static unsigned int drm_num_planes(struct drm_device *dev)
@@ -1582,3 +1603,78 @@ int drm_plane_create_scaling_filter_property(struct drm_plane *plane,
 	return 0;
 }
 EXPORT_SYMBOL(drm_plane_create_scaling_filter_property);
+
+static int hint_dim(int x, int x0, int w, int min, int max)
+{
+	if (x == 0)
+		return min;
+	else if (x == w - 1)
+		return max;
+	else
+		return 1 << (x0 + x);
+}
+
+/**
+ * drm_plane_add_size_hint_property - create a size hint property
+ *
+ * @plane: drm plane
+ * @min_width: minimum width
+ * @min_height: minimum height
+ * @max_width: maximum width
+ * @max_height: maximum height
+ * @cursor_size_ok: function to check if specified size is ok
+ *
+ * Create a size hints property for the plane.
+ *
+ * RETURNS:
+ * Zero for success or -errno
+ */
+int drm_plane_add_size_hints_property(struct drm_plane *plane,
+				      int min_width, int min_height,
+				      int max_width, int max_height,
+				      bool (*size_ok)(struct drm_plane *plane,
+						      int width, int height))
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_property_blob *blob;
+	struct drm_plane_size_hint *hints;
+	int x, y, x0, y0, w, h;
+
+	x0 = fls(min_width) - 1;
+	y0 = fls(min_height) - 1;
+	w = fls(2 * max_width - 1) - x0;
+	h = fls(2 * max_height - 1) - y0;
+
+	blob = drm_property_create_blob(dev,
+					struct_size(hints, bitmap,
+						    DIV_ROUND_UP(w * h, 32)),
+					NULL);
+	if (IS_ERR(blob))
+		return PTR_ERR(blob);
+
+	hints = blob->data;
+
+	hints->min_width = min_width;
+	hints->min_height = min_height;
+	hints->max_width = max_width;
+	hints->max_height = max_height;
+
+	for (y = 0; y < h; y++) {
+		int height = hint_dim(y, y0, h, min_height, max_height);
+
+		for (x = 0; x < w; x++) {
+			int width = hint_dim(x, x0, w, min_width, max_width);
+			unsigned int bit = y * w + x;
+
+			if (size_ok(plane, width, height))
+				hints->bitmap[bit/32] |= 1 << (bit & 31);
+		}
+	}
+
+	drm_object_attach_property(&plane->base, config->size_hints_property,
+				   blob->base.id);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_plane_add_size_hints_property);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index e5b053001d22..d5495c0804c5 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -949,6 +949,11 @@ struct drm_mode_config {
 	 */
 	struct drm_property *modifiers_property;
 
+	/**
+	 * @size_hints_propertty: Plane size hints property.
+	 */
+	struct drm_property *size_hints_property;
+
 	/* cursor size */
 	uint32_t cursor_width, cursor_height;
 
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 51291983ea44..6d7326f15761 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -32,6 +32,7 @@
 #include <drm/drm_util.h>
 
 struct drm_crtc;
+struct drm_plane_size_hint;
 struct drm_printer;
 struct drm_modeset_acquire_ctx;
 
@@ -945,5 +946,10 @@ drm_plane_get_damage_clips(const struct drm_plane_state *state);
 
 int drm_plane_create_scaling_filter_property(struct drm_plane *plane,
 					     unsigned int supported_filters);
+int drm_plane_add_size_hints_property(struct drm_plane *plane,
+				      int min_width, int min_height,
+				      int max_width, int max_height,
+				      bool (*size_ok)(struct drm_plane *plane,
+						      int width, int height));
 
 #endif
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 46becedf5b2f..5a1de6bd80dd 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -849,6 +849,35 @@ struct drm_color_lut {
 	__u16 reserved;
 };
 
+/**
+ * struct drm_plane_size_hint - Plane size hints
+ *
+ * Includes a 2D bitmap in row major order of one bit per plane
+ * size (WxH). The colums represent the possible plane widths as
+ * <min>,<power of two values between min-max>,<max>
+ * the rows do the same for plane heights. The minimums and
+ * maximums need not be power of two themselves. The bits
+ * are walked in lsb->msb order.
+ *
+ * Example:
+ * min_width = 33, min_height = 16
+ * max_width = 128, max_height = 127
+ * bitmap[0] = 0b100111011001
+ *     33 64 128
+ * 16   *
+ * 32   *  *
+ * 64   *  *   *
+ * 127         *
+ *
+ * Indicating support for 33x16,33xx32,
+ * 64x32,64x64,128x64,128x127 sizes.
+ */
+struct drm_plane_size_hint {
+	__u16 min_width, min_height;
+	__u16 max_width, max_height;
+	__u32 bitmap[];
+};
+
 /**
  * struct hdr_metadata_infoframe - HDR Metadata Infoframe Data.
  *
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Intel-gfx] [RFC][PATCH v2 2/3] drm/i915: Adjust cursor_size_ok() func calling convention
  2023-03-21 14:36 [Intel-gfx] [RFC][PATCH v2 0/3] drm: Add plane SIZE_HINTS property Ville Syrjala
  2023-03-21 14:36 ` [Intel-gfx] [RFC][PATCH v2 1/3] drm: Introduce " Ville Syrjala
@ 2023-03-21 14:36 ` Ville Syrjala
  2023-03-21 14:36 ` [Intel-gfx] [RFC][PATCH v2 3/3] drm/i915: Add SIZE_HINTS property for cursors Ville Syrjala
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjala @ 2023-03-21 14:36 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Tweak the parameters we pass to the cursor size_ok() functions
in preparation for using them to populate the SIZE_HINT property.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_cursor.c | 63 +++++++++++----------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index 31bef0427377..edeeb5f9f795 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -65,12 +65,10 @@ static u32 intel_cursor_position(const struct intel_plane_state *plane_state)
 	return pos;
 }
 
-static bool intel_cursor_size_ok(const struct intel_plane_state *plane_state)
+static bool intel_cursor_size_ok(struct drm_i915_private *i915,
+				 int width, int height)
 {
-	const struct drm_mode_config *config =
-		&plane_state->uapi.plane->dev->mode_config;
-	int width = drm_rect_width(&plane_state->uapi.dst);
-	int height = drm_rect_height(&plane_state->uapi.dst);
+	const struct drm_mode_config *config = &i915->drm.mode_config;
 
 	return width > 0 && width <= config->cursor_width &&
 		height > 0 && height <= config->cursor_height;
@@ -198,23 +196,25 @@ static u32 i845_cursor_ctl(const struct intel_crtc_state *crtc_state,
 		CURSOR_STRIDE(plane_state->view.color_plane[0].mapping_stride);
 }
 
-static bool i845_cursor_size_ok(const struct intel_plane_state *plane_state)
+static bool i845_cursor_size_ok(struct drm_plane *plane,
+				int width, int height)
 {
-	int width = drm_rect_width(&plane_state->uapi.dst);
+	struct drm_i915_private *i915 = to_i915(plane->dev);
 
 	/*
 	 * 845g/865g are only limited by the width of their cursors,
 	 * the height is arbitrary up to the precision of the register.
 	 */
-	return intel_cursor_size_ok(plane_state) && IS_ALIGNED(width, 64);
+	return intel_cursor_size_ok(i915, width, height) && IS_ALIGNED(width, 64);
 }
 
 static int i845_check_cursor(struct intel_crtc_state *crtc_state,
 			     struct intel_plane_state *plane_state)
 {
+	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
+	struct drm_i915_private *i915 = to_i915(plane->base.dev);
 	const struct drm_framebuffer *fb = plane_state->hw.fb;
-	struct drm_i915_private *i915 = to_i915(plane_state->uapi.plane->dev);
-	int ret;
+	int ret, width, height;
 
 	ret = intel_check_cursor(crtc_state, plane_state);
 	if (ret)
@@ -224,12 +224,14 @@ static int i845_check_cursor(struct intel_crtc_state *crtc_state,
 	if (!fb)
 		return 0;
 
+	width = drm_rect_width(&plane_state->uapi.dst);
+	height = drm_rect_height(&plane_state->uapi.dst);
+
 	/* Check for which cursor types we support */
-	if (!i845_cursor_size_ok(plane_state)) {
+	if (!i845_cursor_size_ok(&plane->base, width, height)) {
 		drm_dbg_kms(&i915->drm,
 			    "Cursor dimension %dx%d not supported\n",
-			    drm_rect_width(&plane_state->uapi.dst),
-			    drm_rect_height(&plane_state->uapi.dst));
+			    width, height);
 		return -EINVAL;
 	}
 
@@ -386,14 +388,13 @@ static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
 	return cntl;
 }
 
-static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
+static bool i9xx_cursor_size_ok(struct drm_plane *plane,
+				int width, int height,
+				unsigned int rotation)
 {
-	struct drm_i915_private *dev_priv =
-		to_i915(plane_state->uapi.plane->dev);
-	int width = drm_rect_width(&plane_state->uapi.dst);
-	int height = drm_rect_height(&plane_state->uapi.dst);
+	struct drm_i915_private *i915 = to_i915(plane->dev);
 
-	if (!intel_cursor_size_ok(plane_state))
+	if (!intel_cursor_size_ok(i915, width, height))
 		return false;
 
 	/* Cursor width is limited to a few power-of-two sizes */
@@ -412,8 +413,7 @@ static bool i9xx_cursor_size_ok(const struct intel_plane_state *plane_state)
 	 * cursor is not rotated. Everything else requires square
 	 * cursors.
 	 */
-	if (HAS_CUR_FBC(dev_priv) &&
-	    plane_state->hw.rotation & DRM_MODE_ROTATE_0) {
+	if (HAS_CUR_FBC(i915) && rotation & DRM_MODE_ROTATE_0) {
 		if (height < 8 || height > width)
 			return false;
 	} else {
@@ -431,7 +431,7 @@ static int i9xx_check_cursor(struct intel_crtc_state *crtc_state,
 	struct drm_i915_private *dev_priv = to_i915(plane->base.dev);
 	const struct drm_framebuffer *fb = plane_state->hw.fb;
 	enum pipe pipe = plane->pipe;
-	int ret;
+	int ret, width, height;
 
 	ret = intel_check_cursor(crtc_state, plane_state);
 	if (ret)
@@ -441,24 +441,25 @@ static int i9xx_check_cursor(struct intel_crtc_state *crtc_state,
 	if (!fb)
 		return 0;
 
+	width = drm_rect_width(&plane_state->uapi.dst);
+	height = drm_rect_height(&plane_state->uapi.dst);
+
 	/* Check for which cursor types we support */
-	if (!i9xx_cursor_size_ok(plane_state)) {
-		drm_dbg(&dev_priv->drm,
-			"Cursor dimension %dx%d not supported\n",
-			drm_rect_width(&plane_state->uapi.dst),
-			drm_rect_height(&plane_state->uapi.dst));
+	if (!i9xx_cursor_size_ok(&plane->base, width, height,
+				 plane_state->hw.rotation)) {
+		drm_dbg_kms(&dev_priv->drm,
+			    "Cursor dimension %dx%d not supported\n",
+			    width, height);
 		return -EINVAL;
 	}
 
 	drm_WARN_ON(&dev_priv->drm, plane_state->uapi.visible &&
 		    plane_state->view.color_plane[0].mapping_stride != fb->pitches[0]);
 
-	if (fb->pitches[0] !=
-	    drm_rect_width(&plane_state->uapi.dst) * fb->format->cpp[0]) {
+	if (fb->pitches[0] != width * fb->format->cpp[0]) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "Invalid cursor stride (%u) (cursor width %d)\n",
-			    fb->pitches[0],
-			    drm_rect_width(&plane_state->uapi.dst));
+			    fb->pitches[0], width);
 		return -EINVAL;
 	}
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Intel-gfx] [RFC][PATCH v2 3/3] drm/i915: Add SIZE_HINTS property for cursors
  2023-03-21 14:36 [Intel-gfx] [RFC][PATCH v2 0/3] drm: Add plane SIZE_HINTS property Ville Syrjala
  2023-03-21 14:36 ` [Intel-gfx] [RFC][PATCH v2 1/3] drm: Introduce " Ville Syrjala
  2023-03-21 14:36 ` [Intel-gfx] [RFC][PATCH v2 2/3] drm/i915: Adjust cursor_size_ok() func calling convention Ville Syrjala
@ 2023-03-21 14:36 ` Ville Syrjala
  2023-03-21 16:12 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm: Add plane SIZE_HINTS property (rev4) Patchwork
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjala @ 2023-03-21 14:36 UTC (permalink / raw)
  To: dri-devel
  Cc: Simon Ser, intel-gfx, Pekka Paalanen, Jonas Ådahl, Daniel Stone

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Advertize more suitable cursor sizes via the new SIZE_HINTS
plane property.

Here are some examples on various platforms:
ivb+:
            31 SIZE_HINTS:
                    flags: immutable blob
                    blobs:

                    value:
                            4000080000010001ff6f0200
                    size_hints blob decoded:
                            min: 64x8
                            max: 256x256
                            bitmap[0]: 0b100110111111111111
                                    64  128  256
                               8     *    *    *
                              16     *    *    *
                              32     *    *    *
                              64     *    *    *
                             128          *    *
                             256               *
i945+:
            31 SIZE_HINTS:
                    flags: immutable blob
                    blobs:

                    value:
                            400040000001000111010000
                    size_hints blob decoded:
                            min: 64x64
                            max: 256x256
                            bitmap[0]: 0b100010001
                                    64  128  256
                              64     *
                             128          *
                             256               *
i865:
            31 SIZE_HINTS:
                    flags: immutable blob
                    blobs:

                    value:
                            400001000002ff03ffffffffff0f0000
                    size_hints blob decoded:
                            min: 64x1
                            max: 512x1023
                            bitmap[0]: 0b11111111111111111111111111111111
                            bitmap[1]: 0b111111111111
                                    64  128  256  512
                               1     *    *    *    *
                               2     *    *    *    *
                               4     *    *    *    *
                               8     *    *    *    *
                              16     *    *    *    *
                              32     *    *    *    *
                              64     *    *    *    *
                             128     *    *    *    *
                             256     *    *    *    *
                             512     *    *    *    *
                            1023     *    *    *    *

Cc: Simon Ser <contact@emersion.fr>
Cc: Jonas Ådahl <jadahl@redhat.com>
Cc: Daniel Stone <daniel@fooishbar.org>
Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_cursor.c | 43 ++++++++++++++++++---
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index edeeb5f9f795..449860342aea 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -388,9 +388,9 @@ static u32 i9xx_cursor_ctl(const struct intel_crtc_state *crtc_state,
 	return cntl;
 }
 
-static bool i9xx_cursor_size_ok(struct drm_plane *plane,
-				int width, int height,
-				unsigned int rotation)
+static bool _i9xx_cursor_size_ok(struct drm_plane *plane,
+				 int width, int height,
+				 unsigned int rotation)
 {
 	struct drm_i915_private *i915 = to_i915(plane->dev);
 
@@ -424,6 +424,12 @@ static bool i9xx_cursor_size_ok(struct drm_plane *plane,
 	return true;
 }
 
+static bool i9xx_cursor_size_ok(struct drm_plane *plane,
+				int width, int height)
+{
+	return _i9xx_cursor_size_ok(plane, width, height, DRM_MODE_ROTATE_0);
+}
+
 static int i9xx_check_cursor(struct intel_crtc_state *crtc_state,
 			     struct intel_plane_state *plane_state)
 {
@@ -445,8 +451,8 @@ static int i9xx_check_cursor(struct intel_crtc_state *crtc_state,
 	height = drm_rect_height(&plane_state->uapi.dst);
 
 	/* Check for which cursor types we support */
-	if (!i9xx_cursor_size_ok(&plane->base, width, height,
-				 plane_state->hw.rotation)) {
+	if (!_i9xx_cursor_size_ok(&plane->base, width, height,
+				  plane_state->hw.rotation)) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "Cursor dimension %dx%d not supported\n",
 			    width, height);
@@ -757,6 +763,31 @@ static const struct drm_plane_funcs intel_cursor_plane_funcs = {
 	.format_mod_supported = intel_cursor_format_mod_supported,
 };
 
+static void intel_cursor_add_size_hints_property(struct intel_plane *plane)
+{
+	struct drm_i915_private *i915 = to_i915(plane->base.dev);
+	const struct drm_mode_config *config = &i915->drm.mode_config;
+
+	if (IS_I845G(i915) || IS_I865G(i915))
+		drm_plane_add_size_hints_property(&plane->base,
+						  64, 1,
+						  config->cursor_width,
+						  config->cursor_height,
+						  i845_cursor_size_ok);
+	else if (HAS_CUR_FBC(i915))
+		drm_plane_add_size_hints_property(&plane->base,
+						  64, 8,
+						  config->cursor_width,
+						  config->cursor_height,
+						  i9xx_cursor_size_ok);
+	else
+		drm_plane_add_size_hints_property(&plane->base,
+						  64, 64,
+						  config->cursor_width,
+						  config->cursor_height,
+						  i9xx_cursor_size_ok);
+}
+
 struct intel_plane *
 intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 			  enum pipe pipe)
@@ -815,6 +846,8 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 						   DRM_MODE_ROTATE_0 |
 						   DRM_MODE_ROTATE_180);
 
+	intel_cursor_add_size_hints_property(cursor);
+
 	zpos = RUNTIME_INFO(dev_priv)->num_sprites[pipe] + 1;
 	drm_plane_create_zpos_immutable_property(&cursor->base, zpos);
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm: Add plane SIZE_HINTS property (rev4)
  2023-03-21 14:36 [Intel-gfx] [RFC][PATCH v2 0/3] drm: Add plane SIZE_HINTS property Ville Syrjala
                   ` (2 preceding siblings ...)
  2023-03-21 14:36 ` [Intel-gfx] [RFC][PATCH v2 3/3] drm/i915: Add SIZE_HINTS property for cursors Ville Syrjala
@ 2023-03-21 16:12 ` Patchwork
  2023-03-21 16:12 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2023-03-21 16:12 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm: Add plane SIZE_HINTS property (rev4)
URL   : https://patchwork.freedesktop.org/series/113758/
State : warning

== Summary ==

Error: git fetch origin failed



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm: Add plane SIZE_HINTS property (rev4)
  2023-03-21 14:36 [Intel-gfx] [RFC][PATCH v2 0/3] drm: Add plane SIZE_HINTS property Ville Syrjala
                   ` (3 preceding siblings ...)
  2023-03-21 16:12 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm: Add plane SIZE_HINTS property (rev4) Patchwork
@ 2023-03-21 16:12 ` Patchwork
  2023-03-21 16:12 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
  2023-03-21 16:29 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  6 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2023-03-21 16:12 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm: Add plane SIZE_HINTS property (rev4)
URL   : https://patchwork.freedesktop.org/series/113758/
State : warning

== Summary ==

Error: git fetch origin failed



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✗ Fi.CI.DOCS: warning for drm: Add plane SIZE_HINTS property (rev4)
  2023-03-21 14:36 [Intel-gfx] [RFC][PATCH v2 0/3] drm: Add plane SIZE_HINTS property Ville Syrjala
                   ` (4 preceding siblings ...)
  2023-03-21 16:12 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2023-03-21 16:12 ` Patchwork
  2023-03-21 16:29 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  6 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2023-03-21 16:12 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm: Add plane SIZE_HINTS property (rev4)
URL   : https://patchwork.freedesktop.org/series/113758/
State : warning

== Summary ==

Error: git fetch origin failed



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm: Add plane SIZE_HINTS property (rev4)
  2023-03-21 14:36 [Intel-gfx] [RFC][PATCH v2 0/3] drm: Add plane SIZE_HINTS property Ville Syrjala
                   ` (5 preceding siblings ...)
  2023-03-21 16:12 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
@ 2023-03-21 16:29 ` Patchwork
  6 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2023-03-21 16:29 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 11929 bytes --]

== Series Details ==

Series: drm: Add plane SIZE_HINTS property (rev4)
URL   : https://patchwork.freedesktop.org/series/113758/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12884 -> Patchwork_113758v4
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/index.html

Participating hosts (35 -> 36)
------------------------------

  Additional (2): fi-kbl-soraka bat-rpls-2 
  Missing    (1): bat-atsm-1 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_suspend@basic-s3-without-i915:
    - bat-rpls-2:         NOTRUN -> [ABORT][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@i915_suspend@basic-s3-without-i915.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-rpls-2:         NOTRUN -> [SKIP][2] ([i915#7456])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@debugfs_test@basic-hwmon.html

  * igt@fbdev@read:
    - bat-rpls-2:         NOTRUN -> [SKIP][3] ([i915#2582]) +4 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@fbdev@read.html

  * igt@gem_exec_suspend@basic-s3@smem:
    - bat-rpls-1:         NOTRUN -> [ABORT][4] ([i915#6687] / [i915#7978])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-1/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#2190])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][6] ([fdo#109271] / [i915#4613]) +3 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@gem_lmem_swapping@verify-random:
    - bat-rpls-2:         NOTRUN -> [SKIP][7] ([i915#4613]) +3 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_tiled_pread_basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][8] ([i915#3282])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - bat-rpls-2:         NOTRUN -> [SKIP][9] ([i915#7561])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_pm_rps@basic-api:
    - bat-rpls-2:         NOTRUN -> [SKIP][10] ([i915#6621])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@gt_pm:
    - bat-rpls-2:         NOTRUN -> [DMESG-FAIL][11] ([i915#4258])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@i915_selftest@live@gt_pm.html
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][12] ([i915#1886])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@slpc:
    - bat-rpls-2:         NOTRUN -> [DMESG-FAIL][13] ([i915#6367] / [i915#7913])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@i915_selftest@live@slpc.html
    - bat-rpls-1:         NOTRUN -> [DMESG-FAIL][14] ([i915#6367] / [i915#7996])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-1/igt@i915_selftest@live@slpc.html

  * igt@kms_busy@basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][15] ([i915#1845]) +14 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@kms_busy@basic.html

  * igt@kms_chamelium_edid@hdmi-edid-read:
    - bat-rpls-2:         NOTRUN -> [SKIP][16] ([i915#7828]) +7 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@kms_chamelium_edid@hdmi-edid-read.html

  * igt@kms_chamelium_frames@hdmi-crc-fast:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][17] ([fdo#109271]) +16 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/fi-kbl-soraka/igt@kms_chamelium_frames@hdmi-crc-fast.html

  * igt@kms_chamelium_hpd@common-hpd-after-suspend:
    - bat-dg1-6:          NOTRUN -> [SKIP][18] ([i915#7828])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-dg1-6/igt@kms_chamelium_hpd@common-hpd-after-suspend.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - bat-rpls-2:         NOTRUN -> [SKIP][19] ([i915#3637]) +3 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@kms_flip@basic-flip-vs-dpms.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-rpls-2:         NOTRUN -> [SKIP][20] ([fdo#109285])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_frontbuffer_tracking@basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][21] ([i915#1849])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_psr@sprite_plane_onoff:
    - bat-rpls-2:         NOTRUN -> [SKIP][22] ([i915#1072]) +3 similar issues
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@kms_psr@sprite_plane_onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-rpls-2:         NOTRUN -> [SKIP][23] ([i915#3555])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-rpls-2:         NOTRUN -> [SKIP][24] ([fdo#109295] / [i915#1845] / [i915#3708])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-fence-read:
    - bat-rpls-2:         NOTRUN -> [SKIP][25] ([fdo#109295] / [i915#3708]) +3 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-2/igt@prime_vgem@basic-fence-read.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - bat-dg1-6:          [ABORT][26] ([i915#7883]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/bat-dg1-6/igt@i915_pm_rpm@module-reload.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-dg1-6/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@reset:
    - bat-rpls-1:         [ABORT][28] ([i915#4983]) -> [PASS][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/bat-rpls-1/igt@i915_selftest@live@reset.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-rpls-1/igt@i915_selftest@live@reset.html

  
#### Warnings ####

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-dg1-6:          [SKIP][30] ([i915#3555]) -> [SKIP][31] ([i915#3555] / [i915#4579])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/bat-dg1-6/igt@kms_setmode@basic-clone-single-crtc.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-dg1-6/igt@kms_setmode@basic-clone-single-crtc.html
    - fi-rkl-11600:       [SKIP][32] ([i915#3555] / [i915#4098]) -> [SKIP][33] ([i915#3555] / [i915#4098] / [i915#4579])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html
    - bat-dg1-5:          [SKIP][34] ([i915#3555]) -> [SKIP][35] ([i915#3555] / [i915#4579])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/bat-dg1-5/igt@kms_setmode@basic-clone-single-crtc.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-dg1-5/igt@kms_setmode@basic-clone-single-crtc.html
    - fi-tgl-1115g4:      [SKIP][36] ([i915#3555]) -> [SKIP][37] ([i915#3555] / [i915#4579])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/fi-tgl-1115g4/igt@kms_setmode@basic-clone-single-crtc.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/fi-tgl-1115g4/igt@kms_setmode@basic-clone-single-crtc.html
    - bat-dg1-7:          [SKIP][38] ([i915#3555]) -> [SKIP][39] ([i915#3555] / [i915#4579])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12884/bat-dg1-7/igt@kms_setmode@basic-clone-single-crtc.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/bat-dg1-7/igt@kms_setmode@basic-clone-single-crtc.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7883]: https://gitlab.freedesktop.org/drm/intel/issues/7883
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978
  [i915#7996]: https://gitlab.freedesktop.org/drm/intel/issues/7996


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

  * Linux: CI_DRM_12884 -> Patchwork_113758v4

  CI-20190529: 20190529
  CI_DRM_12884: 1d4054731cfcb1cb9810d309b70535ae0b90ecf0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7208: f327c5d77b6ea6adff1ef6d08f21f232dfe093e3 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_113758v4: 1d4054731cfcb1cb9810d309b70535ae0b90ecf0 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

e70208014acd drm/i915: Add SIZE_HINTS property for cursors
351be2babac0 drm/i915: Adjust cursor_size_ok() func calling convention
e73466e0f097 drm: Introduce plane SIZE_HINTS property

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_113758v4/index.html

[-- Attachment #2: Type: text/html, Size: 14824 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [RFC][PATCH v2 1/3] drm: Introduce plane SIZE_HINTS property
  2023-03-21 14:36 ` [Intel-gfx] [RFC][PATCH v2 1/3] drm: Introduce " Ville Syrjala
@ 2023-03-30 10:29   ` Pekka Paalanen
  0 siblings, 0 replies; 9+ messages in thread
From: Pekka Paalanen @ 2023-03-30 10:29 UTC (permalink / raw)
  To: Ville Syrjala
  Cc: Simon Ser, intel-gfx, Daniel Stone, Jonas Ådahl, dri-devel

On Tue, 21 Mar 2023 16:36:41 +0200
Ville Syrjala <ville.syrjala@linux.intel.com> wrote:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add a new immutable plane property by which a plane can advertise
> a handful of recommended plane sizes. This would be mostly exposed
> by cursor planes as a slightly more capable replacement for
> the DRM_CAP_CURSOR_WIDTH/HEIGHT caps, which can only declare
> a one size fits all limit for the whole device.
> 
> Currently eg. amdgpu/i915/nouveau just advertize the max cursor
> size via the cursor size caps. But always using the max sized
> cursor can waste a surprising amount of power, so a better
> stragey is desirable.
> 
> Most other drivers don't specify any cursor size at all, in
> which case the ioctl code just claims that 64x64 is a great
> choice. Whether that is actually true is debatable.
> 
> A poll of various compositor developers informs us that
> blindly probing with setcursor/atomic ioctl to determine
> suitable cursor sizes is not acceptable, thus the
> introduction of the new property to supplant the cursor
> size caps. The compositor will now be free to select a
> more optimal cursor size from the short list of options.
> 
> The blob contains explicit min and max plane sizes, as
> well as a 2D bitmap representing all the square and non-square
> power of two sizes between the min and max.
> 
> Note that the reported sizes (either via the property or the
> caps) make no claims about things such as plane scaling. So
> these things should only really be consulted for simple
> "cursor like" use cases.
> 
> v2: Try to add some docs
> v3: Specify that value 0 is reserved for future use (basic idea from Jonas)
>     Drop the note about typical hardware (Pekka)
> v4: Total redesign to include the 2D bitmap
> 
> Cc: Simon Ser <contact@emersion.fr>
> Cc: Jonas Ådahl <jadahl@redhat.com>
> Cc: Daniel Stone <daniel@fooishbar.org>
> Cc: Pekka Paalanen <pekka.paalanen@collabora.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_mode_config.c |  7 +++
>  drivers/gpu/drm/drm_plane.c       | 96 +++++++++++++++++++++++++++++++
>  include/drm/drm_mode_config.h     |  5 ++
>  include/drm/drm_plane.h           |  6 ++
>  include/uapi/drm/drm_mode.h       | 29 ++++++++++
>  5 files changed, 143 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index 87eb591fe9b5..21860f94a18c 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -374,6 +374,13 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.modifiers_property = prop;
>  
> +	prop = drm_property_create(dev,
> +				   DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB,
> +				   "SIZE_HINTS", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.size_hints_property = prop;
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 24e7998d1731..fb9cee504767 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -140,6 +140,27 @@
>   *     DRM_FORMAT_MOD_LINEAR. Before linux kernel release v5.1 there have been
>   *     various bugs in this area with inconsistencies between the capability
>   *     flag and per-plane properties.
> + *
> + * SIZE_HINTS:
> + *     Blob property which contains the set of recommended plane size
> + *     which can used for simple "cursor like" use cases (eg. no scaling).
> + *     Using these hints frees userspace from extensive probing of
> + *     supported plane sizes through atomic/setcursor ioctls.
> + *
> + *     For optimal usage userspace should pick the smallest size
> + *     that satisfies its own requirements.
> + *
> + *     The blob contains explicit min/max sizes, as well as a 2D bitmap
> + *     representign all square and non-square POT sizes between.
> + *
> + *     Drivers should only attach this property to planes that
> + *     support a very limited set of sizes.
> + *
> + *     Note that property value 0 (ie. no blob) is reserved for potential
> + *     future use. Current userspace is expected to ignore the property
> + *     if the value is 0, and fall back to some other means (eg.
> + *     &DRM_CAP_CURSOR_WIDTH and &DRM_CAP_CURSOR_HEIGHT) to determine
> + *     the appropriate plane size to use.
>   */
>  
>  static unsigned int drm_num_planes(struct drm_device *dev)
> @@ -1582,3 +1603,78 @@ int drm_plane_create_scaling_filter_property(struct drm_plane *plane,
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_plane_create_scaling_filter_property);
> +
> +static int hint_dim(int x, int x0, int w, int min, int max)
> +{
> +	if (x == 0)
> +		return min;
> +	else if (x == w - 1)
> +		return max;
> +	else
> +		return 1 << (x0 + x);
> +}
> +
> +/**
> + * drm_plane_add_size_hint_property - create a size hint property
> + *
> + * @plane: drm plane
> + * @min_width: minimum width
> + * @min_height: minimum height
> + * @max_width: maximum width
> + * @max_height: maximum height
> + * @cursor_size_ok: function to check if specified size is ok
> + *
> + * Create a size hints property for the plane.
> + *
> + * RETURNS:
> + * Zero for success or -errno
> + */
> +int drm_plane_add_size_hints_property(struct drm_plane *plane,
> +				      int min_width, int min_height,
> +				      int max_width, int max_height,
> +				      bool (*size_ok)(struct drm_plane *plane,
> +						      int width, int height))
> +{
> +	struct drm_device *dev = plane->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +	struct drm_property_blob *blob;
> +	struct drm_plane_size_hint *hints;
> +	int x, y, x0, y0, w, h;
> +
> +	x0 = fls(min_width) - 1;
> +	y0 = fls(min_height) - 1;
> +	w = fls(2 * max_width - 1) - x0;
> +	h = fls(2 * max_height - 1) - y0;
> +
> +	blob = drm_property_create_blob(dev,
> +					struct_size(hints, bitmap,
> +						    DIV_ROUND_UP(w * h, 32)),
> +					NULL);
> +	if (IS_ERR(blob))
> +		return PTR_ERR(blob);
> +
> +	hints = blob->data;
> +
> +	hints->min_width = min_width;
> +	hints->min_height = min_height;
> +	hints->max_width = max_width;
> +	hints->max_height = max_height;
> +
> +	for (y = 0; y < h; y++) {
> +		int height = hint_dim(y, y0, h, min_height, max_height);
> +
> +		for (x = 0; x < w; x++) {
> +			int width = hint_dim(x, x0, w, min_width, max_width);
> +			unsigned int bit = y * w + x;
> +
> +			if (size_ok(plane, width, height))
> +				hints->bitmap[bit/32] |= 1 << (bit & 31);
> +		}
> +	}
> +
> +	drm_object_attach_property(&plane->base, config->size_hints_property,
> +				   blob->base.id);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_plane_add_size_hints_property);
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index e5b053001d22..d5495c0804c5 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -949,6 +949,11 @@ struct drm_mode_config {
>  	 */
>  	struct drm_property *modifiers_property;
>  
> +	/**
> +	 * @size_hints_propertty: Plane size hints property.
> +	 */
> +	struct drm_property *size_hints_property;
> +
>  	/* cursor size */
>  	uint32_t cursor_width, cursor_height;
>  
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index 51291983ea44..6d7326f15761 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -32,6 +32,7 @@
>  #include <drm/drm_util.h>
>  
>  struct drm_crtc;
> +struct drm_plane_size_hint;
>  struct drm_printer;
>  struct drm_modeset_acquire_ctx;
>  
> @@ -945,5 +946,10 @@ drm_plane_get_damage_clips(const struct drm_plane_state *state);
>  
>  int drm_plane_create_scaling_filter_property(struct drm_plane *plane,
>  					     unsigned int supported_filters);
> +int drm_plane_add_size_hints_property(struct drm_plane *plane,
> +				      int min_width, int min_height,
> +				      int max_width, int max_height,
> +				      bool (*size_ok)(struct drm_plane *plane,
> +						      int width, int height));
>  
>  #endif
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 46becedf5b2f..5a1de6bd80dd 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -849,6 +849,35 @@ struct drm_color_lut {
>  	__u16 reserved;
>  };
>  
> +/**
> + * struct drm_plane_size_hint - Plane size hints
> + *
> + * Includes a 2D bitmap in row major order of one bit per plane
> + * size (WxH). The colums represent the possible plane widths as
> + * <min>,<power of two values between min-max>,<max>
> + * the rows do the same for plane heights. The minimums and
> + * maximums need not be power of two themselves. The bits
> + * are walked in lsb->msb order.
> + *
> + * Example:
> + * min_width = 33, min_height = 16
> + * max_width = 128, max_height = 127
> + * bitmap[0] = 0b100111011001
> + *     33 64 128
> + * 16   *
> + * 32   *  *
> + * 64   *  *   *
> + * 127         *
> + *
> + * Indicating support for 33x16,33xx32,
> + * 64x32,64x64,128x64,128x127 sizes.
> + */
> +struct drm_plane_size_hint {
> +	__u16 min_width, min_height;
> +	__u16 max_width, max_height;
> +	__u32 bitmap[];
> +};

Hi,

as a compositor developer, I believe I could make use of this somehow.
I would probably pose the optimization problem as "what dimensions
result in the smallest area" in a compositor, iterating over the
possible and allowed widths and checking the smallest possible and
allowed height.

If you think this is a good match to hardware capabilities, it's fine
by me. The power-of-two is a pretty fundamental built-in assumption and
I have no idea how good it is.


Thanks,
pq

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-03-30 10:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-21 14:36 [Intel-gfx] [RFC][PATCH v2 0/3] drm: Add plane SIZE_HINTS property Ville Syrjala
2023-03-21 14:36 ` [Intel-gfx] [RFC][PATCH v2 1/3] drm: Introduce " Ville Syrjala
2023-03-30 10:29   ` Pekka Paalanen
2023-03-21 14:36 ` [Intel-gfx] [RFC][PATCH v2 2/3] drm/i915: Adjust cursor_size_ok() func calling convention Ville Syrjala
2023-03-21 14:36 ` [Intel-gfx] [RFC][PATCH v2 3/3] drm/i915: Add SIZE_HINTS property for cursors Ville Syrjala
2023-03-21 16:12 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm: Add plane SIZE_HINTS property (rev4) Patchwork
2023-03-21 16:12 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2023-03-21 16:12 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2023-03-21 16:29 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).