[PATCH v2 0/4] drm: panel: Fix atomic helper-induced regressions

[PATCH v2 0/4] drm: panel: Fix atomic helper-induced regressions

[Linus Walleij]

commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
"drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
caused a series of regressions in all panels that send
DSI commands in their .prepare() and .unprepare()
callbacks.

As the CRTC is no longer online at bridge_pre_enable()
and gone at brige_post_disable() which maps to the panel
bridge .prepare()/.unprepare() callbacks, any CRTC that
enable/disable the DSI transmitter in it's enable/disable
callbacks will be unable to send any DSI commands in the
.prepare() and .unprepare() callbacks.

This is also evident from device trees with the DSI
inside the CRTC such as this:

mcde@a0350000 {
   status = "okay";
   pinctrl-names = "default";
   pinctrl-0 = <&dsi_default_mode>;

   dsi@a0351000 {
     panel {
       compatible = "hydis,hva40wv1", "novatek,nt35510";
       reg = <0>;
       vdd-supply = <&ab8500_ldo_aux4_reg>;
       vddi-supply = <&ab8500_ldo_aux6_reg>;
    };
  };
};

The panel is inside the DSI which is inside the CRTC
(MCDE).

This is in a way natural, so let's just fix it in all
affected panel drivers that I know of and can test.
Mostly Ux500 phones, and only those with the display
directly on DSI (not e.g. using DPI and SPI).

Other panel drivers may be affected.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Changes in v2:
- Fix half-baked NT35510 patch that was still sending init commands
  in the prepare() callback.
- Link to v1: https://lore.kernel.org/r/20251023-fix-mcde-drm-regression-v1-0-ed9a925db8c7@linaro.org

---
Linus Walleij (4):
      drm: panel: nt355510: Move DSI commands to enable/disable
      drm: panel: s6d16d0: Move DSI commands to enable/disable
      drm: panel: nt35560: Move DSI commands to enable/disable
      drm: panel: s6e63m0: Move DSI commands to enable/disable

 drivers/gpu/drm/panel/panel-novatek-nt35510.c | 31 +++++++++++++++++++-----
 drivers/gpu/drm/panel/panel-novatek-nt35560.c | 24 ++++++++++++------
 drivers/gpu/drm/panel/panel-samsung-s6d16d0.c | 35 ++++++++++++---------------
 drivers/gpu/drm/panel/panel-samsung-s6e63m0.c | 31 +++++++++---------------
 4 files changed, 70 insertions(+), 51 deletions(-)
---
base-commit: 6548d364a3e850326831799d7e3ea2d7bb97ba08
change-id: 20251022-fix-mcde-drm-regression-c9ac0cc20bae

Best regards,
-- 
Linus Walleij <linus.walleij@linaro.org>

[PATCH v2 1/4] drm: panel: nt355510: Move DSI commands to enable/disable

[Linus Walleij]

Due to semantic changes in the bridge core, panels cannot send
any DSI commands in the prepare/unprepare callbacks: there is
no guarantee that the DSI transmitter is available at this
point.

Tested on the Samsung Skomer (GT-S7710) and Kyle (SGH-I407).

Cc: Aradhya Bhatia <a-bhatia1@ti.com>
Fixes: c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/panel/panel-novatek-nt35510.c | 31 +++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
index 3189d89c7ca0..e1c50a3d7dad 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c
@@ -862,7 +862,6 @@ static const struct backlight_ops nt35510_bl_ops = {
  */
 static int nt35510_power_on(struct nt35510 *nt)
 {
-	struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
 	int ret;
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(nt->supplies), nt->supplies);
@@ -884,6 +883,14 @@ static int nt35510_power_on(struct nt35510 *nt)
 		usleep_range(120000, 140000);
 	}
 
+	return 0;
+}
+
+static int nt35510_init(struct nt35510 *nt)
+{
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
+	int ret;
+
 	ret = nt35510_send_long(nt, dsi, MCS_CMD_MTP_READ_PARAM,
 				ARRAY_SIZE(nt35510_mauc_mtp_read_param),
 				nt35510_mauc_mtp_read_param);
@@ -971,6 +978,13 @@ static int nt35510_power_off(struct nt35510 *nt)
 }
 
 static int nt35510_unprepare(struct drm_panel *panel)
