Re: [RFC PATCH v1] drm: rockchip: add drm_plane_create_blend_mode_property

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch incorrectly advertises that all planes only support `DRM_MODE_BLEND_PIXEL_NONE`, but the driver unconditionally performs pre-multiplied alpha blending for formats with an alpha channel on non-primary planes.
--

--- Patch [1]: [RFC PATCH v1] drm: rockchip: add drm_plane_create_blend_mode_property ---
commit b598208b3a4ac16667d04c601badf21d2ed2f848
Author: Johan Jonker <[email protected]>

drm: rockchip: add drm_plane_create_blend_mode_property

This commit fixes warnings in the kernel log by explicitly creating a blend
mode property for the planes. It restricts the supported blend mode to
DRM_MODE_BLEND_PIXEL_NONE.

Link: https://lore.kernel.org/all/[email protected]/

> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 0090d8ff0c79e..0bc5b606f0213 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1850,6 +1850,8 @@ static void vop_plane_add_properties(struct drm_plane *plane,
>  	if (flags)
>  		drm_plane_create_rotation_property(plane, DRM_MODE_ROTATE_0,
>  						   DRM_MODE_ROTATE_0 | flags);
> +	drm_plane_create_blend_mode_property(plane,
> +					     BIT(DRM_MODE_BLEND_PIXEL_NONE));
                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Medium]
Does this incorrectly advertise that all planes only support
DRM_MODE_BLEND_PIXEL_NONE?

Looking at vop_plane_atomic_update() in
drivers/gpu/drm/rockchip/rockchip_drm_vop.c, the driver unconditionally
performs pre-multiplied alpha blending for formats with an alpha channel
on overlay planes:

    if (fb->format->has_alpha && win_index > 0) {
        VOP_WIN_SET(vop, win, dst_alpha_ctl,
                    DST_FACTOR_M0(ALPHA_SRC_INVERSE));
        val = SRC_ALPHA_EN(1) | SRC_COLOR_M0(ALPHA_SRC_PRE_MUL) |
            SRC_ALPHA_M0(ALPHA_STRAIGHT) |
            SRC_BLEND_M0(ALPHA_PER_PIX) |
            SRC_ALPHA_CAL_M0(ALPHA_NO_SATURATION) |
            SRC_FACTOR_M0(ALPHA_ONE);
        VOP_WIN_SET(vop, win, src_alpha_ctl, val);

        VOP_WIN_SET(vop, win, alpha_pre_mul, ALPHA_SRC_PRE_MUL);
        VOP_WIN_SET(vop, win, alpha_mode, ALPHA_PER_PIX);
        VOP_WIN_SET(vop, win, alpha_en, 1);

If userspace compositors query the blend mode property and see only
PIXEL_NONE is supported, they might assume the pixel alpha channel will be
ignored and the plane rendered completely opaque.

If they assign an ARGB8888 buffer to an overlay plane, will the driver
ignore the property's implied opaque semantics and configure the hardware
to perform alpha blending anyway?

>  }
>  
>  static int vop_create_crtc(struct vop *vop)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1