All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] drm/rcar-du: dsi: Convert register bits to BIT()/GENMASK() macros
@ 2025-09-22 18:54 Marek Vasut
  2025-09-22 18:54 ` [PATCH 1/9] drm/rcar-du: dsi: Fix missing parameter in RXSETR_...EN macros Marek Vasut
                   ` (8 more replies)
  0 siblings, 9 replies; 31+ messages in thread
From: Marek Vasut @ 2025-09-22 18:54 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

Convert register bits to BIT() macro and bitfields to GENMASK()/FIELD_PREP() macros.
Most of this patchset is boring mechanical conversion.

Noteworthy patches are 6 and 7 , those introduce handling of DSI mode flags
and convert use of DRM_MODE_FLAG_P.SYNC into DRM_MODE_FLAG_N.SYNC, but that
should not have any adverse effect on existing hardware.

Marek Vasut (9):
  drm/rcar-du: dsi: Fix missing parameter in RXSETR_...EN macros
  drm/rcar-du: dsi: Deduplicate mipi_dsi_pixel_format_to_bpp() usage
  drm/rcar-du: dsi: Clean up VCLKSET register macros
  drm/rcar-du: dsi: Clean up CLOCKSET1 CLKINSEL macros
  drm/rcar-du: dsi: Clean up TXVMPSPHSETR DT macros
  drm/rcar-du: dsi: Respect DSI mode flags
  drm/rcar-du: dsi: Clean up handling of DRM mode flags
  drm/rcar-du: dsi: Convert register bits to BIT() macro
  drm/rcar-du: dsi: Convert register bitfields to GENMASK() macro

 .../gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c   | 101 +++--
 .../drm/renesas/rcar-du/rcar_mipi_dsi_regs.h  | 362 +++++++++---------
 2 files changed, 237 insertions(+), 226 deletions(-)

---
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

-- 
2.51.0


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 1/9] drm/rcar-du: dsi: Fix missing parameter in RXSETR_...EN macros
  2025-09-22 18:54 [PATCH 0/9] drm/rcar-du: dsi: Convert register bits to BIT()/GENMASK() macros Marek Vasut
@ 2025-09-22 18:54 ` Marek Vasut
  2025-09-23 11:42   ` Laurent Pinchart
  2025-09-22 18:54 ` [PATCH 2/9] drm/rcar-du: dsi: Deduplicate mipi_dsi_pixel_format_to_bpp() usage Marek Vasut
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 31+ messages in thread
From: Marek Vasut @ 2025-09-22 18:54 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

The RXSETR_CRCEN(n) and RXSETR_ECCEN(n) macros both take parameter (n),
add the missing macro parameter. Neither of those macros is used by the
driver, so for now the bug is harmless.

Fixes: 685e8dae19df ("drm/rcar-du: dsi: Implement DSI command support")
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
---
 drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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 76521276e2af8..dd871e17dcf53 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
@@ -50,8 +50,8 @@
 #define TXCMPPD3R			0x16c
 
 #define RXSETR				0x200
-#define RXSETR_CRCEN			(((n) & 0xf) << 24)
-#define RXSETR_ECCEN			(((n) & 0xf) << 16)
+#define RXSETR_CRCEN(n)			(((n) & 0xf) << 24)
+#define RXSETR_ECCEN(n)			(((n) & 0xf) << 16)
 #define RXPSETR				0x210
 #define RXPSETR_LPPDACC			(1 << 0)
 #define RXPSR				0x220
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 2/9] drm/rcar-du: dsi: Deduplicate mipi_dsi_pixel_format_to_bpp() usage
  2025-09-22 18:54 [PATCH 0/9] drm/rcar-du: dsi: Convert register bits to BIT()/GENMASK() macros Marek Vasut
  2025-09-22 18:54 ` [PATCH 1/9] drm/rcar-du: dsi: Fix missing parameter in RXSETR_...EN macros Marek Vasut
@ 2025-09-22 18:54 ` Marek Vasut
  2025-09-23  6:47   ` Geert Uytterhoeven
  2025-09-22 18:54 ` [PATCH 3/9] drm/rcar-du: dsi: Clean up VCLKSET register macros Marek Vasut
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 31+ messages in thread
From: Marek Vasut @ 2025-09-22 18:54 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

Call mipi_dsi_pixel_format_to_bpp() once in rcar_mipi_dsi_set_display_timing()
and store the value into a variable. This slightly simplifies the code.

No functional change.

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
---
 drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 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 5c73a513f678e..a550bda6debbe 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
@@ -449,6 +449,7 @@ static void rcar_mipi_dsi_parameters_calc(struct rcar_mipi_dsi *dsi,
 static void rcar_mipi_dsi_set_display_timing(struct rcar_mipi_dsi *dsi,
 					     const struct drm_display_mode *mode)
 {
+	const int dsibpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
 	u32 setr;
 	u32 vprmset0r;
 	u32 vprmset1r;
@@ -457,11 +458,11 @@ static void rcar_mipi_dsi_set_display_timing(struct rcar_mipi_dsi *dsi,
 	u32 vprmset4r;
 
 	/* Configuration for Pixel Stream and Packet Header */
-	if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 24)
+	if (dsibpp == 24)
 		rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB24);
-	else if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 18)
+	else if (dsibpp == 18)
 		rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB18);
-	else if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 16)
+	else if (dsibpp == 16)
 		rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB16);
 	else {
 		dev_warn(dsi->dev, "unsupported format");
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 3/9] drm/rcar-du: dsi: Clean up VCLKSET register macros
  2025-09-22 18:54 [PATCH 0/9] drm/rcar-du: dsi: Convert register bits to BIT()/GENMASK() macros Marek Vasut
  2025-09-22 18:54 ` [PATCH 1/9] drm/rcar-du: dsi: Fix missing parameter in RXSETR_...EN macros Marek Vasut
  2025-09-22 18:54 ` [PATCH 2/9] drm/rcar-du: dsi: Deduplicate mipi_dsi_pixel_format_to_bpp() usage Marek Vasut
@ 2025-09-22 18:54 ` Marek Vasut
  2025-09-23  9:10   ` kernel test robot
  2025-09-23 11:49   ` Laurent Pinchart
  2025-09-22 18:55 ` [PATCH 4/9] drm/rcar-du: dsi: Clean up CLOCKSET1 CLKINSEL macros Marek Vasut
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 31+ messages in thread
From: Marek Vasut @ 2025-09-22 18:54 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

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+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      |  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);
 	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 */
 #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
 #define VCLKSET_LANE(x)			(((x) & 0x3) << 0)
 
 #define VCLKEN				0x1010
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 4/9] drm/rcar-du: dsi: Clean up CLOCKSET1 CLKINSEL macros
  2025-09-22 18:54 [PATCH 0/9] drm/rcar-du: dsi: Convert register bits to BIT()/GENMASK() macros Marek Vasut
                   ` (2 preceding siblings ...)
  2025-09-22 18:54 ` [PATCH 3/9] drm/rcar-du: dsi: Clean up VCLKSET register macros Marek Vasut
@ 2025-09-22 18:55 ` Marek Vasut
  2025-09-23  5:45   ` Biju Das
  2025-09-22 18:55 ` [PATCH 5/9] drm/rcar-du: dsi: Clean up TXVMPSPHSETR DT macros Marek Vasut
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 31+ messages in thread
From: Marek Vasut @ 2025-09-22 18:55 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

Introduce CLOCKSET1_CLKINSEL_MASK macro and remove bitshift from
values to make this bitfield usable with FIELD_PREP(). There are
no users of this bitfield, hence no updates to the DSI driver.

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_regs.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

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 c2cb06ef144ed..808861aaf3bfe 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
@@ -268,9 +268,10 @@
 #define CLOCKSET1			0x101c
 #define CLOCKSET1_LOCK_PHY		(1 << 17)
 #define CLOCKSET1_CLKSEL		(1 << 8)
-#define CLOCKSET1_CLKINSEL_EXTAL	(0 << 2)
-#define CLOCKSET1_CLKINSEL_DIG		(1 << 2)
-#define CLOCKSET1_CLKINSEL_DU		(1 << 3)
+#define CLOCKSET1_CLKINSEL_MASK		(3 << 2)
+#define CLOCKSET1_CLKINSEL_EXTAL	0
+#define CLOCKSET1_CLKINSEL_DIG		1
+#define CLOCKSET1_CLKINSEL_DU		2
 #define CLOCKSET1_SHADOW_CLEAR		(1 << 1)
 #define CLOCKSET1_UPDATEPLL		(1 << 0)
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 5/9] drm/rcar-du: dsi: Clean up TXVMPSPHSETR DT macros
  2025-09-22 18:54 [PATCH 0/9] drm/rcar-du: dsi: Convert register bits to BIT()/GENMASK() macros Marek Vasut
                   ` (3 preceding siblings ...)
  2025-09-22 18:55 ` [PATCH 4/9] drm/rcar-du: dsi: Clean up CLOCKSET1 CLKINSEL macros Marek Vasut
@ 2025-09-22 18:55 ` Marek Vasut
  2025-09-23 11:52   ` Laurent Pinchart
  2025-09-22 18:55 ` [PATCH 6/9] drm/rcar-du: dsi: Respect DSI mode flags Marek Vasut
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 31+ messages in thread
From: Marek Vasut @ 2025-09-22 18:55 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

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
 
 #define TXVMVPRMSET0R			0x1d0
 #define TXVMVPRMSET0R_HSPOL_HIG		(0 << 17)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 6/9] drm/rcar-du: dsi: Respect DSI mode flags
  2025-09-22 18:54 [PATCH 0/9] drm/rcar-du: dsi: Convert register bits to BIT()/GENMASK() macros Marek Vasut
                   ` (4 preceding siblings ...)
  2025-09-22 18:55 ` [PATCH 5/9] drm/rcar-du: dsi: Clean up TXVMPSPHSETR DT macros Marek Vasut
@ 2025-09-22 18:55 ` Marek Vasut
  2025-09-23 11:54   ` Laurent Pinchart
  2025-09-22 18:55 ` [PATCH 7/9] drm/rcar-du: dsi: Clean up handling of DRM " Marek Vasut
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 31+ messages in thread
From: Marek Vasut @ 2025-09-22 18:55 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

Cache DSI mode flags in new mode_flags member of struct rcar_mipi_dsi .
Configure TXVMSETR register based on the content of DSI mode flags in
case the controller operates in video mode.

Rename TXVMSETR_H..BPEN_EN to TXVMSETR_H..BPEN and drop TXVMSETR_H..BPEN_DIS
which resolves to 0. Update TXVMSETR_VSEN in the same manner. Replace
TXVMSETR_SYNSEQ_PULSES with a code comment next to TXVMSETR_SYNSEQ_EVENTS
because TXVMSETR_SYNSEQ_PULSES resolves to 0.

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
---
 .../gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c    | 18 +++++++++++++++---
 .../drm/renesas/rcar-du/rcar_mipi_dsi_regs.h   | 15 +++++----------
 2 files changed, 20 insertions(+), 13 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 1591837ff472c..36bd9de61ce05 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
@@ -71,6 +71,7 @@ struct rcar_mipi_dsi {
 	} clocks;
 
 	enum mipi_dsi_pixel_format format;
+	unsigned long mode_flags;
 	unsigned int num_data_lanes;
 	unsigned int lanes;
 };
@@ -473,9 +474,19 @@ static void rcar_mipi_dsi_set_display_timing(struct rcar_mipi_dsi *dsi,
 	}
 
 	/* Configuration for Blanking sequence and Input Pixel */
-	setr = TXVMSETR_HSABPEN_EN | TXVMSETR_HBPBPEN_EN
-	     | TXVMSETR_HFPBPEN_EN | TXVMSETR_SYNSEQ_PULSES
-	     | TXVMSETR_PIXWDTH | TXVMSETR_VSTPM;
+	setr = TXVMSETR_PIXWDTH | TXVMSETR_VSTPM;
+
+	if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
+		if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE))
+			setr |= TXVMSETR_SYNSEQ_EVENTS;
+		if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP))
+			setr |= TXVMSETR_HFPBPEN;
+		if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HBP))
+			setr |= TXVMSETR_HBPBPEN;
+		if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA))
+			setr |= TXVMSETR_HSABPEN;
+	}
+
 	rcar_mipi_dsi_write(dsi, TXVMSETR, setr);
 
 	/* Configuration for Video Parameters */
@@ -916,6 +927,7 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
 
 	dsi->lanes = device->lanes;
 	dsi->format = device->format;
+	dsi->mode_flags = device->mode_flags;
 
 	dsi->next_bridge = devm_drm_of_get_bridge(dsi->dev, dsi->dev->of_node,
 						  1, 0);
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 1a8d377ea85fc..99a88ea35aacd 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
@@ -140,18 +140,13 @@
  * Video Mode Register
  */
 #define TXVMSETR			0x180
-#define TXVMSETR_SYNSEQ_PULSES		(0 << 16)
-#define TXVMSETR_SYNSEQ_EVENTS		(1 << 16)
+#define TXVMSETR_SYNSEQ_EVENTS		(1 << 16) /* 0:Pulses 1:Events */
 #define TXVMSETR_VSTPM			(1 << 15)
 #define TXVMSETR_PIXWDTH		(1 << 8)
-#define TXVMSETR_VSEN_EN		(1 << 4)
-#define TXVMSETR_VSEN_DIS		(0 << 4)
-#define TXVMSETR_HFPBPEN_EN		(1 << 2)
-#define TXVMSETR_HFPBPEN_DIS		(0 << 2)
-#define TXVMSETR_HBPBPEN_EN		(1 << 1)
-#define TXVMSETR_HBPBPEN_DIS		(0 << 1)
-#define TXVMSETR_HSABPEN_EN		(1 << 0)
-#define TXVMSETR_HSABPEN_DIS		(0 << 0)
+#define TXVMSETR_VSEN			(1 << 4)
+#define TXVMSETR_HFPBPEN		(1 << 2)
+#define TXVMSETR_HBPBPEN		(1 << 1)
+#define TXVMSETR_HSABPEN		(1 << 0)
 
 #define TXVMCR				0x190
 #define TXVMCR_VFCLR			(1 << 12)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 7/9] drm/rcar-du: dsi: Clean up handling of DRM mode flags
  2025-09-22 18:54 [PATCH 0/9] drm/rcar-du: dsi: Convert register bits to BIT()/GENMASK() macros Marek Vasut
                   ` (5 preceding siblings ...)
  2025-09-22 18:55 ` [PATCH 6/9] drm/rcar-du: dsi: Respect DSI mode flags Marek Vasut
@ 2025-09-22 18:55 ` Marek Vasut
  2025-09-23 13:26   ` Laurent Pinchart
  2025-09-22 18:55 ` [PATCH 8/9] drm/rcar-du: dsi: Convert register bits to BIT() macro Marek Vasut
  2025-09-22 18:55 ` [PATCH 9/9] drm/rcar-du: dsi: Convert register bitfields to GENMASK() macro Marek Vasut
  8 siblings, 1 reply; 31+ messages in thread
From: Marek Vasut @ 2025-09-22 18:55 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

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
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+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  | 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;
 
 	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
 
 #define TXVMVPRMSET1R			0x1d4
 #define TXVMVPRMSET1R_VACTIVE(x)	(((x) & 0x7fff) << 16)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 8/9] drm/rcar-du: dsi: Convert register bits to BIT() macro
  2025-09-22 18:54 [PATCH 0/9] drm/rcar-du: dsi: Convert register bits to BIT()/GENMASK() macros Marek Vasut
                   ` (6 preceding siblings ...)
  2025-09-22 18:55 ` [PATCH 7/9] drm/rcar-du: dsi: Clean up handling of DRM " Marek Vasut
