Message-ID: <aVKno9Oh1d1h06m2@fedora>
Date: Mon, 29 Dec 2025 17:09:07 +0100
From: José Expósito <jose.exposito89@...il.com>
To: Louis Chauvet <louis.chauvet@...tlin.com>
Cc: Haneen Mohammed <hamohammed.sa@...il.com>,
	Simona Vetter <simona@...ll.ch>,
	Melissa Wen <melissa.srw@...il.com>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	David Airlie <airlied@...il.com>, Jonathan Corbet <corbet@....net>,
	victoria@...tem76.com, sebastian.wick@...hat.com,
	thomas.petazzoni@...tlin.com, dri-devel@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH v3 18/33] drm/vkms: Introduce configfs for plane format

On Mon, Dec 22, 2025 at 11:11:20AM +0100, Louis Chauvet wrote:
> To allow the userspace to test many hardware configuration, introduce a
> new interface to configure the available formats per planes. VKMS supports
> multiple formats, so the userspace can choose any combination.
> 
> The supported formats are configured by writing the fourcc code in
> supported_formats:
>  # enable AR24 format
>   echo '+AR24' > /config/vkms/DEVICE_1/planes/PLANE_1/supported_formats
>  # disable AR24 format
>   echo '-AR24' > /config/vkms/DEVICE_1/planes/PLANE_1/supported_formats
>  # enable all format supported by VKMS
>   echo '+*' > /config/vkms/DEVICE_1/planes/PLANE_1/supported_formats
>  # disable all formats
>   echo '-*' > /config/vkms/DEVICE_1/planes/PLANE_1/supported_formats
> 
> Signed-off-by: Louis Chauvet <louis.chauvet@...tlin.com>
> ---
>  Documentation/ABI/testing/configfs-vkms         |   9 ++
>  Documentation/gpu/vkms.rst                      |   7 +-
>  drivers/gpu/drm/vkms/tests/Makefile             |   3 +-
>  drivers/gpu/drm/vkms/tests/vkms_configfs_test.c | 102 ++++++++++++++++++++
>  drivers/gpu/drm/vkms/vkms_configfs.c            | 118 ++++++++++++++++++++++++
>  drivers/gpu/drm/vkms/vkms_configfs.h            |   6 ++
>  6 files changed, 243 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/configfs-vkms b/Documentation/ABI/testing/configfs-vkms
> index 2ace79ce848a..48f92538c602 100644
> --- a/Documentation/ABI/testing/configfs-vkms
> +++ b/Documentation/ABI/testing/configfs-vkms
> @@ -153,6 +153,15 @@ Description:
>          Default color range presented to userspace, same
>          values as supported_color_ranges.
>  
> +What:		/sys/kernel/config/vkms/<device>/planes/<plane>/supported_formats
> +Date:		Nov 2025
> +Contact:	dri-devel@...ts.freedesktop.org
> +Description:
> +        List of supported formats for this plane. To add a new
> +        item, write its fourcc code prefixed with '+'. To remove,
> +        use '-' prefix. Use '+*' to add all formats, '-*' to
> +        remove all.
> +
>  What:		/sys/kernel/config/vkms/<device>/planes/<plane>/possible_crtcs
>  Date:		Nov 2025
>  Contact:	dri-devel@...ts.freedesktop.org
> diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
> index ab0eb2f97fc2..15d62ad963c9 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 attributes:
> +Planes have 9 configurable attributes:
>  
>  - type: Plane type: 0 overlay, 1 primary, 2 cursor (same values as those
>    exposed by the "type" property of a plane)
> @@ -109,6 +109,11 @@ Planes have 8 configurable attributes:
>    must be set too.
>  - default_color_range: Default color range presented to the userspace, same
>    values as supported_color_ranges
> +- supported_formats: List of supported formats for this plane. To add a new item in the
> +  list, write it using a plus and fourcc code: +XR24
> +  To remove a format, use a minus and its fourcc: -XR24
> +  To add all formats use +*
> +  To remove all formats, use -*
>  
>  Continue by creating one or more CRTCs::
>  
> diff --git a/drivers/gpu/drm/vkms/tests/Makefile b/drivers/gpu/drm/vkms/tests/Makefile
> index d4d9ba8d4c54..92cfa7262ba4 100644
> --- a/drivers/gpu/drm/vkms/tests/Makefile
> +++ b/drivers/gpu/drm/vkms/tests/Makefile
> @@ -3,6 +3,7 @@
>  vkms-kunit-tests-y := \
>  	vkms_config_test.o \
>  	vkms_format_test.o \
> -	vkms_color_test.o
> +	vkms_color_test.o \
> +	vkms_configfs_test.o \
>  
>  obj-$(CONFIG_DRM_VKMS_KUNIT_TEST) += vkms-kunit-tests.o
> diff --git a/drivers/gpu/drm/vkms/tests/vkms_configfs_test.c b/drivers/gpu/drm/vkms/tests/vkms_configfs_test.c
> new file mode 100644
> index 000000000000..8d02c2c459d9
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/tests/vkms_configfs_test.c
> @@ -0,0 +1,102 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include "linux/printk.h"
> +#include <kunit/test.h>
> +
> +#include "../vkms_configfs.h"
> +
> +MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
> +
> +/**
> + * struct vkms_configfs_parse_format_case - Store test case for format parsing
> + * @str: Contains the string to parse
> + * @str_len: str len
> + * @expected_len: expected len of the matched format
> + * @expected_offset: expected offset in the string for the parsed format
> + */
> +struct vkms_configfs_parse_format_case {
> +	const char *str;
> +	int str_len;
> +	int expected_len;
> +	int expected_offset;
> +};
> +
> +struct vkms_configfs_parse_format_case vkms_configfs_parse_format_test_cases[] = {
> +	{
> +		.str = "+RG24",
> +		.str_len = 6,
> +		.expected_len = 5,
> +		.expected_offset = 0,
> +	}, {
> +		.str = "-RG24",
> +		.str_len = 6,
> +		.expected_len = 5,
> +		.expected_offset = 0
> +	}, {
> +		.str = "  -RG24",
> +		.str_len = 8,
> +		.expected_len = 5,
> +		.expected_offset = 2
> +	}, {
> +		.str = "+*",
> +		.str_len = 3,
> +		.expected_len = 2,
> +		.expected_offset = 0
> +	}, {
> +		.str = "-RG24+RG24",
> +		.str_len = 11,
> +		.expected_len = 5,
> +		.expected_offset = 0
> +	}, {
> +		.str = "-R1+RG24",
> +		.str_len = 9,
> +		.expected_len = 3,
> +		.expected_offset = 0
> +	}, {
> +		.str = "\n-R1",
> +		.str_len = 5,
> +		.expected_len = 3,
> +		.expected_offset = 1
> +	}, {
> +		.str = "-R1111",
> +		.str_len = 3,
> +		.expected_len = 3,
> +		.expected_offset = 0
> +	}
> +};
> +
> +static void vkms_configfs_test_parse_format(struct kunit *test)
> +{
> +	const struct vkms_configfs_parse_format_case *param = test->param_value;
> +	char *out;
> +	int len = vkms_configfs_parse_next_format(param->str, param->str + param->str_len, &out);
> +
> +	KUNIT_EXPECT_EQ(test, len, param->expected_len);
> +	KUNIT_EXPECT_PTR_EQ(test, out, param->str + param->expected_offset);
> +}
> +
> +static void vkms_configfs_test_parse_format_get_desc(struct vkms_configfs_parse_format_case *t,
> +						     char *desc)
> +{
> +	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s", t->str);
> +}
> +
> +KUNIT_ARRAY_PARAM(vkms_configfs_test_parse_format, vkms_configfs_parse_format_test_cases,
> +		  vkms_configfs_test_parse_format_get_desc
> +);
> +
> +static struct kunit_case vkms_configfs_test_cases[] = {
> +	KUNIT_CASE_PARAM(vkms_configfs_test_parse_format,
> +			 vkms_configfs_test_parse_format_gen_params),
> +	{}
> +};
> +
> +static struct kunit_suite vkms_configfs_test_suite = {
> +	.name = "vkms-configfs",
> +	.test_cases = vkms_configfs_test_cases,
> +};
> +
> +kunit_test_suite(vkms_configfs_test_suite);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Kunit test for vkms configfs utility");
> diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> index 7be6d10b2b68..a451d1122acf 100644
> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> @@ -3,6 +3,8 @@
>  #include <linux/configfs.h>
>  #include <linux/mutex.h>
>  #include <linux/slab.h>
> +#include <linux/string.h>
> +#include <kunit/visibility.h>
>  
>  #include "vkms_drv.h"
>  #include "vkms_config.h"
> @@ -628,6 +630,120 @@ static ssize_t plane_default_color_encoding_store(struct config_item *item,
>  	return count;
>  }
>  
> +static ssize_t plane_supported_formats_show(struct config_item *item, char *page)
> +{
> +	struct vkms_configfs_plane *plane;
> +
> +	plane = plane_item_to_vkms_configfs_plane(item);
> +
> +	page[0] = '\0';
> +
> +	scoped_guard(mutex, &plane->dev->lock) {
> +		u32 *formats = vkms_config_plane_get_supported_formats(plane->config);
> +
> +		for (int i = 0;
> +		     i < vkms_config_plane_get_supported_formats_count(plane->config);
> +		     i++) {
> +			char tmp[6] = { 0 };
> +			const ssize_t ret = snprintf(tmp, ARRAY_SIZE(tmp), "%.*s\n",
> +					       (int)sizeof(*formats),
> +					       (char *)&formats[i]);
> +			if (ret < 0)
> +				return ret;
> +			/*
> +			 * Limitation of ConfigFS attributes, an attribute can't be bigger
> +			 * than PAGE_SIZE. This will crop the result if this plane support
> +			 * more than ≈1000 formats.
> +			 */
> +			if (ret + strlen(page) > PAGE_SIZE - 1)
> +				return -ENOMEM;
> +			strncat(page, tmp, ARRAY_SIZE(tmp));
> +		}
> +	}
> +
> +	return strlen(page);
> +}
> +
> +/**
> + * parse_next_format() - Parse the next format in page, skipping all non fourcc-related characters
> + * @page: page to search into
> + * @page_end: last character of the page
> + * @out: Output pointer, will point inside page
> + *
> + * Returns: size of the matched format, @out will point to the + or -
> + */
> +VISIBLE_IF_KUNIT
> +int vkms_configfs_parse_next_format(const char *page, const char *page_end, char **out)
> +{
> +	int count = page - page_end;

Shouldn't this be "page_end - page"? An example logging the variables shows
that "count" takes negative numbers:

$ echo "-XB48" | sudo tee /sys/kernel/config/vkms/gpu1/planes/plane0/supported_formats

[22854.585268] page = 000000001ec38388
[22854.585277] page_end = 000000000b71ccb5
[22854.585279] count = -6
[22854.585281] page = 00000000625917b4
[22854.585282] page_end = 000000000b71ccb5
[22854.585283] count = -1

Jose

> +	char *tmp_plus = strnchr(page, count, '+');
> +	char *tmp_minus = strnchr(page, count, '-');
> +
> +	if (!tmp_plus && !tmp_minus)
> +		return 0;
> +	if (!tmp_plus)
> +		*out = tmp_minus;
> +	else if (!tmp_minus)
> +		*out = tmp_plus;
> +	else
> +		*out = min(tmp_plus, tmp_minus);
> +
> +	char *end = *out + 1;
> +
> +	while (end < page_end) {
> +		if (!isalnum(*end) && *end != '*')
> +			break;
> +		end++;
> +	}
> +
> +	return end - *out;
> +}
> +EXPORT_SYMBOL_IF_KUNIT(vkms_configfs_parse_next_format);
> +
> +static ssize_t plane_supported_formats_store(struct config_item *item,
> +					     const char *page, size_t count)
> +{
> +	struct vkms_configfs_plane *plane;
> +
> +	plane = plane_item_to_vkms_configfs_plane(item);
> +	int ret = 0;
> +	const char *end_page = page + count;
> +
> +	scoped_guard(mutex, &plane->dev->lock) {
> +		while (1) {
> +			char *tmp;
> +			char fmt[4] = {' ', ' ', ' ', ' '};
> +			int len = vkms_configfs_parse_next_format(page, end_page, &tmp);
> +
> +			// No fourcc code found
> +			if (len <= 1 || len > 5)
> +				break;
> +
> +			page = tmp + len;
> +			memcpy(fmt, &tmp[1], min(len - 1, 4));
> +			if (tmp[0] == '+') {
> +				if (fmt[0] == '*') {
> +					ret = vkms_config_plane_add_all_formats(plane->config);
> +					if (ret)
> +						return ret;
> +				} else {
> +					ret = vkms_config_plane_add_format(plane->config,
> +									   *(int *)fmt);
> +					if (ret)
> +						return ret;
> +				}
> +			} else if (tmp[0] == '-') {
> +				if (fmt[0] == '*')
> +					vkms_config_plane_remove_all_formats(plane->config);
> +				else
> +					vkms_config_plane_remove_format(plane->config, *(int *)fmt);
> +			}
> +		}
> +	}
> +
> +	return count;
> +}
> +
>  CONFIGFS_ATTR(plane_, type);
>  CONFIGFS_ATTR(plane_, name);
>  CONFIGFS_ATTR(plane_, supported_rotations);
> @@ -636,6 +752,7 @@ CONFIGFS_ATTR(plane_, supported_color_ranges);
>  CONFIGFS_ATTR(plane_, default_color_range);
>  CONFIGFS_ATTR(plane_, supported_color_encodings);
>  CONFIGFS_ATTR(plane_, default_color_encoding);
> +CONFIGFS_ATTR(plane_, supported_formats);
>  
>  static struct configfs_attribute *plane_item_attrs[] = {
>  	&plane_attr_type,
> @@ -646,6 +763,7 @@ static struct configfs_attribute *plane_item_attrs[] = {
>  	&plane_attr_default_color_range,
>  	&plane_attr_supported_color_encodings,
>  	&plane_attr_default_color_encoding,
> +	&plane_attr_supported_formats,
>  	NULL,
>  };
>  
> diff --git a/drivers/gpu/drm/vkms/vkms_configfs.h b/drivers/gpu/drm/vkms/vkms_configfs.h
> index e9020b0043db..2774655bfcc5 100644
> --- a/drivers/gpu/drm/vkms/vkms_configfs.h
> +++ b/drivers/gpu/drm/vkms/vkms_configfs.h
> @@ -2,7 +2,13 @@
>  #ifndef _VKMS_CONFIGFS_H_
>  #define _VKMS_CONFIGFS_H_
>  
> +#include <linux/types.h>
> +
>  int vkms_configfs_register(void);
>  void vkms_configfs_unregister(void);
>  
> +#if IS_ENABLED(CONFIG_KUNIT)
> +int vkms_configfs_parse_next_format(const char *page, const char *end_page, char **out);
> +#endif
> +
>  #endif /* _VKMS_CONFIGFS_H_ */
> 
> -- 
> 2.51.2
> 

Powered by blists - more mailing lists