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 <dhobs...@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