@ 2025-09-22 18:55 ` Marek Vasut
  2025-09-23 14:16   ` Laurent Pinchart
  2025-09-22 18:55 ` [PATCH 9/9] drm/rcar-du: dsi: Convert register bitfields to GENMASK() macro Marek Vasut
  8 siblings, 1 reply; 31+ messages in thread
From: Marek Vasut @ 2025-09-22 18:55 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

Convert register bits to BIT() macro where applicable. This is done
automatically using regex 's@(1 << \([0-9]\+\))@BIT(\1)', except for
bitfields which are manually updated to use GENMASK().

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
---
 .../drm/renesas/rcar-du/rcar_mipi_dsi_regs.h  | 248 +++++++++---------
 1 file changed, 124 insertions(+), 124 deletions(-)

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 48c3b679b2663..29c806cae3557 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
@@ -9,37 +9,37 @@
 #define __RCAR_MIPI_DSI_REGS_H__
 
 #define LINKSR				0x010
-#define LINKSR_LPBUSY			(1 << 1)
-#define LINKSR_HSBUSY			(1 << 0)
+#define LINKSR_LPBUSY			BIT(1)
+#define LINKSR_HSBUSY			BIT(0)
 
 #define TXSETR				0x100
-#define TXSETR_LANECNT_MASK		(0x3 << 0)
+#define TXSETR_LANECNT_MASK		GENMASK(1, 0)
 
 /*
  * DSI Command Transfer Registers
  */
 #define TXCMSETR			0x110
-#define TXCMSETR_SPDTYP			(1 << 8)	/* 0:HS 1:LP */
-#define TXCMSETR_LPPDACC		(1 << 0)
+#define TXCMSETR_SPDTYP			BIT(8)	/* 0:HS 1:LP */
+#define TXCMSETR_LPPDACC		BIT(0)
 #define TXCMCR				0x120
-#define TXCMCR_BTATYP			(1 << 2)
-#define TXCMCR_BTAREQ			(1 << 1)
-#define TXCMCR_TXREQ			(1 << 0)
+#define TXCMCR_BTATYP			BIT(2)
+#define TXCMCR_BTAREQ			BIT(1)
+#define TXCMCR_TXREQ			BIT(0)
 #define TXCMSR				0x130
-#define TXCMSR_CLSNERR			(1 << 18)
-#define TXCMSR_AXIERR			(1 << 16)
-#define TXCMSR_TXREQEND			(1 << 0)
+#define TXCMSR_CLSNERR			BIT(18)
+#define TXCMSR_AXIERR			BIT(16)
+#define TXCMSR_TXREQEND			BIT(0)
 #define TXCMSCR				0x134
-#define TXCMSCR_CLSNERR			(1 << 18)
-#define TXCMSCR_AXIERR			(1 << 16)
-#define TXCMSCR_TXREQEND		(1 << 0)
+#define TXCMSCR_CLSNERR			BIT(18)
+#define TXCMSCR_AXIERR			BIT(16)
+#define TXCMSCR_TXREQEND		BIT(0)
 #define TXCMIER				0x138
-#define TXCMIER_CLSNERR			(1 << 18)
-#define TXCMIER_AXIERR			(1 << 16)
-#define TXCMIER_TXREQEND		(1 << 0)
+#define TXCMIER_CLSNERR			BIT(18)
+#define TXCMIER_AXIERR			BIT(16)
+#define TXCMIER_TXREQEND		BIT(0)
 #define TXCMADDRSET0R			0x140
 #define TXCMPHDR			0x150
-#define TXCMPHDR_FMT			(1 << 24)	/* 0:SP 1:LP */
+#define TXCMPHDR_FMT			BIT(24)	/* 0:SP 1:LP */
 #define TXCMPHDR_VC(n)			(((n) & 0x3) << 22)
 #define TXCMPHDR_DT(n)			(((n) & 0x3f) << 16)
 #define TXCMPHDR_DATA1(n)		(((n) & 0xff) << 8)
@@ -53,63 +53,63 @@
 #define RXSETR_CRCEN(n)			(((n) & 0xf) << 24)
 #define RXSETR_ECCEN(n)			(((n) & 0xf) << 16)
 #define RXPSETR				0x210
-#define RXPSETR_LPPDACC			(1 << 0)
+#define RXPSETR_LPPDACC			BIT(0)
 #define RXPSR				0x220
-#define RXPSR_ECCERR1B			(1 << 28)
-#define RXPSR_UEXTRGERR			(1 << 25)
-#define RXPSR_RESPTOERR			(1 << 24)
-#define RXPSR_OVRERR			(1 << 23)
-#define RXPSR_AXIERR			(1 << 22)
-#define RXPSR_CRCERR			(1 << 21)
-#define RXPSR_WCERR			(1 << 20)
-#define RXPSR_UEXDTERR			(1 << 19)
-#define RXPSR_UEXPKTERR			(1 << 18)
-#define RXPSR_ECCERR			(1 << 17)
-#define RXPSR_MLFERR			(1 << 16)
-#define RXPSR_RCVACK			(1 << 14)
-#define RXPSR_RCVEOT			(1 << 10)
-#define RXPSR_RCVAKE			(1 << 9)
-#define RXPSR_RCVRESP			(1 << 8)
-#define RXPSR_BTAREQEND			(1 << 0)
+#define RXPSR_ECCERR1B			BIT(28)
+#define RXPSR_UEXTRGERR			BIT(25)
+#define RXPSR_RESPTOERR			BIT(24)
+#define RXPSR_OVRERR			BIT(23)
+#define RXPSR_AXIERR			BIT(22)
+#define RXPSR_CRCERR			BIT(21)
+#define RXPSR_WCERR			BIT(20)
+#define RXPSR_UEXDTERR			BIT(19)
+#define RXPSR_UEXPKTERR			BIT(18)
+#define RXPSR_ECCERR			BIT(17)
+#define RXPSR_MLFERR			BIT(16)
+#define RXPSR_RCVACK			BIT(14)
+#define RXPSR_RCVEOT			BIT(10)
+#define RXPSR_RCVAKE			BIT(9)
+#define RXPSR_RCVRESP			BIT(8)
+#define RXPSR_BTAREQEND			BIT(0)
 #define RXPSCR				0x224
-#define RXPSCR_ECCERR1B			(1 << 28)
-#define RXPSCR_UEXTRGERR		(1 << 25)
-#define RXPSCR_RESPTOERR		(1 << 24)
-#define RXPSCR_OVRERR			(1 << 23)
-#define RXPSCR_AXIERR			(1 << 22)
-#define RXPSCR_CRCERR			(1 << 21)
-#define RXPSCR_WCERR			(1 << 20)
-#define RXPSCR_UEXDTERR			(1 << 19)
-#define RXPSCR_UEXPKTERR		(1 << 18)
-#define RXPSCR_ECCERR			(1 << 17)
-#define RXPSCR_MLFERR			(1 << 16)
-#define RXPSCR_RCVACK			(1 << 14)
-#define RXPSCR_RCVEOT			(1 << 10)
-#define RXPSCR_RCVAKE			(1 << 9)
-#define RXPSCR_RCVRESP			(1 << 8)
-#define RXPSCR_BTAREQEND		(1 << 0)
+#define RXPSCR_ECCERR1B			BIT(28)
+#define RXPSCR_UEXTRGERR		BIT(25)
+#define RXPSCR_RESPTOERR		BIT(24)
+#define RXPSCR_OVRERR			BIT(23)
+#define RXPSCR_AXIERR			BIT(22)
+#define RXPSCR_CRCERR			BIT(21)
+#define RXPSCR_WCERR			BIT(20)
+#define RXPSCR_UEXDTERR			BIT(19)
+#define RXPSCR_UEXPKTERR		BIT(18)
+#define RXPSCR_ECCERR			BIT(17)
+#define RXPSCR_MLFERR			BIT(16)
+#define RXPSCR_RCVACK			BIT(14)
+#define RXPSCR_RCVEOT			BIT(10)
+#define RXPSCR_RCVAKE			BIT(9)
+#define RXPSCR_RCVRESP			BIT(8)
+#define RXPSCR_BTAREQEND		BIT(0)
 #define RXPIER				0x228
-#define RXPIER_ECCERR1B			(1 << 28)
-#define RXPIER_UEXTRGERR		(1 << 25)
-#define RXPIER_RESPTOERR		(1 << 24)
-#define RXPIER_OVRERR			(1 << 23)
-#define RXPIER_AXIERR			(1 << 22)
-#define RXPIER_CRCERR			(1 << 21)
-#define RXPIER_WCERR			(1 << 20)
-#define RXPIER_UEXDTERR			(1 << 19)
-#define RXPIER_UEXPKTERR		(1 << 18)
-#define RXPIER_ECCERR			(1 << 17)
-#define RXPIER_MLFERR			(1 << 16)
-#define RXPIER_RCVACK			(1 << 14)
-#define RXPIER_RCVEOT			(1 << 10)
-#define RXPIER_RCVAKE			(1 << 9)
-#define RXPIER_RCVRESP			(1 << 8)
-#define RXPIER_BTAREQEND		(1 << 0)
+#define RXPIER_ECCERR1B			BIT(28)
+#define RXPIER_UEXTRGERR		BIT(25)
+#define RXPIER_RESPTOERR		BIT(24)
+#define RXPIER_OVRERR			BIT(23)
+#define RXPIER_AXIERR			BIT(22)
+#define RXPIER_CRCERR			BIT(21)
+#define RXPIER_WCERR			BIT(20)
+#define RXPIER_UEXDTERR			BIT(19)
+#define RXPIER_UEXPKTERR		BIT(18)
+#define RXPIER_ECCERR			BIT(17)
+#define RXPIER_MLFERR			BIT(16)
+#define RXPIER_RCVACK			BIT(14)
+#define RXPIER_RCVEOT			BIT(10)
+#define RXPIER_RCVAKE			BIT(9)
+#define RXPIER_RCVRESP			BIT(8)
+#define RXPIER_BTAREQEND		BIT(0)
 #define RXPADDRSET0R			0x230
 #define RXPSIZESETR			0x238
 #define RXPSIZESETR_SIZE(n)		(((n) & 0xf) << 3)
 #define RXPHDR				0x240
-#define RXPHDR_FMT			(1 << 24)	/* 0:SP 1:LP */
+#define RXPHDR_FMT			BIT(24)	/* 0:SP 1:LP */
 #define RXPHDR_VC(n)			(((n) & 0x3) << 22)
 #define RXPHDR_DT(n)			(((n) & 0x3f) << 16)
 #define RXPHDR_DATA1(n)			(((n) & 0xff) << 8)
@@ -128,38 +128,38 @@
 #define TASCR				0x514
 #define TAIER				0x518
 #define TOSR				0x610
-#define TOSR_TATO			(1 << 2)
-#define TOSR_LRXHTO			(1 << 1)
-#define TOSR_HRXTO			(1 << 0)
+#define TOSR_TATO			BIT(2)
+#define TOSR_LRXHTO			BIT(1)
+#define TOSR_HRXTO			BIT(0)
 #define TOSCR				0x614
-#define TOSCR_TATO			(1 << 2)
-#define TOSCR_LRXHTO			(1 << 1)
-#define TOSCR_HRXTO			(1 << 0)
+#define TOSCR_TATO			BIT(2)
+#define TOSCR_LRXHTO			BIT(1)
+#define TOSCR_HRXTO			BIT(0)
 
 /*
  * Video Mode Register
  */
 #define TXVMSETR			0x180
-#define TXVMSETR_SYNSEQ_EVENTS		(1 << 16) /* 0:Pulses 1:Events */
-#define TXVMSETR_VSTPM			(1 << 15)
-#define TXVMSETR_PIXWDTH		(1 << 8)
-#define TXVMSETR_VSEN			(1 << 4)
-#define TXVMSETR_HFPBPEN		(1 << 2)
-#define TXVMSETR_HBPBPEN		(1 << 1)
-#define TXVMSETR_HSABPEN		(1 << 0)
+#define TXVMSETR_SYNSEQ_EVENTS		BIT(16) /* 0:Pulses 1:Events */
+#define TXVMSETR_VSTPM			BIT(15)
+#define TXVMSETR_PIXWDTH		BIT(8)
+#define TXVMSETR_VSEN			BIT(4)
+#define TXVMSETR_HFPBPEN		BIT(2)
+#define TXVMSETR_HBPBPEN		BIT(1)
+#define TXVMSETR_HSABPEN		BIT(0)
 
 #define TXVMCR				0x190
-#define TXVMCR_VFCLR			(1 << 12)
-#define TXVMCR_EN_VIDEO			(1 << 0)
+#define TXVMCR_VFCLR			BIT(12)
+#define TXVMCR_EN_VIDEO			BIT(0)
 
 #define TXVMSR				0x1a0
-#define TXVMSR_STR			(1 << 16)
-#define TXVMSR_VFRDY			(1 << 12)
-#define TXVMSR_ACT			(1 << 8)
-#define TXVMSR_RDY			(1 << 0)
+#define TXVMSR_STR			BIT(16)
+#define TXVMSR_VFRDY			BIT(12)
+#define TXVMSR_ACT			BIT(8)
+#define TXVMSR_RDY			BIT(0)
 
 #define TXVMSCR				0x1a4
-#define TXVMSCR_STR			(1 << 16)
+#define TXVMSCR_STR			BIT(16)
 
 #define TXVMPSPHSETR			0x1c0
 #define TXVMPSPHSETR_DT_MASK		(0x3f << 16)
@@ -170,10 +170,10 @@
 #define TXVMPSPHSETR_DT_YCBCR16		0x2c
 
 #define TXVMVPRMSET0R			0x1d0
-#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_HSPOL_LOW		BIT(17) /* 0:High 1:Low */
+#define TXVMVPRMSET0R_VSPOL_LOW		BIT(16) /* 0:High 1:Low */
+#define TXVMVPRMSET0R_CSPC_YCbCr	BIT(4) /* 0:RGB 1:YCbCr */
+#define TXVMVPRMSET0R_BPP_MASK		GENMASK(2, 0)
 #define TXVMVPRMSET0R_BPP_16		0
 #define TXVMVPRMSET0R_BPP_18		1
 #define TXVMVPRMSET0R_BPP_24		2
@@ -198,51 +198,51 @@
  * PHY-Protocol Interface (PPI) Registers
  */
 #define PPISETR				0x700
-#define PPISETR_DLEN_MASK		(0xf << 0)
-#define PPISETR_CLEN			(1 << 8)
+#define PPISETR_DLEN_MASK		GENMASK(3, 0)
+#define PPISETR_CLEN			BIT(8)
 
 #define PPICLCR				0x710
-#define PPICLCR_TXREQHS			(1 << 8)
-#define PPICLCR_TXULPSEXT		(1 << 1)
-#define PPICLCR_TXULPSCLK		(1 << 0)
+#define PPICLCR_TXREQHS			BIT(8)
+#define PPICLCR_TXULPSEXT		BIT(1)
+#define PPICLCR_TXULPSCLK		BIT(0)
 
 #define PPICLSR				0x720
-#define PPICLSR_HSTOLP			(1 << 27)
-#define PPICLSR_TOHS			(1 << 26)
-#define PPICLSR_STPST			(1 << 0)
+#define PPICLSR_HSTOLP			BIT(27)
+#define PPICLSR_TOHS			BIT(26)
+#define PPICLSR_STPST			BIT(0)
 
 #define PPICLSCR			0x724
-#define PPICLSCR_HSTOLP			(1 << 27)
-#define PPICLSCR_TOHS			(1 << 26)
+#define PPICLSCR_HSTOLP			BIT(27)
+#define PPICLSCR_TOHS			BIT(26)
 
 #define PPIDL0SR			0x740
-#define PPIDL0SR_DIR			(1 << 10)
-#define PPIDL0SR_STPST			(1 << 6)
+#define PPIDL0SR_DIR			BIT(10)
+#define PPIDL0SR_STPST			BIT(6)
 
 #define PPIDLSR				0x760
-#define PPIDLSR_STPST			(0xf << 0)
+#define PPIDLSR_STPST			GENMASK(3, 0)
 
 /*
  * Clocks registers
  */
 #define LPCLKSET			0x1000
-#define LPCLKSET_CKEN			(1 << 8)
+#define LPCLKSET_CKEN			BIT(8)
 #define LPCLKSET_LPCLKDIV(x)		(((x) & 0x3f) << 0)
 
 #define CFGCLKSET			0x1004
-#define CFGCLKSET_CKEN			(1 << 8)
+#define CFGCLKSET_CKEN			BIT(8)
 #define CFGCLKSET_CFGCLKDIV(x)		(((x) & 0x3f) << 0)
 
 #define DOTCLKDIV			0x1008
-#define DOTCLKDIV_CKEN			(1 << 8)
+#define DOTCLKDIV_CKEN			BIT(8)
 #define DOTCLKDIV_DOTCLKDIV(x)		(((x) & 0x3f) << 0)
 
 #define VCLKSET				0x100c
-#define VCLKSET_CKEN			(1 << 16)
-#define VCLKSET_COLOR_YCC		(1 << 8) /* 0:RGB 1:YCbCr */
+#define VCLKSET_CKEN			BIT(16)
+#define VCLKSET_COLOR_YCC		BIT(8) /* 0:RGB 1:YCbCr */
 #define VCLKSET_DIV_V3U(x)		(((x) & 0x3) << 4)
 #define VCLKSET_DIV_V4H(x)		(((x) & 0x7) << 4)
-#define VCLKSET_BPP_MASK		(3 << 2)
+#define VCLKSET_BPP_MASK		GENMASK(3, 2)
 #define VCLKSET_BPP_16			0
 #define VCLKSET_BPP_18			1
 #define VCLKSET_BPP_18L			2
@@ -250,24 +250,24 @@
 #define VCLKSET_LANE(x)			(((x) & 0x3) << 0)
 
 #define VCLKEN				0x1010
-#define VCLKEN_CKEN			(1 << 0)
+#define VCLKEN_CKEN			BIT(0)
 
 #define PHYSETUP			0x1014
 #define PHYSETUP_HSFREQRANGE(x)		(((x) & 0x7f) << 16)
-#define PHYSETUP_HSFREQRANGE_MASK	(0x7f << 16)
+#define PHYSETUP_HSFREQRANGE_MASK	GENMASK(22, 16)
 #define PHYSETUP_CFGCLKFREQRANGE(x)	(((x) & 0x3f) << 8)
-#define PHYSETUP_SHUTDOWNZ		(1 << 1)
-#define PHYSETUP_RSTZ			(1 << 0)
+#define PHYSETUP_SHUTDOWNZ		BIT(1)
+#define PHYSETUP_RSTZ			BIT(0)
 
 #define CLOCKSET1			0x101c
-#define CLOCKSET1_LOCK_PHY		(1 << 17)
-#define CLOCKSET1_CLKSEL		(1 << 8)
-#define CLOCKSET1_CLKINSEL_MASK		(3 << 2)
+#define CLOCKSET1_LOCK_PHY		BIT(17)
+#define CLOCKSET1_CLKSEL		BIT(8)
+#define CLOCKSET1_CLKINSEL_MASK		GENMASK(3, 2)
 #define CLOCKSET1_CLKINSEL_EXTAL	0
 #define CLOCKSET1_CLKINSEL_DIG		1
 #define CLOCKSET1_CLKINSEL_DU		2
-#define CLOCKSET1_SHADOW_CLEAR		(1 << 1)
-#define CLOCKSET1_UPDATEPLL		(1 << 0)
+#define CLOCKSET1_SHADOW_CLEAR		BIT(1)
+#define CLOCKSET1_UPDATEPLL		BIT(0)
 
 #define CLOCKSET2			0x1020
 #define CLOCKSET2_M(x)			(((x) & 0xfff) << 16)
@@ -281,15 +281,15 @@
 #define CLOCKSET3_GMP_CNTRL(x)		(((x) & 0x3) << 0)
 
 #define PHTW				0x1034
-#define PHTW_DWEN			(1 << 24)
+#define PHTW_DWEN			BIT(24)
 #define PHTW_TESTDIN_DATA(x)		(((x) & 0xff) << 16)
-#define PHTW_CWEN			(1 << 8)
+#define PHTW_CWEN			BIT(8)
 #define PHTW_TESTDIN_CODE(x)		(((x) & 0xff) << 0)
 
 #define PHTR				0x1038
-#define PHTR_TEST			(1 << 16)
+#define PHTR_TEST			BIT(16)
 
 #define PHTC				0x103c
-#define PHTC_TESTCLR			(1 << 0)
+#define PHTC_TESTCLR			BIT(0)
 
 #endif /* __RCAR_MIPI_DSI_REGS_H__ */
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 9/9] drm/rcar-du: dsi: Convert register bitfields to GENMASK() macro
  2025-09-22 18:54 [PATCH 0/9] drm/rcar-du: dsi: Convert register bits to BIT()/GENMASK() macros Marek Vasut
                   ` (7 preceding siblings ...)
  2025-09-22 18:55 ` [PATCH 8/9] drm/rcar-du: dsi: Convert register bits to BIT() macro Marek Vasut
@ 2025-09-22 18:55 ` Marek Vasut
  8 siblings, 0 replies; 31+ messages in thread
