Re: [PATCH v2 3/3] drm/vc4: dsi: Serialize host transfers

[Dave Stevenson]

On Mon, 19 Oct 2026 at 18:55, Henry Zhang <henryzhang62@xxxxxxxxx> wrote:
>
> 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, and the timeout recovery
> added in the previous patch could reset the block under a transfer
> that is still in flight.
>
> Hold a mutex for the whole host transfer, including the recovery.
>
> Signed-off-by: Henry Zhang <henryzhang62@xxxxxxxxx>

Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>

> ---
>
> Notes:
>     New in v2 (Sashiko review).
>
>  drivers/gpu/drm/vc4/vc4_dsi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c
> index 2d985bc..627adf0 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,8 @@ struct vc4_dsi {
>          */
>         struct clk *pixel_clock;
>
> +       /* Serializes host transfers, including the recovery after a timeout. */
> +       struct mutex transfer_lock;
>         struct completion xfer_completion;
>         int xfer_result;
>         int irq;
> @@ -1434,6 +1437,8 @@ static ssize_t vc4_dsi_host_transfer(struct mipi_dsi_host *host,
>         u32 disp0_ctrl;
>         ssize_t ret;
>
> +       guard(mutex)(&dsi->transfer_lock);
> +
>         ret = vc4_dsi_transfer(dsi, msg);
>         if (ret != -ETIMEDOUT)
>                 return ret;
> @@ -1911,6 +1916,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))
> @@ -1918,6 +1924,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
>