drm/mcde: Enable the DSI link with display

The MCDE DSI link hardware which is modeled like a bridge
in DRM, connected further to the panel bridge, creating
a pipeline.

We have been using the .pre_enable(), .enable(),
.disable() and .post_disable() callbacks from the bridge
to set this up in a chained manner: first the display
controller goes online and then in successive order
each bridge in the pipeline. Inside DRM it works
like this:

drm_atomic_helper_commit_tail()
drm_atomic_helper_commit_modeset_enables()
struct drm_crtc_helper_funcs .atomic_enable()
struct drm_simple_display_pipe_funcs .enable()
MCDE display enable call
drm_atomic_bridge_chain_enable()
struct drm_bridge_funcs .pre_enable()
mcde_dsi_bridge_pre_enable()
panel_bridge_pre_enable()
struct drm_panel_funcs .prepare()
struct drm_bridge_funcs .enable()
mcde_dsi_bridge_enable()
panel_bridge_enable()
struct drm_panel_funcs .enable()

A similar sequence is executed for disabling.

Unfortunately this is not what the hardware needs: at
a certain stage in the enablement of the display
controller the DSI link needs to come up to support
video mode, else something (like a FIFO flow) locks
up the hardware and we never get picture.

Fix this by simply leaving the pre|enable and
post|disable callbacks unused, and establish two
cross-calls from the display controller to bring up
the DSI link at the right place in the display
bring-up sequence and vice versa in the shutdown
sequence.

For command mode displays, it works just fine to
also enable the display flow early. The only time
we hold it back right now is in one-shot mode,
on-demand display updates.

When combined with the previous patch and some patches
for the S6E63M0 display controller to support DSI
mode, this gives working display on the Samsung
GT-I8190 (Golden) phone. It has also been tested working
on the Samsung GT-S7710 (Skomer) phone.

Signed-off-by: Linus Walleij <[e-mail]>
Acked-by: Daniel Vetter <[e-mail]>
Cc: [e-mail]
Cc: Stephan Gerhold <[e-mail]>
Link: https://patchwork.freedesktop.org/patch/msgid/[e-mail]

drivers/gpu/drm/mcde/mcde_display.c

···
 	mcde_configure_fifo(mcde, MCDE_FIFO_A, MCDE_DSI_FORMATTER_0,
 			    fifo_wtrmrk);
 
+	/*
+	 * This brings up the DSI bridge which is tightly connected
+	 * to the MCDE DSI formatter.
+	 *
+	 * FIXME: if we want to use another formatter, such as DPI,
+	 * we need to be more elaborate here and select the appropriate
+	 * bridge.
+	 */
+	mcde_dsi_enable(mcde->bridge);
+
 	/* Configure the DSI formatter 0 for the DSI panel output */
 	mcde_configure_dsi_formatter(mcde, MCDE_DSI_FORMATTER_0,
 				     formatter_frame, pkt_size);
···
 
 	drm_crtc_vblank_on(crtc);
 