From: Marek Vasut @ 2025-09-22 18:55 UTC (permalink / raw)
  To: dri-devel
  Cc: Marek Vasut, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

Convert register bitfields to GENMASK() macro where applicable.
Use FIELD_PREP() throughout the driver.

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
---
 .../gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c   | 46 +++++------
 .../drm/renesas/rcar-du/rcar_mipi_dsi_regs.h  | 77 +++++++++----------
 2 files changed, 61 insertions(+), 62 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 f91cc35423758..67bab7fbd9288 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
@@ -496,17 +496,17 @@ static void rcar_mipi_dsi_set_display_timing(struct rcar_mipi_dsi *dsi,
 	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
 		vprmset0r |= TXVMVPRMSET0R_HSPOL_LOW;
 
-	vprmset1r = TXVMVPRMSET1R_VACTIVE(mode->vdisplay)
-		  | TXVMVPRMSET1R_VSA(mode->vsync_end - mode->vsync_start);
+	vprmset1r = FIELD_PREP(TXVMVPRMSET1R_VACTIVE_MASK, mode->vdisplay)
+		  | FIELD_PREP(TXVMVPRMSET1R_VSA_MASK, mode->vsync_end - mode->vsync_start);
 
-	vprmset2r = TXVMVPRMSET2R_VFP(mode->vsync_start - mode->vdisplay)
-		  | TXVMVPRMSET2R_VBP(mode->vtotal - mode->vsync_end);
+	vprmset2r = FIELD_PREP(TXVMVPRMSET2R_VFP_MASK, mode->vsync_start - mode->vdisplay)
+		  | FIELD_PREP(TXVMVPRMSET2R_VBP_MASK, mode->vtotal - mode->vsync_end);
 
-	vprmset3r = TXVMVPRMSET3R_HACTIVE(mode->hdisplay)
-		  | TXVMVPRMSET3R_HSA(mode->hsync_end - mode->hsync_start);
+	vprmset3r = FIELD_PREP(TXVMVPRMSET3R_HACTIVE_MASK, mode->hdisplay)
+		  | FIELD_PREP(TXVMVPRMSET3R_HSA_MASK, mode->hsync_end - mode->hsync_start);
 
-	vprmset4r = TXVMVPRMSET4R_HFP(mode->hsync_start - mode->hdisplay)
-		  | TXVMVPRMSET4R_HBP(mode->htotal - mode->hsync_end);
+	vprmset4r = FIELD_PREP(TXVMVPRMSET4R_HFP_MASK, mode->hsync_start - mode->hdisplay)
+		  | FIELD_PREP(TXVMVPRMSET4R_HBP_MASK, mode->htotal - mode->hsync_end);
 
 	rcar_mipi_dsi_write(dsi, TXVMVPRMSET0R, vprmset0r);
 	rcar_mipi_dsi_write(dsi, TXVMVPRMSET1R, vprmset1r);
@@ -553,7 +553,7 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 	/* PHY setting */
 	phy_setup = rcar_mipi_dsi_read(dsi, PHYSETUP);
 	phy_setup &= ~PHYSETUP_HSFREQRANGE_MASK;
-	phy_setup |= PHYSETUP_HSFREQRANGE(setup_info.hsfreqrange);
+	phy_setup |= FIELD_PREP(PHYSETUP_HSFREQRANGE_MASK, setup_info.hsfreqrange);
 	rcar_mipi_dsi_write(dsi, PHYSETUP, phy_setup);
 
 	switch (dsi->info->model) {
@@ -576,13 +576,13 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 	rcar_mipi_dsi_set(dsi, CLOCKSET1, CLOCKSET1_SHADOW_CLEAR);
 	rcar_mipi_dsi_clr(dsi, CLOCKSET1, CLOCKSET1_SHADOW_CLEAR);
 
-	clockset2 = CLOCKSET2_M(setup_info.m - dsi->info->clockset2_m_offset)
-		  | CLOCKSET2_N(setup_info.n - 1)
-		  | CLOCKSET2_VCO_CNTRL(setup_info.clkset->vco_cntrl);
-	clockset3 = CLOCKSET3_PROP_CNTRL(setup_info.clkset->prop_cntrl)
-		  | CLOCKSET3_INT_CNTRL(setup_info.clkset->int_cntrl)
-		  | CLOCKSET3_CPBIAS_CNTRL(setup_info.clkset->cpbias_cntrl)
-		  | CLOCKSET3_GMP_CNTRL(setup_info.clkset->gmp_cntrl);
+	clockset2 = FIELD_PREP(CLOCKSET2_M_MASK, setup_info.m - dsi->info->clockset2_m_offset)
+		  | FIELD_PREP(CLOCKSET2_N_MASK, setup_info.n - 1)
+		  | FIELD_PREP(CLOCKSET2_VCO_CNTRL_MASK, setup_info.clkset->vco_cntrl);
+	clockset3 = FIELD_PREP(CLOCKSET3_PROP_CNTRL_MASK, setup_info.clkset->prop_cntrl)
+		  | FIELD_PREP(CLOCKSET3_INT_CNTRL_MASK, setup_info.clkset->int_cntrl)
+		  | FIELD_PREP(CLOCKSET3_CPBIAS_CNTRL_MASK, setup_info.clkset->cpbias_cntrl)
+		  | FIELD_PREP(CLOCKSET3_GMP_CNTRL_MASK, setup_info.clkset->gmp_cntrl);
 	rcar_mipi_dsi_write(dsi, CLOCKSET2, clockset2);
 	rcar_mipi_dsi_write(dsi, CLOCKSET3, clockset3);
 
@@ -647,16 +647,16 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
 		return -EINVAL;
 	}
 
-	vclkset |= VCLKSET_LANE(dsi->lanes - 1);
+	vclkset |= FIELD_PREP(VCLKSET_LANE_MASK, dsi->lanes - 1);
 
 	switch (dsi->info->model) {
 	case RCAR_DSI_V3U:
 	default:
-		vclkset |= VCLKSET_DIV_V3U(__ffs(setup_info.vclk_divider));
+		vclkset |= FIELD_PREP(VCLKSET_DIV_V3U_MASK, __ffs(setup_info.vclk_divider));
 		break;
 
 	case RCAR_DSI_V4H:
-		vclkset |= VCLKSET_DIV_V4H(__ffs(setup_info.vclk_divider) - 1);
+		vclkset |= FIELD_PREP(VCLKSET_DIV_V4H_MASK, __ffs(setup_info.vclk_divider) - 1);
 		break;
 	}
 
@@ -988,10 +988,10 @@ static ssize_t rcar_mipi_dsi_host_tx_transfer(struct mipi_dsi_host *host,
 	 */
 	rcar_mipi_dsi_write(dsi, TXCMPHDR,
 			    (is_tx_long ? TXCMPHDR_FMT : 0) |
-			    TXCMPHDR_VC(msg->channel) |
-			    TXCMPHDR_DT(msg->type) |
-			    TXCMPHDR_DATA1(packet.header[2]) |
-			    TXCMPHDR_DATA0(packet.header[1]));
+			    FIELD_PREP(TXCMPHDR_VC_MASK, msg->channel) |
+			    FIELD_PREP(TXCMPHDR_DT_MASK, msg->type) |
+			    FIELD_PREP(TXCMPHDR_DATA1_MASK, packet.header[2]) |
+			    FIELD_PREP(TXCMPHDR_DATA0_MASK, packet.header[1]));
 
 	if (is_tx_long) {
 		memcpy(payload, packet.payload,
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 29c806cae3557..c605e7a964dd7 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
@@ -40,18 +40,18 @@
 #define TXCMADDRSET0R			0x140
 #define TXCMPHDR			0x150
 #define TXCMPHDR_FMT			BIT(24)	/* 0:SP 1:LP */
-#define TXCMPHDR_VC(n)			(((n) & 0x3) << 22)
-#define TXCMPHDR_DT(n)			(((n) & 0x3f) << 16)
-#define TXCMPHDR_DATA1(n)		(((n) & 0xff) << 8)
-#define TXCMPHDR_DATA0(n)		(((n) & 0xff) << 0)
+#define TXCMPHDR_VC_MASK		GENMASK(23, 22)
+#define TXCMPHDR_DT_MASK		GENMASK(21, 16)
+#define TXCMPHDR_DATA1_MASK		GENMASK(15, 8)
+#define TXCMPHDR_DATA0_MASK		GENMASK(7, 0)
 #define TXCMPPD0R			0x160
 #define TXCMPPD1R			0x164
 #define TXCMPPD2R			0x168
 #define TXCMPPD3R			0x16c
 
 #define RXSETR				0x200
-#define RXSETR_CRCEN(n)			(((n) & 0xf) << 24)
-#define RXSETR_ECCEN(n)			(((n) & 0xf) << 16)
+#define RXSETR_CRCEN_MASK		GENMASK(27, 24)
+#define RXSETR_ECCEN_MASK		GENMASK(19, 16)
 #define RXPSETR				0x210
 #define RXPSETR_LPPDACC			BIT(0)
 #define RXPSR				0x220
@@ -107,21 +107,21 @@
 #define RXPIER_BTAREQEND		BIT(0)
 #define RXPADDRSET0R			0x230
 #define RXPSIZESETR			0x238
-#define RXPSIZESETR_SIZE(n)		(((n) & 0xf) << 3)
+#define RXPSIZESETR_SIZE_MASK		GENMASK(6, 3)
 #define RXPHDR				0x240
 #define RXPHDR_FMT			BIT(24)	/* 0:SP 1:LP */
-#define RXPHDR_VC(n)			(((n) & 0x3) << 22)
-#define RXPHDR_DT(n)			(((n) & 0x3f) << 16)
-#define RXPHDR_DATA1(n)			(((n) & 0xff) << 8)
-#define RXPHDR_DATA0(n)			(((n) & 0xff) << 0)
+#define RXPHDR_VC_MASK			GENMASK(23, 22)
+#define RXPHDR_DT_MASK			GENMASK(21, 16)
+#define RXPHDR_DATA1_MASK		GENMASK(15, 8)
+#define RXPHDR_DATA0_MASK		GENMASK(7, 0)
 #define RXPPD0R				0x250
 #define RXPPD1R				0x254
 #define RXPPD2R				0x258
 #define RXPPD3R				0x25c
 #define AKEPR				0x300
-#define AKEPR_VC(n)			(((n) & 0x3) << 22)
-#define AKEPR_DT(n)			(((n) & 0x3f) << 16)
-#define AKEPR_ERRRPT(n)			(((n) & 0xffff) << 0)
+#define AKEPR_VC_MASK			GENMASK(23, 22)
+#define AKEPR_DT_MASK			GENMASK(21, 16)
+#define AKEPR_ERRRPT_MASK		GENMASK(15, 0)
 #define RXRESPTOSETR			0x400
 #define TACR				0x500
 #define TASR				0x510
@@ -179,20 +179,20 @@
 #define TXVMVPRMSET0R_BPP_24		2
 
 #define TXVMVPRMSET1R			0x1d4
-#define TXVMVPRMSET1R_VACTIVE(x)	(((x) & 0x7fff) << 16)
-#define TXVMVPRMSET1R_VSA(x)		(((x) & 0xfff) << 0)
+#define TXVMVPRMSET1R_VACTIVE_MASK	GENMASK(30, 16)
+#define TXVMVPRMSET1R_VSA_MASK		GENMASK(11, 0)
 
 #define TXVMVPRMSET2R			0x1d8
-#define TXVMVPRMSET2R_VFP(x)		(((x) & 0x1fff) << 16)
-#define TXVMVPRMSET2R_VBP(x)		(((x) & 0x1fff) << 0)
+#define TXVMVPRMSET2R_VFP_MASK		GENMASK(28, 16)
+#define TXVMVPRMSET2R_VBP_MASK		GENMASK(12, 0)
 
 #define TXVMVPRMSET3R			0x1dc
-#define TXVMVPRMSET3R_HACTIVE(x)	(((x) & 0x7fff) << 16)
-#define TXVMVPRMSET3R_HSA(x)		(((x) & 0xfff) << 0)
+#define TXVMVPRMSET3R_HACTIVE_MASK	GENMASK(30, 16)
+#define TXVMVPRMSET3R_HSA_MASK		GENMASK(11, 0)
 
 #define TXVMVPRMSET4R			0x1e0
-#define TXVMVPRMSET4R_HFP(x)		(((x) & 0x1fff) << 16)
-#define TXVMVPRMSET4R_HBP(x)		(((x) & 0x1fff) << 0)
+#define TXVMVPRMSET4R_HFP_MASK		GENMASK(28, 16)
+#define TXVMVPRMSET4R_HBP_MASK		GENMASK(12, 0)
 
 /*
  * PHY-Protocol Interface (PPI) Registers
@@ -227,35 +227,34 @@
  */
 #define LPCLKSET			0x1000
 #define LPCLKSET_CKEN			BIT(8)
-#define LPCLKSET_LPCLKDIV(x)		(((x) & 0x3f) << 0)
+#define LPCLKSET_LPCLKDIV_MASK		GENMASK(5, 0)
 
 #define CFGCLKSET			0x1004
 #define CFGCLKSET_CKEN			BIT(8)
-#define CFGCLKSET_CFGCLKDIV(x)		(((x) & 0x3f) << 0)
+#define CFGCLKSET_CFGCLKDIV_MASK	GENMASK(5, 0)
 
 #define DOTCLKDIV			0x1008
 #define DOTCLKDIV_CKEN			BIT(8)
-#define DOTCLKDIV_DOTCLKDIV(x)		(((x) & 0x3f) << 0)
+#define DOTCLKDIV_DOTCLKDIV_MASK	GENMASK(5, 0)
 
 #define VCLKSET				0x100c
 #define VCLKSET_CKEN			BIT(16)
 #define VCLKSET_COLOR_YCC		BIT(8) /* 0:RGB 1:YCbCr */
-#define VCLKSET_DIV_V3U(x)		(((x) & 0x3) << 4)
-#define VCLKSET_DIV_V4H(x)		(((x) & 0x7) << 4)
+#define VCLKSET_DIV_V3U_MASK		GENMASK(5, 4)
+#define VCLKSET_DIV_V4H_MASK		GENMASK(6, 4)
 #define VCLKSET_BPP_MASK		GENMASK(3, 2)
 #define VCLKSET_BPP_16			0
 #define VCLKSET_BPP_18			1
 #define VCLKSET_BPP_18L			2
 #define VCLKSET_BPP_24			3
-#define VCLKSET_LANE(x)			(((x) & 0x3) << 0)
+#define VCLKSET_LANE_MASK		GENMASK(1, 0)
 
 #define VCLKEN				0x1010
 #define VCLKEN_CKEN			BIT(0)
 
 #define PHYSETUP			0x1014
-#define PHYSETUP_HSFREQRANGE(x)		(((x) & 0x7f) << 16)
 #define PHYSETUP_HSFREQRANGE_MASK	GENMASK(22, 16)
-#define PHYSETUP_CFGCLKFREQRANGE(x)	(((x) & 0x3f) << 8)
+#define PHYSETUP_CFGCLKFREQRANGE_MASK	GENMASK(13, 8)
 #define PHYSETUP_SHUTDOWNZ		BIT(1)
 #define PHYSETUP_RSTZ			BIT(0)
 
@@ -270,21 +269,21 @@
 #define CLOCKSET1_UPDATEPLL		BIT(0)
 
 #define CLOCKSET2			0x1020
-#define CLOCKSET2_M(x)			(((x) & 0xfff) << 16)
-#define CLOCKSET2_VCO_CNTRL(x)		(((x) & 0x3f) << 8)
-#define CLOCKSET2_N(x)			(((x) & 0xf) << 0)
+#define CLOCKSET2_M_MASK		GENMASK(27, 16)
+#define CLOCKSET2_VCO_CNTRL_MASK	GENMASK(13, 8)
+#define CLOCKSET2_N_MASK		GENMASK(3, 0)
 
 #define CLOCKSET3			0x1024
-#define CLOCKSET3_PROP_CNTRL(x)		(((x) & 0x3f) << 24)
-#define CLOCKSET3_INT_CNTRL(x)		(((x) & 0x3f) << 16)
-#define CLOCKSET3_CPBIAS_CNTRL(x)	(((x) & 0x7f) << 8)
-#define CLOCKSET3_GMP_CNTRL(x)		(((x) & 0x3) << 0)
+#define CLOCKSET3_PROP_CNTRL_MASK	GENMASK(29, 24)
+#define CLOCKSET3_INT_CNTRL_MASK	GENMASK(21, 16)
+#define CLOCKSET3_CPBIAS_CNTRL_MASK	GENMASK(14, 8)
+#define CLOCKSET3_GMP_CNTRL_MASK	GENMASK(1, 0)
 
 #define PHTW				0x1034
 #define PHTW_DWEN			BIT(24)
-#define PHTW_TESTDIN_DATA(x)		(((x) & 0xff) << 16)
+#define PHTW_TESTDIN_DATA_MASK		GENMASK(23, 16)
 #define PHTW_CWEN			BIT(8)
-#define PHTW_TESTDIN_CODE(x)		(((x) & 0xff) << 0)
+#define PHTW_TESTDIN_CODE_MASK		GENMASK(15, 0)
 
 #define PHTR				0x1038
 #define PHTR_TEST			BIT(16)
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* RE: [PATCH 4/9] drm/rcar-du: dsi: Clean up CLOCKSET1 CLKINSEL macros
  2025-09-22 18:55 ` [PATCH 4/9] drm/rcar-du: dsi: Clean up CLOCKSET1 CLKINSEL macros Marek Vasut
@ 2025-09-23  5:45   ` Biju Das
  2025-09-23  8:58     ` Marek Vasut
  0 siblings, 1 reply; 31+ messages in thread
From: Biju Das @ 2025-09-23  5:45 UTC (permalink / raw)
  To: Marek Vasut, dri-devel@lists.freedesktop.org
  Cc: David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, magnus.damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc@vger.kernel.org

Hi Marek,

> -----Original Message-----
> From: Marek Vasut <marek.vasut+renesas@mailbox.org>
> Sent: 22 September 2025 19:55
> Subject: [PATCH 4/9] drm/rcar-du: dsi: Clean up CLOCKSET1 CLKINSEL macros
> 
> Introduce CLOCKSET1_CLKINSEL_MASK macro and remove bitshift from values to make this bitfield usable
> with FIELD_PREP(). There are no users of this bitfield, hence no updates to the DSI driver.
> 
> 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_regs.h | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> 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 c2cb06ef144ed..808861aaf3bfe 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
> @@ -268,9 +268,10 @@
>  #define CLOCKSET1			0x101c
>  #define CLOCKSET1_LOCK_PHY		(1 << 17)
>  #define CLOCKSET1_CLKSEL		(1 << 8)
> -#define CLOCKSET1_CLKINSEL_EXTAL	(0 << 2)

0
> -#define CLOCKSET1_CLKINSEL_DIG		(1 << 2)
4
> -#define CLOCKSET1_CLKINSEL_DU		(1 << 3)
8
> +#define CLOCKSET1_CLKINSEL_MASK		(3 << 2)
> +#define CLOCKSET1_CLKINSEL_EXTAL	0
> +#define CLOCKSET1_CLKINSEL_DIG		1
> +#define CLOCKSET1_CLKINSEL_DU		2


Looks like this patch breaks existing functionality,
as the macro values are different. 

Cheers,
Biju

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/9] drm/rcar-du: dsi: Deduplicate mipi_dsi_pixel_format_to_bpp() usage
  2025-09-22 18:54 ` [PATCH 2/9] drm/rcar-du: dsi: Deduplicate mipi_dsi_pixel_format_to_bpp() usage Marek Vasut
@ 2025-09-23  6:47   ` Geert Uytterhoeven
  2025-09-23  8:55     ` Marek Vasut
  0 siblings, 1 reply; 31+ messages in thread
From: Geert Uytterhoeven @ 2025-09-23  6:47 UTC (permalink / raw)
  To: Marek Vasut
  Cc: dri-devel, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

Hi Marek,

On Mon, 22 Sept 2025 at 20:58, Marek Vasut
<marek.vasut+renesas@mailbox.org> wrote:
> Call mipi_dsi_pixel_format_to_bpp() once in rcar_mipi_dsi_set_display_timing()
> and store the value into a variable. This slightly simplifies the code.
>
> No functional change.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>

Thanks for your patch!

> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> @@ -449,6 +449,7 @@ static void rcar_mipi_dsi_parameters_calc(struct rcar_mipi_dsi *dsi,
>  static void rcar_mipi_dsi_set_display_timing(struct rcar_mipi_dsi *dsi,
>                                              const struct drm_display_mode *mode)
>  {
> +       const int dsibpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
>         u32 setr;
>         u32 vprmset0r;
>         u32 vprmset1r;
> @@ -457,11 +458,11 @@ static void rcar_mipi_dsi_set_display_timing(struct rcar_mipi_dsi *dsi,
>         u32 vprmset4r;
>
>         /* Configuration for Pixel Stream and Packet Header */
> -       if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 24)
> +       if (dsibpp == 24)
>                 rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB24);
> -       else if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 18)
> +       else if (dsibpp == 18)
>                 rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB18);
> -       else if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 16)
> +       else if (dsibpp == 16)

What about using the switch() statement instead?

>                 rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB16);
>         else {
>                 dev_warn(dsi->dev, "unsupported format");

The current code even has a default case ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/9] drm/rcar-du: dsi: Deduplicate mipi_dsi_pixel_format_to_bpp() usage
  2025-09-23  6:47   ` Geert Uytterhoeven
@ 2025-09-23  8:55     ` Marek Vasut
  2025-09-23  8:57       ` Laurent Pinchart
  0 siblings, 1 reply; 31+ messages in thread
From: Marek Vasut @ 2025-09-23  8:55 UTC (permalink / raw)
  To: Geert Uytterhoeven, Marek Vasut
  Cc: dri-devel, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

On 9/23/25 8:47 AM, Geert Uytterhoeven wrote:

Hello Geert,

>> @@ -457,11 +458,11 @@ static void rcar_mipi_dsi_set_display_timing(struct rcar_mipi_dsi *dsi,
>>          u32 vprmset4r;
>>
>>          /* Configuration for Pixel Stream and Packet Header */
>> -       if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 24)
>> +       if (dsibpp == 24)
>>                  rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB24);
>> -       else if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 18)
>> +       else if (dsibpp == 18)
>>                  rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB18);
>> -       else if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 16)
>> +       else if (dsibpp == 16)
> 
> What about using the switch() statement instead?

Not for single-line bodies in the conditionals. The switch {} statement 
would require additional break; in each case and that's not worth it 
here, it would only add noise into the code.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/9] drm/rcar-du: dsi: Deduplicate mipi_dsi_pixel_format_to_bpp() usage
  2025-09-23  8:55     ` Marek Vasut
