Hi Marek, On Mon, Sep 22, 2025 at 08:55:03PM +0200, Marek Vasut wrote: > Introduce TXVMVPRMSET0R_BPP_MASK macro and use FIELD_PREP() to generate > appropriate bitfield from mask and value without bitshift, assign this > value into vprmset0r. Remove TXVMVPRMSET0R_CSPC_RGB which is never used, > replace it with code comment next to TXVMVPRMSET0R_CSPC_YCbCr. > > Replace (mode->flags & DRM_MODE_FLAG_P.SYNC) test with inverted conditional > (mode->flags & DRM_MODE_FLAG_N.SYNC) and bitwise orr vprmset0r with either
I wonder if the DRM_MODE_FLAG_P[HV]SYNC flags are always the exact opposite of DRM_MODE_FLAG_N[HV]SYNC. It's probably fine to assume that here. A quick grep showed one panel driver setting both the N and P flags (drivers/gpu/drm/panel/panel-sitronix-st7789v.c, see t28cp45tn89_mode, which I assume is a bug - Sebastian, could you check that ?). > or both TXVMVPRMSET0R_HSPOL_LOW and TXVMVPRMSET0R_VSPOL_LOW if conditional > matches. > > 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 | 12 ++++++------ > .../gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h | 16 +++++++--------- > 2 files changed, 13 insertions(+), 15 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 36bd9de61ce05..f91cc35423758 100644 > --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c > +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c > @@ -489,12 +489,12 @@ static void rcar_mipi_dsi_set_display_timing(struct > rcar_mipi_dsi *dsi, > > rcar_mipi_dsi_write(dsi, TXVMSETR, setr); > > - /* Configuration for Video Parameters */ > - vprmset0r = (mode->flags & DRM_MODE_FLAG_PVSYNC ? > - TXVMVPRMSET0R_VSPOL_HIG : TXVMVPRMSET0R_VSPOL_LOW) > - | (mode->flags & DRM_MODE_FLAG_PHSYNC ? > - TXVMVPRMSET0R_HSPOL_HIG : TXVMVPRMSET0R_HSPOL_LOW) > - | TXVMVPRMSET0R_CSPC_RGB | TXVMVPRMSET0R_BPP_24; > + /* Configuration for Video Parameters, input is always RGB888 */ > + vprmset0r = FIELD_PREP(TXVMVPRMSET0R_BPP_MASK, TXVMVPRMSET0R_BPP_24); > + if (mode->flags & DRM_MODE_FLAG_NVSYNC) > + vprmset0r |= TXVMVPRMSET0R_VSPOL_LOW; > + if (mode->flags & DRM_MODE_FLAG_NHSYNC) > + vprmset0r |= TXVMVPRMSET0R_HSPOL_LOW; Looks good. > > vprmset1r = TXVMVPRMSET1R_VACTIVE(mode->vdisplay) > | TXVMVPRMSET1R_VSA(mode->vsync_end - mode->vsync_start); > 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 99a88ea35aacd..48c3b679b2663 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 > @@ -170,15 +170,13 @@ > #define TXVMPSPHSETR_DT_YCBCR16 0x2c > > #define TXVMVPRMSET0R 0x1d0 > -#define TXVMVPRMSET0R_HSPOL_HIG (0 << 17) > -#define TXVMVPRMSET0R_HSPOL_LOW (1 << 17) > -#define TXVMVPRMSET0R_VSPOL_HIG (0 << 16) > -#define TXVMVPRMSET0R_VSPOL_LOW (1 << 16) > -#define TXVMVPRMSET0R_CSPC_RGB (0 << 4) > -#define TXVMVPRMSET0R_CSPC_YCbCr (1 << 4) > -#define TXVMVPRMSET0R_BPP_16 (0 << 0) > -#define TXVMVPRMSET0R_BPP_18 (1 << 0) > -#define TXVMVPRMSET0R_BPP_24 (2 << 0) > +#define TXVMVPRMSET0R_HSPOL_LOW (1 << 17) /* 0:High 1:Low */ > +#define TXVMVPRMSET0R_VSPOL_LOW (1 << 16) /* 0:High 1:Low */ > +#define TXVMVPRMSET0R_CSPC_YCbCr (1 << 4) /* 0:RGB 1:YCbCr */ > +#define TXVMVPRMSET0R_BPP_MASK (7 << 0) > +#define TXVMVPRMSET0R_BPP_16 0 > +#define TXVMVPRMSET0R_BPP_18 1 > +#define TXVMVPRMSET0R_BPP_24 2 Same comment as in previous patches regarding usage of FIELD_PREP(). The rest looks fine. > > #define TXVMVPRMSET1R 0x1d4 > #define TXVMVPRMSET1R_VACTIVE(x) (((x) & 0x7fff) << 16) -- Regards, Laurent Pinchart
