Re: [PATCH v2 4/4] drm/vboxvideo: create blend mode property on planes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The plane's pixel_blend_mode atomic state defaults to DRM_MODE_BLEND_PREMULTI, but the driver only exposes DRM_MODE_BLEND_COVERAGE, resulting in an invalid initial atomic state that causes atomic commits to fail.
--

commit e1e7e08f37513293cd7928eb76a6d3c06de1c985
Author: Qinyun Tan <qinyuntan@linux.alibaba.com>

drm/vboxvideo: create blend mode property on planes

This commit exposes a "pixel blend mode" property advertising only
DRM_MODE_BLEND_COVERAGE for vboxvideo primary and cursor planes to match
host compositing semantics and silence a DRM core warning.

> diff --git a/drivers/gpu/drm/vboxvideo/vbox_mode.c b/drivers/gpu/drm/vboxvideo/vbox_mode.c
> index 8e4e5fc9d3c5a..3c41238a82685 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_mode.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_mode.c
[ ... ]
> @@ -540,6 +541,9 @@ static struct drm_plane *vbox_create_plane(struct vbox_private *vbox,
>  
>  	drm_plane_helper_add(plane, helper_funcs);
>  
> +	drm_plane_create_blend_mode_property(plane,
> +					     BIT(DRM_MODE_BLEND_COVERAGE));
> +

[Severity: High]
Will this cause atomic commits to fail with -EINVAL due to an invalid
initial atomic state?

The blend mode property is restricted here to allow only
DRM_MODE_BLEND_COVERAGE. However, the primary plane uses
drm_atomic_helper_plane_reset() and the cursor plane uses
DRM_GEM_SHADOW_PLANE_FUNCS (which provides drm_gem_reset_shadow_plane()).

Both of these default reset hooks rely on
__drm_atomic_helper_plane_state_reset(), which unconditionally hardcodes
the initial state to PREMULTI:

__drm_atomic_helper_plane_state_reset() {
    ...
    state->pixel_blend_mode = DRM_MODE_BLEND_PREMULTI;
    ...
}

Since the driver does not provide a custom reset hook to override this
default and explicitly set plane_state->pixel_blend_mode to
DRM_MODE_BLEND_COVERAGE, does this leave the initial atomic state set to a
mode (PREMULTI) that isn't supported by the property? 

If generic compositors read this initial state and pass it back during
commits, will the DRM core's validation reject it because the property
only exposes COVERAGE?

>  	return plane;
>  
>  free_plane:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901083234.1828755-1-qinyuntan@linux.alibaba.com?part=4