Hi Jacopo,
> Subject: Re: [PATCH v10 3/4] drm: renesas: Add RZ/G2L DU Support
>
> Hi Biju
>
> On Mon, Sep 18, 2023 at 08:09:58AM +0000, Biju Das wrote:
> > Hi Jacopo Mondi,
> >
> > Looks like you are happy with my response for V10. I will send V11 soon.
>
> Sorry for the late reply..
>
> See below, I only see two "controversial" points
>
> >
> > Cheers,
> > Biju
> >
> > > -----Original Message-----
> > > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > Sent: Friday, September 8, 2023 2:24 PM
> > > Subject: RE: [PATCH v10 3/4] drm: renesas: Add RZ/G2L DU Support
> > >
>
> [snip]
>
> > > >
> > > > > +
> > > > > + ditr0 = (DU_DITR0_DEMD_HIGH
> > > >
> > > > I see most registers definition in rzg2l_du_regs.h being only used
> > > > by the crtc driver (some of them are not even used). Why a
> > > > separate header file ?
> > >
> > > For consistency I added header file similar to R-Car. Please let me
> > > know this to be added in .c ?
> > >
>
>
> I would say up to you, as R-Car does the same. In general, if a symbol
> doesn't need to be exported to any other file, it could very well live in
> the .c file.
Agreed.
>
> > > >
> > > > > + | ((mode->flags & DRM_MODE_FLAG_PVSYNC) ?
> DU_DITR0_VSPOL : 0)
> > > > > + | ((mode->flags & DRM_MODE_FLAG_PHSYNC) ?
> DU_DITR0_HSPOL :
>
> [snip]
>
> > > > ---------
> > > > > + * Format helpers
> > > > > + */
> > > > > +
> > > > > +static const struct rzg2l_du_format_info rzg2l_du_format_infos[] =
> {
> > > > > + {
> > > > > + .fourcc = DRM_FORMAT_RGB565,
> > > > > + .v4l2 = V4L2_PIX_FMT_RGB565,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_ARGB1555,
> > > > > + .v4l2 = V4L2_PIX_FMT_ARGB555,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_XRGB1555,
> > > > > + .v4l2 = V4L2_PIX_FMT_XRGB555,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_XRGB8888,
> > > > > + .v4l2 = V4L2_PIX_FMT_XBGR32,
> > > > > + .bpp = 32,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_ARGB8888,
> > > > > + .v4l2 = V4L2_PIX_FMT_ABGR32,
> > > > > + .bpp = 32,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_UYVY,
> > > > > + .v4l2 = V4L2_PIX_FMT_UYVY,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 2,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_YUYV,
> > > > > + .v4l2 = V4L2_PIX_FMT_YUYV,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 2,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_NV12,
> > > > > + .v4l2 = V4L2_PIX_FMT_NV12M,
> > > > > + .bpp = 12,
> > > > > + .planes = 2,
> > > > > + .hsub = 2,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_NV21,
> > > > > + .v4l2 = V4L2_PIX_FMT_NV21M,
> > > > > + .bpp = 12,
> > > > > + .planes = 2,
> > > > > + .hsub = 2,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_NV16,
> > > > > + .v4l2 = V4L2_PIX_FMT_NV16M,
> > > > > + .bpp = 16,
> > > > > + .planes = 2,
> > > > > + .hsub = 2,
> > > > > + },
> > > > > + {
> > > > > + .fourcc = DRM_FORMAT_RGB332,
> > > > > + .v4l2 = V4L2_PIX_FMT_RGB332,
> > > > > + .bpp = 8,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_ARGB4444,
> > > > > + .v4l2 = V4L2_PIX_FMT_ARGB444,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_XRGB4444,
> > > > > + .v4l2 = V4L2_PIX_FMT_XRGB444,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_RGBA4444,
> > > > > + .v4l2 = V4L2_PIX_FMT_RGBA444,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_RGBX4444,
> > > > > + .v4l2 = V4L2_PIX_FMT_RGBX444,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_ABGR4444,
> > > > > + .v4l2 = V4L2_PIX_FMT_ABGR444,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_XBGR4444,
> > > > > + .v4l2 = V4L2_PIX_FMT_XBGR444,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_BGRA4444,
> > > > > + .v4l2 = V4L2_PIX_FMT_BGRA444,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_BGRX4444,
> > > > > + .v4l2 = V4L2_PIX_FMT_BGRX444,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_RGBA5551,
> > > > > + .v4l2 = V4L2_PIX_FMT_RGBA555,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_RGBX5551,
> > > > > + .v4l2 = V4L2_PIX_FMT_RGBX555,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_ABGR1555,
> > > > > + .v4l2 = V4L2_PIX_FMT_ABGR555,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_XBGR1555,
> > > > > + .v4l2 = V4L2_PIX_FMT_XBGR555,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_BGRA5551,
> > > > > + .v4l2 = V4L2_PIX_FMT_BGRA555,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_BGRX5551,
> > > > > + .v4l2 = V4L2_PIX_FMT_BGRX555,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_BGR888,
> > > > > + .v4l2 = V4L2_PIX_FMT_RGB24,
> > > > > + .bpp = 24,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_RGB888,
> > > > > + .v4l2 = V4L2_PIX_FMT_BGR24,
> > > > > + .bpp = 24,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_RGBA8888,
> > > > > + .v4l2 = V4L2_PIX_FMT_BGRA32,
> > > > > + .bpp = 32,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_RGBX8888,
> > > > > + .v4l2 = V4L2_PIX_FMT_BGRX32,
> > > > > + .bpp = 32,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_ABGR8888,
> > > > > + .v4l2 = V4L2_PIX_FMT_RGBA32,
> > > > > + .bpp = 32,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_XBGR8888,
> > > > > + .v4l2 = V4L2_PIX_FMT_RGBX32,
> > > > > + .bpp = 32,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_BGRA8888,
> > > > > + .v4l2 = V4L2_PIX_FMT_ARGB32,
> > > > > + .bpp = 32,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_BGRX8888,
> > > > > + .v4l2 = V4L2_PIX_FMT_XRGB32,
> > > > > + .bpp = 32,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_RGBX1010102,
> > > > > + .v4l2 = V4L2_PIX_FMT_RGBX1010102,
> > > > > + .bpp = 32,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_RGBA1010102,
> > > > > + .v4l2 = V4L2_PIX_FMT_RGBA1010102,
> > > > > + .bpp = 32,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_ARGB2101010,
> > > > > + .v4l2 = V4L2_PIX_FMT_ARGB2101010,
> > > > > + .bpp = 32,
> > > > > + .planes = 1,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_YVYU,
> > > > > + .v4l2 = V4L2_PIX_FMT_YVYU,
> > > > > + .bpp = 16,
> > > > > + .planes = 1,
> > > > > + .hsub = 2,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_NV61,
> > > > > + .v4l2 = V4L2_PIX_FMT_NV61M,
> > > > > + .bpp = 16,
> > > > > + .planes = 2,
> > > > > + .hsub = 2,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_YUV420,
> > > > > + .v4l2 = V4L2_PIX_FMT_YUV420M,
> > > > > + .bpp = 12,
> > > > > + .planes = 3,
> > > > > + .hsub = 2,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_YVU420,
> > > > > + .v4l2 = V4L2_PIX_FMT_YVU420M,
> > > > > + .bpp = 12,
> > > > > + .planes = 3,
> > > > > + .hsub = 2,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_YUV422,
> > > > > + .v4l2 = V4L2_PIX_FMT_YUV422M,
> > > > > + .bpp = 16,
> > > > > + .planes = 3,
> > > > > + .hsub = 2,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_YVU422,
> > > > > + .v4l2 = V4L2_PIX_FMT_YVU422M,
> > > > > + .bpp = 16,
> > > > > + .planes = 3,
> > > > > + .hsub = 2,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_YUV444,
> > > > > + .v4l2 = V4L2_PIX_FMT_YUV444M,
> > > > > + .bpp = 24,
> > > > > + .planes = 3,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_YVU444,
> > > > > + .v4l2 = V4L2_PIX_FMT_YVU444M,
> > > > > + .bpp = 24,
> > > > > + .planes = 3,
> > > > > + .hsub = 1,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_Y210,
> > > > > + .v4l2 = V4L2_PIX_FMT_Y210,
> > > > > + .bpp = 32,
> > > > > + .planes = 1,
> > > > > + .hsub = 2,
> > > > > + }, {
> > > > > + .fourcc = DRM_FORMAT_Y212,
> > > > > + .v4l2 = V4L2_PIX_FMT_Y212,
> > > > > + .bpp = 32,
> > > > > + .planes = 1,
> > > > > + .hsub = 2,
> > > > > + },
> > > > > +};
> > > >
> > > > I see listed as supported formats in the DU features list
> > > >
> > > > Input data format (from VSPD): RGB888, RGB666 (not supports
> > > > dithering of
> > > > RGB565)
> > > > − Output data format: same as Input data format
> > > >
> > > > Am I missing something ?
> > >
> > > If you see the pipeline below, the Image buffer is read by RPF and
> > > finally rendered to DU as the VSP is the compositor.
> > >
> > > Imagebuffer (YUV) --> RPF(YUV)-->WPF(YUV)-->LIF(RGB)-->DU(RGB)
>
> I see, aren't RPF/WPF and LIF part of VSP ? Why do you need to list YUV
> formats here if the DU only accepts RGB as input ?
Agreed. Will send V11.
Cheers,
Biju
