Merge tag 'drm-rcar-next-20230325' of...

Merge tag 'drm-rcar-next-20230325' of git://git.kernel.org/pub/scm/linux/kernel/git/pinchartl/linux into drm-next

Miscellaneous fixes and improvements for rcar-du
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20230325204922.GD19335@pendragon.ideasonboard.com

drivers/gpu/drm/rcar-du/rcar_du_crtc.c

@@ -298,13 +298,26 @@ static void rcar_du_crtc_set_display_timing(struct rcar_du_crtc *rcrtc)
 		escr = params.escr;
 	}
 
-	if (rcdu->info->gen < 4) {
+	/*
+	 * The ESCR register only exists in DU channels that can output to an
+	 * LVDS or DPAT, and the OTAR register in DU channels that can output
+	 * to a DPAD.
+	 */
+	if ((rcdu->info->routes[RCAR_DU_OUTPUT_DPAD0].possible_crtcs |
+	     rcdu->info->routes[RCAR_DU_OUTPUT_DPAD1].possible_crtcs |
+	     rcdu->info->routes[RCAR_DU_OUTPUT_LVDS0].possible_crtcs |
+	     rcdu->info->routes[RCAR_DU_OUTPUT_LVDS1].possible_crtcs) &
+	    BIT(rcrtc->index)) {
 		dev_dbg(rcrtc->dev->dev, "%s: ESCR 0x%08x\n", __func__, escr);
 
 		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? ESCR13 : ESCR02, escr);
-		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
 	}
 
+	if ((rcdu->info->routes[RCAR_DU_OUTPUT_DPAD0].possible_crtcs |
+	     rcdu->info->routes[RCAR_DU_OUTPUT_DPAD1].possible_crtcs) &
+	    BIT(rcrtc->index))
+		rcar_du_crtc_write(rcrtc, rcrtc->index % 2 ? OTAR13 : OTAR02, 0);
+
 	/* Signal polarities */
 	dsmr = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0)
 	     | ((mode->flags & DRM_MODE_FLAG_PHSYNC) ? DSMR_HSL : 0)
@@ -749,16 +762,17 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc *crtc,
 
 	/*
 	 * On D3/E3 the dot clock is provided by the LVDS encoder attached to
-	 * the DU channel. We need to enable its clock output explicitly if
-	 * the LVDS output is disabled.
+	 * the DU channel. We need to enable its clock output explicitly before
+	 * starting the CRTC, as the bridge hasn't been enabled by the atomic
+	 * helpers yet.
 	 */
-	if (rcdu->info->lvds_clk_mask & BIT(rcrtc->index) &&
-	    rstate->outputs == BIT(RCAR_DU_OUTPUT_DPAD0)) {
+	if (rcdu->info->lvds_clk_mask & BIT(rcrtc->index)) {
+		bool dot_clk_only = rstate->outputs == BIT(RCAR_DU_OUTPUT_DPAD0);
 		struct drm_bridge *bridge = rcdu->lvds[rcrtc->index];
 		const struct drm_display_mode *mode =
 			&crtc->state->adjusted_mode;
 
-		rcar_lvds_pclk_enable(bridge, mode->clock * 1000);
+		rcar_lvds_pclk_enable(bridge, mode->clock * 1000, dot_clk_only);
 	}
 
 	/*
@@ -795,15 +809,16 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
 	rcar_du_crtc_stop(rcrtc);
 	rcar_du_crtc_put(rcrtc);
 
-	if (rcdu->info->lvds_clk_mask & BIT(rcrtc->index) &&
-	    rstate->outputs == BIT(RCAR_DU_OUTPUT_DPAD0)) {
+	if (rcdu->info->lvds_clk_mask & BIT(rcrtc->index)) {
+		bool dot_clk_only = rstate->outputs == BIT(RCAR_DU_OUTPUT_DPAD0);
 		struct drm_bridge *bridge = rcdu->lvds[rcrtc->index];
 
 		/*
 		 * Disable the LVDS clock output, see
-		 * rcar_du_crtc_atomic_enable().
+		 * rcar_du_crtc_atomic_enable(). When the LVDS output is used,
+		 * this also disables the LVDS encoder.
 		 */
-		rcar_lvds_pclk_disable(bridge);
+		rcar_lvds_pclk_disable(bridge, dot_clk_only);
 	}
 
 	if ((rcdu->info->dsi_clk_mask & BIT(rcrtc->index)) &&
@@ -815,7 +830,6 @@ static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc,
 		 * Disable the DSI clock output, see
 		 * rcar_du_crtc_atomic_enable().
 		 */
-
 		rcar_mipi_dsi_pclk_disable(bridge);
 	}
 

