[PATCH 13/22] drm/vkms: Introduce configfs for plane zpos property

Louis Chauvet posted 22 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH 13/22] drm/vkms: Introduce configfs for plane zpos property
Posted by Louis Chauvet 3 months, 3 weeks ago
Modern compositor rely on zpos managment to offload some processing to
deticated hardware. In order to test multiple configurations, add zpos
configuration to configFS.

Introduce multiple attributes to configure zpos:
- zpos_enabled - Create or not the zpos property. If not created, the zpos
  is undefined.
- zpos_mutable - If the zpos property is created, allow or not the
  userspace to modify it
- zpos_initial - Intial value for zpos property. Must be between zpos_min
  and zpos_max
- zpos_min - Minimal zpos value for this plane. Must be smaller than or
  equals to zpos_max
- zpos_max - Maximal zpos value for this plane. Must be greater than or
  equals to zpos_min

Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
 Documentation/gpu/vkms.rst           |   9 +-
 drivers/gpu/drm/vkms/vkms_configfs.c | 199 +++++++++++++++++++++++++++++++++++
 2 files changed, 207 insertions(+), 1 deletion(-)

diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index deb14e7c48ea..d4ad4af45414 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -87,7 +87,7 @@ Start by creating one or more planes::
 
   sudo mkdir /config/vkms/my-vkms/planes/plane0
 
-Planes have 8 configurable attribute:
+Planes have 13 configurable attribute:
 
 - type: Plane type: 0 overlay, 1 primary, 2 cursor (same values as those
   exposed by the "type" property of a plane)
@@ -111,6 +111,13 @@ Planes have 8 configurable attribute:
   To remove a format, use a minus and its fourcc: -XR24
   To add all formats use +*
   To remove all formats, use -*
+- zpos_enabled: Enable or not the zpos property: 1 enable, 0 disable
+- zpos_mutable: Create the zpos property as a mutable or imutable property: 1 mutable,
+  0 disable. No effect if zpos_enabled is not set.
+- zpos_initial: Set the initial zpos value. Must be between zpos_min and zpos_max. No
+  effect if zpos_enabled is not set.
+- zpos_min: Set the minimal zpos value. No effect if zpos_enabled is not set.
+- zpos_max: Set the maximal zpos value. No effect if zpos_enabled is not set.
 
 Continue by creating one or more CRTCs::
 
diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
index 528f22fa2df1..fd1be7292058 100644
--- a/drivers/gpu/drm/vkms/vkms_configfs.c
+++ b/drivers/gpu/drm/vkms/vkms_configfs.c
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0+
+#include "asm-generic/errno-base.h"
 #include <linux/cleanup.h>
 #include <linux/configfs.h>
 #include <linux/mutex.h>
@@ -679,6 +680,194 @@ static ssize_t plane_supported_formats_store(struct config_item *item,
 	return count;
 }
 
