Re: [PATCH 5/9] drm/rcar-du: dsi: Clean up TXVMPSPHSETR DT macros

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Mon, Sep 22, 2025 at 08:55:01PM +0200, Marek Vasut wrote:
> Introduce TXVMPSPHSETR_DT_MASK macro and use FIELD_PREP() to generate
> appropriate bitfield from mask and value without bitshift.
> 
> Do not convert bits and bitfields to BIT() and GENMASK() yet, to be
> consisten with the current style. Conversion to BIT() and GENMASK()
> macros is done at the very end of this series in the last two patches.
> 
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: David Airlie <airlied@gmail.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> NOTE: No functional change expected, this is a preparatory patch which
> partly removes macros which evaluate to zeroes from rcar_mipi_dsi_regs.h .
> The other patches in this series proceed with that job, piece by piece,
> to make it all reviewable.
> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c | 17 ++++++++++-------
>  .../drm/renesas/rcar-du/rcar_mipi_dsi_regs.h    | 11 ++++++-----
>  2 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> index 2374cbe3768f2..1591837ff472c 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> @@ -458,13 +458,16 @@ static void rcar_mipi_dsi_set_display_timing(struct rcar_mipi_dsi *dsi,
>  	u32 vprmset4r;
>  
>  	/* Configuration for Pixel Stream and Packet Header */
> -	if (dsibpp == 24)
> -		rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB24);
> -	else if (dsibpp == 18)
> -		rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB18);
> -	else if (dsibpp == 16)
> -		rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB16);
> -	else {
> +	if (dsibpp == 24) {
> +		rcar_mipi_dsi_write(dsi, TXVMPSPHSETR,
> +				    FIELD_PREP(TXVMPSPHSETR_DT_MASK, TXVMPSPHSETR_DT_RGB24));
> +	} else if (dsibpp == 18) {
> +		rcar_mipi_dsi_write(dsi, TXVMPSPHSETR,
> +				    FIELD_PREP(TXVMPSPHSETR_DT_MASK, TXVMPSPHSETR_DT_RGB18));
> +	} else if (dsibpp == 16) {
> +		rcar_mipi_dsi_write(dsi, TXVMPSPHSETR,
> +				    FIELD_PREP(TXVMPSPHSETR_DT_MASK, TXVMPSPHSETR_DT_RGB16));
> +	} else {
>  		dev_warn(dsi->dev, "unsupported format");
>  		return;
>  	}
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
> index 808861aaf3bfe..1a8d377ea85fc 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h
> @@ -167,11 +167,12 @@
>  #define TXVMSCR_STR			(1 << 16)
>  
>  #define TXVMPSPHSETR			0x1c0
> -#define TXVMPSPHSETR_DT_RGB16		(0x0e << 16)
> -#define TXVMPSPHSETR_DT_RGB18		(0x1e << 16)
> -#define TXVMPSPHSETR_DT_RGB18_LS	(0x2e << 16)
> -#define TXVMPSPHSETR_DT_RGB24		(0x3e << 16)
> -#define TXVMPSPHSETR_DT_YCBCR16		(0x2c << 16)
> +#define TXVMPSPHSETR_DT_MASK		(0x3f << 16)
> +#define TXVMPSPHSETR_DT_RGB16		0x0e
> +#define TXVMPSPHSETR_DT_RGB18		0x1e
> +#define TXVMPSPHSETR_DT_RGB18_LS	0x2e
> +#define TXVMPSPHSETR_DT_RGB24		0x3e
> +#define TXVMPSPHSETR_DT_YCBCR16		0x2c

As commented on 3/9, I'm really not thrilled, sorry. I think this
decreased readability in the .c file without any other advantage. If
it's the shift that bothers you you could use FIELD_PREP() in the macro
itself, but I wouldn't do that.

>  
>  #define TXVMVPRMSET0R			0x1d0
>  #define TXVMVPRMSET0R_HSPOL_HIG		(0 << 17)

-- 
Regards,

Laurent Pinchart