@ 2025-09-23  8:57       ` Laurent Pinchart
  2025-09-23  9:02         ` Geert Uytterhoeven
  0 siblings, 1 reply; 31+ messages in thread
From: Laurent Pinchart @ 2025-09-23  8:57 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Geert Uytterhoeven, Marek Vasut, dri-devel, David Airlie,
	Geert Uytterhoeven, Kieran Bingham, Maarten Lankhorst,
	Magnus Damm, Maxime Ripard, Simona Vetter, Thomas Zimmermann,
	Tomi Valkeinen, linux-renesas-soc

On Tue, Sep 23, 2025 at 10:55:20AM +0200, Marek Vasut wrote:
> On 9/23/25 8:47 AM, Geert Uytterhoeven wrote:
> 
> Hello Geert,
> 
> >> @@ -457,11 +458,11 @@ static void rcar_mipi_dsi_set_display_timing(struct rcar_mipi_dsi *dsi,
> >>          u32 vprmset4r;
> >>
> >>          /* Configuration for Pixel Stream and Packet Header */
> >> -       if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 24)
> >> +       if (dsibpp == 24)
> >>                  rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB24);
> >> -       else if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 18)
> >> +       else if (dsibpp == 18)
> >>                  rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB18);
> >> -       else if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 16)
> >> +       else if (dsibpp == 16)
> > 
> > What about using the switch() statement instead?
> 
> Not for single-line bodies in the conditionals. The switch {} statement 
> would require additional break; in each case and that's not worth it 
> here, it would only add noise into the code.

