[PATCH] drm/bridge: dw-mipi-dsi: fix cmd mode transition when disabling

[PATCH] drm/bridge: dw-mipi-dsi: fix cmd mode transition when disabling

[Icenowy Zheng]

With pre_enable_prev_first flag set, the atomic_post_disable hook of the
next bridge (which should be the DSI panel bridge and calls panel
unprepare hook) is called before the hook of the DSI controller bridge,
which leads to the switch to command mode happening after the panel
unprepare. In this case some panels' unprepare hooks sliently fails
because they expect the DSI controller to be in command mode.

As the DSI controller atomic_disable hook happens after panel bridge
atomic_disable but before panel bridge atomic_post_disable, it seems to
be a good position for the transition to the command mode.

Move the mode transition from the atomic_post_disable hook to a newly
created atomic_disable hook.

Fixes: 05aa61334592 ("drm: bridge: dw-mipi-dsi: Fix enable/disable of DSI controller")
Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
---
Tested on Lichee Console 4A with a pending glue driver for T-Head TH1520
DW MIPI DSI. This device has a always-powered panel (no switchable
supply and no reset pin), so correct execution of unprepare commands is
critical.

 drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 23 ++++++++++++-------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 5dd475fcaacf8..49b6228ca69e5 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -944,14 +944,6 @@ static void dw_mipi_dsi_bridge_post_atomic_disable(struct drm_bridge *bridge,
 	struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
 	const struct dw_mipi_dsi_phy_ops *phy_ops = dsi->plat_data->phy_ops;
 
-	/*
-	 * Switch to command mode before panel-bridge post_disable &
-	 * panel unprepare.
-	 * Note: panel-bridge disable & panel disable has been called
-	 * before by the drm framework.
-	 */
-	dw_mipi_dsi_set_mode(dsi, 0);
-
 	if (phy_ops->power_off)
 		phy_ops->power_off(dsi->plat_data->priv_data);
 
@@ -1058,6 +1050,20 @@ static void dw_mipi_dsi_bridge_atomic_enable(struct drm_bridge *bridge,
 		dw_mipi_dsi_set_mode(dsi->slave, MIPI_DSI_MODE_VIDEO);
 }
 
+static void dw_mipi_dsi_bridge_atomic_disable(struct drm_bridge *bridge,
+					      struct drm_atomic_commit *state)
+{
+	struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
+
+	/*
+	 * Switch to command mode before panel-bridge post_disable &
+	 * panel unprepare.
+	 * Note: panel-bridge disable & panel disable has been called
+	 * before by the drm framework.
+	 */
+	dw_mipi_dsi_set_mode(dsi, 0);
+}
+
 static enum drm_mode_status
 dw_mipi_dsi_bridge_mode_valid(struct drm_bridge *bridge,
 			      const struct drm_display_info *info,
@@ -1098,6 +1104,7 @@ static const struct drm_bridge_funcs dw_mipi_dsi_bridge_funcs = {
 	.atomic_create_state		= drm_atomic_helper_bridge_create_state,
 	.atomic_pre_enable	= dw_mipi_dsi_bridge_atomic_pre_enable,
 	.atomic_enable		= dw_mipi_dsi_bridge_atomic_enable,
+	.atomic_disable		= dw_mipi_dsi_bridge_atomic_disable,
 	.atomic_post_disable	= dw_mipi_dsi_bridge_post_atomic_disable,
 	.mode_set		= dw_mipi_dsi_bridge_mode_set,
 	.mode_valid		= dw_mipi_dsi_bridge_mode_valid,
-- 
2.56.0

Re: [PATCH] drm/bridge: dw-mipi-dsi: fix cmd mode transition when disabling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/bridge: dw-mipi-dsi: missing dsi->slave cmd mode switch in atomic_disable()
--

commit 99d2bade79250c503a435eb9a6a3898d6ebd22f7
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>

drm/bridge: dw-mipi-dsi: fix cmd mode transition when disabling

This commit moves the command mode transition from the atomic_post_disable
hook into a new atomic_disable hook so that the DSI controller switches to
command mode before the panel bridge runs its atomic_post_disable and
panel unprepare callbacks.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 22d1b8cbe1d57..7cd0ca76b86ee 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c

[ ... ]

> @@ -1057,6 +1049,20 @@ static void dw_mipi_dsi_bridge_atomic_enable(struct drm_bridge *bridge,
>  		dw_mipi_dsi_set_mode(dsi->slave, MIPI_DSI_MODE_VIDEO);
>  }
>  
> +static void dw_mipi_dsi_bridge_atomic_disable(struct drm_bridge *bridge,
> +					      struct drm_atomic_commit *state)
> +{
> +	struct dw_mipi_dsi *dsi = bridge_to_dsi(bridge);
> +
> +	/*
> +	 * Switch to command mode before panel-bridge post_disable &
> +	 * panel unprepare.
> +	 * Note: panel-bridge disable & panel disable has been called
> +	 * before by the drm framework.
> +	 */
> +	dw_mipi_dsi_set_mode(dsi, 0);
> +}

[Severity: Medium]
Should dw_mipi_dsi_bridge_atomic_disable() also switch dsi->slave to command
mode when operating in a dual-DSI configuration?

In dw_mipi_dsi_bridge_atomic_enable(), both the master and slave controllers
are switched to video mode:

	dw_mipi_dsi_set_mode(dsi, MIPI_DSI_MODE_VIDEO);
	if (dsi->slave)
		dw_mipi_dsi_set_mode(dsi->slave, MIPI_DSI_MODE_VIDEO);

And when the panel subsequently sends DSI commands during unprepare,
dw_mipi_dsi_host_transfer() transmits the packet through both controllers:

	dw_mipi_message_config(dsi, msg);
	if (dsi->slave)
		dw_mipi_message_config(dsi->slave, msg);

	ret = dw_mipi_dsi_write(dsi, &packet);
	if (ret)
		return ret;
	if (dsi->slave) {
		ret = dw_mipi_dsi_write(dsi->slave, &packet);
		if (ret)
			return ret;
	}

Without switching dsi->slave back to command mode in
dw_mipi_dsi_bridge_atomic_disable(), won't commands sent during panel
unprepare reach the slave controller while it is still in video mode?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030055806.4102994-1-zhengxingda@iscas.ac.cn?part=1