Re: [PATCH i-g-t 07/39] lib/vkms: Test plane default files

[Louis Chauvet <louis.chauvet@bootlin.com>]

Le 18/02/2025 à 17:49, José Expósito a écrit :
> Add a helper to create a plane and a test checking that the default
> files and directories are created.
> 
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> ---
>   lib/igt_vkms.c             | 52 ++++++++++++++++++++++++++++++++++++++
>   lib/igt_vkms.h             |  2 ++
>   tests/vkms/vkms_configfs.c | 33 ++++++++++++++++++++++++
>   3 files changed, 87 insertions(+)
> 
> diff --git a/lib/igt_vkms.c b/lib/igt_vkms.c
> index 1fdfb09f7..a7b395a71 100644
> --- a/lib/igt_vkms.c
> +++ b/lib/igt_vkms.c
> @@ -24,6 +24,13 @@
>   #define VKMS_ROOT_DIR_NAME		"vkms"
>   #define VKMS_FILE_ENABLED		"enabled"
>   
> +enum vkms_pipeline_item {
> +	VKMS_PIPELINE_ITEM_PLANE,
> +	VKMS_PIPELINE_ITEM_CRTC,
> +	VKMS_PIPELINE_ITEM_ENCODER,
> +	VKMS_PIPELINE_ITEM_CONNECTOR,
> +};
> +
>   /**
>    * SECTION:igt_vkms
>    * @short_description: Helpers to create and configure VKMS devices
> @@ -93,6 +100,39 @@ static void write_bool(const char *path, bool value)
>   	write_int(path, value ? 1 : 0);
>   }
>   
> +static const char *get_pipeline_item_dir_name(enum vkms_pipeline_item item)
> +{
> +	switch (item) {
> +	case VKMS_PIPELINE_ITEM_PLANE:
> +		return "planes";
> +	case VKMS_PIPELINE_ITEM_CRTC:
> +		return "crtcs";
> +	case VKMS_PIPELINE_ITEM_ENCODER:
> +		return "encoders";
> +	case VKMS_PIPELINE_ITEM_CONNECTOR:
> +		return "connectors";
> +	}
> +
> +	igt_assert(!"Cannot be reached: Unknown VKMS pipeline item type");
> +}
> +
> +static void add_pipeline_item(igt_vkms_t *dev, enum vkms_pipeline_item item,
> +			      const char *name)
> +{
> +	const char *item_dir_name;
> +	char path[PATH_MAX];
> +	int ret;
> +
> +	item_dir_name = get_pipeline_item_dir_name(item);
> +	snprintf(path, sizeof(path), "%s/%s/%s", dev->path, item_dir_name,
> +		 name);

Again, it costs nothing to add a check igt_assert_le(snprintf, 
PATH_MAX). Same for all the snprintf in other patches.

For the rest of the patches (8..39), my R-by are valid with or without 
this change.

> +
> +	ret = mkdir(path, 0777);
> +	igt_assert_f(ret == 0,
> +		     "Unable to mkdir directory '%s'. Got errno=%d (%s)\n",
> +		     path, errno, strerror(errno));
> +}
> +
>   /**
>    * igt_require_vkms_configfs:
>    *
> @@ -269,3 +309,15 @@ void igt_vkms_device_set_enabled(igt_vkms_t *dev, bool enabled)
>   
>   	write_bool(path, enabled);
>   }
> +
> +/**
> + * igt_vkms_device_add_plane:
> + * @dev: Device to add the plane to
> + * @name: Plane name
> + *
> + * Add a new plane to the VKMS device.
> + */
> +void igt_vkms_device_add_plane(igt_vkms_t *dev, const char *name)
> +{
> +	add_pipeline_item(dev, VKMS_PIPELINE_ITEM_PLANE, name);
> +}
> diff --git a/lib/igt_vkms.h b/lib/igt_vkms.h
> index dab51852a..9e16ae7eb 100644
> --- a/lib/igt_vkms.h
> +++ b/lib/igt_vkms.h
> @@ -29,4 +29,6 @@ void igt_vkms_destroy_all_devices(void);
>   bool igt_vkms_device_is_enabled(igt_vkms_t *dev);
>   void igt_vkms_device_set_enabled(igt_vkms_t *dev, bool enabled);
>   
> +void igt_vkms_device_add_plane(igt_vkms_t *dev, const char *name);
> +
>   #endif /* __IGT_VKMS_H__ */
> diff --git a/tests/vkms/vkms_configfs.c b/tests/vkms/vkms_configfs.c
> index 79023359a..ec334641f 100644
> --- a/tests/vkms/vkms_configfs.c
> +++ b/tests/vkms/vkms_configfs.c
> @@ -170,6 +170,38 @@ static void test_device_wrong_values(void)
>   	igt_vkms_device_destroy(dev);
>   }
>   
> +/**
> + * SUBTEST: plane-default-files
> + * Description: Test that creating a plane creates the default files and
> + *              directories.
> + */
> +
> +static void test_plane_default_files(void)
> +{
> +	igt_vkms_t *dev;
> +	char path[PATH_MAX];
> +
> +	const char *files[] = {
> +		"type",
> +	};
> +
> +	const char *dirs[] = {
> +		"possible_crtcs",
> +	};
> +
> +	dev = igt_vkms_device_create(__func__);
> +	igt_assert(dev);
> +
> +	igt_vkms_device_add_plane(dev, "plane0");
> +
> +	snprintf(path, sizeof(path), "%s/planes/plane0", dev->path);
> +	assert_default_files(path,
> +			     files, ARRAY_SIZE(files),
> +			     dirs, ARRAY_SIZE(dirs));
> +
> +	igt_vkms_device_destroy(dev);
> +}
> +
>   igt_main
>   {
>   	struct {
> @@ -179,6 +211,7 @@ igt_main
>   		{ "device-default-files", test_device_default_files },
>   		{ "device-default-values", test_device_default_values },
>   		{ "device-wrong-values", test_device_wrong_values },
> +		{ "plane-default-files", test_plane_default_files },
>   	};
>   
>   	igt_fixture {

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com