drivers/gpu/drm/rcar-du/rcar_du_encoder.c

@@ -109,8 +109,8 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
 	renc = drmm_encoder_alloc(&rcdu->ddev, struct rcar_du_encoder, base,
 				  &rcar_du_encoder_funcs, DRM_MODE_ENCODER_NONE,
 				  NULL);
-	if (!renc)
-		return -ENOMEM;
+	if (IS_ERR(renc))
+		return PTR_ERR(renc);
 
 	renc->output = output;
 

drivers/gpu/drm/rcar-du/rcar_du_group.c

@@ -138,6 +138,7 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
 {
 	struct rcar_du_device *rcdu = rgrp->dev;
 	u32 defr7 = DEFR7_CODE;
+	u32 dorcr;
 
 	/* Enable extended features */
 	rcar_du_group_write(rgrp, DEFR, DEFR_CODE | DEFR_DEFE);
@@ -174,8 +175,15 @@ static void rcar_du_group_setup(struct rcar_du_group *rgrp)
 	/*
 	 * Use DS1PR and DS2PR to configure planes priorities and connects the
 	 * superposition 0 to DU0 pins. DU1 pins will be configured dynamically.
+	 *
+	 * Groups that have a single channel have a hardcoded configuration. On
+	 * Gen3 and newer, the documentation requires PG1T, DK1S and PG1D_DS1 to
+	 * always be set in this case.
 	 */
-	rcar_du_group_write(rgrp, DORCR, DORCR_PG1D_DS1 | DORCR_DPRS);
+	dorcr = DORCR_PG0D_DS0 | DORCR_DPRS;
+	if (rcdu->info->gen >= 3 && rgrp->num_crtcs == 1)
+		dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
+	rcar_du_group_write(rgrp, DORCR, dorcr);
 
 	/* Apply planes to CRTCs association. */
 	mutex_lock(&rgrp->lock);
@@ -349,7 +357,7 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
 	struct rcar_du_device *rcdu = rgrp->dev;
 	u32 dorcr = rcar_du_group_read(rgrp, DORCR);
 
-	dorcr &= ~(DORCR_PG2T | DORCR_DK2S | DORCR_PG2D_MASK);
+	dorcr &= ~(DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_MASK);
 
 	/*
 	 * Set the DPAD1 pins sources. Select CRTC 0 if explicitly requested and
@@ -357,9 +365,9 @@ int rcar_du_group_set_routing(struct rcar_du_group *rgrp)
 	 * by default.
 	 */
 	if (rcdu->dpad1_source == rgrp->index * 2)
-		dorcr |= DORCR_PG2D_DS1;
+		dorcr |= DORCR_PG1D_DS0;
 	else
-		dorcr |= DORCR_PG2T | DORCR_DK2S | DORCR_PG2D_DS2;
+		dorcr |= DORCR_PG1T | DORCR_DK1S | DORCR_PG1D_DS1;
 
 	rcar_du_group_write(rgrp, DORCR, dorcr);
 

drivers/gpu/drm/rcar-du/rcar_du_regs.h

@@ -511,19 +511,19 @@
  */
 
 #define DORCR			0x11000
-#define DORCR_PG2T		(1 << 30)
-#define DORCR_DK2S		(1 << 28)
-#define DORCR_PG2D_DS1		(0 << 24)
-#define DORCR_PG2D_DS2		(1 << 24)
-#define DORCR_PG2D_FIX0		(2 << 24)
-#define DORCR_PG2D_DOOR		(3 << 24)
-#define DORCR_PG2D_MASK		(3 << 24)
-#define DORCR_DR1D		(1 << 21)
-#define DORCR_PG1D_DS1		(0 << 16)
-#define DORCR_PG1D_DS2		(1 << 16)
-#define DORCR_PG1D_FIX0		(2 << 16)
-#define DORCR_PG1D_DOOR		(3 << 16)
-#define DORCR_PG1D_MASK		(3 << 16)
+#define DORCR_PG1T		(1 << 30)
+#define DORCR_DK1S		(1 << 28)
+#define DORCR_PG1D_DS0		(0 << 24)
+#define DORCR_PG1D_DS1		(1 << 24)
+#define DORCR_PG1D_FIX0		(2 << 24)
+#define DORCR_PG1D_DOOR		(3 << 24)
+#define DORCR_PG1D_MASK		(3 << 24)
+#define DORCR_DR0D		(1 << 21)
+#define DORCR_PG0D_DS0		(0 << 16)
+#define DORCR_PG0D_DS1		(1 << 16)
+#define DORCR_PG0D_FIX0		(2 << 16)
+#define DORCR_PG0D_DOOR		(3 << 16)
+#define DORCR_PG0D_MASK		(3 << 16)
 #define DORCR_RGPV		(1 << 4)
 #define DORCR_DPRS		(1 << 0)
 

drivers/gpu/drm/rcar-du/rcar_du_vsp.c

@@ -73,7 +73,7 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
 			.src.y2 = mode->vdisplay << 16,
 			.zpos = 0,
 		},
