Hi Laurent,
Thank you for reviewing the patch.
> 2022/07/31 7:36、Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>のメール:
>
> 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@xxxxxxxxxx>
>> ---
>> 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.
You’re right. I’ll make the suggested changes and submit V2.
>
>> +
>> 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
Cheers,
Takanari Hayama, Ph.D. <taki@xxxxxxxxxx>
IGEL Co., Ltd.
https://www.igel.co.jp/
