[PATCH] drm/vc4: dsi: Serialize register writes through the DMA workaround

[PATCH] drm/vc4: dsi: Serialize register writes through the DMA workaround

[Henry Zhang]

DSI1 on BCM2835/6/7 cannot take register writes from the ARM, so
dsi_dma_workaround_write() stores the value in the single coherent
buffer reg_dma_mem and has the DMA engine copy it to the register.
Nothing serializes these writes. The threaded interrupt handler writes
INT_STAT through the same buffer while process context may be
programming the block (in a transfer, in its timeout path or in the
bridge callbacks), so one write can overwrite the buffer before the DMA
of the other has run, and the wrong value lands in a register.

Take a mutex around the buffer write and the DMA. The writes already
sleep, which is why the handler runs in a thread on this port.

Fixes: 4078f5757144 ("drm/vc4: Add DSI driver")
Signed-off-by: Henry Zhang <henryzhang62@gmail.com>
---
Sashiko pointed this out in its review of my vc4 DSI series [1]. The
race is older than the series and not limited to its timeout path, so
I am sending the fix on its own. It applies to drm-misc-next and on top
of the series.

Compile-tested only (arm64 defconfig, DRM_VC4=m, W=1); I have no
BCM2835/6/7 board with a DSI1 panel to run it on.

[1] https://sashiko.dev/#/patchset/20261028164539.3471833-1-henryzhang62@gmail.com?part=5

 drivers/gpu/drm/vc4/vc4_dsi.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
index 5ed3c35ee..b81b46829 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>
@@ -562,6 +563,11 @@ struct vc4_dsi {
 	struct dma_chan *reg_dma_chan;
 	dma_addr_t reg_dma_paddr;
 	u32 *reg_dma_mem;
+	/*
+	 * Serializes the register writes through the DMA workaround, which
+	 * all share the single reg_dma_mem buffer.
+	 */
+	struct mutex reg_dma_lock;
 	dma_addr_t reg_paddr;
 
 	const struct vc4_dsi_variant *variant;
@@ -625,6 +631,8 @@ dsi_dma_workaround_write(struct vc4_dsi *dsi, u32 offset, u32 val)
 		return;
 	}
 
+	guard(mutex)(&dsi->reg_dma_lock);
+
 	*dsi->reg_dma_mem = val;
 
 	tx = chan->device->device_prep_dma_memcpy(chan,
@@ -1667,6 +1675,10 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data)
 	if (dsi->variant->broken_axi_workaround) {
 		dma_cap_mask_t dma_mask;
 
+		ret = devm_mutex_init(dev, &dsi->reg_dma_lock);
+		if (ret)
+			return ret;
+
 		dsi->reg_dma_mem = dma_alloc_coherent(dev, 4,
 						      &dsi->reg_dma_paddr,
 						      GFP_KERNEL);
-- 
2.47.3