Hello Hayama-san,
Thank you for the patch (and sorry for the long delay).
On Mon, Jul 04, 2022 at 11:52:29AM +0900, Takanari Hayama wrote:
> In order to support DRM blend mode, we need to able to override pixel
> alpha flag. When DRM_MODE_BLEND_PIXEL_NONE is desired, we'd like to let
> VSP1 to ignore the pixel alpha by overriding the flag.
>
> Signed-off-by: Takanari Hayama <t...@igel.co.jp>
> ---
> drivers/media/platform/renesas/vsp1/vsp1_drm.c | 1 +
> drivers/media/platform/renesas/vsp1/vsp1_rpf.c | 6 +++---
> drivers/media/platform/renesas/vsp1/vsp1_rwpf.h | 2 ++
> 3 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> index 0c2507dc03d6..9ec3ac835987 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
> @@ -600,6 +600,7 @@ static int vsp1_du_pipeline_set_rwpf_format(struct
> vsp1_device *vsp1,
> rwpf->format.num_planes = fmtinfo->planes;
> rwpf->format.plane_fmt[0].bytesperline = pitch;
> rwpf->format.plane_fmt[1].bytesperline = pitch / chroma_hsub;
> + rwpf->pixel_alpha = fmtinfo->alpha;
>
> return 0;
> }
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> index 75083cb234fe..e6bd813dc68c 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> @@ -152,13 +152,13 @@ static void rpf_configure_stream(struct vsp1_entity
> *entity,
> * In all cases, disable color keying.
> */
> vsp1_rpf_write(rpf, dlb, VI6_RPF_ALPH_SEL, VI6_RPF_ALPH_SEL_AEXT_EXT |
> - (fmtinfo->alpha ? VI6_RPF_ALPH_SEL_ASEL_PACKED
> - : VI6_RPF_ALPH_SEL_ASEL_FIXED));
> + (rpf->pixel_alpha ? VI6_RPF_ALPH_SEL_ASEL_PACKED
> + : VI6_RPF_ALPH_SEL_ASEL_FIXED));
>
> if (entity->vsp1->info->gen == 3) {
> u32 mult;
>
> - if (fmtinfo->alpha) {
> + if (rpf->pixel_alpha) {
> /*
> * When the input contains an alpha channel enable the
> * alpha multiplier. If the input is premultiplied we
>From a DRM point of view this looks fine.
vsp1_du_pipeline_set_rwpf_format() is called upon .atomic_update(), and
rpf_configure_stream() upon .atomic_flush(), and every update is
guaranteed to be followed by a flush (through code paths that are way
too convoluted, but that's a story for a different day).
However, VSP instances that are exposed through V4L2 will see a
regression with this patch. rwpf->pixel_alpha will never be set, so
alpha-enabled formats will break. Several test case from
https://git.ideasonboard.com/renesas/vsp-tests.git report failures, for
instance
# ./vsp-unit-test-0001.sh
Testing WPF packing in RGB332: pass
Testing WPF packing in ARGB555: pass
Testing WPF packing in XRGB555: pass
Testing WPF packing in RGB565: pass
Testing WPF packing in BGR24: pass
Testing WPF packing in RGB24: pass
Testing WPF packing in ABGR32: fail
Testing WPF packing in ARGB32: fail
Testing WPF packing in XBGR32: pass
Testing WPF packing in XRGB32: pass
It seems that this could be fixed fairly easily by setting
rpf->pixel_alpha in vsp1_video_set_format():
diff --git a/drivers/media/platform/renesas/vsp1/vsp1_video.c
b/drivers/media/platform/renesas/vsp1/vsp1_video.c
index 51219b1b6ea9..b518873d3280 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_video.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_video.c
@@ -1015,6 +1015,7 @@ vsp1_video_set_format(struct file *file, void *fh, struct
v4l2_format *format)
video->rwpf->format = format->fmt.pix_mp;
video->rwpf->fmtinfo = info;
+ video->rwpf->pixel_alpha = info->alpha;
done:
mutex_unlock(&video->lock);
I've tested that and it fixes the tests, so with this change,
Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
No need to submit a v2 if no other changes are needed in subsequent
patches, I can apply this change when merging the patches.
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
> b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
> index eac5c04c2239..07ddebb78dfa 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h
> @@ -43,6 +43,8 @@ struct vsp1_rwpf {
> const struct vsp1_format_info *fmtinfo;
> unsigned int brx_input;
>
> + bool pixel_alpha;
> +
> unsigned int alpha;
>
> u32 mult_alpha;
--
Regards,
Laurent Pinchart
