Hi Marek,
Thank you for the patch.
On Mon, Sep 22, 2025 at 08:54:59PM +0200, Marek Vasut wrote:
> Introduce VCLKSET_BPP_MASK macro and use FIELD_PREP() to generate
> appropriate bitfield from mask and value without bitshift. Remove
> VCLKSET_COLOR_RGB which is never used, replace it with code comment.
>
> 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+rene...@mailbox.org>
> ---
> Cc: David Airlie <airl...@gmail.com>
> Cc: Geert Uytterhoeven <geert+rene...@glider.be>
> Cc: Kieran Bingham <kieran.bingham+rene...@ideasonboard.com>
> Cc: Laurent Pinchart <laurent.pinchart+rene...@ideasonboard.com>
> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Cc: Magnus Damm <magnus.d...@gmail.com>
> Cc: Maxime Ripard <mrip...@kernel.org>
> Cc: Simona Vetter <sim...@ffwll.ch>
> Cc: Thomas Zimmermann <tzimmerm...@suse.de>
> Cc: Tomi Valkeinen <tomi.valkeinen+rene...@ideasonboard.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-renesas-...@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 | 9 +++++----
> drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h | 12 ++++++------
> 2 files changed, 11 insertions(+), 10 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 a550bda6debbe..2374cbe3768f2 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> @@ -621,18 +621,19 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi
> *dsi,
> vclkset = VCLKSET_CKEN;
> rcar_mipi_dsi_write(dsi, VCLKSET, vclkset);
>
> + /* Output is always RGB, never YCbCr */
> if (dsi_format == 24)
> - vclkset |= VCLKSET_BPP_24;
> + vclkset |= FIELD_PREP(VCLKSET_BPP_MASK, VCLKSET_BPP_24);
> else if (dsi_format == 18)
> - vclkset |= VCLKSET_BPP_18;
> + vclkset |= FIELD_PREP(VCLKSET_BPP_MASK, VCLKSET_BPP_18);
> else if (dsi_format == 16)
> - vclkset |= VCLKSET_BPP_16;
> + vclkset |= FIELD_PREP(VCLKSET_BPP_MASK, VCLKSET_BPP_16);
I personally find this less readable.
> else {
> dev_warn(dsi->dev, "unsupported format");
> return -EINVAL;
> }
>
> - vclkset |= VCLKSET_COLOR_RGB | VCLKSET_LANE(dsi->lanes - 1);
> + vclkset |= VCLKSET_LANE(dsi->lanes - 1);
>
> switch (dsi->info->model) {
> case RCAR_DSI_V3U:
> 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 dd871e17dcf53..c2cb06ef144ed 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
> @@ -245,14 +245,14 @@
>
> #define VCLKSET 0x100c
> #define VCLKSET_CKEN (1 << 16)
> -#define VCLKSET_COLOR_RGB (0 << 8)
> -#define VCLKSET_COLOR_YCC (1 << 8)
> +#define VCLKSET_COLOR_YCC (1 << 8) /* 0:RGB 1:YCbCr */
This I like.
> #define VCLKSET_DIV_V3U(x) (((x) & 0x3) << 4)
> #define VCLKSET_DIV_V4H(x) (((x) & 0x7) << 4)
> -#define VCLKSET_BPP_16 (0 << 2)
> -#define VCLKSET_BPP_18 (1 << 2)
> -#define VCLKSET_BPP_18L (2 << 2)
> -#define VCLKSET_BPP_24 (3 << 2)
> +#define VCLKSET_BPP_MASK (3 << 2)
> +#define VCLKSET_BPP_16 0
> +#define VCLKSET_BPP_18 1
> +#define VCLKSET_BPP_18L 2
> +#define VCLKSET_BPP_24 3
If you'd rather get rid of the shifts, why not
#define VCLKSET_BPP_16 FIELD_PREP(VCLKSET_BPP_MASK, 0)
#define VCLKSET_BPP_18 FIELD_PREP(VCLKSET_BPP_MASK, 1)
#define VCLKSET_BPP_18L FIELD_PREP(VCLKSET_BPP_MASK, 2)
#define VCLKSET_BPP_24 FIELD_PREP(VCLKSET_BPP_MASK, 3)
without touching the users ? Personally I'd keep the shifts though.
> #define VCLKSET_LANE(x) (((x) & 0x3) << 0)
>
> #define VCLKEN 0x1010
--
Regards,
Laurent Pinchart
