Re: [PATCH i-g-t 12/39] lib/vkms: Test CRTC invalid values

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

Le 18/02/2025 à 17:49, José Expósito a écrit :
> For a VKMS CRTC, it is only possible to set invalid values in the
> writeback connector status.
> 
> Test that setting wrong values fails and that the CRTC status is not
> accidentally changed.
> 
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> ---
>   lib/igt_vkms.c             | 20 ++++++++++++++++++++
>   lib/igt_vkms.h             |  2 ++
>   tests/vkms/vkms_configfs.c | 27 +++++++++++++++++++++++++++
>   3 files changed, 49 insertions(+)
> 
> diff --git a/lib/igt_vkms.c b/lib/igt_vkms.c
> index 4021f9aea..d881a2417 100644
> --- a/lib/igt_vkms.c
> +++ b/lib/igt_vkms.c
> @@ -396,3 +396,23 @@ bool igt_vkms_crtc_is_writeback_enabled(igt_vkms_t *dev, const char *name)
>   
>   	return read_bool(path);
>   }
> +
> +/**
> + * igt_vkms_crtc_set_writeback_enabled:
> + * @dev: Device the CRTC belongs to
> + * @name: CRTC name
> + * @writeback: Enable or disable the writeback connector
> + *
> + * Set the VKMS CRTC writeback connector is status.
> + */
> +void igt_vkms_crtc_set_writeback_enabled(igt_vkms_t *dev, const char *name,
> +					 bool writeback)
> +{
> +	char path[PATH_MAX];
> +
> +	snprintf(path, sizeof(path), "%s/%s/%s/%s", dev->path,
> +		 get_pipeline_item_dir_name(VKMS_PIPELINE_ITEM_CRTC), name,
> +		 VKMS_FILE_CRTC_WRITEBACK);
> +
> +	write_bool(path, writeback);

Can fail silently because of the implementation of write_int.

> +}
> diff --git a/lib/igt_vkms.h b/lib/igt_vkms.h
> index 59f6a9775..cb0278544 100644
> --- a/lib/igt_vkms.h
> +++ b/lib/igt_vkms.h
> @@ -35,5 +35,7 @@ void igt_vkms_plane_set_type(igt_vkms_t *dev, const char *name, int type);
>   
>   void igt_vkms_device_add_crtc(igt_vkms_t *dev, const char *name);
>   bool igt_vkms_crtc_is_writeback_enabled(igt_vkms_t *dev, const char *name);
> +void igt_vkms_crtc_set_writeback_enabled(igt_vkms_t *dev, const char *name,
> +					 bool writeback);
>   
>   #endif /* __IGT_VKMS_H__ */
> diff --git a/tests/vkms/vkms_configfs.c b/tests/vkms/vkms_configfs.c
> index 95779d336..af42e364d 100644
> --- a/tests/vkms/vkms_configfs.c
> +++ b/tests/vkms/vkms_configfs.c
> @@ -319,6 +319,32 @@ static void test_crtc_default_values(void)
>   	igt_vkms_device_destroy(dev);
>   }
>   
> +/**
> + * SUBTEST: crtc-wrong-values
> + * Description: Check that setting unexpected values doesn't work.
> + */
> +
> +static void test_crtc_wrong_values(void)
> +{
> +	igt_vkms_t *dev;
> +	char path[PATH_MAX];
> +
> +	/* Test invalid values for "writeback" */
> +	dev = igt_vkms_device_create(__func__);
> +	igt_assert(dev);
> +
> +	igt_vkms_device_add_crtc(dev, "crtc0");
> +	igt_vkms_crtc_set_writeback_enabled(dev, "crtc0", true);
> +	igt_assert(igt_vkms_crtc_is_writeback_enabled(dev, "crtc0"));
> +
> +	snprintf(path, sizeof(path), "%s/crtcs/crtc0/writeback", dev->path);
> +
> +	assert_wrong_bool_values(path);
> +	igt_assert(igt_vkms_crtc_is_writeback_enabled(dev, "crtc0"));
> +
> +	igt_vkms_device_destroy(dev);
> +}
> +
>   igt_main
>   {
>   	struct {
> @@ -333,6 +359,7 @@ igt_main
>   		{ "plane-wrong-values", test_plane_wrong_values },
>   		{ "crtc-default-files", test_crtc_default_files },
>   		{ "crtc-default-values", test_crtc_default_values },
> +		{ "crtc-wrong-values", test_crtc_wrong_values },
>   	};
>   
>   	igt_fixture {

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