I'm a bit surprised. I don't mind much as I don't work on this driver
myself, but for what it's worth I would find a switch statement to be
more readable too. Coding style is of course a matter of personal
preference in many cases.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 4/9] drm/rcar-du: dsi: Clean up CLOCKSET1 CLKINSEL macros
  2025-09-23  5:45   ` Biju Das
@ 2025-09-23  8:58     ` Marek Vasut
  2025-09-23  9:04       ` Biju Das
  0 siblings, 1 reply; 31+ messages in thread
From: Marek Vasut @ 2025-09-23  8:58 UTC (permalink / raw)
  To: Biju Das, Marek Vasut, dri-devel@lists.freedesktop.org
  Cc: David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, magnus.damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc@vger.kernel.org

On 9/23/25 7:45 AM, Biju Das wrote:

Hello Biju,

>> Introduce CLOCKSET1_CLKINSEL_MASK macro and remove bitshift from values to make this bitfield usable
>> with FIELD_PREP(). There are no users of this bitfield, hence no updates to the DSI driver.

...

>> -#define CLOCKSET1_CLKINSEL_EXTAL	(0 << 2)
> 
> 0
>> -#define CLOCKSET1_CLKINSEL_DIG		(1 << 2)
> 4
>> -#define CLOCKSET1_CLKINSEL_DU		(1 << 3)
> 8
>> +#define CLOCKSET1_CLKINSEL_MASK		(3 << 2)
>> +#define CLOCKSET1_CLKINSEL_EXTAL	0
>> +#define CLOCKSET1_CLKINSEL_DIG		1
>> +#define CLOCKSET1_CLKINSEL_DU		2
> 
> 
> Looks like this patch breaks existing functionality,

There are no users of this bitfield, hence no updates to the DSI driver. 
(see commit message). Therefore there is no breakage.

> as the macro values are different.
Use FIELD_PREP(CLOCKSET1_CLKINSEL_MASK, 
CLOCKSET1_CLKINSEL_{EXTAL,DIG,DU}) with these updated macros, that will 
place the value in the correct location .

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/9] drm/rcar-du: dsi: Deduplicate mipi_dsi_pixel_format_to_bpp() usage
  2025-09-23  8:57       ` Laurent Pinchart
@ 2025-09-23  9:02         ` Geert Uytterhoeven
  0 siblings, 0 replies; 31+ messages in thread
From: Geert Uytterhoeven @ 2025-09-23  9:02 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, Marek Vasut, dri-devel, David Airlie,
	Geert Uytterhoeven, Kieran Bingham, Maarten Lankhorst,
	Magnus Damm, Maxime Ripard, Simona Vetter, Thomas Zimmermann,
	Tomi Valkeinen, linux-renesas-soc

On Tue, 23 Sept 2025 at 10:57, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Tue, Sep 23, 2025 at 10:55:20AM +0200, Marek Vasut wrote:
> > On 9/23/25 8:47 AM, Geert Uytterhoeven wrote:
> > >> @@ -457,11 +458,11 @@ static void rcar_mipi_dsi_set_display_timing(struct rcar_mipi_dsi *dsi,
> > >>          u32 vprmset4r;
> > >>
> > >>          /* Configuration for Pixel Stream and Packet Header */
> > >> -       if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 24)
> > >> +       if (dsibpp == 24)
> > >>                  rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB24);
> > >> -       else if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 18)
> > >> +       else if (dsibpp == 18)
> > >>                  rcar_mipi_dsi_write(dsi, TXVMPSPHSETR, TXVMPSPHSETR_DT_RGB18);
> > >> -       else if (mipi_dsi_pixel_format_to_bpp(dsi->format) == 16)
> > >> +       else if (dsibpp == 16)
> > >
> > > What about using the switch() statement instead?
> >
> > Not for single-line bodies in the conditionals. The switch {} statement
> > would require additional break; in each case and that's not worth it
> > here, it would only add noise into the code.
>
> I'm a bit surprised. I don't mind much as I don't work on this driver
> myself, but for what it's worth I would find a switch statement to be
> more readable too. Coding style is of course a matter of personal
> preference in many cases.

Exactly.
And you would no longer need the dsibpp local variable.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 31+ messages in thread

* RE: [PATCH 4/9] drm/rcar-du: dsi: Clean up CLOCKSET1 CLKINSEL macros
  2025-09-23  8:58     ` Marek Vasut
@ 2025-09-23  9:04       ` Biju Das
  2025-09-23  9:11         ` Marek Vasut
  0 siblings, 1 reply; 31+ messages in thread
From: Biju Das @ 2025-09-23  9:04 UTC (permalink / raw)
  To: marek.vasut@mailbox.org, Marek Vasut,
	dri-devel@lists.freedesktop.org
  Cc: David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, magnus.damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc@vger.kernel.org

Hi Marek,

> -----Original Message-----
> From: Marek Vasut <marek.vasut@mailbox.org>
> Sent: 23 September 2025 09:59
> Subject: Re: [PATCH 4/9] drm/rcar-du: dsi: Clean up CLOCKSET1 CLKINSEL macros
> 
> On 9/23/25 7:45 AM, Biju Das wrote:
> 
> Hello Biju,
> 
> >> Introduce CLOCKSET1_CLKINSEL_MASK macro and remove bitshift from
> >> values to make this bitfield usable with FIELD_PREP(). There are no users of this bitfield, hence no
> updates to the DSI driver.
> 
> ...
> 
> >> -#define CLOCKSET1_CLKINSEL_EXTAL	(0 << 2)
> >
> > 0
> >> -#define CLOCKSET1_CLKINSEL_DIG		(1 << 2)
> > 4
> >> -#define CLOCKSET1_CLKINSEL_DU		(1 << 3)
> > 8
> >> +#define CLOCKSET1_CLKINSEL_MASK		(3 << 2)
> >> +#define CLOCKSET1_CLKINSEL_EXTAL	0
> >> +#define CLOCKSET1_CLKINSEL_DIG		1
> >> +#define CLOCKSET1_CLKINSEL_DU		2
> >
> >
> > Looks like this patch breaks existing functionality,
> 
> There are no users of this bitfield, hence no updates to the DSI driver.
> (see commit message). Therefore there is no breakage.

Can we remove it, if there are no users? Or you still prefer to keep it.

Cheers,
Biju



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/9] drm/rcar-du: dsi: Clean up VCLKSET register macros
  2025-09-22 18:54 ` [PATCH 3/9] drm/rcar-du: dsi: Clean up VCLKSET register macros Marek Vasut
@ 2025-09-23  9:10   ` kernel test robot
  2025-09-23 11:49   ` Laurent Pinchart
  1 sibling, 0 replies; 31+ messages in thread
From: kernel test robot @ 2025-09-23  9:10 UTC (permalink / raw)
  To: Marek Vasut, dri-devel
  Cc: oe-kbuild-all, Marek Vasut, David Airlie, Geert Uytterhoeven,
	Kieran Bingham, Laurent Pinchart, Maarten Lankhorst, Magnus Damm,
	Maxime Ripard, Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

Hi Marek,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next drm-tip/drm-tip next-20250922]
[cannot apply to linus/master v6.17-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Marek-Vasut/drm-rcar-du-dsi-Fix-missing-parameter-in-RXSETR_-EN-macros/20250923-025951
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20250922185740.153759-4-marek.vasut%2Brenesas%40mailbox.org
patch subject: [PATCH 3/9] drm/rcar-du: dsi: Clean up VCLKSET register macros
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20250923/202509231609.HYEkeR1s-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250923/202509231609.HYEkeR1s-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509231609.HYEkeR1s-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c: In function 'rcar_mipi_dsi_startup':
>> drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c:626:28: error: implicit declaration of function 'FIELD_PREP' [-Wimplicit-function-declaration]
     626 |                 vclkset |= FIELD_PREP(VCLKSET_BPP_MASK, VCLKSET_BPP_24);
         |                            ^~~~~~~~~~


vim +/FIELD_PREP +626 drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c

   503	
   504	static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
   505					 const struct drm_display_mode *mode)
   506	{
   507		struct dsi_setup_info setup_info = {};
   508		unsigned int timeout;
   509		int ret;
   510		int dsi_format;
   511		u32 phy_setup;
   512		u32 clockset2, clockset3;
   513		u32 ppisetr;
   514		u32 vclkset;
   515	
   516		/* Checking valid format */
   517		dsi_format = mipi_dsi_pixel_format_to_bpp(dsi->format);
   518		if (dsi_format < 0) {
   519			dev_warn(dsi->dev, "invalid format");
   520			return -EINVAL;
   521		}
   522	
   523		/* Parameters Calculation */
   524		rcar_mipi_dsi_parameters_calc(dsi, dsi->clocks.pll,
   525					      mode->clock * 1000, &setup_info);
   526	
   527		/* LPCLK enable */
   528		rcar_mipi_dsi_set(dsi, LPCLKSET, LPCLKSET_CKEN);
   529	
   530		/* CFGCLK enabled */
   531		rcar_mipi_dsi_set(dsi, CFGCLKSET, CFGCLKSET_CKEN);
   532	
   533		rcar_mipi_dsi_clr(dsi, PHYSETUP, PHYSETUP_RSTZ);
   534		rcar_mipi_dsi_clr(dsi, PHYSETUP, PHYSETUP_SHUTDOWNZ);
   535	
   536		rcar_mipi_dsi_set(dsi, PHTC, PHTC_TESTCLR);
   537		rcar_mipi_dsi_clr(dsi, PHTC, PHTC_TESTCLR);
   538	
   539		/* PHY setting */
   540		phy_setup = rcar_mipi_dsi_read(dsi, PHYSETUP);
   541		phy_setup &= ~PHYSETUP_HSFREQRANGE_MASK;
   542		phy_setup |= PHYSETUP_HSFREQRANGE(setup_info.hsfreqrange);
   543		rcar_mipi_dsi_write(dsi, PHYSETUP, phy_setup);
   544	
   545		switch (dsi->info->model) {
   546		case RCAR_DSI_V3U:
   547		default:
   548			ret = rcar_mipi_dsi_init_phtw_v3u(dsi);
   549			if (ret < 0)
   550				return ret;
   551			break;
   552	
   553		case RCAR_DSI_V4H:
   554			ret = rcar_mipi_dsi_init_phtw_v4h(dsi, &setup_info);
   555			if (ret < 0)
   556				return ret;
   557			break;
   558		}
   559	
   560		/* PLL Clock Setting */
   561		rcar_mipi_dsi_clr(dsi, CLOCKSET1, CLOCKSET1_SHADOW_CLEAR);
   562		rcar_mipi_dsi_set(dsi, CLOCKSET1, CLOCKSET1_SHADOW_CLEAR);
   563		rcar_mipi_dsi_clr(dsi, CLOCKSET1, CLOCKSET1_SHADOW_CLEAR);
   564	
   565		clockset2 = CLOCKSET2_M(setup_info.m - dsi->info->clockset2_m_offset)
   566			  | CLOCKSET2_N(setup_info.n - 1)
   567			  | CLOCKSET2_VCO_CNTRL(setup_info.clkset->vco_cntrl);
   568		clockset3 = CLOCKSET3_PROP_CNTRL(setup_info.clkset->prop_cntrl)
   569			  | CLOCKSET3_INT_CNTRL(setup_info.clkset->int_cntrl)
   570			  | CLOCKSET3_CPBIAS_CNTRL(setup_info.clkset->cpbias_cntrl)
   571			  | CLOCKSET3_GMP_CNTRL(setup_info.clkset->gmp_cntrl);
   572		rcar_mipi_dsi_write(dsi, CLOCKSET2, clockset2);
   573		rcar_mipi_dsi_write(dsi, CLOCKSET3, clockset3);
   574	
   575		rcar_mipi_dsi_clr(dsi, CLOCKSET1, CLOCKSET1_UPDATEPLL);
   576		rcar_mipi_dsi_set(dsi, CLOCKSET1, CLOCKSET1_UPDATEPLL);
   577		udelay(10);
   578		rcar_mipi_dsi_clr(dsi, CLOCKSET1, CLOCKSET1_UPDATEPLL);
   579	
   580		rcar_mipi_dsi_clr(dsi, TXSETR, TXSETR_LANECNT_MASK);
   581		rcar_mipi_dsi_set(dsi, TXSETR, dsi->lanes - 1);
   582	
   583		ppisetr = ((BIT(dsi->lanes) - 1) & PPISETR_DLEN_MASK) | PPISETR_CLEN;
   584		rcar_mipi_dsi_write(dsi, PPISETR, ppisetr);
   585	
   586		rcar_mipi_dsi_set(dsi, PHYSETUP, PHYSETUP_SHUTDOWNZ);
   587		rcar_mipi_dsi_set(dsi, PHYSETUP, PHYSETUP_RSTZ);
   588		usleep_range(400, 500);
   589	
   590		/* Checking PPI clock status register */
   591		for (timeout = 10; timeout > 0; --timeout) {
   592			if ((rcar_mipi_dsi_read(dsi, PPICLSR) & PPICLSR_STPST) &&
   593			    (rcar_mipi_dsi_read(dsi, PPIDLSR) & PPIDLSR_STPST) &&
   594			    (rcar_mipi_dsi_read(dsi, CLOCKSET1) & CLOCKSET1_LOCK_PHY))
   595				break;
   596	
   597			usleep_range(1000, 2000);
   598		}
   599	
   600		if (!timeout) {
   601			dev_err(dsi->dev, "failed to enable PPI clock\n");
   602			return -ETIMEDOUT;
   603		}
   604	
   605		switch (dsi->info->model) {
   606		case RCAR_DSI_V3U:
   607		default:
   608			ret = rcar_mipi_dsi_post_init_phtw_v3u(dsi);
   609			if (ret < 0)
   610				return ret;
   611			break;
   612	
   613		case RCAR_DSI_V4H:
   614			ret = rcar_mipi_dsi_post_init_phtw_v4h(dsi, &setup_info);
   615			if (ret < 0)
   616				return ret;
   617			break;
   618		}
   619	
   620		/* Enable DOT clock */
   621		vclkset = VCLKSET_CKEN;
   622		rcar_mipi_dsi_write(dsi, VCLKSET, vclkset);
   623	
   624		/* Output is always RGB, never YCbCr */
   625		if (dsi_format == 24)
 > 626			vclkset |= FIELD_PREP(VCLKSET_BPP_MASK, VCLKSET_BPP_24);
   627		else if (dsi_format == 18)
   628			vclkset |= FIELD_PREP(VCLKSET_BPP_MASK, VCLKSET_BPP_18);
   629		else if (dsi_format == 16)
   630			vclkset |= FIELD_PREP(VCLKSET_BPP_MASK, VCLKSET_BPP_16);
   631		else {
   632			dev_warn(dsi->dev, "unsupported format");
   633			return -EINVAL;
   634		}
   635	
   636		vclkset |= VCLKSET_LANE(dsi->lanes - 1);
   637	
   638		switch (dsi->info->model) {
   639		case RCAR_DSI_V3U:
   640		default:
   641			vclkset |= VCLKSET_DIV_V3U(__ffs(setup_info.vclk_divider));
   642			break;
   643	
   644		case RCAR_DSI_V4H:
   645			vclkset |= VCLKSET_DIV_V4H(__ffs(setup_info.vclk_divider) - 1);
   646			break;
   647		}
   648	
   649		rcar_mipi_dsi_write(dsi, VCLKSET, vclkset);
   650	
   651		/* After setting VCLKSET register, enable VCLKEN */
   652		rcar_mipi_dsi_set(dsi, VCLKEN, VCLKEN_CKEN);
   653	
   654		dev_dbg(dsi->dev, "DSI device is started\n");
   655	
   656		return 0;
   657	}
   658	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 4/9] drm/rcar-du: dsi: Clean up CLOCKSET1 CLKINSEL macros
  2025-09-23  9:04       ` Biju Das