-		.format = rcar_du_format_info(DRM_FORMAT_ARGB8888),
+		.format = rcar_du_format_info(DRM_FORMAT_XRGB8888),
 		.source = RCAR_DU_PLANE_VSPD1,
 		.colorkey = 0,
 	};

drivers/gpu/drm/rcar-du/rcar_lvds.c

@@ -269,8 +269,8 @@ static void rcar_lvds_d3_e3_pll_calc(struct rcar_lvds *lvds, struct clk *clk,
 		pll->pll_m, pll->pll_n, pll->pll_e, pll->div);
 }
 
-static void __rcar_lvds_pll_setup_d3_e3(struct rcar_lvds *lvds,
-					unsigned int freq, bool dot_clock_only)
+static void rcar_lvds_pll_setup_d3_e3(struct rcar_lvds *lvds,
+				      unsigned int freq, bool dot_clock_only)
 {
 	struct pll_info pll = { .diff = (unsigned long)-1 };
 	u32 lvdpllcr;
@@ -305,52 +305,8 @@ static void __rcar_lvds_pll_setup_d3_e3(struct rcar_lvds *lvds,
 		rcar_lvds_write(lvds, LVDDIV, 0);
 }
 
-static void rcar_lvds_pll_setup_d3_e3(struct rcar_lvds *lvds, unsigned int freq)
-{
-	__rcar_lvds_pll_setup_d3_e3(lvds, freq, false);
-}
-
 /* -----------------------------------------------------------------------------
- * Clock - D3/E3 only
- */
-
-int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq)
-{
-	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
-	int ret;
-
-	if (WARN_ON(!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)))
-		return -ENODEV;
-
-	dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
-
-	ret = pm_runtime_resume_and_get(lvds->dev);
-	if (ret)
-		return ret;
-
-	__rcar_lvds_pll_setup_d3_e3(lvds, freq, true);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(rcar_lvds_pclk_enable);
-
-void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
-{
-	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
-
-	if (WARN_ON(!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)))
-		return;
-
-	dev_dbg(lvds->dev, "disabling LVDS PLL\n");
-
-	rcar_lvds_write(lvds, LVDPLLCR, 0);
-
-	pm_runtime_put_sync(lvds->dev);
-}
-EXPORT_SYMBOL_GPL(rcar_lvds_pclk_disable);
-
-/* -----------------------------------------------------------------------------
- * Bridge
+ * Enable/disable
  */
 
 static enum rcar_lvds_mode rcar_lvds_get_lvds_mode(struct rcar_lvds *lvds,
@@ -394,10 +350,10 @@ static enum rcar_lvds_mode rcar_lvds_get_lvds_mode(struct rcar_lvds *lvds,
 	return mode;
 }
 
-static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
-				      struct drm_atomic_state *state,
-				      struct drm_crtc *crtc,
-				      struct drm_connector *connector)
+static void rcar_lvds_enable(struct drm_bridge *bridge,
+			     struct drm_atomic_state *state,
+			     struct drm_crtc *crtc,
+			     struct drm_connector *connector)
 {
 	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
 	u32 lvdhcr;
@@ -410,8 +366,7 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
 
 	/* Enable the companion LVDS encoder in dual-link mode. */
 	if (lvds->link_type != RCAR_LVDS_SINGLE_LINK && lvds->companion)
-		__rcar_lvds_atomic_enable(lvds->companion, state, crtc,
-					  connector);
+		rcar_lvds_enable(lvds->companion, state, crtc, connector);
 
 	/*
 	 * Hardcode the channels and control signals routing for now.
@@ -465,8 +420,12 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
 	/*
 	 * PLL clock configuration on all instances but the companion in
 	 * dual-link mode.
+	 *
+	 * The extended PLL has been turned on by an explicit call to
+	 * rcar_lvds_pclk_enable() from the DU driver.
 	 */
-	if (lvds->link_type == RCAR_LVDS_SINGLE_LINK || lvds->companion) {
+	if ((lvds->link_type == RCAR_LVDS_SINGLE_LINK || lvds->companion) &&
+	    !(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)) {
 		const struct drm_crtc_state *crtc_state =
 			drm_atomic_get_new_crtc_state(state, crtc);
 		const struct drm_display_mode *mode =
@@ -531,22 +490,7 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge *bridge,
 	rcar_lvds_write(lvds, LVDCR0, lvdcr0);
 }
 
-static void rcar_lvds_atomic_enable(struct drm_bridge *bridge,
-				    struct drm_bridge_state *old_bridge_state)
-{
-	struct drm_atomic_state *state = old_bridge_state->base.state;
-	struct drm_connector *connector;
-	struct drm_crtc *crtc;
-
-	connector = drm_atomic_get_new_connector_for_encoder(state,
-							     bridge->encoder);
-	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
-
-	__rcar_lvds_atomic_enable(bridge, state, crtc, connector);
-}
-
-static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
-				     struct drm_bridge_state *old_bridge_state)
+static void rcar_lvds_disable(struct drm_bridge *bridge)
 {
 	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
 	u32 lvdcr0;
@@ -578,15 +522,99 @@ static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
 
 	rcar_lvds_write(lvds, LVDCR0, 0);
 	rcar_lvds_write(lvds, LVDCR1, 0);
-	rcar_lvds_write(lvds, LVDPLLCR, 0);
+
+	/* The extended PLL is turned off in rcar_lvds_pclk_disable(). */
+	if (!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL))
+		rcar_lvds_write(lvds, LVDPLLCR, 0);
 
 	/* Disable the companion LVDS encoder in dual-link mode. */
 	if (lvds->link_type != RCAR_LVDS_SINGLE_LINK && lvds->companion)
-		lvds->companion->funcs->atomic_disable(lvds->companion,
-						       old_bridge_state);
+		rcar_lvds_disable(lvds->companion);
+
+	pm_runtime_put_sync(lvds->dev);
+}
+
+/* -----------------------------------------------------------------------------
+ * Clock - D3/E3 only
+ */
+
+int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq,
+			  bool dot_clk_only)
+{
+	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
+	int ret;
+
+	if (WARN_ON(!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)))
+		return -ENODEV;
+
+	dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
+
+	ret = pm_runtime_resume_and_get(lvds->dev);
+	if (ret)
+		return ret;
+
+	rcar_lvds_pll_setup_d3_e3(lvds, freq, dot_clk_only);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rcar_lvds_pclk_enable);
+
+void rcar_lvds_pclk_disable(struct drm_bridge *bridge, bool dot_clk_only)
+{
+	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
+
+	if (WARN_ON(!(lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)))
+		return;
+
+	dev_dbg(lvds->dev, "disabling LVDS PLL\n");
+
+	if (!dot_clk_only)
+		rcar_lvds_disable(bridge);
+
+	rcar_lvds_write(lvds, LVDPLLCR, 0);
 
 	pm_runtime_put_sync(lvds->dev);
 }
