On Wed Oct 29, 2025 at 3:36 PM CET, 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 <[email protected]>
> --- a/Documentation/gpu/vkms.rst
> +++ b/Documentation/gpu/vkms.rst
> @@ -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_range
> +- supported_formats: List of supported formats for this plane. To add a new
> item in the
> + list, write it using a plus its fourcc code: +XR24
^ and
> +++ 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
> + * @data: Contains the string to parse
> + * @data_len: data 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 *data;
This is a string, why not calling it 'string', or 'str'?
> +struct vkms_configfs_parse_format_case
> vkms_configfs_parse_format_test_cases[] = {
> + {
> + .data = "+RG24",
> + .data_len = 6,
> + .expected_len = 5,
> + .expected_offset = 0,
> + }, {
> + .data = "-RG24",
> + .data_len = 6,
> + .expected_len = 5,
> + .expected_offset = 0
> + }, {
> + .data = " -RG24",
> + .data_len = 8,
> + .expected_len = 5,
> + .expected_offset = 2
> + }, {
> + .data = "+*",
> + .data_len = 3,
> + .expected_len = 2,
> + .expected_offset = 0
> + }, {
> + .data = "-RG24+RG24",
> + .data_len = 11,
> + .expected_len = 5,
> + .expected_offset = 0
> + }, {
> + .data = "-R1+RG24",
> + .data_len = 9,
> + .expected_len = 3,
> + .expected_offset = 0
> + }, {
> + .data = "\n-R1",
> + .data_len = 5,
> + .expected_len = 3,
> + .expected_offset = 1
> + }, {
> + .data = "-R1111",
> + .data_len = 3,
The string is longer than 3 chars, is this intended?
> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> +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)
> + break;
> +
> + page = tmp + len;
> + memcpy(tmp, &fmt[1], min(len - 1, 4));
Should this be instead: fmt tmp
?
Also I think it would be good to reject strings longer than 4 chars (len >
5), because they cannot br fourccs.
Otherwise looks good.
BTW, I feel your pain in implementing the parsing!
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
