On Sat, Oct 18, 2025 at 04:01:05AM +0200, Louis Chauvet wrote:
> VKMS driver supports all the color encoding on planes, but for testing it
> can be useful to only advertise few of them. This new configuration
> interface will allow configuring the color encoding per planes.
>
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
> drivers/gpu/drm/vkms/vkms_config.c | 15 +++++++++++++++
> drivers/gpu/drm/vkms/vkms_config.h | 30 ++++++++++++++++++++++++++++++
> drivers/gpu/drm/vkms/vkms_plane.c | 6 ++----
> 3 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_config.c b/drivers/gpu/drm/vkms/vkms_config.c
> index b8cabe6f6cf8..5353719a476d 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.c
> +++ b/drivers/gpu/drm/vkms/vkms_config.c
> @@ -156,6 +156,13 @@ static bool valid_plane_properties(const struct vkms_config *config)
> drm_info(dev, "Configured default rotation is not supported by the plane\n");
> return false;
> }
> +
> + if ((BIT(vkms_config_plane_get_default_color_encoding(plane_cfg)) &
> + vkms_config_plane_get_supported_color_encoding(plane_cfg)) !=
> + BIT(vkms_config_plane_get_default_color_encoding(plane_cfg))) {
Could you document what kind of value should be stored when using
vkms_config_plane_set_default_color_encoding(), please?
I'm confused by the use of BIT() here but not in the next commit
("drm/vkms: Introduce configfs for plane color encoding") during
a similar validation.
> + drm_info(dev, "Configured default color encoding is not supported by the plane\n");
> + return false;
> + }
> }
> return true;
> }
> @@ -375,6 +382,10 @@ static int vkms_config_show(struct seq_file *m, void *data)
> vkms_config_plane_get_supported_rotations(plane_cfg));
> seq_printf(m, "\tdefault rotation: 0x%x\n",
> vkms_config_plane_get_default_rotation(plane_cfg));
> + seq_printf(m, "\tsupported color encoding: 0x%x\n",
> + vkms_config_plane_get_supported_color_encoding(plane_cfg));
> + seq_printf(m, "\tdefault color encoding: %d\n",
Is this one expressed in decimal rather than hex on purpose?
> + vkms_config_plane_get_default_color_encoding(plane_cfg));
> }
>
> vkms_config_for_each_crtc(vkmsdev->config, crtc_cfg) {
> @@ -418,6 +429,10 @@ struct vkms_config_plane *vkms_config_create_plane(struct vkms_config *config)
> vkms_config_plane_set_name(plane_cfg, NULL);
> vkms_config_plane_set_supported_rotations(plane_cfg, DRM_MODE_ROTATE_MASK);
> vkms_config_plane_set_default_rotation(plane_cfg, DRM_MODE_ROTATE_0);
> + vkms_config_plane_set_supported_color_encoding(plane_cfg, BIT(DRM_COLOR_YCBCR_BT601) |
> + BIT(DRM_COLOR_YCBCR_BT709) |
> + BIT(DRM_COLOR_YCBCR_BT2020));
> + vkms_config_plane_set_default_color_encoding(plane_cfg, DRM_COLOR_YCBCR_BT601);
>
> xa_init_flags(&plane_cfg->possible_crtcs, XA_FLAGS_ALLOC);
>
> diff --git a/drivers/gpu/drm/vkms/vkms_config.h b/drivers/gpu/drm/vkms/vkms_config.h
> index 3c3cbefcc67f..11160c3c13bc 100644
> --- a/drivers/gpu/drm/vkms/vkms_config.h
> +++ b/drivers/gpu/drm/vkms/vkms_config.h
> @@ -45,6 +45,8 @@ struct vkms_config {
> * It can be used to store a temporary reference to a VKMS plane during
> * device creation. This pointer is not managed by the configuration and
> * must be managed by other means.
> + * @default_color_encoding: Default color encoding that should be used by this plane
> + * @supported_color_encoding: Color encoding that this plane will support
To be consistent with "supported_rotations", should this prop
be "supported_color_encodings"?
> */
> struct vkms_config_plane {
> struct list_head link;
> @@ -54,6 +56,8 @@ struct vkms_config_plane {
> enum drm_plane_type type;
> unsigned int default_rotation;
> unsigned int supported_rotations;
> + enum drm_color_encoding default_color_encoding;
> + unsigned int supported_color_encoding;
> struct xarray possible_crtcs;
>
> /* Internal usage */
> @@ -344,6 +348,32 @@ vkms_config_plane_set_supported_rotations(struct vkms_config_plane *plane_cfg,
> plane_cfg->supported_rotations = supported_rotations;
> }
>
> +static inline enum drm_color_encoding
> +vkms_config_plane_get_default_color_encoding(struct vkms_config_plane *plane_cfg)
> +{
> + return plane_cfg->default_color_encoding;
> +}
> +
> +static inline void
> +vkms_config_plane_set_default_color_encoding(struct vkms_config_plane *plane_cfg,
> + enum drm_color_encoding default_color_encoding)
> +{
> + plane_cfg->default_color_encoding = default_color_encoding;
> +}
> +
> +static inline unsigned int
> +vkms_config_plane_get_supported_color_encoding(struct vkms_config_plane *plane_cfg)
> +{
> + return plane_cfg->supported_color_encoding;
> +}
> +
> +static inline void
> +vkms_config_plane_set_supported_color_encoding(struct vkms_config_plane *plane_cfg,
> + unsigned int supported_color_encoding)
> +{
> + plane_cfg->supported_color_encoding = supported_color_encoding;
> +}
> +
> /**
> * vkms_config_plane_set_name() - Set the plane name
> * @plane_cfg: Plane to set the name to
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> index 6e6bfe1b7400..5869000415e4 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -239,12 +239,10 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> vkms_config_plane_get_supported_rotations(config));
>
> drm_plane_create_color_properties(&plane->base,
> - BIT(DRM_COLOR_YCBCR_BT601) |
> - BIT(DRM_COLOR_YCBCR_BT709) |
> - BIT(DRM_COLOR_YCBCR_BT2020),
> + vkms_config_plane_get_supported_color_encoding(config),
> BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
> BIT(DRM_COLOR_YCBCR_FULL_RANGE),
> - DRM_COLOR_YCBCR_BT601,
> + vkms_config_plane_get_default_color_encoding(config),
> DRM_COLOR_YCBCR_FULL_RANGE);
>
> return plane;
>
> --
> 2.51.0
>