RE: [PATCH v2 4/7] drm: rcar-du: Move rcar_du_output_name() to rcar_du_common.c

[Biju Das <biju.das.jz@bp.renesas.com>]

Hi Laurent,

Thanks for the feedback.

> Subject: Re: [PATCH v2 4/7] drm: rcar-du: Move rcar_du_output_name() to
> rcar_du_common.c
> 
> Hi Biju,
> 
> Thank you for the patch.
> 
> On Wed, Mar 16, 2022 at 01:10:57PM +0000, Biju Das wrote:
> > RZ/G2L SoC's does not have group/plane registers compared to RCar,
> > hence it needs a different CRTC implementation.
> >
> > Move rcar_du_output_name() to a new common file rcar_du_common.c, So
> > that the same function can be reused by RZ/G2L SoC later.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v1->v2:
> >  * No change
> > RFC->v1:
> >  New patch
> > ---
> >  drivers/gpu/drm/rcar-du/Makefile         |  1 +
> >  drivers/gpu/drm/rcar-du/rcar_du_common.c | 30 ++++++++++++++++++++++++
> >  drivers/gpu/drm/rcar-du/rcar_du_drv.c    | 20 ----------------
> >  3 files changed, 31 insertions(+), 20 deletions(-)  create mode
> > 100644 drivers/gpu/drm/rcar-du/rcar_du_common.c
> >
> > diff --git a/drivers/gpu/drm/rcar-du/Makefile
> > b/drivers/gpu/drm/rcar-du/Makefile
> > index e7275b5e7ec8..331e12d65a6b 100644
> > --- a/drivers/gpu/drm/rcar-du/Makefile
> > +++ b/drivers/gpu/drm/rcar-du/Makefile
> > @@ -1,5 +1,6 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  rcar-du-drm-y := rcar_du_crtc.o \
> > +		 rcar_du_common.o \
> >  		 rcar_du_drv.o \
> >  		 rcar_du_encoder.o \
> >  		 rcar_du_group.o \
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_common.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_common.c
> > new file mode 100644
> > index 000000000000..f9f9908cda6d
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_common.c
> > @@ -0,0 +1,30 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * rcar_du_common.c  --  R-Car Display Unit Common
> > + *
> > + * Copyright (C) 2013-2022 Renesas Electronics Corporation
> > + *
> > + * Contact: Laurent Pinchart (laurent.pinchart@ideasonboard.com)
> > + */
> > +
> > +#include "rcar_du_drv.h"
> > +
> > +const char *rcar_du_output_name(enum rcar_du_output output) {
> > +	static const char * const names[] = {
> > +		[RCAR_DU_OUTPUT_DPAD0] = "DPAD0",
> > +		[RCAR_DU_OUTPUT_DPAD1] = "DPAD1",
> > +		[RCAR_DU_OUTPUT_DSI0] = "DSI0",
> > +		[RCAR_DU_OUTPUT_DSI1] = "DSI1",
> > +		[RCAR_DU_OUTPUT_HDMI0] = "HDMI0",
> > +		[RCAR_DU_OUTPUT_HDMI1] = "HDMI1",
> > +		[RCAR_DU_OUTPUT_LVDS0] = "LVDS0",
> > +		[RCAR_DU_OUTPUT_LVDS1] = "LVDS1",
> > +		[RCAR_DU_OUTPUT_TCON] = "TCON",
> > +	};
> > +
> > +	if (output >= ARRAY_SIZE(names) || !names[output])
> > +		return "UNKNOWN";
> > +
> > +	return names[output];
> > +}
> 
> As we have nothing else than this function in this file, how about moving
> it to rcar_du_drv.c instead, to avoid adding a new file ?

OK, since it is small change will add the same in rzg2l_du_drv.c file.

Cheers,
Biju

> You also need to add a declaration for rcar_du_output_name() in the
> appropriate header.
> 
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > index 4640c356a532..f6e234dafb72 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> > @@ -591,26 +591,6 @@ static const struct of_device_id
> > rcar_du_of_table[] = {
> >
> >  MODULE_DEVICE_TABLE(of, rcar_du_of_table);
> >
> > -const char *rcar_du_output_name(enum rcar_du_output output) -{
> > -	static const char * const names[] = {
> > -		[RCAR_DU_OUTPUT_DPAD0] = "DPAD0",
> > -		[RCAR_DU_OUTPUT_DPAD1] = "DPAD1",
> > -		[RCAR_DU_OUTPUT_DSI0] = "DSI0",
> > -		[RCAR_DU_OUTPUT_DSI1] = "DSI1",
> > -		[RCAR_DU_OUTPUT_HDMI0] = "HDMI0",
> > -		[RCAR_DU_OUTPUT_HDMI1] = "HDMI1",
> > -		[RCAR_DU_OUTPUT_LVDS0] = "LVDS0",
> > -		[RCAR_DU_OUTPUT_LVDS1] = "LVDS1",
> > -		[RCAR_DU_OUTPUT_TCON] = "TCON",
> > -	};
> > -
> > -	if (output >= ARRAY_SIZE(names) || !names[output])
> > -		return "UNKNOWN";
> > -
> > -	return names[output];
> > -}
> > -
> >  /* ---------------------------------------------------------------------
> --------
> >   * DRM operations
> >   */
> 
> --
> Regards,
> 
> Laurent Pinchart