Re: [PATCH v1 4/4] drm/vkms: Add DRIVER_PLANE_COLOR_PIPELINE driver feature

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Enabling the global plane color pipeline feature breaks color management on configfs planes.
- [High] Implicit BT601 YUV-to-RGB conversion is unconditionally applied before the color pipeline, breaking BT709/BT2020 support.
--

commit 131af6dd345d5805b19305ccac226f92ac37e694
Author: Robert Mader <robert.mader@collabora.com>

drm/vkms: Add DRIVER_PLANE_COLOR_PIPELINE driver feature

Which is now required for DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE. Only
enable it if the enable_plane_pipeline option is enabled so otherwise
the legacy flags COLOR_ENCODING and COLOR_RANGE continue to be
supported.

> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 434c295f44ba6..33caf3c0c671a 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -91,7 +91,8 @@ static void vkms_atomic_commit_tail(struct drm_atomic_state *old_state)
>  }
>  
>  static const struct drm_driver vkms_driver = {
> -	.driver_features	= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM,
> +	.driver_features	= DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_GEM |
> +				  DRIVER_PLANE_COLOR_PIPELINE,

[Severity: High]
When DRIVER_PLANE_COLOR_PIPELINE is enabled, does it cause an implicit
BT601 YUV-to-RGB conversion that breaks BT709/BT2020 support?

Enabling DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE locks color_encoding to BT601.
This causes vkms_plane_atomic_update() to unconditionally populate the
conversion matrix with BT601:

drivers/gpu/drm/vkms/vkms_plane.c:vkms_plane_atomic_update() {
    ...
    get_conversion_matrix_to_argb_u16(fmt, new_state->color_encoding,
                                      new_state->color_range,
                                      &vkms_plane_state->conversion_matrix);
    ...
}

Then in blend_line(), the YUV-to-RGB conversion and clamping occurs before
the color pipeline is executed:

drivers/gpu/drm/vkms/vkms_composer.c:blend_line() {
    ...
    current_plane->pixel_read_line(current_plane, src_x_start, src_y_start,
                                   direction, pixel_count,
                                   &stage_buffer->pixels[dst_x_start]);
    pre_blend_color_transform(current_plane, stage_buffer);
    ...
}

Will this unconditionally convert and clamp YUV buffers to RGB using the
BT601 matrix before the pipeline can process them, making it impossible
to correctly handle BT709 or BT2020 content?

>  	.fops			= &vkms_driver_fops,
>  	DRM_GEM_SHMEM_DRIVER_OPS,
>  	DRM_FBDEV_SHMEM_DRIVER_OPS,

[ ... ]

> @@ -184,6 +185,9 @@ int vkms_create(struct vkms_config *config)
>  	vkms_device->config = config;
>  	config->dev = vkms_device;
>  
> +	if (!enable_plane_pipeline)
> +		vkms_device->drm.driver_features &= ~DRIVER_PLANE_COLOR_PIPELINE;
> +

[Severity: High]
Does enabling this global driver feature break color management for configfs
planes?

When the pipeline feature is enabled, DRM core hides legacy COLOR_ENCODING
and COLOR_RANGE. However, in vkms_config_create_plane(), the pipeline is
hardcoded to false for dynamically created planes:

drivers/gpu/drm/vkms/vkms_config.c:vkms_config_create_plane() {
    ...
    plane_cfg->default_pipeline = false;
    ...
}

Will this cause configfs-created planes to completely lack any color
management properties since they do not get COLOR_PIPELINE initialized?

>  	ret = dma_coerce_mask_and_coherent(vkms_device->drm.dev,
>  					   DMA_BIT_MASK(64));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630084229.529682-1-robert.mader@collabora.com?part=4