On 11/02/25 - 12:09, José Expósito wrote:
> From: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
>
> As the configuration will be used by userspace, add a validator to avoid
> creating a broken DRM device.
>
> For the moment, the function always returns true, but rules will be
> added in future patches.
>
> Reviewed-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
> Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
> Co-developed-by: José Expósito <jose.exposito89@xxxxxxxxx>
> Signed-off-by: José Expósito <jose.exposito89@xxxxxxxxx>
The compilation is broken when building as module:
diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
index b9267aef4804..82335006c94a 100644
--- a/drivers/gpu/drm/vkms/vkms_config.c
+++ b/drivers/gpu/drm/vkms/vkms_config.c
@@ -55,6 +55,7 @@ bool vkms_config_is_valid(struct vkms_config *config)
{
return true;
}
+EXPORT_SYMBOL_IF_KUNIT(vkms_config_is_valid);
static int vkms_config_show(struct seq_file *m, void *data)
{
> ---
> drivers/gpu/drm/vkms/tests/vkms_config_test.c | 2 ++
> drivers/gpu/drm/vkms/vkms_config.c | 5 +++++
> drivers/gpu/drm/vkms/vkms_config.h | 10 ++++++++++
> drivers/gpu/drm/vkms/vkms_drv.h | 2 +-
> drivers/gpu/drm/vkms/vkms_output.c | 3 +++
> 5 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vkms/tests/vkms_config_test.c b/drivers/gpu/drm/vkms/tests/vkms_config_test.c
> index 92798590051b..6e07139d261c 100644
> --- a/drivers/gpu/drm/vkms/tests/vkms_config_test.c
> +++ b/drivers/gpu/drm/vkms/tests/vkms_config_test.c
> @@ -54,6 +54,8 @@ static void vkms_config_test_default_config(struct kunit *test)
> KUNIT_EXPECT_EQ(test, config->writeback, params->enable_writeback);
> KUNIT_EXPECT_EQ(test, config->overlay, params->enable_overlay);
>
> + KUNIT_EXPECT_TRUE(test, vkms_config_is_valid(config));
> +
> vkms_config_destroy(config);
> }
>
> diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
> index 11b0e539920b..67f71d29596e 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.c
> +++ b/drivers/gpu/drm/vkms/vkms_config.c
> @@ -47,6 +47,11 @@ void vkms_config_destroy(struct vkms_config *config)
> kfree(config);
> }
>
> +bool vkms_config_is_valid(struct vkms_config *config)
> +{
> + return true;
> +}
> +
> static int vkms_config_show(struct seq_file *m, void *data)
> {
> struct drm_debugfs_entry *entry = m->private;
> diff --git a/drivers/gpu/drm/vkms/vkms_config.h b/drivers/gpu/drm/vkms/vkms_config.h
> index fcaa909fb2e0..0376dceaf6be 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.h
> +++ b/drivers/gpu/drm/vkms/vkms_config.h
> @@ -67,6 +67,16 @@ vkms_config_get_device_name(struct vkms_config *config)
> return config->dev_name;
> }
>
> +/**
> + * vkms_config_is_valid() - Validate a configuration
> + * @config: Configuration to validate
> + *
> + * Returns:
> + * Whether the configuration is valid or not.
> + * For example, a configuration without primary planes is not valid.
> + */
> +bool vkms_config_is_valid(struct vkms_config *config);
> +
I think here we can take a const pointer.
> /**
> * vkms_config_register_debugfs() - Register a debugfs file to show the device's
> * configuration
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index a74a7fc3a056..95afc39ce985 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -204,7 +204,7 @@ struct vkms_config;
> struct vkms_device {
> struct drm_device drm;
> struct platform_device *platform;
> - const struct vkms_config *config;
> + struct vkms_config *config;
So we can keep a const pointer here (for me the device should never modify
vkms_config)
> };
>
> /*
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c
> index 068a7f87ecec..414cc933af41 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -16,6 +16,9 @@ int vkms_output_init(struct vkms_device *vkmsdev)
> int writeback;
> unsigned int n;
>
> + if (!vkms_config_is_valid(vkmsdev->config))
> + return -EINVAL;
> +
> /*
> * Initialize used plane. One primary plane is required to perform the composition.
> *
> --
> 2.48.1
>