@ 2025-09-23  9:11         ` Marek Vasut
  0 siblings, 0 replies; 31+ messages in thread
From: Marek Vasut @ 2025-09-23  9:11 UTC (permalink / raw)
  To: Biju Das, dri-devel@lists.freedesktop.org
  Cc: David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Laurent Pinchart, Maarten Lankhorst, magnus.damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc@vger.kernel.org

On 9/23/25 11:04 AM, Biju Das wrote:

Hello Biju,

>>>> -#define CLOCKSET1_CLKINSEL_EXTAL	(0 << 2)
>>>
>>> 0
>>>> -#define CLOCKSET1_CLKINSEL_DIG		(1 << 2)
>>> 4
>>>> -#define CLOCKSET1_CLKINSEL_DU		(1 << 3)
>>> 8
>>>> +#define CLOCKSET1_CLKINSEL_MASK		(3 << 2)
>>>> +#define CLOCKSET1_CLKINSEL_EXTAL	0
>>>> +#define CLOCKSET1_CLKINSEL_DIG		1
>>>> +#define CLOCKSET1_CLKINSEL_DU		2
>>>
>>>
>>> Looks like this patch breaks existing functionality,
>>
>> There are no users of this bitfield, hence no updates to the DSI driver.
>> (see commit message). Therefore there is no breakage.
> 
> Can we remove it, if there are no users? Or you still prefer to keep it.
The header does contain a complete list of bitfields, that's why I kept it.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/9] drm/rcar-du: dsi: Fix missing parameter in RXSETR_...EN macros
  2025-09-22 18:54 ` [PATCH 1/9] drm/rcar-du: dsi: Fix missing parameter in RXSETR_...EN macros Marek Vasut
@ 2025-09-23 11:42   ` Laurent Pinchart
  0 siblings, 0 replies; 31+ messages in thread
From: Laurent Pinchart @ 2025-09-23 11:42 UTC (permalink / raw)
  To: Marek Vasut
  Cc: dri-devel, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Maarten Lankhorst, Magnus Damm, Maxime Ripard, Simona Vetter,
	Thomas Zimmermann, Tomi Valkeinen, linux-renesas-soc

On Mon, Sep 22, 2025 at 08:54:57PM +0200, Marek Vasut wrote:
> The RXSETR_CRCEN(n) and RXSETR_ECCEN(n) macros both take parameter (n),
> add the missing macro parameter. Neither of those macros is used by the
> driver, so for now the bug is harmless.
> 
> Fixes: 685e8dae19df ("drm/rcar-du: dsi: Implement DSI command support")
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> 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
> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi_regs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 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 76521276e2af8..dd871e17dcf53 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
> @@ -50,8 +50,8 @@
>  #define TXCMPPD3R			0x16c
>  
>  #define RXSETR				0x200
> -#define RXSETR_CRCEN			(((n) & 0xf) << 24)
> -#define RXSETR_ECCEN			(((n) & 0xf) << 16)
> +#define RXSETR_CRCEN(n)			(((n) & 0xf) << 24)
> +#define RXSETR_ECCEN(n)			(((n) & 0xf) << 16)
>  #define RXPSETR				0x210
>  #define RXPSETR_LPPDACC			(1 << 0)
>  #define RXPSR				0x220

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/9] drm/rcar-du: dsi: Clean up VCLKSET register macros
  2025-09-22 18:54 ` [PATCH 3/9] drm/rcar-du: dsi: Clean up VCLKSET register macros Marek Vasut
  2025-09-23  9:10   ` kernel test robot
@ 2025-09-23 11:49   ` Laurent Pinchart
  2025-09-24  0:00     ` Marek Vasut
  1 sibling, 1 reply; 31+ messages in thread
From: Laurent Pinchart @ 2025-09-23 11:49 UTC (permalink / raw)
  To: Marek Vasut
  Cc: dri-devel, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Maarten Lankhorst, Magnus Damm, Maxime Ripard, Simona Vetter,
	Thomas Zimmermann, Tomi Valkeinen, linux-renesas-soc

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+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      |  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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 5/9] drm/rcar-du: dsi: Clean up TXVMPSPHSETR DT macros
  2025-09-22 18:55 ` [PATCH 5/9] drm/rcar-du: dsi: Clean up TXVMPSPHSETR DT macros Marek Vasut
@ 2025-09-23 11:52   ` Laurent Pinchart
  0 siblings, 0 replies; 31+ messages in thread
From: Laurent Pinchart @ 2025-09-23 11:52 UTC (permalink / raw)
  To: Marek Vasut
  Cc: dri-devel, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Maarten Lankhorst, Magnus Damm, Maxime Ripard, Simona Vetter,
	Thomas Zimmermann, Tomi Valkeinen, linux-renesas-soc

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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 6/9] drm/rcar-du: dsi: Respect DSI mode flags
  2025-09-22 18:55 ` [PATCH 6/9] drm/rcar-du: dsi: Respect DSI mode flags Marek Vasut
@ 2025-09-23 11:54   ` Laurent Pinchart
  0 siblings, 0 replies; 31+ messages in thread
From: Laurent Pinchart @ 2025-09-23 11:54 UTC (permalink / raw)
  To: Marek Vasut
  Cc: dri-devel, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Maarten Lankhorst, Magnus Damm, Maxime Ripard, Simona Vetter,
	Thomas Zimmermann, Tomi Valkeinen, linux-renesas-soc

Hi Marek,

Thank you for the patch.

On Mon, Sep 22, 2025 at 08:55:02PM +0200, Marek Vasut wrote:
> Cache DSI mode flags in new mode_flags member of struct rcar_mipi_dsi .
> Configure TXVMSETR register based on the content of DSI mode flags in
> case the controller operates in video mode.
> 
> Rename TXVMSETR_H..BPEN_EN to TXVMSETR_H..BPEN and drop TXVMSETR_H..BPEN_DIS
> which resolves to 0. Update TXVMSETR_VSEN in the same manner. Replace
> TXVMSETR_SYNSEQ_PULSES with a code comment next to TXVMSETR_SYNSEQ_EVENTS
> because TXVMSETR_SYNSEQ_PULSES resolves to 0.
> 
> 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>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> 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
> ---
>  .../gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c    | 18 +++++++++++++++---
>  .../drm/renesas/rcar-du/rcar_mipi_dsi_regs.h   | 15 +++++----------
>  2 files changed, 20 insertions(+), 13 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 1591837ff472c..36bd9de61ce05 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_mipi_dsi.c
> @@ -71,6 +71,7 @@ struct rcar_mipi_dsi {
>  	} clocks;
>  
>  	enum mipi_dsi_pixel_format format;
> +	unsigned long mode_flags;
>  	unsigned int num_data_lanes;
>  	unsigned int lanes;
>  };
> @@ -473,9 +474,19 @@ static void rcar_mipi_dsi_set_display_timing(struct rcar_mipi_dsi *dsi,
>  	}
>  
>  	/* Configuration for Blanking sequence and Input Pixel */
> -	setr = TXVMSETR_HSABPEN_EN | TXVMSETR_HBPBPEN_EN
> -	     | TXVMSETR_HFPBPEN_EN | TXVMSETR_SYNSEQ_PULSES
> -	     | TXVMSETR_PIXWDTH | TXVMSETR_VSTPM;
> +	setr = TXVMSETR_PIXWDTH | TXVMSETR_VSTPM;
> +
> +	if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
> +		if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE))
> +			setr |= TXVMSETR_SYNSEQ_EVENTS;
> +		if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HFP))
> +			setr |= TXVMSETR_HFPBPEN;
> +		if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HBP))
> +			setr |= TXVMSETR_HBPBPEN;
> +		if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA))
> +			setr |= TXVMSETR_HSABPEN;
> +	}
> +
>  	rcar_mipi_dsi_write(dsi, TXVMSETR, setr);
>  
>  	/* Configuration for Video Parameters */
> @@ -916,6 +927,7 @@ static int rcar_mipi_dsi_host_attach(struct mipi_dsi_host *host,
>  
>  	dsi->lanes = device->lanes;
>  	dsi->format = device->format;
> +	dsi->mode_flags = device->mode_flags;
>  
>  	dsi->next_bridge = devm_drm_of_get_bridge(dsi->dev, dsi->dev->of_node,
>  						  1, 0);
> 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 1a8d377ea85fc..99a88ea35aacd 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
> @@ -140,18 +140,13 @@
>   * Video Mode Register
>   */
>  #define TXVMSETR			0x180
> -#define TXVMSETR_SYNSEQ_PULSES		(0 << 16)
> -#define TXVMSETR_SYNSEQ_EVENTS		(1 << 16)
> +#define TXVMSETR_SYNSEQ_EVENTS		(1 << 16) /* 0:Pulses 1:Events */
>  #define TXVMSETR_VSTPM			(1 << 15)
>  #define TXVMSETR_PIXWDTH		(1 << 8)
> -#define TXVMSETR_VSEN_EN		(1 << 4)
> -#define TXVMSETR_VSEN_DIS		(0 << 4)
> -#define TXVMSETR_HFPBPEN_EN		(1 << 2)
> -#define TXVMSETR_HFPBPEN_DIS		(0 << 2)
> -#define TXVMSETR_HBPBPEN_EN		(1 << 1)
> -#define TXVMSETR_HBPBPEN_DIS		(0 << 1)
> -#define TXVMSETR_HSABPEN_EN		(1 << 0)
> -#define TXVMSETR_HSABPEN_DIS		(0 << 0)
> +#define TXVMSETR_VSEN			(1 << 4)
> +#define TXVMSETR_HFPBPEN		(1 << 2)
> +#define TXVMSETR_HBPBPEN		(1 << 1)
> +#define TXVMSETR_HSABPEN		(1 << 0)
>  
>  #define TXVMCR				0x190
>  #define TXVMCR_VFCLR			(1 << 12)

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 7/9] drm/rcar-du: dsi: Clean up handling of DRM mode flags
  2025-09-22 18:55 ` [PATCH 7/9] drm/rcar-du: dsi: Clean up handling of DRM " Marek Vasut
@ 2025-09-23 13:26   ` Laurent Pinchart
  2025-09-24  0:12     ` Marek Vasut
  2025-09-24  1:18     ` Sebastian Reichel
  0 siblings, 2 replies; 31+ messages in thread
From: Laurent Pinchart @ 2025-09-23 13:26 UTC (permalink / raw)
  To: Marek Vasut
  Cc: dri-devel, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Maarten Lankhorst, Magnus Damm, Maxime Ripard, Simona Vetter,
	Thomas Zimmermann, Tomi Valkeinen, linux-renesas-soc,
	Sebastian Reichel

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+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  | 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

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/9] drm/rcar-du: dsi: Convert register bits to BIT() macro
  2025-09-22 18:55 ` [PATCH 8/9] drm/rcar-du: dsi: Convert register bits to BIT() macro Marek Vasut
@ 2025-09-23 14:16   ` Laurent Pinchart
  2025-09-24  0:34     ` Marek Vasut
  0 siblings, 1 reply; 31+ messages in thread
From: Laurent Pinchart @ 2025-09-23 14:16 UTC (permalink / raw)
  To: Marek Vasut
  Cc: dri-devel, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Maarten Lankhorst, Magnus Damm, Maxime Ripard, Simona Vetter,
	Thomas Zimmermann, Tomi Valkeinen, linux-renesas-soc

