Re: [RFC][PATCH v2 1/3] drm: Introduce plane SIZE_HINTS property
On Tue, 21 Mar 2023 16:36:41 +0200
Ville Syrjala wrote:
> From: Ville Syrjälä
>
> 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
> Cc: Jonas Ådahl
> Cc: Daniel Stone
> Cc: Pekka Paalanen
> Signed-off-by: Ville Syrjälä
> ---
> 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.
> + * _CAP_CURSOR_WIDTH and _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:
[RFC][PATCH v2 1/3] drm: Introduce plane SIZE_HINTS property
From: Ville Syrjälä
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
Cc: Jonas Ådahl
Cc: Daniel Stone
Cc: Pekka Paalanen
Signed-off-by: Ville Syrjälä
---
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.
+ * _CAP_CURSOR_WIDTH and _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,
+
