Re: [PATCH 1/2] drm: rcar-du: Add more formats to DRM_MODE_BLEND_PIXEL_NONE support

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

On Fri, Aug 04, 2023 at 02:47:04AM +0300, Laurent Pinchart wrote:
> Hi Damian,
> 
> Thank you for the patch.
> 
> On Fri, Jul 28, 2023 at 04:07:13PM -0400, Damian Hobson-Garcia wrote:
> > Add additional pixel formats for which blending is disabling when
> 
> Did you mean "disabled" instead of "disabling" ?
> 
> > DRM_MODE_BLEND_PIXEL_NONE is set.
> > 
> > Refactor the fourcc selection into a separate function to handle the
> > increased number of formats.
> > 
> > Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> > ---
> >  drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c | 49 ++++++++++++-------
> >  1 file changed, 32 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > index 45c05d0ffc70..96241c03b60f 100644
> > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_vsp.c
> > @@ -176,6 +176,37 @@ static const u32 rcar_du_vsp_formats_gen4[] = {
> >  	DRM_FORMAT_Y212,
> >  };
> >  
> > +static u32 rcar_du_vsp_state_get_format(struct rcar_du_vsp_plane_state *state)
> > +{
> > +	u32 fourcc = state->format->fourcc;
> > +
> > +	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > +		switch (fourcc) {
> > +		case DRM_FORMAT_ARGB1555:
> > +			fourcc = DRM_FORMAT_XRGB1555;
> > +			break;
> > +
> > +		case DRM_FORMAT_ARGB4444:
> > +			fourcc = DRM_FORMAT_XRGB4444;
> > +			break;
> > +
> > +		case DRM_FORMAT_ARGB8888:
> > +			fourcc = DRM_FORMAT_XRGB8888;
> > +			break;
> > +
> > +		case DRM_FORMAT_BGRA8888:
> > +			fourcc = DRM_FORMAT_BGRX8888;
> > +			break;
> > +
> > +		case DRM_FORMAT_RGBA1010102:
> > +			fourcc = DRM_FORMAT_RGBX1010102;
> > +			break;
> 
> Should DRM_FORMAT_ARGB2101010 be added as well, or did you leave it out
> intentionally ?

It looks like DRM_FORMAT_ARGB2101010 will require a bit more work, as
DRM_FORMAT_XRGB2101010 is not handled by the DU driver at the moment.
Let's do so with a patch on top of this series. There's no need to send
a v2, I can handle the simple change in the commit message if you let me
know whether my comment is right or wrong.

> > +		}
> > +	}
> > +
> > +	return fourcc;
> > +}
> > +
> >  static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> >  {
> >  	struct rcar_du_vsp_plane_state *state =
> > @@ -189,7 +220,7 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> >  		.alpha = state->state.alpha >> 8,
> >  		.zpos = state->state.zpos,
> >  	};
> > -	u32 fourcc = state->format->fourcc;
> > +	u32 fourcc = rcar_du_vsp_state_get_format(state);
> >  	unsigned int i;
> >  
> >  	cfg.src.left = state->state.src.x1 >> 16;
> > @@ -206,22 +237,6 @@ static void rcar_du_vsp_plane_setup(struct rcar_du_vsp_plane *plane)
> >  		cfg.mem[i] = sg_dma_address(state->sg_tables[i].sgl)
> >  			   + fb->offsets[i];
> >  
> > -	if (state->state.pixel_blend_mode == DRM_MODE_BLEND_PIXEL_NONE) {
> > -		switch (fourcc) {
> > -		case DRM_FORMAT_ARGB1555:
> > -			fourcc = DRM_FORMAT_XRGB1555;
> > -			break;
> > -
> > -		case DRM_FORMAT_ARGB4444:
> > -			fourcc = DRM_FORMAT_XRGB4444;
> > -			break;
> > -
> > -		case DRM_FORMAT_ARGB8888:
> > -			fourcc = DRM_FORMAT_XRGB8888;
> > -			break;
> > -		}
> > -	}
> > -
> >  	format = rcar_du_format_info(fourcc);
> >  	cfg.pixelformat = format->v4l2;
> >  

-- 
Regards,

Laurent Pinchart