On Mon, Sep 22, 2025 at 08:55:04PM +0200, Marek Vasut wrote:
> Convert register bits to BIT() macro where applicable. This is done
> automatically using regex 's@(1 << \([0-9]\+\))@BIT(\1)', except for
> bitfields which are manually updated to use GENMASK().
> 
> 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
> ---
>  .../drm/renesas/rcar-du/rcar_mipi_dsi_regs.h  | 248 +++++++++---------
>  1 file changed, 124 insertions(+), 124 deletions(-)
> 
> 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 48c3b679b2663..29c806cae3557 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
> @@ -9,37 +9,37 @@
>  #define __RCAR_MIPI_DSI_REGS_H__
>  
>  #define LINKSR				0x010
> -#define LINKSR_LPBUSY			(1 << 1)
> -#define LINKSR_HSBUSY			(1 << 0)
> +#define LINKSR_LPBUSY			BIT(1)
> +#define LINKSR_HSBUSY			BIT(0)
>  
>  #define TXSETR				0x100
> -#define TXSETR_LANECNT_MASK		(0x3 << 0)
> +#define TXSETR_LANECNT_MASK		GENMASK(1, 0)
>  
>  /*
>   * DSI Command Transfer Registers
>   */
>  #define TXCMSETR			0x110
> -#define TXCMSETR_SPDTYP			(1 << 8)	/* 0:HS 1:LP */
> -#define TXCMSETR_LPPDACC		(1 << 0)
> +#define TXCMSETR_SPDTYP			BIT(8)	/* 0:HS 1:LP */
> +#define TXCMSETR_LPPDACC		BIT(0)
>  #define TXCMCR				0x120
> -#define TXCMCR_BTATYP			(1 << 2)
> -#define TXCMCR_BTAREQ			(1 << 1)
> -#define TXCMCR_TXREQ			(1 << 0)
> +#define TXCMCR_BTATYP			BIT(2)
> +#define TXCMCR_BTAREQ			BIT(1)
> +#define TXCMCR_TXREQ			BIT(0)
>  #define TXCMSR				0x130
> -#define TXCMSR_CLSNERR			(1 << 18)
> -#define TXCMSR_AXIERR			(1 << 16)
> -#define TXCMSR_TXREQEND			(1 << 0)
> +#define TXCMSR_CLSNERR			BIT(18)
> +#define TXCMSR_AXIERR			BIT(16)
> +#define TXCMSR_TXREQEND			BIT(0)
>  #define TXCMSCR				0x134
> -#define TXCMSCR_CLSNERR			(1 << 18)
> -#define TXCMSCR_AXIERR			(1 << 16)
> -#define TXCMSCR_TXREQEND		(1 << 0)
> +#define TXCMSCR_CLSNERR			BIT(18)
> +#define TXCMSCR_AXIERR			BIT(16)
> +#define TXCMSCR_TXREQEND		BIT(0)
>  #define TXCMIER				0x138
> -#define TXCMIER_CLSNERR			(1 << 18)
> -#define TXCMIER_AXIERR			(1 << 16)
> -#define TXCMIER_TXREQEND		(1 << 0)
> +#define TXCMIER_CLSNERR			BIT(18)
> +#define TXCMIER_AXIERR			BIT(16)
> +#define TXCMIER_TXREQEND		BIT(0)
>  #define TXCMADDRSET0R			0x140
>  #define TXCMPHDR			0x150
> -#define TXCMPHDR_FMT			(1 << 24)	/* 0:SP 1:LP */
> +#define TXCMPHDR_FMT			BIT(24)	/* 0:SP 1:LP */
>  #define TXCMPHDR_VC(n)			(((n) & 0x3) << 22)
>  #define TXCMPHDR_DT(n)			(((n) & 0x3f) << 16)
>  #define TXCMPHDR_DATA1(n)		(((n) & 0xff) << 8)
> @@ -53,63 +53,63 @@
>  #define RXSETR_CRCEN(n)			(((n) & 0xf) << 24)
>  #define RXSETR_ECCEN(n)			(((n) & 0xf) << 16)
>  #define RXPSETR				0x210
> -#define RXPSETR_LPPDACC			(1 << 0)
> +#define RXPSETR_LPPDACC			BIT(0)
>  #define RXPSR				0x220
> -#define RXPSR_ECCERR1B			(1 << 28)
> -#define RXPSR_UEXTRGERR			(1 << 25)
> -#define RXPSR_RESPTOERR			(1 << 24)
> -#define RXPSR_OVRERR			(1 << 23)
> -#define RXPSR_AXIERR			(1 << 22)
> -#define RXPSR_CRCERR			(1 << 21)
> -#define RXPSR_WCERR			(1 << 20)
> -#define RXPSR_UEXDTERR			(1 << 19)
> -#define RXPSR_UEXPKTERR			(1 << 18)
> -#define RXPSR_ECCERR			(1 << 17)
> -#define RXPSR_MLFERR			(1 << 16)
> -#define RXPSR_RCVACK			(1 << 14)
> -#define RXPSR_RCVEOT			(1 << 10)
> -#define RXPSR_RCVAKE			(1 << 9)
> -#define RXPSR_RCVRESP			(1 << 8)
> -#define RXPSR_BTAREQEND			(1 << 0)
> +#define RXPSR_ECCERR1B			BIT(28)
> +#define RXPSR_UEXTRGERR			BIT(25)
> +#define RXPSR_RESPTOERR			BIT(24)
> +#define RXPSR_OVRERR			BIT(23)
> +#define RXPSR_AXIERR			BIT(22)
> +#define RXPSR_CRCERR			BIT(21)
> +#define RXPSR_WCERR			BIT(20)
> +#define RXPSR_UEXDTERR			BIT(19)
> +#define RXPSR_UEXPKTERR			BIT(18)
> +#define RXPSR_ECCERR			BIT(17)
> +#define RXPSR_MLFERR			BIT(16)
> +#define RXPSR_RCVACK			BIT(14)
> +#define RXPSR_RCVEOT			BIT(10)
> +#define RXPSR_RCVAKE			BIT(9)
> +#define RXPSR_RCVRESP			BIT(8)
> +#define RXPSR_BTAREQEND			BIT(0)
>  #define RXPSCR				0x224
> -#define RXPSCR_ECCERR1B			(1 << 28)
> -#define RXPSCR_UEXTRGERR		(1 << 25)
> -#define RXPSCR_RESPTOERR		(1 << 24)
> -#define RXPSCR_OVRERR			(1 << 23)
> -#define RXPSCR_AXIERR			(1 << 22)
> -#define RXPSCR_CRCERR			(1 << 21)
> -#define RXPSCR_WCERR			(1 << 20)
> -#define RXPSCR_UEXDTERR			(1 << 19)
> -#define RXPSCR_UEXPKTERR		(1 << 18)
> -#define RXPSCR_ECCERR			(1 << 17)
> -#define RXPSCR_MLFERR			(1 << 16)
> -#define RXPSCR_RCVACK			(1 << 14)
> -#define RXPSCR_RCVEOT			(1 << 10)
> -#define RXPSCR_RCVAKE			(1 << 9)
> -#define RXPSCR_RCVRESP			(1 << 8)
> -#define RXPSCR_BTAREQEND		(1 << 0)
> +#define RXPSCR_ECCERR1B			BIT(28)
> +#define RXPSCR_UEXTRGERR		BIT(25)
> +#define RXPSCR_RESPTOERR		BIT(24)
> +#define RXPSCR_OVRERR			BIT(23)
> +#define RXPSCR_AXIERR			BIT(22)
> +#define RXPSCR_CRCERR			BIT(21)
> +#define RXPSCR_WCERR			BIT(20)
> +#define RXPSCR_UEXDTERR			BIT(19)
> +#define RXPSCR_UEXPKTERR		BIT(18)
> +#define RXPSCR_ECCERR			BIT(17)
> +#define RXPSCR_MLFERR			BIT(16)
> +#define RXPSCR_RCVACK			BIT(14)
> +#define RXPSCR_RCVEOT			BIT(10)
> +#define RXPSCR_RCVAKE			BIT(9)
> +#define RXPSCR_RCVRESP			BIT(8)
> +#define RXPSCR_BTAREQEND		BIT(0)
>  #define RXPIER				0x228
> -#define RXPIER_ECCERR1B			(1 << 28)
> -#define RXPIER_UEXTRGERR		(1 << 25)
> -#define RXPIER_RESPTOERR		(1 << 24)
> -#define RXPIER_OVRERR			(1 << 23)
> -#define RXPIER_AXIERR			(1 << 22)
> -#define RXPIER_CRCERR			(1 << 21)
> -#define RXPIER_WCERR			(1 << 20)
> -#define RXPIER_UEXDTERR			(1 << 19)
> -#define RXPIER_UEXPKTERR		(1 << 18)
> -#define RXPIER_ECCERR			(1 << 17)
> -#define RXPIER_MLFERR			(1 << 16)
> -#define RXPIER_RCVACK			(1 << 14)
> -#define RXPIER_RCVEOT			(1 << 10)
> -#define RXPIER_RCVAKE			(1 << 9)
> -#define RXPIER_RCVRESP			(1 << 8)
> -#define RXPIER_BTAREQEND		(1 << 0)
> +#define RXPIER_ECCERR1B			BIT(28)
> +#define RXPIER_UEXTRGERR		BIT(25)
> +#define RXPIER_RESPTOERR		BIT(24)
> +#define RXPIER_OVRERR			BIT(23)
> +#define RXPIER_AXIERR			BIT(22)
> +#define RXPIER_CRCERR			BIT(21)
> +#define RXPIER_WCERR			BIT(20)
> +#define RXPIER_UEXDTERR			BIT(19)
> +#define RXPIER_UEXPKTERR		BIT(18)
> +#define RXPIER_ECCERR			BIT(17)
> +#define RXPIER_MLFERR			BIT(16)
> +#define RXPIER_RCVACK			BIT(14)
> +#define RXPIER_RCVEOT			BIT(10)
> +#define RXPIER_RCVAKE			BIT(9)
> +#define RXPIER_RCVRESP			BIT(8)
> +#define RXPIER_BTAREQEND		BIT(0)
>  #define RXPADDRSET0R			0x230
>  #define RXPSIZESETR			0x238
>  #define RXPSIZESETR_SIZE(n)		(((n) & 0xf) << 3)
>  #define RXPHDR				0x240
> -#define RXPHDR_FMT			(1 << 24)	/* 0:SP 1:LP */
> +#define RXPHDR_FMT			BIT(24)	/* 0:SP 1:LP */
>  #define RXPHDR_VC(n)			(((n) & 0x3) << 22)
>  #define RXPHDR_DT(n)			(((n) & 0x3f) << 16)
>  #define RXPHDR_DATA1(n)			(((n) & 0xff) << 8)
> @@ -128,38 +128,38 @@
>  #define TASCR				0x514
>  #define TAIER				0x518
>  #define TOSR				0x610
> -#define TOSR_TATO			(1 << 2)
> -#define TOSR_LRXHTO			(1 << 1)
> -#define TOSR_HRXTO			(1 << 0)
> +#define TOSR_TATO			BIT(2)
> +#define TOSR_LRXHTO			BIT(1)
> +#define TOSR_HRXTO			BIT(0)
>  #define TOSCR				0x614
> -#define TOSCR_TATO			(1 << 2)
> -#define TOSCR_LRXHTO			(1 << 1)
> -#define TOSCR_HRXTO			(1 << 0)
> +#define TOSCR_TATO			BIT(2)
> +#define TOSCR_LRXHTO			BIT(1)
> +#define TOSCR_HRXTO			BIT(0)
>  
>  /*
>   * Video Mode Register
>   */
>  #define TXVMSETR			0x180
> -#define TXVMSETR_SYNSEQ_EVENTS		(1 << 16) /* 0:Pulses 1:Events */
> -#define TXVMSETR_VSTPM			(1 << 15)
> -#define TXVMSETR_PIXWDTH		(1 << 8)
> -#define TXVMSETR_VSEN			(1 << 4)
> -#define TXVMSETR_HFPBPEN		(1 << 2)
> -#define TXVMSETR_HBPBPEN		(1 << 1)
> -#define TXVMSETR_HSABPEN		(1 << 0)
> +#define TXVMSETR_SYNSEQ_EVENTS		BIT(16) /* 0:Pulses 1:Events */
> +#define TXVMSETR_VSTPM			BIT(15)
> +#define TXVMSETR_PIXWDTH		BIT(8)

This is a 3 bits field according to the documentation.

> +#define TXVMSETR_VSEN			BIT(4)
> +#define TXVMSETR_HFPBPEN		BIT(2)
> +#define TXVMSETR_HBPBPEN		BIT(1)
> +#define TXVMSETR_HSABPEN		BIT(0)
>  
>  #define TXVMCR				0x190
> -#define TXVMCR_VFCLR			(1 << 12)
> -#define TXVMCR_EN_VIDEO			(1 << 0)
> +#define TXVMCR_VFCLR			BIT(12)
> +#define TXVMCR_EN_VIDEO			BIT(0)
>  
>  #define TXVMSR				0x1a0
> -#define TXVMSR_STR			(1 << 16)
> -#define TXVMSR_VFRDY			(1 << 12)
> -#define TXVMSR_ACT			(1 << 8)
> -#define TXVMSR_RDY			(1 << 0)
> +#define TXVMSR_STR			BIT(16)
> +#define TXVMSR_VFRDY			BIT(12)
> +#define TXVMSR_ACT			BIT(8)
> +#define TXVMSR_RDY			BIT(0)
>  
>  #define TXVMSCR				0x1a4
> -#define TXVMSCR_STR			(1 << 16)
> +#define TXVMSCR_STR			BIT(16)
>  
>  #define TXVMPSPHSETR			0x1c0
>  #define TXVMPSPHSETR_DT_MASK		(0x3f << 16)
> @@ -170,10 +170,10 @@
>  #define TXVMPSPHSETR_DT_YCBCR16		0x2c
>  
>  #define TXVMVPRMSET0R			0x1d0
> -#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_HSPOL_LOW		BIT(17) /* 0:High 1:Low */
> +#define TXVMVPRMSET0R_VSPOL_LOW		BIT(16) /* 0:High 1:Low */
> +#define TXVMVPRMSET0R_CSPC_YCbCr	BIT(4) /* 0:RGB 1:YCbCr */

This is a 2 bits field.

> +#define TXVMVPRMSET0R_BPP_MASK		GENMASK(2, 0)
>  #define TXVMVPRMSET0R_BPP_16		0
>  #define TXVMVPRMSET0R_BPP_18		1
>  #define TXVMVPRMSET0R_BPP_24		2
> @@ -198,51 +198,51 @@
>   * PHY-Protocol Interface (PPI) Registers
>   */
>  #define PPISETR				0x700
> -#define PPISETR_DLEN_MASK		(0xf << 0)
> -#define PPISETR_CLEN			(1 << 8)
> +#define PPISETR_DLEN_MASK		GENMASK(3, 0)
> +#define PPISETR_CLEN			BIT(8)

While at it I'd swap those two lines, to sort fields by decreasing bit
position as in the rest of the file.

>  
>  #define PPICLCR				0x710
> -#define PPICLCR_TXREQHS			(1 << 8)
> -#define PPICLCR_TXULPSEXT		(1 << 1)
> -#define PPICLCR_TXULPSCLK		(1 << 0)
> +#define PPICLCR_TXREQHS			BIT(8)
> +#define PPICLCR_TXULPSEXT		BIT(1)
> +#define PPICLCR_TXULPSCLK		BIT(0)
>  
>  #define PPICLSR				0x720
> -#define PPICLSR_HSTOLP			(1 << 27)
> -#define PPICLSR_TOHS			(1 << 26)
> -#define PPICLSR_STPST			(1 << 0)
> +#define PPICLSR_HSTOLP			BIT(27)
> +#define PPICLSR_TOHS			BIT(26)
> +#define PPICLSR_STPST			BIT(0)
>  
>  #define PPICLSCR			0x724
> -#define PPICLSCR_HSTOLP			(1 << 27)
> -#define PPICLSCR_TOHS			(1 << 26)
> +#define PPICLSCR_HSTOLP			BIT(27)
> +#define PPICLSCR_TOHS			BIT(26)
>  
>  #define PPIDL0SR			0x740
> -#define PPIDL0SR_DIR			(1 << 10)
> -#define PPIDL0SR_STPST			(1 << 6)
> +#define PPIDL0SR_DIR			BIT(10)
> +#define PPIDL0SR_STPST			BIT(6)
>  
>  #define PPIDLSR				0x760
> -#define PPIDLSR_STPST			(0xf << 0)
> +#define PPIDLSR_STPST			GENMASK(3, 0)
>  
>  /*
>   * Clocks registers
>   */
>  #define LPCLKSET			0x1000
> -#define LPCLKSET_CKEN			(1 << 8)
> +#define LPCLKSET_CKEN			BIT(8)
>  #define LPCLKSET_LPCLKDIV(x)		(((x) & 0x3f) << 0)
>  
>  #define CFGCLKSET			0x1004
> -#define CFGCLKSET_CKEN			(1 << 8)
> +#define CFGCLKSET_CKEN			BIT(8)
>  #define CFGCLKSET_CFGCLKDIV(x)		(((x) & 0x3f) << 0)
>  
>  #define DOTCLKDIV			0x1008
> -#define DOTCLKDIV_CKEN			(1 << 8)
> +#define DOTCLKDIV_CKEN			BIT(8)
>  #define DOTCLKDIV_DOTCLKDIV(x)		(((x) & 0x3f) << 0)
>  
>  #define VCLKSET				0x100c
> -#define VCLKSET_CKEN			(1 << 16)
> -#define VCLKSET_COLOR_YCC		(1 << 8) /* 0:RGB 1:YCbCr */
> +#define VCLKSET_CKEN			BIT(16)
> +#define VCLKSET_COLOR_YCC		BIT(8) /* 0:RGB 1:YCbCr */
>  #define VCLKSET_DIV_V3U(x)		(((x) & 0x3) << 4)
>  #define VCLKSET_DIV_V4H(x)		(((x) & 0x7) << 4)
> -#define VCLKSET_BPP_MASK		(3 << 2)
> +#define VCLKSET_BPP_MASK		GENMASK(3, 2)
>  #define VCLKSET_BPP_16			0
>  #define VCLKSET_BPP_18			1
>  #define VCLKSET_BPP_18L			2
> @@ -250,24 +250,24 @@
>  #define VCLKSET_LANE(x)			(((x) & 0x3) << 0)
>  
>  #define VCLKEN				0x1010
> -#define VCLKEN_CKEN			(1 << 0)
> +#define VCLKEN_CKEN			BIT(0)
>  
>  #define PHYSETUP			0x1014
>  #define PHYSETUP_HSFREQRANGE(x)		(((x) & 0x7f) << 16)
> -#define PHYSETUP_HSFREQRANGE_MASK	(0x7f << 16)
> +#define PHYSETUP_HSFREQRANGE_MASK	GENMASK(22, 16)
>  #define PHYSETUP_CFGCLKFREQRANGE(x)	(((x) & 0x3f) << 8)
> -#define PHYSETUP_SHUTDOWNZ		(1 << 1)
> -#define PHYSETUP_RSTZ			(1 << 0)
> +#define PHYSETUP_SHUTDOWNZ		BIT(1)
> +#define PHYSETUP_RSTZ			BIT(0)
>  
>  #define CLOCKSET1			0x101c
> -#define CLOCKSET1_LOCK_PHY		(1 << 17)
> -#define CLOCKSET1_CLKSEL		(1 << 8)
> -#define CLOCKSET1_CLKINSEL_MASK		(3 << 2)
> +#define CLOCKSET1_LOCK_PHY		BIT(17)
> +#define CLOCKSET1_CLKSEL		BIT(8)

