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

[Laurent Pinchart]

On Fri, Aug 04, 2023 at 02:53:17AM +0300, Laurent Pinchart wrote:
> 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 at 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.

Replying to myself again, the datasheet doesn't explicitly list
DRM_FORMAT_XRGB2101010 as supported, but the generic mechanism to
specify the location of the components should work fine for that format.
Is this something you would be able to test ?

> 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