Re: [PATCH 05/22] drm/vkms: Introduce config for plane color encoding

["José Expósito" <jose.exposito89@gmail.com>]

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
>