+EXPORT_SYMBOL_GPL(rcar_lvds_pclk_disable);
+
+/* -----------------------------------------------------------------------------
+ * Bridge
+ */
+
+static void rcar_lvds_atomic_enable(struct drm_bridge *bridge,
+				    struct drm_bridge_state *old_bridge_state)
+{
+	struct drm_atomic_state *state = old_bridge_state->base.state;
+	struct drm_connector *connector;
+	struct drm_crtc *crtc;
+
+	connector = drm_atomic_get_new_connector_for_encoder(state,
+							     bridge->encoder);
+	crtc = drm_atomic_get_new_connector_state(state, connector)->crtc;
+
+	rcar_lvds_enable(bridge, state, crtc, connector);
+}
+
+static void rcar_lvds_atomic_disable(struct drm_bridge *bridge,
+				     struct drm_bridge_state *old_bridge_state)
+{
+	struct rcar_lvds *lvds = bridge_to_rcar_lvds(bridge);
+
+	/*
+	 * For D3 and E3, disabling the LVDS encoder before the DU would stall
+	 * the DU, causing a vblank wait timeout when stopping the DU. This has
+	 * been traced to clearing the LVEN bit, but the exact reason is
+	 * unknown. Keep the encoder enabled, it will be disabled by an explicit
+	 * call to rcar_lvds_pclk_disable() from the DU driver.
+	 *
+	 * We could clear the LVRES bit already to disable the LVDS output, but
+	 * that's likely pointless.
+	 */
+	if (lvds->info->quirks & RCAR_LVDS_QUIRK_EXT_PLL)
+		return;
+
+	rcar_lvds_disable(bridge);
+}
 
 static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
 				 const struct drm_display_mode *mode,
