* [PATCH v3 1/4] drm/vc4: dsi: Don't return error reports as read data
2026-10-26 19:14 [PATCH v3 0/4] drm/vc4: dsi: Make failed reads visible and recoverable Henry Zhang
@ 2026-10-26 19:14 ` Henry Zhang
2026-10-26 19:14 ` [PATCH v3 2/4] drm/vc4: dsi: Rate-limit error interrupt messages Henry Zhang
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Henry Zhang @ 2026-10-26 19:14 UTC (permalink / raw)
To: Maxime Ripard, Dave Stevenson
Cc: Henry Zhang, Maíra Canal, Raspberry Pi Kernel Maintenance,
Maarten Lankhorst, Thomas Zimmermann, David Airlie,
Simona Vetter, dri-devel, linux-kernel
When a DSI peripheral detects an error, for example contention during
the bus turnaround, it may answer a read request with an Acknowledge
and Error Report short packet instead of the read response.
vc4_dsi_host_transfer() does not check the type of the received packet
(there is a FINISHME for it). It copies the two bytes of the error
report into the caller's buffer and returns success, so the caller
gets the error bits as if they were register contents. Packets
received with a checksum error, an uncorrectable ECC error or
incomplete are passed on the same way.
Return -EIO in these cases so that callers can tell a failed read from
data, and retry if they want to.
Signed-off-by: Henry Zhang <henryzhang62@gmail.com>
---
drivers/gpu/drm/vc4/vc4_dsi.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index 5ed3c35ee..5452b727a 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -39,6 +39,8 @@
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
+#include <video/mipi_display.h>
+
#include "vc4_drv.h"
#include "vc4_regs.h"
@@ -1299,6 +1301,24 @@ static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host,
u32 rxpkt1h = DSI_PORT_READ(RXPKT1H);
u8 *msg_rx = msg->rx_buf;
+ /*
+ * A peripheral that detected an error, e.g. contention during
+ * the bus turnaround, may answer with an Acknowledge and Error
+ * Report instead of the read response. Don't hand that, or a
+ * packet received with errors, to the caller as read data.
+ * Bits 7:6 of the Data ID are the virtual channel.
+ */
+ if ((rxpkt1h & (DSI_RXPKT1H_CRC_ERR | DSI_RXPKT1H_DET_ERR |
+ DSI_RXPKT1H_INCOMP_PKT)) ||
+ (!(rxpkt1h & DSI_RXPKT1H_PKT_TYPE_LONG) &&
+ (VC4_GET_FIELD(rxpkt1h, DSI_RXPKT1H_DT_LP_CMD) & 0x3f) ==
+ MIPI_DSI_RX_ACKNOWLEDGE_AND_ERROR_REPORT)) {
+ drm_dbg_driver(drm, "DSI read rejected, RXPKT1H 0x%08x\n",
+ rxpkt1h);
+ ret = -EIO;
+ goto reset_fifo_and_return;
+ }
+
if (rxpkt1h & DSI_RXPKT1H_PKT_TYPE_LONG) {
u32 rxlen = VC4_GET_FIELD(rxpkt1h,
DSI_RXPKT1H_BC_PARAM);
@@ -1313,8 +1333,6 @@ static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host,
for (i = 0; i < msg->rx_len; i++)
msg_rx[i] = DSI_READ(DSI1_RXPKT_FIFO);
} else {
- /* FINISHME: Handle AWER */
-
msg_rx[0] = VC4_GET_FIELD(rxpkt1h,
DSI_RXPKT1H_SHORT_0);
if (msg->rx_len > 1) {
--
2.47.3
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v3 3/4] drm/vc4: dsi: Serialize host transfers
2026-10-26 19:14 [PATCH v3 0/4] drm/vc4: dsi: Make failed reads visible and recoverable Henry Zhang
2026-10-26 19:14 ` [PATCH v3 1/4] drm/vc4: dsi: Don't return error reports as read data Henry Zhang
2026-10-26 19:14 ` [PATCH v3 2/4] drm/vc4: dsi: Rate-limit error interrupt messages Henry Zhang
@ 2026-10-26 19:14 ` Henry Zhang
2026-10-26 19:14 ` [PATCH v3 4/4] drm/vc4: dsi: Recover from a timed-out transfer Henry Zhang
3 siblings, 0 replies; 5+ messages in thread
From: Henry Zhang @ 2026-10-26 19:14 UTC (permalink / raw)
To: Maxime Ripard, Dave Stevenson
Cc: Henry Zhang, Maíra Canal, Raspberry Pi Kernel Maintenance,
Maarten Lankhorst, Thomas Zimmermann, David Airlie,
Simona Vetter, dri-devel, linux-kernel
vc4_dsi_host_transfer() fills the command FIFOs and the TXPKT registers
and waits on a single completion without any locking. Two callers
sending at the same time, for example a panel driver and a backlight
update, can overwrite each other's packets. Nothing keeps a transfer
from running while the bridge callbacks reprogram the block either,
for example while pre_enable sets up the PHY timings.
Add a mutex and hold it for the whole host transfer and in the bridge
pre_enable, enable, disable and post_disable callbacks. None of these
callbacks calls into the DSI device, so the lock is never taken
recursively.
Signed-off-by: Henry Zhang <henryzhang62@gmail.com>
---
drivers/gpu/drm/vc4/vc4_dsi.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index face44a23..ea3a6d06b 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -25,6 +25,7 @@
#include <linux/dma-mapping.h>
#include <linux/dmaengine.h>
#include <linux/io.h>
+#include <linux/mutex.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/platform_device.h>
@@ -595,6 +596,11 @@ struct vc4_dsi {
*/
struct clk *pixel_clock;
+ /*
+ * Serializes host transfers against each other and against the
+ * bridge enable/disable paths, which reprogram the block.
+ */
+ struct mutex transfer_lock;
struct completion xfer_completion;
int xfer_result;
@@ -805,6 +811,8 @@ static void vc4_dsi_bridge_disable(struct drm_bridge *bridge,
struct vc4_dsi *dsi = bridge_to_vc4_dsi(bridge);
u32 disp0_ctrl;
+ guard(mutex)(&dsi->transfer_lock);
+
disp0_ctrl = DSI_PORT_READ(DISP0_CTRL);
disp0_ctrl &= ~DSI_DISP0_ENABLE;
DSI_PORT_WRITE(DISP0_CTRL, disp0_ctrl);
@@ -816,6 +824,8 @@ static void vc4_dsi_bridge_post_disable(struct drm_bridge *bridge,
struct vc4_dsi *dsi = bridge_to_vc4_dsi(bridge);
struct device *dev = &dsi->pdev->dev;
+ guard(mutex)(&dsi->transfer_lock);
+
clk_disable_unprepare(dsi->pll_phy_clock);
clk_disable_unprepare(dsi->escape_clock);
clk_disable_unprepare(dsi->pixel_clock);
@@ -891,6 +901,8 @@ static void vc4_dsi_bridge_pre_enable(struct drm_bridge *bridge,
unsigned long phy_clock;
int ret;
+ guard(mutex)(&dsi->transfer_lock);
+
ret = pm_runtime_resume_and_get(dev);
if (ret) {
drm_err(bridge->dev, "Failed to runtime PM enable on DSI%d\n", dsi->variant->port);
@@ -1148,6 +1160,8 @@ static void vc4_dsi_bridge_enable(struct drm_bridge *bridge,
bool debug_dump_regs = false;
u32 disp0_ctrl;
+ guard(mutex)(&dsi->transfer_lock);
+
disp0_ctrl = DSI_PORT_READ(DISP0_CTRL);
disp0_ctrl |= DSI_DISP0_ENABLE;
DSI_PORT_WRITE(DISP0_CTRL, disp0_ctrl);
@@ -1181,6 +1195,8 @@ static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host,
bool is_long = mipi_dsi_packet_format_is_long(msg->type);
u32 cmd_fifo_len = 0, pix_fifo_len = 0;
+ guard(mutex)(&dsi->transfer_lock);
+
mipi_dsi_create_packet(&packet, msg);
pkth |= VC4_SET_FIELD(packet.header[0], DSI_TXPKT1H_BC_DT);
@@ -1807,6 +1823,7 @@ static int vc4_dsi_dev_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct vc4_dsi *dsi;
+ int ret;
dsi = devm_drm_bridge_alloc(&pdev->dev, struct vc4_dsi, bridge, &vc4_dsi_bridge_funcs);
if (IS_ERR(dsi))
@@ -1814,6 +1831,9 @@ static int vc4_dsi_dev_probe(struct platform_device *pdev)
dev_set_drvdata(dev, dsi);
dsi->pdev = pdev;
+ ret = devm_mutex_init(dev, &dsi->transfer_lock);
+ if (ret)
+ return ret;
#ifdef CONFIG_OF
dsi->bridge.of_node = dev->of_node;
#endif
--
2.47.3
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v3 4/4] drm/vc4: dsi: Recover from a timed-out transfer
2026-10-26 19:14 [PATCH v3 0/4] drm/vc4: dsi: Make failed reads visible and recoverable Henry Zhang
` (2 preceding siblings ...)
2026-10-26 19:14 ` [PATCH v3 3/4] drm/vc4: dsi: Serialize host transfers Henry Zhang
@ 2026-10-26 19:14 ` Henry Zhang
3 siblings, 0 replies; 5+ messages in thread
From: Henry Zhang @ 2026-10-26 19:14 UTC (permalink / raw)
To: Maxime Ripard, Dave Stevenson
Cc: Henry Zhang, Maíra Canal, Raspberry Pi Kernel Maintenance,
Maarten Lankhorst, Thomas Zimmermann, David Airlie,
Simona Vetter, dri-devel, linux-kernel
When a transfer times out, vc4_dsi_host_transfer() disables the
command, resets the FIFOs and returns -ETIMEDOUT. That is not always
enough: after an LP receive timeout during a read (the peripheral did
not complete the bus turnaround), every following transfer times out
as well, with no interrupt and STAT reading back 0. Only a modeset,
which goes through post_disable/pre_enable, brings the block back.
Reset the DSI block and its analog front end after a timeout and restore
the configuration programmed in pre_enable, in the same order as
pre_enable: the block is ungated and the AFE released only after the
timings are programmed. Before resetting, wait for the interrupt
handler, so that a late completion interrupt of the timed-out transfer
cannot complete the next one, and mask the interrupts until the AFE is
out of reset: the error interrupts otherwise fire continuously while it
is held in reset. The timed-out message is not resent, as it may already
have been executed by the peripheral; the error is returned and the
caller decides whether to retry.
Only do this while the block is enabled. Between post_disable and
pre_enable the clocks are off and there is nothing to restore; a
transfer then times out as before. The transfer lock keeps the
recovery from running in the middle of pre_enable or post_disable.
This does not prevent the timeouts themselves. With video running,
vc4 does not keep commands within vertical blanking, and transfers can
still time out, sometimes several in a row after a reset. That is a
separate issue.
Signed-off-by: Henry Zhang <henryzhang62@gmail.com>
---
drivers/gpu/drm/vc4/vc4_dsi.c | 130 ++++++++++++++++++++++++++++++++--
1 file changed, 123 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index ea3a6d06b..42f50d2ca 100644
--- a/drivers/gpu/drm/vc4/vc4_dsi.c
+++ b/drivers/gpu/drm/vc4/vc4_dsi.c
@@ -601,8 +601,11 @@ struct vc4_dsi {
* bridge enable/disable paths, which reprogram the block.
*/
struct mutex transfer_lock;
+ /* The block is powered and configured. Protected by transfer_lock. */
+ bool enabled;
struct completion xfer_completion;
int xfer_result;
+ int irq;
struct debugfs_regset32 regset;
};
@@ -826,6 +829,8 @@ static void vc4_dsi_bridge_post_disable(struct drm_bridge *bridge,
guard(mutex)(&dsi->transfer_lock);
+ dsi->enabled = false;
+
clk_disable_unprepare(dsi->pll_phy_clock);
clk_disable_unprepare(dsi->escape_clock);
clk_disable_unprepare(dsi->pixel_clock);
@@ -1151,6 +1156,8 @@ static void vc4_dsi_bridge_pre_enable(struct drm_bridge *bridge,
DSI_PORT_WRITE(DISP0_CTRL,
DSI_DISP0_COMMAND_MODE);
}
+
+ dsi->enabled = true;
}
static void vc4_dsi_bridge_enable(struct drm_bridge *bridge,
@@ -1184,10 +1191,9 @@ static int vc4_dsi_bridge_attach(struct drm_bridge *bridge,
&dsi->bridge, flags);
}
-static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host,
- const struct mipi_dsi_msg *msg)
+static ssize_t vc4_dsi_transfer(struct vc4_dsi *dsi,
+ const struct mipi_dsi_msg *msg)
{
- struct vc4_dsi *dsi = host_to_dsi(host);
struct drm_device *drm = dsi->bridge.dev;
struct mipi_dsi_packet packet;
u32 pkth = 0, pktc = 0;
@@ -1195,8 +1201,6 @@ static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host,
bool is_long = mipi_dsi_packet_format_is_long(msg->type);
u32 cmd_fifo_len = 0, pix_fifo_len = 0;
- guard(mutex)(&dsi->transfer_lock);
-
mipi_dsi_create_packet(&packet, msg);
pkth |= VC4_SET_FIELD(packet.header[0], DSI_TXPKT1H_BC_DT);
@@ -1374,6 +1378,114 @@ static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host,
return ret;
}
+/*
+ * Reset the DSI block and its analog front end, then restore the
+ * configuration done in pre_enable. Resetting the FIFOs alone does not
+ * clear the state the block can be left in after a transfer timed out
+ * (e.g. on an LP receive timeout): every following transfer then times
+ * out as well, until the next modeset.
+ */
+static void vc4_dsi_reset_and_restore(struct vc4_dsi *dsi)
+{
+ u32 en = dsi->variant->port ? DSI1_CTRL_EN : DSI0_CTRL_CTRL0;
+ u32 ctrl = DSI_PORT_READ(CTRL);
+ u32 afec0 = DSI_PORT_READ(PHY_AFEC0);
+ u32 afec1 = DSI_PORT_READ(PHY_AFEC1);
+ u32 disp0 = DSI_PORT_READ(DISP0_CTRL) & ~DSI_DISP0_ENABLE;
+ u32 disp1 = DSI_PORT_READ(DISP1_CTRL);
+ u32 hstx_to = DSI_PORT_READ(HSTX_TO_CNT);
+ u32 lprx_to = DSI_PORT_READ(LPRX_TO_CNT);
+ u32 ta_to = DSI_PORT_READ(TA_TO_CNT);
+ u32 pr_to = DSI_PORT_READ(PR_TO_CNT);
+ u32 phyc = DSI_PORT_READ(PHYC);
+ u32 clt0 = DSI_PORT_READ(HS_CLT0);
+ u32 clt1 = DSI_PORT_READ(HS_CLT1);
+ u32 clt2 = DSI_PORT_READ(HS_CLT2);
+ u32 dlt3 = DSI_PORT_READ(HS_DLT3);
+ u32 dlt4 = DSI_PORT_READ(HS_DLT4);
+ u32 dlt5 = DSI_PORT_READ(HS_DLT5);
+ u32 dlt6 = DSI_PORT_READ(HS_DLT6);
+ u32 dlt7 = DSI_PORT_READ(HS_DLT7);
+
+ /*
+ * Mask the interrupts while the block and its AFE are in reset: the
+ * error interrupts would otherwise fire continuously.
+ */
+ DSI_PORT_WRITE(INT_EN, 0);
+ DSI_PORT_WRITE(DISP0_CTRL, disp0);
+ DSI_PORT_WRITE(TXPKT1C, 0);
+ DSI_PORT_WRITE(CTRL, DSI_CTRL_SOFT_RESET_CFG |
+ DSI_PORT_BIT(CTRL_RESET_FIFOS));
+ DSI_PORT_WRITE(PHY_AFEC0, afec0 | DSI_PORT_BIT(PHY_AFEC0_RESET));
+ /* AFEC reset hold time */
+ mdelay(1);
+
+ /*
+ * Same order as pre_enable: the block stays gated and the AFE in
+ * reset until the timings are programmed.
+ */
+ DSI_PORT_WRITE(CTRL, ctrl & ~(DSI_CTRL_CAL_BYTE | en));
+ DSI_PORT_WRITE(STAT, DSI_PORT_READ(STAT));
+ DSI_PORT_WRITE(PHY_AFEC1, afec1);
+ DSI_PORT_WRITE(HS_CLT0, clt0);
+ DSI_PORT_WRITE(HS_CLT1, clt1);
+ DSI_PORT_WRITE(HS_CLT2, clt2);
+ DSI_PORT_WRITE(HS_DLT3, dlt3);
+ DSI_PORT_WRITE(HS_DLT4, dlt4);
+ DSI_PORT_WRITE(HS_DLT5, dlt5);
+ DSI_PORT_WRITE(HS_DLT6, dlt6);
+ DSI_PORT_WRITE(HS_DLT7, dlt7);
+ DSI_PORT_WRITE(PHYC, phyc);
+ DSI_PORT_WRITE(CTRL, ctrl & ~en);
+ DSI_PORT_WRITE(HSTX_TO_CNT, hstx_to);
+ DSI_PORT_WRITE(LPRX_TO_CNT, lprx_to);
+ DSI_PORT_WRITE(TA_TO_CNT, ta_to);
+ DSI_PORT_WRITE(PR_TO_CNT, pr_to);
+ DSI_PORT_WRITE(DISP1_CTRL, disp1);
+ /* Ungate the block. */
+ DSI_PORT_WRITE(CTRL, ctrl);
+ /* Bring AFE out of reset. */
+ DSI_PORT_WRITE(PHY_AFEC0, afec0 & ~DSI_PORT_BIT(PHY_AFEC0_RESET));
+ DSI_PORT_WRITE(DISP0_CTRL, disp0);
+
+ DSI_PORT_WRITE(INT_STAT, DSI_PORT_READ(INT_STAT));
+ DSI_PORT_WRITE(INT_EN, DSI_PORT_BIT(INTERRUPTS_ALWAYS_ENABLED));
+}
+
+static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host,
+ const struct mipi_dsi_msg *msg)
+{
+ struct vc4_dsi *dsi = host_to_dsi(host);
+ u32 disp0_ctrl;
+ ssize_t ret;
+
+ guard(mutex)(&dsi->transfer_lock);
+
+ ret = vc4_dsi_transfer(dsi, msg);
+ /* Only a running block can be reset and restored. */
+ if (ret != -ETIMEDOUT || !dsi->enabled)
+ return ret;
+
+ /*
+ * Leave the block usable for the next transfer. The message is not
+ * resent: it may already have reached the peripheral, and only the
+ * caller knows whether it is safe to repeat.
+ */
+ disp0_ctrl = DSI_PORT_READ(DISP0_CTRL);
+ dev_warn(&dsi->pdev->dev,
+ "transfer timed out (stat 0x%08x), resetting DSI\n",
+ DSI_PORT_READ(STAT));
+ /*
+ * The completion interrupt of the timed-out transfer may still be
+ * in flight; let it finish so that it cannot complete the next one.
+ */
+ synchronize_irq(dsi->irq);
+ vc4_dsi_reset_and_restore(dsi);
+ DSI_PORT_WRITE(DISP0_CTRL, disp0_ctrl);
+
+ return ret;
+}
+
static const struct component_ops vc4_dsi_ops;
static int vc4_dsi_host_attach(struct mipi_dsi_host *host,
struct mipi_dsi_device *device)
@@ -1743,14 +1855,18 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data)
/* Clear any existing interrupt state. */
DSI_PORT_WRITE(INT_STAT, DSI_PORT_READ(INT_STAT));
+ dsi->irq = platform_get_irq(pdev, 0);
+ if (dsi->irq < 0)
+ return dsi->irq;
+
if (dsi->reg_dma_mem)
- ret = devm_request_threaded_irq(dev, platform_get_irq(pdev, 0),
+ ret = devm_request_threaded_irq(dev, dsi->irq,
vc4_dsi_irq_defer_to_thread_handler,
vc4_dsi_irq_handler,
IRQF_ONESHOT,
"vc4 dsi", dsi);
else
- ret = devm_request_irq(dev, platform_get_irq(pdev, 0),
+ ret = devm_request_irq(dev, dsi->irq,
vc4_dsi_irq_handler, 0, "vc4 dsi", dsi);
if (ret) {
if (ret != -EPROBE_DEFER)
--
2.47.3
^ permalink raw reply [flat|nested] 5+ messages in thread