Hi Geert Uytterhoeven,
Thanks for the feedback.
> Subject: Re: [PATCH v11 3/4] drm: renesas: Add RZ/G2L DU Support
>
> Hi Biju,
>
> On Mon, Oct 2, 2023 at 2:28 PM Biju Das <biju.das...@bp.renesas.com>
> wrote:
> > The LCD controller is composed of Frame Compression Processor (FCPVD),
> > Video Signal Processor (VSPD), and Display Unit (DU).
> >
> > It has DPI/DSI interfaces and supports a maximum resolution of 1080p
> > along with 2 RPFs to support the blending of two picture layers and
> > raster operations (ROPs).
> >
> > The DU module is connected to VSPD. Add RZ/G2L DU support for RZ/G2L
> > alike SoCs.
> >
> > Signed-off-by: Biju Das <biju.das...@bp.renesas.com>
>
> Thanks for your patch!
>
> > v9->v10:
>
> > * Added rzg2l_du_write() wrapper function.
>
> > --- /dev/null
> > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c
>
> > +static inline void rzg2l_du_write(struct rzg2l_du_device *rcdu, u32
> > +reg, u32 data) {
> > + writel(data, rcdu->mmio + reg); }
>
> What is the added value of this wrapper?
I think, for debugging we can add some prints here and check reg values. Other
than I don't see
any benefit here. Laurent/ Jacopo please confirm.
> The order of the reg/data parameters is the inverse of the standard
> writel() operation...
OK.
>
> > + rzg2l_du_write(rcdu, DU_DITR0, ditr0);
>
> ... and using it actually needs one more keystroke than open-coding it:
>
> - writel(ditr0, rcdu->mmio + DU_DITR0);
>
> Sorry for missing this before.
I have changed this based on review comment from Laurent and Jacopo. If the
wrapper
is not adding value, I am happy to use writel instead.
Please confirm.
Cheers,
Biju