-	if (mcde_flow_is_video(mcde)) {
-		/*
-		 * Keep FIFO permanently enabled in video mode,
-		 * otherwise MCDE will stop feeding data to the panel.
-		 */
+	/*
+	 * If we're using oneshot mode we don't start the flow
+	 * until each time the display is given an update, and
+	 * then we disable it immediately after. For all other
+	 * modes (command or video) we start the FIFO flow
+	 * right here. This is necessary for the hardware to
+	 * behave right.
+	 */
+	if (mcde->flow_mode != MCDE_COMMAND_ONESHOT_FLOW) {
 		mcde_enable_fifo(mcde, MCDE_FIFO_A);
 		dev_dbg(mcde->dev, "started MCDE video FIFO flow\n");
 	}
 
-	/* Enable automatic clock gating */
+	/* Enable MCDE with automatic clock gating */
 	val = readl(mcde->regs + MCDE_CR);
 	val |= MCDE_CR_MCDEEN | MCDE_CR_AUTOCLKG_EN;
 	writel(val, mcde->regs + MCDE_CR);
···
 
 	/* Disable FIFO A flow */
 	mcde_disable_fifo(mcde, MCDE_FIFO_A, true);
+
+	/* This disables the DSI bridge */
+	mcde_dsi_disable(mcde->bridge);
 
 	event = crtc->state->event;
 	if (event) {
···
 	if (fb) {
 		mcde_set_extsrc(mcde, drm_fb_cma_get_gem_addr(fb, pstate, 0));
 		dev_info_once(mcde->dev, "first update of display contents\n");
-		/* The flow is already active in video mode */
-		if (!mcde_flow_is_video(mcde) && mcde->flow_active == 0)
+		/*
+		 * Usually the flow is already active, unless we are in
+		 * oneshot mode, then we need to kick the flow right here.
+		 */
+		if (mcde->flow_active == 0)
 			mcde_start_flow(mcde);
 	} else {
 		/*

drivers/gpu/drm/mcde/mcde_drm.h

···
 
 bool mcde_dsi_irq(struct mipi_dsi_device *mdsi);
 void mcde_dsi_te_request(struct mipi_dsi_device *mdsi);
+void mcde_dsi_enable(struct drm_bridge *bridge);
+void mcde_dsi_disable(struct drm_bridge *bridge);
 extern struct platform_driver mcde_dsi_driver;
 
 void mcde_display_irq(struct mcde *mcde);

drivers/gpu/drm/mcde/mcde_dsi.c

···
 	dev_info(d->dev, "DSI link enabled\n");
 }
 
-
-static void mcde_dsi_bridge_enable(struct drm_bridge *bridge)
-{
-	struct mcde_dsi *d = bridge_to_mcde_dsi(bridge);
-	u32 val;
-
-	if (d->mdsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
-		/* Enable video mode */
-		val = readl(d->regs + DSI_MCTL_MAIN_DATA_CTL);
-		val |= DSI_MCTL_MAIN_DATA_CTL_VID_EN;
-		writel(val, d->regs + DSI_MCTL_MAIN_DATA_CTL);
-	}
-
-	dev_info(d->dev, "enable DSI master\n");
-};
-
-static void mcde_dsi_bridge_pre_enable(struct drm_bridge *bridge)
+/*
+ * Notice that this is called from inside the display controller
+ * and not from the bridge callbacks.
+ */
+void mcde_dsi_enable(struct drm_bridge *bridge)
 {
 	struct mcde_dsi *d = bridge_to_mcde_dsi(bridge);
 	unsigned long hs_freq, lp_freq;
···
 		val |= DSI_VID_MODE_STS_CTL_ERR_MISSING_VSYNC;
 		val |= DSI_VID_MODE_STS_CTL_ERR_MISSING_DATA;
 		writel(val, d->regs + DSI_VID_MODE_STS_CTL);
+
+		/* Enable video mode */
+		val = readl(d->regs + DSI_MCTL_MAIN_DATA_CTL);
+		val |= DSI_MCTL_MAIN_DATA_CTL_VID_EN;
+		writel(val, d->regs + DSI_MCTL_MAIN_DATA_CTL);
 	} else {
 		/* Command mode, clear IF1 ID */
 		val = readl(d->regs + DSI_CMD_MODE_CTL);
···
 		val &= ~DSI_CMD_MODE_CTL_IF1_ID_MASK;
 		writel(val, d->regs + DSI_CMD_MODE_CTL);
 	}
+
+	dev_info(d->dev, "enabled MCDE DSI master\n");
 }
 
 static void mcde_dsi_bridge_mode_set(struct drm_bridge *bridge,
···
 	}
 }
 
-static void mcde_dsi_bridge_disable(struct drm_bridge *bridge)
+/*
+ * Notice that this is called from inside the display controller
+ * and not from the bridge callbacks.
+ */
+void mcde_dsi_disable(struct drm_bridge *bridge)
 {
 	struct mcde_dsi *d = bridge_to_mcde_dsi(bridge);
 	u32 val;
···
 		/* Stop command mode */
 		mcde_dsi_wait_for_command_mode_stop(d);
 	}
-}
-
-static void mcde_dsi_bridge_post_disable(struct drm_bridge *bridge)
-{
-	struct mcde_dsi *d = bridge_to_mcde_dsi(bridge);
 
 	/*
 	 * Stop clocks and terminate any DSI traffic here so the panel can
···
 static const struct drm_bridge_funcs mcde_dsi_bridge_funcs = {
 	.attach = mcde_dsi_bridge_attach,
 	.mode_set = mcde_dsi_bridge_mode_set,
-	.disable = mcde_dsi_bridge_disable,
-	.enable = mcde_dsi_bridge_enable,
-	.pre_enable = mcde_dsi_bridge_pre_enable,
-	.post_disable = mcde_dsi_bridge_post_disable,
 };
 
 static int mcde_dsi_bind(struct device *dev, struct device *master,