+{
+	struct nt35510 *nt = panel_to_nt35510(panel);
+
+	return nt35510_power_off(nt);
+}
+
+static int nt35510_disable(struct drm_panel *panel)
 {
 	struct nt35510 *nt = panel_to_nt35510(panel);
 	struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
@@ -993,20 +1007,23 @@ static int nt35510_unprepare(struct drm_panel *panel)
 	/* Wait 4 frames, how much is that 5ms in the vendor driver */
 	usleep_range(5000, 10000);
 
-	ret = nt35510_power_off(nt);
-	if (ret)
-		return ret;
-
 	return 0;
 }
 
 static int nt35510_prepare(struct drm_panel *panel)
+{
+	struct nt35510 *nt = panel_to_nt35510(panel);
+
+	return nt35510_power_on(nt);
+}
+
+static int nt35510_enable(struct drm_panel *panel)
 {
 	struct nt35510 *nt = panel_to_nt35510(panel);
 	struct mipi_dsi_device *dsi = to_mipi_dsi_device(nt->dev);
 	int ret;
 
-	ret = nt35510_power_on(nt);
+	ret = nt35510_init(nt);
 	if (ret)
 		return ret;
 
@@ -1078,6 +1095,8 @@ static int nt35510_get_modes(struct drm_panel *panel,
 static const struct drm_panel_funcs nt35510_drm_funcs = {
 	.unprepare = nt35510_unprepare,
 	.prepare = nt35510_prepare,
+	.disable = nt35510_disable,
+	.enable = nt35510_enable,
 	.get_modes = nt35510_get_modes,
 };
 

-- 
2.51.0

[PATCH v2 2/4] drm: panel: s6d16d0: Move DSI commands to enable/disable

[Linus Walleij]

Due to semantic changes in the bridge core, panels cannot send
any DSI commands in the prepare/unprepare callbacks: there is
no guarantee that the DSI transmitter is available at this
point.

Cc: Aradhya Bhatia <a-bhatia1@ti.com>
Fixes: c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/panel/panel-samsung-s6d16d0.c | 35 ++++++++++++---------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
index ba1a02000bb9..0b343e627500 100644
--- a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
+++ b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
@@ -48,15 +48,6 @@ static inline struct s6d16d0 *panel_to_s6d16d0(struct drm_panel *panel)
 static int s6d16d0_unprepare(struct drm_panel *panel)
 {
 	struct s6d16d0 *s6 = panel_to_s6d16d0(panel);
-	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
-	int ret;
-
-	/* Enter sleep mode */
-	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
-	if (ret) {
-		dev_err(s6->dev, "failed to enter sleep mode (%d)\n", ret);
-		return ret;
-	}
 
 	/* Assert RESET */
 	gpiod_set_value_cansleep(s6->reset_gpio, 1);
@@ -68,7 +59,6 @@ static int s6d16d0_unprepare(struct drm_panel *panel)
 static int s6d16d0_prepare(struct drm_panel *panel)
 {
 	struct s6d16d0 *s6 = panel_to_s6d16d0(panel);
-	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
 	int ret;
 
 	ret = regulator_enable(s6->supply);
@@ -84,6 +74,15 @@ static int s6d16d0_prepare(struct drm_panel *panel)
 	gpiod_set_value_cansleep(s6->reset_gpio, 0);
 	msleep(120);
 
+	return 0;
+}
+
+static int s6d16d0_enable(struct drm_panel *panel)
+{
+	struct s6d16d0 *s6 = panel_to_s6d16d0(panel);
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
+	int ret;
+
 	/* Enabe tearing mode: send TE (tearing effect) at VBLANK */
 	ret = mipi_dsi_dcs_set_tear_on(dsi,
 				       MIPI_DSI_DCS_TEAR_MODE_VBLANK);
@@ -98,15 +97,6 @@ static int s6d16d0_prepare(struct drm_panel *panel)
 		return ret;
 	}
 
-	return 0;
-}
-
-static int s6d16d0_enable(struct drm_panel *panel)
-{
-	struct s6d16d0 *s6 = panel_to_s6d16d0(panel);
-	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
-	int ret;
-
 	ret = mipi_dsi_dcs_set_display_on(dsi);
 	if (ret) {
 		dev_err(s6->dev, "failed to turn display on (%d)\n", ret);
@@ -128,6 +118,13 @@ static int s6d16d0_disable(struct drm_panel *panel)
 		return ret;
 	}
 
+	/* Enter sleep mode */
+	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
+	if (ret) {
+		dev_err(s6->dev, "failed to enter sleep mode (%d)\n", ret);
+		return ret;
+	}
+
 	return 0;
 }
 

-- 
2.51.0

[PATCH v2 3/4] drm: panel: nt35560: Move DSI commands to enable/disable

[Linus Walleij]

Due to semantic changes in the bridge core, panels cannot send
any DSI commands in the prepare/unprepare callbacks: there is
no guarantee that the DSI transmitter is available at this
point.

Cc: Aradhya Bhatia <a-bhatia1@ti.com>
Fixes: c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/panel/panel-novatek-nt35560.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35560.c b/drivers/gpu/drm/panel/panel-novatek-nt35560.c
index 561e6643dcbb..b0b11d3e26fe 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt35560.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt35560.c
@@ -278,16 +278,18 @@ static void nt35560_power_off(struct nt35560 *nt)
 }
 
 static int nt35560_prepare(struct drm_panel *panel)
+{
+	struct nt35560 *nt = panel_to_nt35560(panel);
+
+	return nt35560_power_on(nt);
+}
+
+static int nt35560_enable(struct drm_panel *panel)
 {
 	struct nt35560 *nt = panel_to_nt35560(panel);
 	struct mipi_dsi_multi_context dsi_ctx = {
 		.dsi = to_mipi_dsi_device(nt->dev)
 	};
-	int ret;
-
-	ret = nt35560_power_on(nt);
-	if (ret)
-		return ret;
 
 	nt35560_read_id(&dsi_ctx);
 
@@ -317,7 +319,7 @@ static int nt35560_prepare(struct drm_panel *panel)
 	return dsi_ctx.accum_err;
 }
 
-static int nt35560_unprepare(struct drm_panel *panel)
+static int nt35560_disable(struct drm_panel *panel)
 {
 	struct nt35560 *nt = panel_to_nt35560(panel);
 	struct mipi_dsi_multi_context dsi_ctx = {
@@ -332,12 +334,18 @@ static int nt35560_unprepare(struct drm_panel *panel)
 
 	msleep(85);
 
+	return 0;
+}
+
+static int nt35560_unprepare(struct drm_panel *panel)
+{
+	struct nt35560 *nt = panel_to_nt35560(panel);
+
 	nt35560_power_off(nt);
 
 	return 0;
 }
 
-
 static int nt35560_get_modes(struct drm_panel *panel,
 			     struct drm_connector *connector)
 {
@@ -369,6 +377,8 @@ static int nt35560_get_modes(struct drm_panel *panel,
 static const struct drm_panel_funcs nt35560_drm_funcs = {
 	.unprepare = nt35560_unprepare,
 	.prepare = nt35560_prepare,
+	.enable = nt35560_enable,
+	.disable = nt35560_disable,
 	.get_modes = nt35560_get_modes,
 };
 

-- 
2.51.0

[PATCH v2 4/4] drm: panel: s6e63m0: Move DSI commands to enable/disable

[Linus Walleij]

Due to semantic changes in the bridge core, panels cannot send
any DSI commands in the prepare/unprepare callbacks: there is
no guarantee that the DSI transmitter is available at this
point.

This will affect also SPI-based S6E63M0 displays, but that
should be fine.

Tested on the Samsung Golden (GT-I8190).

Cc: Aradhya Bhatia <a-bhatia1@ti.com>
Fixes: c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/panel/panel-samsung-s6e63m0.c | 31 +++++++++++----------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c b/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c
index ea241c89593b..7e000f30b124 100644
--- a/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c
+++ b/drivers/gpu/drm/panel/panel-samsung-s6e63m0.c
@@ -508,32 +508,30 @@ static int s6e63m0_disable(struct drm_panel *panel)
 	s6e63m0_dcs_write_seq_static(ctx, MIPI_DCS_ENTER_SLEEP_MODE);
 	msleep(120);
 
+	s6e63m0_clear_error(ctx);
+
 	return 0;
 }
 
 static int s6e63m0_unprepare(struct drm_panel *panel)
 {
 	struct s6e63m0 *ctx = panel_to_s6e63m0(panel);
-	int ret;
 
-	s6e63m0_clear_error(ctx);
+	return s6e63m0_power_off(ctx);
+}
 
-	ret = s6e63m0_power_off(ctx);
-	if (ret < 0)
-		return ret;
+static int s6e63m0_prepare(struct drm_panel *panel)
+{
+	struct s6e63m0 *ctx = panel_to_s6e63m0(panel);
 
-	return 0;
+	return s6e63m0_power_on(ctx);
 }
 
-static int s6e63m0_prepare(struct drm_panel *panel)
+static int s6e63m0_enable(struct drm_panel *panel)
 {
 	struct s6e63m0 *ctx = panel_to_s6e63m0(panel);
 	int ret;
 
-	ret = s6e63m0_power_on(ctx);
-	if (ret < 0)
-		return ret;
-
 	/* Magic to unlock level 2 control of the display */
 	s6e63m0_dcs_write_seq_static(ctx, MCS_LEVEL_2_KEY, 0x5a, 0x5a);
 	/* Magic to unlock MTP reading */
@@ -547,15 +545,10 @@ static int s6e63m0_prepare(struct drm_panel *panel)
 
 	ret = s6e63m0_clear_error(ctx);
 
-	if (ret < 0)
+	if (ret < 0) {
 		s6e63m0_unprepare(panel);
-
-	return ret;
-}
-
-static int s6e63m0_enable(struct drm_panel *panel)
-{
-	struct s6e63m0 *ctx = panel_to_s6e63m0(panel);
+		return ret;
+	}
 
 	s6e63m0_dcs_write_seq_static(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
 	msleep(120);

-- 
2.51.0

Re: [PATCH v2 0/4] drm: panel: Fix atomic helper-induced regressions

[Tomi Valkeinen]

Hi,

On 26/10/2025 19:39, Linus Walleij wrote:
> commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
> "drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
> caused a series of regressions in all panels that send
> DSI commands in their .prepare() and .unprepare()
> callbacks.
> 
> As the CRTC is no longer online at bridge_pre_enable()
> and gone at brige_post_disable() which maps to the panel
> bridge .prepare()/.unprepare() callbacks, any CRTC that
> enable/disable the DSI transmitter in it's enable/disable
> callbacks will be unable to send any DSI commands in the
> .prepare() and .unprepare() callbacks.

Well, that wasn't the intention...

We also have this pre_enable_prev_first hack in the drm_bridge, but I
guess that doesn't help in this case?

More generally speaking, I think the core issue is that we have the DSI
video stream as a dependency for DSI commands. It would be better if the
DSI hosts dealt with DSI commands independent of the video stream, in
other words, one could send a DSI command at any time.

Unfortunately in some cases that might be impossible, if the DSI host
depends on the incoming pixel clock to function...

In this particular case, if the same driver is dealing with the crtc and
the dsi host, wouldn't it be possible to make the
mipi_dsi_host_ops.transfer() work regardless of the enable/disable status?

 Tomi

> This is also evident from device trees with the DSI
> inside the CRTC such as this:
> 
> mcde@a0350000 {
>    status = "okay";
>    pinctrl-names = "default";
>    pinctrl-0 = <&dsi_default_mode>;
> 
>    dsi@a0351000 {
>      panel {
>        compatible = "hydis,hva40wv1", "novatek,nt35510";
>        reg = <0>;
>        vdd-supply = <&ab8500_ldo_aux4_reg>;
>        vddi-supply = <&ab8500_ldo_aux6_reg>;
>     };
>   };
> };
> 
> The panel is inside the DSI which is inside the CRTC
> (MCDE).
> 
> This is in a way natural, so let's just fix it in all
> affected panel drivers that I know of and can test.
> Mostly Ux500 phones, and only those with the display
> directly on DSI (not e.g. using DPI and SPI).
> 
> Other panel drivers may be affected.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> Changes in v2:
> - Fix half-baked NT35510 patch that was still sending init commands
>   in the prepare() callback.
> - Link to v1: https://lore.kernel.org/r/20251023-fix-mcde-drm-regression-v1-0-ed9a925db8c7@linaro.org
> 
> ---
> Linus Walleij (4):
>       drm: panel: nt355510: Move DSI commands to enable/disable
>       drm: panel: s6d16d0: Move DSI commands to enable/disable
>       drm: panel: nt35560: Move DSI commands to enable/disable
>       drm: panel: s6e63m0: Move DSI commands to enable/disable
> 
>  drivers/gpu/drm/panel/panel-novatek-nt35510.c | 31 +++++++++++++++++++-----
>  drivers/gpu/drm/panel/panel-novatek-nt35560.c | 24 ++++++++++++------
>  drivers/gpu/drm/panel/panel-samsung-s6d16d0.c | 35 ++++++++++++---------------
>  drivers/gpu/drm/panel/panel-samsung-s6e63m0.c | 31 +++++++++---------------
>  4 files changed, 70 insertions(+), 51 deletions(-)
> ---
> base-commit: 6548d364a3e850326831799d7e3ea2d7bb97ba08
> change-id: 20251022-fix-mcde-drm-regression-c9ac0cc20bae
> 
> Best regards,

Re: [PATCH v2 0/4] drm: panel: Fix atomic helper-induced regressions

[Linus Walleij]

On Tue, Oct 28, 2025 at 11:32 AM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:

> > As the CRTC is no longer online at bridge_pre_enable()
> > and gone at brige_post_disable() which maps to the panel
> > bridge .prepare()/.unprepare() callbacks, any CRTC that
> > enable/disable the DSI transmitter in it's enable/disable
> > callbacks will be unable to send any DSI commands in the
> > .prepare() and .unprepare() callbacks.
>
> Well, that wasn't the intention...
>
> We also have this pre_enable_prev_first hack in the drm_bridge, but I
> guess that doesn't help in this case?

I don't know what that's for, the panel bridge isn't
using it so it doesn't help any panel driver?

> More generally speaking, I think the core issue is that we have the DSI
> video stream as a dependency for DSI commands.

That's right, I think.

> It would be better if the
> DSI hosts dealt with DSI commands independent of the video stream, in
> other words, one could send a DSI command at any time.
>
> Unfortunately in some cases that might be impossible, if the DSI host
> depends on the incoming pixel clock to function...

This is the case with the MCDE driver. It even cannot use the
existing .enable/.disable callbacks in it's bridge because it really
needs to bring the DSI transmission block up at the right time
in the initialization sequence.

I'm not making it up: I really, really tried hard to just initialized
it independently. It just won't, it's too tightly coupled with the
DSI video/command stream generator.

> In this particular case, if the same driver is dealing with the crtc and
> the dsi host, wouldn't it be possible to make the
> mipi_dsi_host_ops.transfer() work regardless of the enable/disable status?

Sadly no.

FWIW I think it is fine to require the DSI panels to only send
DSI commands in .enable/.disable and not in .prepare/.unprepare,
it's intuitive in a way, if we go for this semantic I can send a doc
update after the fixes and try to look over the panel drivers a bit
and see if there are more offenders.

Yours,
Linus Walleij

Re: [PATCH v2 0/4] drm: panel: Fix atomic helper-induced regressions

[Tomi Valkeinen]

Hi,

On 28/10/2025 16:27, Linus Walleij wrote:
> On Tue, Oct 28, 2025 at 11:32 AM Tomi Valkeinen
> <tomi.valkeinen@ideasonboard.com> wrote:
> 
>>> As the CRTC is no longer online at bridge_pre_enable()
>>> and gone at brige_post_disable() which maps to the panel
>>> bridge .prepare()/.unprepare() callbacks, any CRTC that
>>> enable/disable the DSI transmitter in it's enable/disable
>>> callbacks will be unable to send any DSI commands in the
>>> .prepare() and .unprepare() callbacks.
>>
>> Well, that wasn't the intention...
>>
>> We also have this pre_enable_prev_first hack in the drm_bridge, but I
>> guess that doesn't help in this case?
> 
> I don't know what that's for, the panel bridge isn't
> using it so it doesn't help any panel driver?

What do you mean? drivers/gpu/drm/bridge/panel.c uses it, see
"panel->prepare_prev_first". I do find it confusing, and I'm not sure if
I should say more about it until I read the code to refresh my memory =)

But I think the idea is that the panel's prepare is called after the DSI
hosts pre_enable. Probably that doesn't help here, as you need the crtc
to be active, and, afaik, the point with the whole pre-enable/prepare
sequence is that the video is not active at that point:

 * The display pipe (i.e. clocks and timing signals) feeding this bridge
 * will not yet be running when the @atomic_pre_enable is called.

>> More generally speaking, I think the core issue is that we have the DSI
>> video stream as a dependency for DSI commands.
> 
> That's right, I think.
> 
>> It would be better if the
>> DSI hosts dealt with DSI commands independent of the video stream, in
>> other words, one could send a DSI command at any time.
>>
>> Unfortunately in some cases that might be impossible, if the DSI host
>> depends on the incoming pixel clock to function...
> 
> This is the case with the MCDE driver. It even cannot use the
> existing .enable/.disable callbacks in it's bridge because it really
> needs to bring the DSI transmission block up at the right time
> in the initialization sequence.
> 
> I'm not making it up: I really, really tried hard to just initialized
> it independently. It just won't, it's too tightly coupled with the
> DSI video/command stream generator.
> 
>> In this particular case, if the same driver is dealing with the crtc and
>> the dsi host, wouldn't it be possible to make the
>> mipi_dsi_host_ops.transfer() work regardless of the enable/disable status?
> 
> Sadly no.
> 
> FWIW I think it is fine to require the DSI panels to only send
> DSI commands in .enable/.disable and not in .prepare/.unprepare,
> it's intuitive in a way, if we go for this semantic I can send a doc
> update after the fixes and try to look over the panel drivers a bit
> and see if there are more offenders.
No, I don't think that works generally. In panel's enable() the video
stream is already on, and the DSI link is in HS mode. You might need to
configure the panel first (at least the first part of the setup) in LP
mode. Say, to set the number of lanes. Or, see
"20251021-tc358768-v1-0-d590dc6a1a0c@ideasonboard.com", tc358768 can
send long DSI commands, but only if the video stream is not enabled.

Conceptually it makes sense to do configuration in panel's prepare(),
and do the final enablement in panel's enable(). But unfortunately I
don't have any good ideas how to sort this out. Feels like whatever we
do, it's not ok for some bridge/panel...

 Tomi

Re: [PATCH v2 0/4] drm: panel: Fix atomic helper-induced regressions

[Linus Walleij]

On Tue, Oct 28, 2025 at 4:13 PM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:

> > I don't know what that's for, the panel bridge isn't
> > using it so it doesn't help any panel driver?
>
> What do you mean? drivers/gpu/drm/bridge/panel.c uses it, see
> "panel->prepare_prev_first". I do find it confusing, and I'm not sure if
> I should say more about it until I read the code to refresh my memory =)

OK yes it is there, I did a misgrep...

panel_bridge->bridge.pre_enable_prev_first = panel->prepare_prev_first;

> But I think the idea is that the panel's prepare is called after the DSI
> hosts pre_enable. Probably that doesn't help here, as you need the crtc
> to be active, and, afaik, the point with the whole pre-enable/prepare
> sequence is that the video is not active at that point:
>
>  * The display pipe (i.e. clocks and timing signals) feeding this bridge
>  * will not yet be running when the @atomic_pre_enable is called.

Indeed.

So what is needed is for .enable on the preceding bridge to have
been called first.

> > FWIW I think it is fine to require the DSI panels to only send
> > DSI commands in .enable/.disable and not in .prepare/.unprepare,
> > it's intuitive in a way, if we go for this semantic I can send a doc
> > update after the fixes and try to look over the panel drivers a bit
> > and see if there are more offenders.
>
> No, I don't think that works generally. In panel's enable() the video
> stream is already on, and the DSI link is in HS mode. You might need to
> configure the panel first (at least the first part of the setup) in LP
> mode. Say, to set the number of lanes. Or, see
> "20251021-tc358768-v1-0-d590dc6a1a0c@ideasonboard.com", tc358768 can
> send long DSI commands, but only if the video stream is not enabled.
>
> Conceptually it makes sense to do configuration in panel's prepare(),
> and do the final enablement in panel's enable(). But unfortunately I
> don't have any good ideas how to sort this out. Feels like whatever we
> do, it's not ok for some bridge/panel...

So what shall we do with this regression?

Shall we just revert
commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
"drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
while we still can?

Yours,
Linus Walleij