This is a two bits field.

> +#define CLOCKSET1_CLKINSEL_MASK		GENMASK(3, 2)
>  #define CLOCKSET1_CLKINSEL_EXTAL	0
>  #define CLOCKSET1_CLKINSEL_DIG		1
>  #define CLOCKSET1_CLKINSEL_DU		2
> -#define CLOCKSET1_SHADOW_CLEAR		(1 << 1)
> -#define CLOCKSET1_UPDATEPLL		(1 << 0)
> +#define CLOCKSET1_SHADOW_CLEAR		BIT(1)
> +#define CLOCKSET1_UPDATEPLL		BIT(0)
>  
>  #define CLOCKSET2			0x1020
>  #define CLOCKSET2_M(x)			(((x) & 0xfff) << 16)
> @@ -281,15 +281,15 @@
>  #define CLOCKSET3_GMP_CNTRL(x)		(((x) & 0x3) << 0)
>  
>  #define PHTW				0x1034
> -#define PHTW_DWEN			(1 << 24)
> +#define PHTW_DWEN			BIT(24)
>  #define PHTW_TESTDIN_DATA(x)		(((x) & 0xff) << 16)
> -#define PHTW_CWEN			(1 << 8)
> +#define PHTW_CWEN			BIT(8)
>  #define PHTW_TESTDIN_CODE(x)		(((x) & 0xff) << 0)
>  
>  #define PHTR				0x1038
> -#define PHTR_TEST			(1 << 16)
> +#define PHTR_TEST			BIT(16)

And this is a 8 bits field.

>  
>  #define PHTC				0x103c
> -#define PHTC_TESTCLR			(1 << 0)
> +#define PHTC_TESTCLR			BIT(0)
>  
>  #endif /* __RCAR_MIPI_DSI_REGS_H__ */

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/9] drm/rcar-du: dsi: Clean up VCLKSET register macros
  2025-09-23 11:49   ` Laurent Pinchart
@ 2025-09-24  0:00     ` Marek Vasut
  0 siblings, 0 replies; 31+ messages in thread
From: Marek Vasut @ 2025-09-24  0:00 UTC (permalink / raw)
  To: Laurent Pinchart, Marek Vasut
  Cc: dri-devel, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Maarten Lankhorst, Magnus Damm, Maxime Ripard, Simona Vetter,
	Thomas Zimmermann, Tomi Valkeinen, linux-renesas-soc

On 9/23/25 1:49 PM, Laurent Pinchart wrote:

Hello Laurent,

[...]

>> +#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.
I'll opt for the former option then . It fits with the GENMASK() macros.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 7/9] drm/rcar-du: dsi: Clean up handling of DRM mode flags
  2025-09-23 13:26   ` Laurent Pinchart
@ 2025-09-24  0:12     ` Marek Vasut
  2025-09-24  1:18     ` Sebastian Reichel
  1 sibling, 0 replies; 31+ messages in thread
From: Marek Vasut @ 2025-09-24  0:12 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Maarten Lankhorst, Magnus Damm, Maxime Ripard, Simona Vetter,
	Thomas Zimmermann, Tomi Valkeinen, linux-renesas-soc,
	Sebastian Reichel

On 9/23/25 3:26 PM, Laurent Pinchart wrote:
> Hi Marek,

Hello Laurent,

> 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.

Maybe not _exact_ opposite.

DRM_MODE_FLAG_P[HV]SYNC set -- devices with HVsync pulse active high
DRM_MODE_FLAG_N[HV]SYNC set -- devices with HVsync pulse active low
Neither flag set -- devices without HVsync pulse, like DE panels or so
Both flags set -- invalid

> 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 ?).
This looks like a bug indeed. Also, the ST77xx are DSI-to-TCON chips, 
they directly drive the TFT matrix, I don't think they have a concept of 
sync signal polarity at all.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 8/9] drm/rcar-du: dsi: Convert register bits to BIT() macro
  2025-09-23 14:16   ` Laurent Pinchart
@ 2025-09-24  0:34     ` Marek Vasut
  0 siblings, 0 replies; 31+ messages in thread
From: Marek Vasut @ 2025-09-24  0:34 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: dri-devel, David Airlie, Geert Uytterhoeven, Kieran Bingham,
	Maarten Lankhorst, Magnus Damm, Maxime Ripard, Simona Vetter,
	Thomas Zimmermann, Tomi Valkeinen, linux-renesas-soc

On 9/23/25 4:16 PM, Laurent Pinchart wrote:

Hello Laurent,

[...]

>>   #define CLOCKSET1			0x101c
>> -#define CLOCKSET1_LOCK_PHY		(1 << 17)
>> -#define CLOCKSET1_CLKSEL		(1 << 8)
>> -#define CLOCKSET1_CLKINSEL_MASK		(3 << 2)
>> +#define CLOCKSET1_LOCK_PHY		BIT(17)
>> +#define CLOCKSET1_CLKSEL		BIT(8)
> 
> This is a two bits field.

Fixed in v2 2/10 .

>> +#define CLOCKSET1_CLKINSEL_MASK		GENMASK(3, 2)
>>   #define CLOCKSET1_CLKINSEL_EXTAL	0
>>   #define CLOCKSET1_CLKINSEL_DIG		1
>>   #define CLOCKSET1_CLKINSEL_DU		2
>> -#define CLOCKSET1_SHADOW_CLEAR		(1 << 1)
>> -#define CLOCKSET1_UPDATEPLL		(1 << 0)
>> +#define CLOCKSET1_SHADOW_CLEAR		BIT(1)
>> +#define CLOCKSET1_UPDATEPLL		BIT(0)
>>   
>>   #define CLOCKSET2			0x1020
>>   #define CLOCKSET2_M(x)			(((x) & 0xfff) << 16)
>> @@ -281,15 +281,15 @@
>>   #define CLOCKSET3_GMP_CNTRL(x)		(((x) & 0x3) << 0)
>>   
>>   #define PHTW				0x1034
>> -#define PHTW_DWEN			(1 << 24)
>> +#define PHTW_DWEN			BIT(24)
>>   #define PHTW_TESTDIN_DATA(x)		(((x) & 0xff) << 16)
>> -#define PHTW_CWEN			(1 << 8)
>> +#define PHTW_CWEN			BIT(8)
>>   #define PHTW_TESTDIN_CODE(x)		(((x) & 0xff) << 0)
>>   
>>   #define PHTR				0x1038
>> -#define PHTR_TEST			(1 << 16)
>> +#define PHTR_TEST			BIT(16)
> 
> And this is a 8 bits field.
I'll fix this in v3, once I get further feedback on V2, I missed this one.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 7/9] drm/rcar-du: dsi: Clean up handling of DRM mode flags
  2025-09-23 13:26   ` Laurent Pinchart
  2025-09-24  0:12     ` Marek Vasut
@ 2025-09-24  1:18     ` Sebastian Reichel
  2025-09-24  2:36       ` Marek Vasut
  1 sibling, 1 reply; 31+ messages in thread
From: Sebastian Reichel @ 2025-09-24  1:18 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, dri-devel, David Airlie, Geert Uytterhoeven,
	Kieran Bingham, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]

Hi,

On Tue, Sep 23, 2025 at 04:26:16PM +0300, Laurent Pinchart wrote:
> 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 ?).

Yeah, it was supposed to be 

static const struct drm_display_mode t28cp45tn89_mode = {
    ...
    .flags = DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NHSYNC
};

The driver works with the panel, because the driver defaults to
DRM_MODE_FLAG_N[HV]SYNC and ignores the related flags, so just the
DRM_MODE_FLAG_P[HV]SYNC one is applied:

static int st7789v_prepare(struct drm_panel *panel)
{
    ...

	if (ctx->info->mode->flags & DRM_MODE_FLAG_PVSYNC)
		polarity |= ST7789V_RGBCTRL_VSYNC_HIGH;
	if (ctx->info->mode->flags & DRM_MODE_FLAG_PHSYNC)
		polarity |= ST7789V_RGBCTRL_HSYNC_HIGH;

    ...
}

Greetings and thanks for the bug report,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 7/9] drm/rcar-du: dsi: Clean up handling of DRM mode flags
  2025-09-24  1:18     ` Sebastian Reichel
@ 2025-09-24  2:36       ` Marek Vasut
  2025-09-24 20:53         ` Laurent Pinchart
  0 siblings, 1 reply; 31+ messages in thread
From: Marek Vasut @ 2025-09-24  2:36 UTC (permalink / raw)
  To: Sebastian Reichel, Laurent Pinchart
  Cc: Marek Vasut, dri-devel, David Airlie, Geert Uytterhoeven,
	Kieran Bingham, Maarten Lankhorst, Magnus Damm, Maxime Ripard,
	Simona Vetter, Thomas Zimmermann, Tomi Valkeinen,
	linux-renesas-soc

On 9/24/25 3:18 AM, Sebastian Reichel wrote:

Hello Sebastian,

> On Tue, Sep 23, 2025 at 04:26:16PM +0300, Laurent Pinchart wrote:
>> 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 ?).
> 
> Yeah, it was supposed to be
> 
> static const struct drm_display_mode t28cp45tn89_mode = {
>      ...
>      .flags = DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NHSYNC
> };
> 
> The driver works with the panel, because the driver defaults to
> DRM_MODE_FLAG_N[HV]SYNC and ignores the related flags, so just the
> DRM_MODE_FLAG_P[HV]SYNC one is applied:
> 
> static int st7789v_prepare(struct drm_panel *panel)
> {
>      ...
> 
> 	if (ctx->info->mode->flags & DRM_MODE_FLAG_PVSYNC)
> 		polarity |= ST7789V_RGBCTRL_VSYNC_HIGH;
> 	if (ctx->info->mode->flags & DRM_MODE_FLAG_PHSYNC)
> 		polarity |= ST7789V_RGBCTRL_HSYNC_HIGH;
> 
>      ...
> }
> 
> Greetings and thanks for the bug report,
Will you send a patch ? If so, please CC me, so I can provide RB.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 7/9] drm/rcar-du: dsi: Clean up handling of DRM mode flags
  2025-09-24  2:36       ` Marek Vasut
@ 2025-09-24 20:53         ` Laurent Pinchart
  0 siblings, 0 replies; 31+ messages in thread
From: Laurent Pinchart @ 2025-09-24 20:53 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Sebastian Reichel, Marek Vasut, dri-devel, David Airlie,
	Geert Uytterhoeven, Kieran Bingham, Maarten Lankhorst,
	Magnus Damm, Maxime Ripard, Simona Vetter, Thomas Zimmermann,
	Tomi Valkeinen, linux-renesas-soc

On Wed, Sep 24, 2025 at 04:36:00AM +0200, Marek Vasut wrote:
> On 9/24/25 3:18 AM, Sebastian Reichel wrote:
> 
> Hello Sebastian,
> 
> > On Tue, Sep 23, 2025 at 04:26:16PM +0300, Laurent Pinchart wrote:
> >> 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 ?).
> > 
> > Yeah, it was supposed to be
> > 
> > static const struct drm_display_mode t28cp45tn89_mode = {
> >      ...
> >      .flags = DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NHSYNC
> > };
> > 
> > The driver works with the panel, because the driver defaults to
> > DRM_MODE_FLAG_N[HV]SYNC and ignores the related flags, so just the
> > DRM_MODE_FLAG_P[HV]SYNC one is applied:
> > 
> > static int st7789v_prepare(struct drm_panel *panel)
> > {
> >      ...
> > 
> > 	if (ctx->info->mode->flags & DRM_MODE_FLAG_PVSYNC)
> > 		polarity |= ST7789V_RGBCTRL_VSYNC_HIGH;
> > 	if (ctx->info->mode->flags & DRM_MODE_FLAG_PHSYNC)
> > 		polarity |= ST7789V_RGBCTRL_HSYNC_HIGH;
> > 
> >      ...
> > }
> > 
> > Greetings and thanks for the bug report,
> 
> Will you send a patch ? If so, please CC me, so I can provide RB.

Feel free to include my

    Reported-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

in that patch.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2025-09-24 20:53 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-22 18:54 [PATCH 0/9] drm/rcar-du: dsi: Convert register bits to BIT()/GENMASK() macros Marek Vasut
2025-09-22 18:54 ` [PATCH 1/9] drm/rcar-du: dsi: Fix missing parameter in RXSETR_...EN macros Marek Vasut
2025-09-23 11:42   ` Laurent Pinchart
2025-09-22 18:54 ` [PATCH 2/9] drm/rcar-du: dsi: Deduplicate mipi_dsi_pixel_format_to_bpp() usage Marek Vasut
2025-09-23  6:47   ` Geert Uytterhoeven
2025-09-23  8:55     ` Marek Vasut
2025-09-23  8:57       ` Laurent Pinchart
2025-09-23  9:02         ` Geert Uytterhoeven
2025-09-22 18:54 ` [PATCH 3/9] drm/rcar-du: dsi: Clean up VCLKSET register macros Marek Vasut
2025-09-23  9:10   ` kernel test robot
2025-09-23 11:49   ` Laurent Pinchart
2025-09-24  0:00     ` Marek Vasut
2025-09-22 18:55 ` [PATCH 4/9] drm/rcar-du: dsi: Clean up CLOCKSET1 CLKINSEL macros Marek Vasut
2025-09-23  5:45   ` Biju Das
2025-09-23  8:58     ` Marek Vasut
2025-09-23  9:04       ` Biju Das
2025-09-23  9:11         ` Marek Vasut
2025-09-22 18:55 ` [PATCH 5/9] drm/rcar-du: dsi: Clean up TXVMPSPHSETR DT macros Marek Vasut
2025-09-23 11:52   ` Laurent Pinchart
2025-09-22 18:55 ` [PATCH 6/9] drm/rcar-du: dsi: Respect DSI mode flags Marek Vasut
2025-09-23 11:54   ` Laurent Pinchart
2025-09-22 18:55 ` [PATCH 7/9] drm/rcar-du: dsi: Clean up handling of DRM " Marek Vasut
2025-09-23 13:26   ` Laurent Pinchart
2025-09-24  0:12     ` Marek Vasut
2025-09-24  1:18     ` Sebastian Reichel
2025-09-24  2:36       ` Marek Vasut
2025-09-24 20:53         ` Laurent Pinchart
2025-09-22 18:55 ` [PATCH 8/9] drm/rcar-du: dsi: Convert register bits to BIT() macro Marek Vasut
2025-09-23 14:16   ` Laurent Pinchart
2025-09-24  0:34     ` Marek Vasut
2025-09-22 18:55 ` [PATCH 9/9] drm/rcar-du: dsi: Convert register bitfields to GENMASK() macro Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.