Re: [PATCH 2/3] media: vsp1: add blend mode support

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Hayama-san,

Thank you for the patch.

On Mon, Jul 04, 2022 at 11:52:30AM +0900, Takanari Hayama wrote:
> To support DRM blend mode in R-Car DU driver, we must add blend mode
> support in VSP1. Although VSP1 hardware is capable to support all blend
> mode defined in DRM, the current R-Car DU driver implicitly supports
> DRM_MODE_BLEND_COVERAGE only.
> 
> We add a new property to vsp1_du_atomic_config, so that R-Car DU driver
> can pass the desired blend mode.
> 
> Signed-off-by: Takanari Hayama <taki@igel.co.jp>
> ---
>  drivers/media/platform/renesas/vsp1/vsp1_drm.c | 11 +++++++++++
>  include/media/vsp1.h                           | 14 ++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> index 9ec3ac835987..ed0cf552fce2 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> @@ -861,6 +861,17 @@ int vsp1_du_atomic_update(struct device *dev, unsigned int pipe_index,
>  	vsp1->drm->inputs[rpf_index].compose = cfg->dst;
>  	vsp1->drm->inputs[rpf_index].zpos = cfg->zpos;
>  
> +	switch (cfg->blend_mode) {
> +	case VSP1_DU_BLEND_MODE_PREMULTI:
> +		rpf->format.flags = V4L2_PIX_FMT_FLAG_PREMUL_ALPHA;
> +		break;
> +	case VSP1_DU_BLEND_MODE_PIXEL_NONE:
> +		rpf->pixel_alpha = false;
> +		fallthrough;
> +	case VSP1_DU_BLEND_MODE_COVERAGE:
> +		rpf->format.flags = 0;
> +	}

This should work, but wouldn't it be simpler to override the format
passed in cfg->pixelformat in rcar_du_vsp_plane_setup() with the
non-alpha variant when state->state.pixel_blend_mode is set to
DRM_MODE_BLEND_PIXEL_NONE ? That way you could drop rpf->pixel_alpha,
turn cfg->blend_mode into a premult bool flag, and drop the
vsp1_du_blend_mode enum. There's only three formats with an alpha
channel that the rcar-du driver supports (DRM_FORMAT_ARGB4444,
DRM_FORMAT_ARGB1555 and DRM_FORMAT_ARGB8888), so the override could be
as simple as a switch (state->format->fourcc) when the blend mode is
NONE.

> +
>  	drm_pipe->pipe.inputs[rpf_index] = rpf;
>  
>  	return 0;
> diff --git a/include/media/vsp1.h b/include/media/vsp1.h
> index cc1b0d42ce95..1ba7459b7a06 100644
> --- a/include/media/vsp1.h
> +++ b/include/media/vsp1.h
> @@ -42,6 +42,18 @@ struct vsp1_du_lif_config {
>  int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
>  		      const struct vsp1_du_lif_config *cfg);
>  
> +/**
> + * enum vsp1_du_blend_mode - Pixel blend mode
> + * @VSP1_DU_BLEND_MODE_PREMULTI: Pixel alpha is pre-mutiplied
> + * @VSP1_DU_BLEND_MODE_COVERAGE: Pixel alpha is not pre-mutiplied
> + * @VSP1_DU_BLEND_MODE_PIXEL_NONE: Ignores the pixel alpha
> + */
> +enum vsp1_du_blend_mode {
> +	VSP1_DU_BLEND_MODE_PREMULTI,
> +	VSP1_DU_BLEND_MODE_COVERAGE,
> +	VSP1_DU_BLEND_MODE_PIXEL_NONE,
> +};
> +
>  /**
>   * struct vsp1_du_atomic_config - VSP atomic configuration parameters
>   * @pixelformat: plane pixel format (V4L2 4CC)
> @@ -51,6 +63,7 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
>   * @dst: destination rectangle on the display (integer coordinates)
>   * @alpha: alpha value (0: fully transparent, 255: fully opaque)
>   * @zpos: Z position of the plane (from 0 to number of planes minus 1)
> + * @blend_mode: Pixel blend mode of the plane
>   */
>  struct vsp1_du_atomic_config {
>  	u32 pixelformat;
> @@ -60,6 +73,7 @@ struct vsp1_du_atomic_config {
>  	struct v4l2_rect dst;
>  	unsigned int alpha;
>  	unsigned int zpos;
> +	enum vsp1_du_blend_mode blend_mode;
>  };
>  
>  /**

-- 
Regards,

Laurent Pinchart