+static ssize_t plane_zpos_enabled_show(struct config_item *item, char *page)
+{
+	struct vkms_configfs_plane *plane;
+	bool enabled;
+
+	plane = plane_item_to_vkms_configfs_plane(item);
+
+	scoped_guard(mutex, &plane->dev->lock)
+		enabled = vkms_config_plane_get_zpos_enabled(plane->config);
+
+	return sprintf(page, "%d\n", enabled);
+}
+
+static ssize_t plane_zpos_enabled_store(struct config_item *item, const char *page,
+					size_t count)
+{
+	struct vkms_configfs_plane *plane;
+	bool enabled;
+
+	plane = plane_item_to_vkms_configfs_plane(item);
+
+	if (kstrtobool(page, &enabled))
+		return -EINVAL;
+
+	scoped_guard(mutex, &plane->dev->lock) {
+		if (plane->dev->enabled)
+			return -EBUSY;
+
+		vkms_config_plane_set_zpos_enabled(plane->config, enabled);
+	}
+
+	return (ssize_t)count;
+}
+
+static ssize_t plane_zpos_mutable_show(struct config_item *item, char *page)
+{
+	struct vkms_configfs_plane *plane;
+	bool mutable;
+
+	plane = plane_item_to_vkms_configfs_plane(item);
+
+	scoped_guard(mutex, &plane->dev->lock)
+		mutable = vkms_config_plane_get_zpos_mutable(plane->config);
+
+	return sprintf(page, "%d\n", mutable);
+}
+
+static ssize_t plane_zpos_mutable_store(struct config_item *item, const char *page,
+					size_t count)
+{
+	struct vkms_configfs_plane *plane;
+	bool mutable;
+
+	plane = plane_item_to_vkms_configfs_plane(item);
+
+	if (kstrtobool(page, &mutable))
+		return -EINVAL;
+
+	scoped_guard(mutex, &plane->dev->lock) {
+		if (plane->dev->enabled)
+			return -EBUSY;
+
+		vkms_config_plane_set_zpos_mutable(plane->config, mutable);
+	}
+
+	return (ssize_t)count;
+}
+
+static ssize_t plane_zpos_initial_show(struct config_item *item, char *page)
+{
+	struct vkms_configfs_plane *plane;
+	unsigned int initial;
+
+	plane = plane_item_to_vkms_configfs_plane(item);
+
+	scoped_guard(mutex, &plane->dev->lock)
+		initial = vkms_config_plane_get_zpos_initial(plane->config);
+
+	return sprintf(page, "%u\n", initial);
+}
+
+static ssize_t plane_zpos_initial_store(struct config_item *item, const char *page,
+					size_t count)
+{
+	struct vkms_configfs_plane *plane;
+	unsigned int initial;
+
+	plane = plane_item_to_vkms_configfs_plane(item);
+
+	if (kstrtouint(page, 10, &initial))
+		return -EINVAL;
+
+	scoped_guard(mutex, &plane->dev->lock) {
+		if (plane->dev->enabled)
+			return -EBUSY;
+
+		if (initial > vkms_config_plane_get_zpos_max(plane->config))
+			return -EINVAL;
+
+		if (initial < vkms_config_plane_get_zpos_min(plane->config))
+			return -EINVAL;
+
+		vkms_config_plane_set_zpos_initial(plane->config, initial);
+	}
+
+	return (ssize_t)count;
+}
+
+static ssize_t plane_zpos_min_show(struct config_item *item, char *page)
+{
+	struct vkms_configfs_plane *plane;
+	unsigned int min;
+
+	plane = plane_item_to_vkms_configfs_plane(item);
+
+	scoped_guard(mutex, &plane->dev->lock)
+		min = vkms_config_plane_get_zpos_min(plane->config);
+
+	return sprintf(page, "%u\n", min);
+}
+
+static ssize_t plane_zpos_min_store(struct config_item *item, const char *page,
+				    size_t count)
+{
+	struct vkms_configfs_plane *plane;
+	unsigned int min;
+
+	plane = plane_item_to_vkms_configfs_plane(item);
+
+	if (kstrtouint(page, 10, &min))
+		return -EINVAL;
+
+	scoped_guard(mutex, &plane->dev->lock) {
+		if (plane->dev->enabled)
+			return -EBUSY;
+
+		if (min > vkms_config_plane_get_zpos_max(plane->config))
+			return -EINVAL;
+
+		if (min > vkms_config_plane_get_zpos_initial(plane->config))
+			return -EINVAL;
+
+		vkms_config_plane_set_zpos_min(plane->config, min);
+	}
+
+	return (ssize_t)count;
+}
+
+static ssize_t plane_zpos_max_show(struct config_item *item, char *page)
+{
+	struct vkms_configfs_plane *plane;
+	unsigned int max;
+
+	plane = plane_item_to_vkms_configfs_plane(item);
+
+	scoped_guard(mutex, &plane->dev->lock)
+		max = vkms_config_plane_get_zpos_max(plane->config);
+
+	return sprintf(page, "%u\n", max);
+}
+
+static ssize_t plane_zpos_max_store(struct config_item *item, const char *page,
+				    size_t count)
+{
+	struct vkms_configfs_plane *plane;
+	unsigned int max;
+
+	plane = plane_item_to_vkms_configfs_plane(item);
+
+	if (kstrtouint(page, 10, &max))
+		return -EINVAL;
+
+	scoped_guard(mutex, &plane->dev->lock) {
+		if (plane->dev->enabled)
+			return -EBUSY;
+
+		if (max < vkms_config_plane_get_zpos_min(plane->config))
+			return -EINVAL;
+
+		if (max < vkms_config_plane_get_zpos_initial(plane->config))
+			return -EINVAL;
+
+		vkms_config_plane_set_zpos_max(plane->config, max);
+	}
+
+	return (ssize_t)count;
+}
+
 CONFIGFS_ATTR(plane_, type);
 CONFIGFS_ATTR(plane_, supported_rotations);
 CONFIGFS_ATTR(plane_, default_rotation);
