[PATCH v4] drm/omap: dsi: avoid sending bta sync all the time in writes

[PATCH v4] drm/omap: dsi: avoid sending bta sync all the time in writes

[Andreas Kemnade]

Some chips need configuration commands to be sent first, before they can
send data. TC358762 for example needs PPI_LPTXTIMECNT configured
and PPI_STARTPPI set to 1 to be able to transmit anything. To be able to
configure such chips, do not send bta sync during writes if no acks are
requested. Instead just wait for the packet to be sent to avoid FIFO
overflows. There might be more to do about acks, but there seem to be
virtually no users of that flag.

This came to light when fiddling with the Epson Moverio BT-200 display
which consists of 2 TC358762 bridges with SPI funneled through
to the unknown display chip. With that patch the bridge can be accessed,
Reading back registers works, when the above-mentioned registers are set.

In Command-Mode update, there was a nop sent, apparently the most
relevant part was the bta sync to actually force low power mode.

Video mode panel at OMAP4 (BT-200) and video mode at OMAP5 was tested.

Fixes: e70965386353e ("drm/omap: dsi: simplify write function")
Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
Changes in v4:
- wait on completition on all packets (was limited to long packets only,
  because I had the wrong impression that there is no confirmation on
  these)
- Link to v3: https://patch.msgid.link/20260628-vm-upstr-v3-1-9e9add93378b@kemnade.info

Changes in v3:

- Link to v2: https://patch.msgid.link/20260529-vm-upstr-v2-1-24c30671719f@kernel.org
- fix things mentioned by claude here:
  https://lore.gitlab.freedesktop.org/drm-ai-reviews/review-patch1-20260529-vm-upstr-v2-1-24c30671719f@kernel.org/
  - fix typos
  - register ISR before sending packet
  - check for RX_FIFO_NOT_EMPTY also in for short packets

Changes in v2:
- fix commandmode update, need bta sync there
- do not wait on short packets
- Link to v1: https://patch.msgid.link/20260528-vm-upstr-v1-1-fb93ef8cbe47@kernel.org