@@ -922,14 +950,12 @@ static const struct rcar_lvds_device_info rcar_lvds_r8a77990_info = {
 	.gen = 3,
 	.quirks = RCAR_LVDS_QUIRK_GEN3_LVEN | RCAR_LVDS_QUIRK_EXT_PLL
 		| RCAR_LVDS_QUIRK_DUAL_LINK,
-	.pll_setup = rcar_lvds_pll_setup_d3_e3,
 };
 
 static const struct rcar_lvds_device_info rcar_lvds_r8a77995_info = {
 	.gen = 3,
 	.quirks = RCAR_LVDS_QUIRK_GEN3_LVEN | RCAR_LVDS_QUIRK_PWD
 		| RCAR_LVDS_QUIRK_EXT_PLL | RCAR_LVDS_QUIRK_DUAL_LINK,
-	.pll_setup = rcar_lvds_pll_setup_d3_e3,
 };
 
 static const struct of_device_id rcar_lvds_of_table[] = {

drivers/gpu/drm/rcar-du/rcar_lvds.h

@@ -13,17 +13,21 @@
 struct drm_bridge;
 
 #if IS_ENABLED(CONFIG_DRM_RCAR_LVDS)
-int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq);
-void rcar_lvds_pclk_disable(struct drm_bridge *bridge);
+int rcar_lvds_pclk_enable(struct drm_bridge *bridge, unsigned long freq,
+			  bool dot_clk_only);
+void rcar_lvds_pclk_disable(struct drm_bridge *bridge, bool dot_clk_only);
 bool rcar_lvds_dual_link(struct drm_bridge *bridge);
 bool rcar_lvds_is_connected(struct drm_bridge *bridge);
 #else
 static inline int rcar_lvds_pclk_enable(struct drm_bridge *bridge,
-					unsigned long freq)
+					unsigned long freq, bool dot_clk_only)
 {
 	return -ENOSYS;
 }
-static inline void rcar_lvds_pclk_disable(struct drm_bridge *bridge) { }
+static inline void rcar_lvds_pclk_disable(struct drm_bridge *bridge,
+					  bool dot_clock_only)
+{
+}
 static inline bool rcar_lvds_dual_link(struct drm_bridge *bridge)
 {
 	return false;