@@ -687,6 +876,11 @@ CONFIGFS_ATTR(plane_, default_color_range);
 CONFIGFS_ATTR(plane_, supported_color_encoding);
 CONFIGFS_ATTR(plane_, default_color_encoding);
 CONFIGFS_ATTR(plane_, supported_formats);
+CONFIGFS_ATTR(plane_, zpos_enabled);
+CONFIGFS_ATTR(plane_, zpos_mutable);
+CONFIGFS_ATTR(plane_, zpos_initial);
+CONFIGFS_ATTR(plane_, zpos_min);
+CONFIGFS_ATTR(plane_, zpos_max);
 
 static struct configfs_attribute *plane_item_attrs[] = {
 	&plane_attr_type,
@@ -697,6 +891,11 @@ static struct configfs_attribute *plane_item_attrs[] = {
 	&plane_attr_supported_color_encoding,
 	&plane_attr_default_color_encoding,
 	&plane_attr_supported_formats,
+	&plane_attr_zpos_enabled,
+	&plane_attr_zpos_mutable,
+	&plane_attr_zpos_initial,
+	&plane_attr_zpos_min,
+	&plane_attr_zpos_max,
 	NULL,
 };
 

-- 
2.51.0
Re: [PATCH 13/22] drm/vkms: Introduce configfs for plane zpos property
Posted by José Expósito 3 months, 2 weeks ago
On Sat, Oct 18, 2025 at 04:01:13AM +0200, Louis Chauvet wrote:
> Modern compositor rely on zpos managment to offload some processing to
> deticated hardware. In order to test multiple configurations, add zpos
> configuration to configFS.
> 
> Introduce multiple attributes to configure zpos:
> - zpos_enabled - Create or not the zpos property. If not created, the zpos
>   is undefined.
> - zpos_mutable - If the zpos property is created, allow or not the
>   userspace to modify it
> - zpos_initial - Intial value for zpos property. Must be between zpos_min
>   and zpos_max
> - zpos_min - Minimal zpos value for this plane. Must be smaller than or
>   equals to zpos_max
> - zpos_max - Maximal zpos value for this plane. Must be greater than or
>   equals to zpos_min
> 
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
>  Documentation/gpu/vkms.rst           |   9 +-
>  drivers/gpu/drm/vkms/vkms_configfs.c | 199 +++++++++++++++++++++++++++++++++++
>  2 files changed, 207 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> index deb14e7c48ea..d4ad4af45414 100644
> --- a/Documentation/gpu/vkms.rst
> +++ b/Documentation/gpu/vkms.rst
> @@ -87,7 +87,7 @@ Start by creating one or more planes::
>  
>    sudo mkdir /config/vkms/my-vkms/planes/plane0
>  
> -Planes have 8 configurable attribute:
> +Planes have 13 configurable attribute:
>  
>  - type: Plane type: 0 overlay, 1 primary, 2 cursor (same values as those
>    exposed by the "type" property of a plane)
> @@ -111,6 +111,13 @@ Planes have 8 configurable attribute:
>    To remove a format, use a minus and its fourcc: -XR24
>    To add all formats use +*
>    To remove all formats, use -*
> +- zpos_enabled: Enable or not the zpos property: 1 enable, 0 disable
> +- zpos_mutable: Create the zpos property as a mutable or imutable property: 1 mutable,
> +  0 disable. No effect if zpos_enabled is not set.
> +- zpos_initial: Set the initial zpos value. Must be between zpos_min and zpos_max. No
> +  effect if zpos_enabled is not set.
> +- zpos_min: Set the minimal zpos value. No effect if zpos_enabled is not set.
> +- zpos_max: Set the maximal zpos value. No effect if zpos_enabled is not set.

Aren't zpos_min/max also ignored when zpos_mutable is false?

>  Continue by creating one or more CRTCs::
>  
> diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> index 528f22fa2df1..fd1be7292058 100644
> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> @@ -1,4 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0+
> +#include "asm-generic/errno-base.h"

Accidentally included by IDE?

>  #include <linux/cleanup.h>
>  #include <linux/configfs.h>
>  #include <linux/mutex.h>
> @@ -679,6 +680,194 @@ static ssize_t plane_supported_formats_store(struct config_item *item,
>  	return count;
>  }
>  
> +static ssize_t plane_zpos_enabled_show(struct config_item *item, char *page)
> +{
> +	struct vkms_configfs_plane *plane;
> +	bool enabled;
> +
> +	plane = plane_item_to_vkms_configfs_plane(item);
> +
> +	scoped_guard(mutex, &plane->dev->lock)
> +		enabled = vkms_config_plane_get_zpos_enabled(plane->config);
> +
> +	return sprintf(page, "%d\n", enabled);
> +}
> +
> +static ssize_t plane_zpos_enabled_store(struct config_item *item, const char *page,
> +					size_t count)
> +{
> +	struct vkms_configfs_plane *plane;
> +	bool enabled;
> +
> +	plane = plane_item_to_vkms_configfs_plane(item);
> +
> +	if (kstrtobool(page, &enabled))
> +		return -EINVAL;
> +
> +	scoped_guard(mutex, &plane->dev->lock) {
> +		if (plane->dev->enabled)
> +			return -EBUSY;
> +
> +		vkms_config_plane_set_zpos_enabled(plane->config, enabled);
> +	}
> +
> +	return (ssize_t)count;
> +}
> +
> +static ssize_t plane_zpos_mutable_show(struct config_item *item, char *page)
> +{
> +	struct vkms_configfs_plane *plane;
> +	bool mutable;
> +
> +	plane = plane_item_to_vkms_configfs_plane(item);
> +
> +	scoped_guard(mutex, &plane->dev->lock)
> +		mutable = vkms_config_plane_get_zpos_mutable(plane->config);
> +
> +	return sprintf(page, "%d\n", mutable);
> +}
> +
> +static ssize_t plane_zpos_mutable_store(struct config_item *item, const char *page,
> +					size_t count)
> +{
> +	struct vkms_configfs_plane *plane;
> +	bool mutable;
> +
> +	plane = plane_item_to_vkms_configfs_plane(item);
> +
> +	if (kstrtobool(page, &mutable))
> +		return -EINVAL;
> +
> +	scoped_guard(mutex, &plane->dev->lock) {
> +		if (plane->dev->enabled)
> +			return -EBUSY;
> +
> +		vkms_config_plane_set_zpos_mutable(plane->config, mutable);
> +	}
> +
> +	return (ssize_t)count;
> +}
> +
> +static ssize_t plane_zpos_initial_show(struct config_item *item, char *page)
> +{
> +	struct vkms_configfs_plane *plane;
> +	unsigned int initial;
> +
> +	plane = plane_item_to_vkms_configfs_plane(item);
> +
> +	scoped_guard(mutex, &plane->dev->lock)
> +		initial = vkms_config_plane_get_zpos_initial(plane->config);
> +
> +	return sprintf(page, "%u\n", initial);
> +}
> +
> +static ssize_t plane_zpos_initial_store(struct config_item *item, const char *page,
> +					size_t count)
> +{
> +	struct vkms_configfs_plane *plane;
> +	unsigned int initial;
> +
> +	plane = plane_item_to_vkms_configfs_plane(item);
> +
> +	if (kstrtouint(page, 10, &initial))
> +		return -EINVAL;
> +
> +	scoped_guard(mutex, &plane->dev->lock) {
> +		if (plane->dev->enabled)
> +			return -EBUSY;
> +
> +		if (initial > vkms_config_plane_get_zpos_max(plane->config))
> +			return -EINVAL;
> +
> +		if (initial < vkms_config_plane_get_zpos_min(plane->config))
> +			return -EINVAL;
> +
> +		vkms_config_plane_set_zpos_initial(plane->config, initial);
> +	}
> +
> +	return (ssize_t)count;
> +}
> +
> +static ssize_t plane_zpos_min_show(struct config_item *item, char *page)
> +{
> +	struct vkms_configfs_plane *plane;
> +	unsigned int min;
> +
> +	plane = plane_item_to_vkms_configfs_plane(item);
> +
> +	scoped_guard(mutex, &plane->dev->lock)
> +		min = vkms_config_plane_get_zpos_min(plane->config);
> +
> +	return sprintf(page, "%u\n", min);
> +}
> +
> +static ssize_t plane_zpos_min_store(struct config_item *item, const char *page,
> +				    size_t count)
> +{
> +	struct vkms_configfs_plane *plane;
> +	unsigned int min;
> +
> +	plane = plane_item_to_vkms_configfs_plane(item);
> +
> +	if (kstrtouint(page, 10, &min))
> +		return -EINVAL;
> +
> +	scoped_guard(mutex, &plane->dev->lock) {
> +		if (plane->dev->enabled)
> +			return -EBUSY;
> +
> +		if (min > vkms_config_plane_get_zpos_max(plane->config))
> +			return -EINVAL;
> +
> +		if (min > vkms_config_plane_get_zpos_initial(plane->config))
> +			return -EINVAL;
> +
> +		vkms_config_plane_set_zpos_min(plane->config, min);
> +	}
> +
> +	return (ssize_t)count;
> +}
> +
> +static ssize_t plane_zpos_max_show(struct config_item *item, char *page)
> +{
> +	struct vkms_configfs_plane *plane;
> +	unsigned int max;
> +
> +	plane = plane_item_to_vkms_configfs_plane(item);
> +
> +	scoped_guard(mutex, &plane->dev->lock)
> +		max = vkms_config_plane_get_zpos_max(plane->config);
> +
> +	return sprintf(page, "%u\n", max);
> +}
> +
> +static ssize_t plane_zpos_max_store(struct config_item *item, const char *page,
> +				    size_t count)
> +{
> +	struct vkms_configfs_plane *plane;
> +	unsigned int max;
> +
> +	plane = plane_item_to_vkms_configfs_plane(item);
> +
> +	if (kstrtouint(page, 10, &max))
> +		return -EINVAL;
> +
> +	scoped_guard(mutex, &plane->dev->lock) {
> +		if (plane->dev->enabled)
> +			return -EBUSY;
> +
> +		if (max < vkms_config_plane_get_zpos_min(plane->config))
> +			return -EINVAL;
> +
> +		if (max < vkms_config_plane_get_zpos_initial(plane->config))
> +			return -EINVAL;
> +
> +		vkms_config_plane_set_zpos_max(plane->config, max);
> +	}
> +
> +	return (ssize_t)count;
> +}
> +
>  CONFIGFS_ATTR(plane_, type);
>  CONFIGFS_ATTR(plane_, supported_rotations);
>  CONFIGFS_ATTR(plane_, default_rotation);
> @@ -687,6 +876,11 @@ CONFIGFS_ATTR(plane_, default_color_range);
>  CONFIGFS_ATTR(plane_, supported_color_encoding);
>  CONFIGFS_ATTR(plane_, default_color_encoding);
>  CONFIGFS_ATTR(plane_, supported_formats);
> +CONFIGFS_ATTR(plane_, zpos_enabled);
> +CONFIGFS_ATTR(plane_, zpos_mutable);
> +CONFIGFS_ATTR(plane_, zpos_initial);
> +CONFIGFS_ATTR(plane_, zpos_min);
> +CONFIGFS_ATTR(plane_, zpos_max);
>  
>  static struct configfs_attribute *plane_item_attrs[] = {
>  	&plane_attr_type,
> @@ -697,6 +891,11 @@ static struct configfs_attribute *plane_item_attrs[] = {
>  	&plane_attr_supported_color_encoding,
>  	&plane_attr_default_color_encoding,
>  	&plane_attr_supported_formats,
> +	&plane_attr_zpos_enabled,
> +	&plane_attr_zpos_mutable,
> +	&plane_attr_zpos_initial,
> +	&plane_attr_zpos_min,
> +	&plane_attr_zpos_max,

LGTM, other than my previous comment on adding validation in this layer.
It can force to change attributtes in a specific order. I think we should
only validate in vkms_config_is_valid().

>  	NULL,
>  };
>  
> 
> -- 
> 2.51.0
>