To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Sebastian Reichel <sebastian.reichel@collabora.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/gpu/drm/omapdrm/dss/dsi.c | 53 ++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
index 27fe7bca9e2c..3f22929c36e2 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -2194,28 +2194,38 @@ static int dsi_vc_send_null(struct dsi_data *dsi, int vc, int channel)
 static int dsi_vc_write_common(struct omap_dss_device *dssdev, int vc,
 			       const struct mipi_dsi_msg *msg)
 {
+	DECLARE_COMPLETION_ONSTACK(completion);
 	struct dsi_data *dsi = to_dsi_data(dssdev);
 	int r;
 
+	/* wait for IRQ for packet transmission confirmation */
+	r = dsi_register_isr_vc(dsi, vc, dsi_completion_handler,
+				&completion, DSI_VC_IRQ_PACKET_SENT);
+	if (r)
+		return r;
+
 	if (mipi_dsi_packet_format_is_short(msg->type))
 		r = dsi_vc_send_short(dsi, vc, msg);
 	else
 		r = dsi_vc_send_long(dsi, vc, msg);
 
-	if (r < 0)
+	if ((!r) && wait_for_completion_timeout(&completion,
+				msecs_to_jiffies(500)) == 0)
+		r = -EIO;
+
+	dsi_unregister_isr_vc(dsi, vc, dsi_completion_handler,
+			      &completion, DSI_VC_IRQ_PACKET_SENT);
+	if (r)
 		return r;
 
-	/*
-	 * TODO: we do not always have to do the BTA sync, for example
-	 * we can improve performance by setting the update window
-	 * information without sending BTA sync between the commands.
-	 * In that case we can return early.
-	 */
+	/* TODO: find out if more needs to be done for MIPI_DSI_MSG_REQ_ACK */
 
-	r = dsi_vc_send_bta_sync(dssdev, vc);
-	if (r) {
-		DSSERR("bta sync failed\n");
-		return r;
+	if (msg->flags & MIPI_DSI_MSG_REQ_ACK) {
+		r = dsi_vc_send_bta_sync(dssdev, vc);
+		if (r) {
+			DSSERR("bta sync failed\n");
+			return r;
+		}
 	}
 
 	/* RX_FIFO_NOT_EMPTY */
@@ -3233,21 +3243,6 @@ static int _dsi_update(struct dsi_data *dsi)
 	return 0;
 }
 
-static int _dsi_send_nop(struct dsi_data *dsi, int vc, int channel)
-{
-	const u8 payload[] = { MIPI_DCS_NOP };
-	const struct mipi_dsi_msg msg = {
-		.channel = channel,
-		.type = MIPI_DSI_DCS_SHORT_WRITE,
-		.tx_len = 1,
-		.tx_buf = payload,
-	};
-
-	WARN_ON(!dsi_bus_is_locked(dsi));
-
-	return _omap_dsi_host_transfer(dsi, vc, &msg);
-}
-
 static int dsi_update_channel(struct omap_dss_device *dssdev, int vc)
 {
 	struct dsi_data *dsi = to_dsi_data(dssdev);
@@ -3268,13 +3263,13 @@ static int dsi_update_channel(struct omap_dss_device *dssdev, int vc)
 	DSSDBG("dsi_update_channel: %d", vc);
 
 	/*
-	 * Send NOP between the frames. If we don't send something here, the
+	 * Transition to LP here. If we don't send something here, the
 	 * updates stop working. This is probably related to DSI spec stating
 	 * that the DSI host should transition to LP at least once per frame.
 	 */
-	r = _dsi_send_nop(dsi, VC_CMD, dsi->dsidev->channel);
+	r = dsi_vc_send_bta_sync(dssdev, vc);
 	if (r < 0) {
-		DSSWARN("failed to send nop between frames: %d\n", r);
+		DSSWARN("failed to send bta sync between frames: %d\n", r);
 		goto err;
 	}
 

---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260528-vm-upstr-c8e7634ebf56

Best regards,
--  
Andreas Kemnade <andreas@kemnade.info>

Re: [PATCH v4] drm/omap: dsi: avoid sending bta sync all the time in writes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Stack Use-After-Free due to race between IRQ handler and unregistering on-stack completion
- [Medium] Regression for command-mode panels due to missing DCS NOP and incorrect channel for BTA
- [Medium] Missing Error IRQ registration causes a 500ms stall on hardware transmission errors
--

commit 30f75c50c586dd648e75b97e7fe46e6f3d854488
Author: Andreas Kemnade <andreas@kemnade.info>
Subject: drm/omap: dsi: avoid sending bta sync all the time in writes

This patch avoids sending a BTA sync during writes if no ACKs are requested,
instead waiting for the packet to be sent to prevent FIFO overflows. It also
replaces the NOP sent between frames with a BTA sync during channel updates.

> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index 27fe7bca9e2cf..3f22929c36e2a 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -2194,28 +2194,38 @@ static int dsi_vc_send_null(struct dsi_data *dsi, int vc, int channel)
>  static int dsi_vc_write_common(struct omap_dss_device *dssdev, int vc,
>  			       const struct mipi_dsi_msg *msg)
>  {
> +	DECLARE_COMPLETION_ONSTACK(completion);
>  	struct dsi_data *dsi = to_dsi_data(dssdev);
>  	int r;
>  
> +	/* wait for IRQ for packet transmission confirmation */
> +	r = dsi_register_isr_vc(dsi, vc, dsi_completion_handler,
> +				&completion, DSI_VC_IRQ_PACKET_SENT);

[Severity: Medium]
Will this stall for the full 500ms timeout if a hardware transmission error
occurs?

Since this is only registered for DSI_VC_IRQ_PACKET_SENT, an error interrupt
(like DSI_IRQ_HS_TX_TIMEOUT) wouldn't wake up the completion. Other functions
like dsi_vc_send_bta_sync() also register for DSI_IRQ_ERROR_MASK to handle
this case.

> +	if (r)
> +		return r;
> +
>  	if (mipi_dsi_packet_format_is_short(msg->type))
>  		r = dsi_vc_send_short(dsi, vc, msg);
>  	else
>  		r = dsi_vc_send_long(dsi, vc, msg);
>  
> -	if (r < 0)
> +	if ((!r) && wait_for_completion_timeout(&completion,
> +				msecs_to_jiffies(500)) == 0)
> +		r = -EIO;
> +
> +	dsi_unregister_isr_vc(dsi, vc, dsi_completion_handler,
> +			      &completion, DSI_VC_IRQ_PACKET_SENT);
> +	if (r)
>  		return r;

[Severity: High]
Could this lead to a stack use-after-free if the timeout is reached?

The wait_for_completion_timeout() can return 0 (timeout), after which
dsi_unregister_isr_vc() unregisters the ISR and the function returns,
destroying the on-stack completion. 

However, dsi_unregister_isr_vc() doesn't appear to synchronize with the IRQ
handler. If the hardware IRQ fires right after the timeout and the handler
executes from its lockless copy of the ISR table, could it call
dsi_completion_handler() with a dangling pointer to the destroyed stack
frame?

[ ... ]

> @@ -3268,13 +3263,13 @@ static int dsi_update_channel(struct omap_dss_device *dssdev, int vc)
>  	DSSDBG("dsi_update_channel: %d", vc);
>  
>  	/*
> -	 * Send NOP between the frames. If we don't send something here, the
> +	 * Transition to LP here. If we don't send something here, the
>  	 * updates stop working. This is probably related to DSI spec stating
>  	 * that the DSI host should transition to LP at least once per frame.
>  	 */
> -	r = _dsi_send_nop(dsi, VC_CMD, dsi->dsidev->channel);
> +	r = dsi_vc_send_bta_sync(dssdev, vc);

[Severity: Medium]
Will this break command-mode panels?

The original code explicitly sent a DCS NOP on VC_CMD (which is typically
configured for Low Power/CPU). This replaces it with a BTA sync on the vc
passed to the function, which is usually VC_VIDEO (configured as High
Speed/Video Port).

Is a BTA sync on a Video Port channel valid hardware behavior here? Also,
will panels that depend on receiving the DCS NOP payload to transition to
LP mode stop updating?

>  	if (r < 0) {
> -		DSSWARN("failed to send nop between frames: %d\n", r);
> +		DSSWARN("failed to send bta sync between frames: %d\n", r);
>  		goto err;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-vm-upstr-v4-1-9f0c62d8808e@kemnade.info?part=1

Re: [PATCH v4] drm/omap: dsi: avoid sending bta sync all the time in writes

[Andreas Kemnade]

On Mon, 29 Jun 2026 16:04:47 +0000
sashiko-bot@kernel.org wrote:

> > @@ -3268,13 +3263,13 @@ static int dsi_update_channel(struct omap_dss_device *dssdev, int vc)
> >  	DSSDBG("dsi_update_channel: %d", vc);
> >  
> >  	/*
> > -	 * Send NOP between the frames. If we don't send something here, the
> > +	 * Transition to LP here. If we don't send something here, the
> >  	 * updates stop working. This is probably related to DSI spec stating
> >  	 * that the DSI host should transition to LP at least once per frame.
> >  	 */
> > -	r = _dsi_send_nop(dsi, VC_CMD, dsi->dsidev->channel);
> > +	r = dsi_vc_send_bta_sync(dssdev, vc);  
> 
> [Severity: Medium]
> Will this break command-mode panels?
> 
this was introduced to fix command mode panels in v2 and confirmed to do that.

> The original code explicitly sent a DCS NOP on VC_CMD (which is typically
> configured for Low Power/CPU). This replaces it with a BTA sync on the vc
> passed to the function, which is usually VC_VIDEO (configured as High
> Speed/Video Port).
>
original code indirectly also has sent a bta sync.
 
> Is a BTA sync on a Video Port channel valid hardware behavior here? Also,
> will panels that depend on receiving the DCS NOP payload to transition to
> LP mode stop updating?

The NOP packet (with the bta sync) was solely introduced to transition
to LP mode. It was confirmed that updating works with just the BTA sync.

Regards,
Andreas

Re: [PATCH v4] drm/omap: dsi: avoid sending bta sync all the time in writes

[Andreas Kemnade]

On Mon, 29 Jun 2026 16:04:47 +0000
sashiko-bot@kernel.org wrote:

> > -	if (r < 0)
> > +	if ((!r) && wait_for_completion_timeout(&completion,
> > +				msecs_to_jiffies(500)) == 0)
> > +		r = -EIO;
> > +
> > +	dsi_unregister_isr_vc(dsi, vc, dsi_completion_handler,
> > +			      &completion, DSI_VC_IRQ_PACKET_SENT);
> > +	if (r)
> >  		return r;  
> 
> [Severity: High]
> Could this lead to a stack use-after-free if the timeout is reached?
> 
> The wait_for_completion_timeout() can return 0 (timeout), after which
> dsi_unregister_isr_vc() unregisters the ISR and the function returns,
> destroying the on-stack completion. 
> 
> However, dsi_unregister_isr_vc() doesn't appear to synchronize with the IRQ
> handler. If the hardware IRQ fires right after the timeout and the handler
> executes from its lockless copy of the ISR table, could it call
> dsi_completion_handler() with a dangling pointer to the destroyed stack
> frame?

There are several places in this file having that problem. So it is not new.
I have sent a fix here:

https://lore.kernel.org/lkml/20260629-dsi-uaf-v1-1-a5e894f4e4d1@kemnade.info/T/#u

Regards,
Andreas

Re: [PATCH v4] drm/omap: dsi: avoid sending bta sync all the time in writes

[Ivaylo Dimitrov]

Hi,
With this version of the patch I see:

[  102.760894] DSI: omapdss DSI error: Framedone not received for 250ms!
[  303.860321] DSI: omapdss DSI error: Framedone not received for 250ms!
[  307.071624] DSI: omapdss DSI error: Framedone not received for 250ms!
[  307.360626] DSI: omapdss DSI error: Framedone not received for 250ms!
[  307.671142] DSI: omapdss DSI error: Framedone not received for 250ms!
[  308.660339] DSI: omapdss DSI error: Framedone not received for 250ms!
[  308.961059] DSI: omapdss DSI error: Framedone not received for 250ms!
[  313.191314] DSI: omapdss DSI error: Framedone not received for 250ms!
[  313.700378] DSI: omapdss DSI error: Failed to set dsi_if_enable to 0
[  313.980346] DSI: omapdss DSI error: Framedone not received for 250ms!
[  314.791564] DSI: omapdss DSI error: Framedone not received for 250ms!
[  315.090362] DSI: omapdss DSI error: Framedone not received for 250ms!
[  322.850952] DSI: omapdss DSI error: Framedone not received for 250ms!
[  323.111175] DSI: omapdss DSI error: Framedone not received for 250ms!

while scrolling the (hildon)desktop on droid4, so I would say the change 
broke command mode.

Regards,
Ivo

On 29.06.26 г. 18:50 ч., Andreas Kemnade wrote:
> Some chips need configuration commands to be sent first, before they can
> send data. TC358762 for example needs PPI_LPTXTIMECNT configured
> and PPI_STARTPPI set to 1 to be able to transmit anything. To be able to
> configure such chips, do not send bta sync during writes if no acks are
> requested. Instead just wait for the packet to be sent to avoid FIFO
> overflows. There might be more to do about acks, but there seem to be
> virtually no users of that flag.
> 
> This came to light when fiddling with the Epson Moverio BT-200 display
> which consists of 2 TC358762 bridges with SPI funneled through
> to the unknown display chip. With that patch the bridge can be accessed,
> Reading back registers works, when the above-mentioned registers are set.
> 
> In Command-Mode update, there was a nop sent, apparently the most
> relevant part was the bta sync to actually force low power mode.
> 
> Video mode panel at OMAP4 (BT-200) and video mode at OMAP5 was tested.
> 
> Fixes: e70965386353e ("drm/omap: dsi: simplify write function")
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
> Changes in v4:
> - wait on completition on all packets (was limited to long packets only,
>    because I had the wrong impression that there is no confirmation on
>    these)
> - Link to v3: https://patch.msgid.link/20260628-vm-upstr-v3-1-9e9add93378b@kemnade.info
> 
> Changes in v3:
> 
> - Link to v2: https://patch.msgid.link/20260529-vm-upstr-v2-1-24c30671719f@kernel.org
> - fix things mentioned by claude here:
>    https://lore.gitlab.freedesktop.org/drm-ai-reviews/review-patch1-20260529-vm-upstr-v2-1-24c30671719f@kernel.org/
>    - fix typos
>    - register ISR before sending packet
>    - check for RX_FIFO_NOT_EMPTY also in for short packets
> 
> Changes in v2:
> - fix commandmode update, need bta sync there
> - do not wait on short packets
> - Link to v1: https://patch.msgid.link/20260528-vm-upstr-v1-1-fb93ef8cbe47@kernel.org
> 
> To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> To: Maxime Ripard <mripard@kernel.org>
> To: Thomas Zimmermann <tzimmermann@suse.de>
> To: David Airlie <airlied@gmail.com>
> To: Simona Vetter <simona@ffwll.ch>
> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> To: Sebastian Reichel <sebastian.reichel@collabora.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> ---
>   drivers/gpu/drm/omapdrm/dss/dsi.c | 53 ++++++++++++++++++---------------------
>   1 file changed, 24 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index 27fe7bca9e2c..3f22929c36e2 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -2194,28 +2194,38 @@ static int dsi_vc_send_null(struct dsi_data *dsi, int vc, int channel)
>   static int dsi_vc_write_common(struct omap_dss_device *dssdev, int vc,
>   			       const struct mipi_dsi_msg *msg)
>   {
> +	DECLARE_COMPLETION_ONSTACK(completion);
>   	struct dsi_data *dsi = to_dsi_data(dssdev);
>   	int r;
>   
> +	/* wait for IRQ for packet transmission confirmation */
> +	r = dsi_register_isr_vc(dsi, vc, dsi_completion_handler,
> +				&completion, DSI_VC_IRQ_PACKET_SENT);
> +	if (r)
> +		return r;
> +
>   	if (mipi_dsi_packet_format_is_short(msg->type))
>   		r = dsi_vc_send_short(dsi, vc, msg);
>   	else
>   		r = dsi_vc_send_long(dsi, vc, msg);
>   
> -	if (r < 0)
> +	if ((!r) && wait_for_completion_timeout(&completion,
> +				msecs_to_jiffies(500)) == 0)
> +		r = -EIO;
> +
> +	dsi_unregister_isr_vc(dsi, vc, dsi_completion_handler,
> +			      &completion, DSI_VC_IRQ_PACKET_SENT);
> +	if (r)
>   		return r;
>   
> -	/*
> -	 * TODO: we do not always have to do the BTA sync, for example
> -	 * we can improve performance by setting the update window
> -	 * information without sending BTA sync between the commands.
> -	 * In that case we can return early.
> -	 */
> +	/* TODO: find out if more needs to be done for MIPI_DSI_MSG_REQ_ACK */
>   
> -	r = dsi_vc_send_bta_sync(dssdev, vc);
> -	if (r) {
> -		DSSERR("bta sync failed\n");
> -		return r;
> +	if (msg->flags & MIPI_DSI_MSG_REQ_ACK) {
> +		r = dsi_vc_send_bta_sync(dssdev, vc);
> +		if (r) {
> +			DSSERR("bta sync failed\n");
> +			return r;
> +		}
>   	}
>   
>   	/* RX_FIFO_NOT_EMPTY */
> @@ -3233,21 +3243,6 @@ static int _dsi_update(struct dsi_data *dsi)
>   	return 0;
>   }
>   
> -static int _dsi_send_nop(struct dsi_data *dsi, int vc, int channel)
> -{
> -	const u8 payload[] = { MIPI_DCS_NOP };
> -	const struct mipi_dsi_msg msg = {
> -		.channel = channel,
> -		.type = MIPI_DSI_DCS_SHORT_WRITE,
> -		.tx_len = 1,
> -		.tx_buf = payload,
> -	};
> -
> -	WARN_ON(!dsi_bus_is_locked(dsi));
> -
> -	return _omap_dsi_host_transfer(dsi, vc, &msg);
> -}
> -
>   static int dsi_update_channel(struct omap_dss_device *dssdev, int vc)
>   {
>   	struct dsi_data *dsi = to_dsi_data(dssdev);
> @@ -3268,13 +3263,13 @@ static int dsi_update_channel(struct omap_dss_device *dssdev, int vc)
>   	DSSDBG("dsi_update_channel: %d", vc);
>   
>   	/*
> -	 * Send NOP between the frames. If we don't send something here, the
> +	 * Transition to LP here. If we don't send something here, the
>   	 * updates stop working. This is probably related to DSI spec stating
>   	 * that the DSI host should transition to LP at least once per frame.
>   	 */
> -	r = _dsi_send_nop(dsi, VC_CMD, dsi->dsidev->channel);
> +	r = dsi_vc_send_bta_sync(dssdev, vc);
>   	if (r < 0) {
> -		DSSWARN("failed to send nop between frames: %d\n", r);
> +		DSSWARN("failed to send bta sync between frames: %d\n", r);
>   		goto err;
>   	}
>   
> 
> ---
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
> change-id: 20260528-vm-upstr-c8e7634ebf56
> 
> Best regards,
> --
> Andreas Kemnade <andreas@kemnade.info>
>