On Wed, Apr 2, 2025 at 4:36 PM AngeloGioacchino Del Regno
<angelogioacchino.delre...@collabora.com> wrote:
>
> The RDMA driver is installing an ISR in the probe function but, if
> the component is not bound yet, the interrupt handler may call the
> vblank_cb ahead of time (while probing other drivers) or too late
> (while removing other drivers), possibly accessing memory that it
> should not try to access by reusing stale pointers.
>
> In order to fix this, like done in the OVL driver, add a new `irq`
> member to struct mtk_disp_ovl and then set the NOAUTOEN flag to
> the irq before installing the ISR to manually disable and clear
> the hwirqs with register writes, and enable_irq() and disable_irq()
> in the bind and unbind callbacks respectively.
>
> Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
> Signed-off-by: AngeloGioacchino Del Regno
> <angelogioacchino.delre...@collabora.com>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 35 ++++++++++++++----------
> 1 file changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> index bf47790e4d6b..8c5021365a04 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> @@ -81,6 +81,7 @@ struct mtk_disp_rdma_data {
> struct mtk_disp_rdma {
> struct clk *clk;
> void __iomem *regs;
> + int irq;
> struct cmdq_client_reg cmdq_reg;
> const struct mtk_disp_rdma_data *data;
> void (*vblank_cb)(void *data);
> @@ -295,13 +296,23 @@ void mtk_rdma_layer_config(struct device *dev, unsigned
> int idx,
> static int mtk_disp_rdma_bind(struct device *dev, struct device *master,
> void *data)
> {
> - return 0;
> + struct mtk_disp_rdma *priv = dev_get_drvdata(dev);
> +
> + /* Disable and clear pending interrupts */
> + writel(0x0, priv->regs + DISP_REG_RDMA_INT_ENABLE);
> + writel(0x0, priv->regs + DISP_REG_RDMA_INT_STATUS);
>
> + enable_irq(priv->irq);
> +
> + return 0;
> }
>
> static void mtk_disp_rdma_unbind(struct device *dev, struct device *master,
> void *data)
> {
> + struct mtk_disp_rdma *priv = dev_get_drvdata(dev);
> +
> + disable_irq(priv->irq);
> }
>
> static const struct component_ops mtk_disp_rdma_component_ops = {
> @@ -314,16 +325,15 @@ static int mtk_disp_rdma_probe(struct platform_device
> *pdev)
> struct device *dev = &pdev->dev;
> struct mtk_disp_rdma *priv;
> struct resource *res;
> - int irq;
> int ret;
>
> priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> if (!priv)
> return -ENOMEM;
>
> - irq = platform_get_irq(pdev, 0);
> - if (irq < 0)
> - return irq;
> + priv->irq = platform_get_irq(pdev, 0);
> + if (priv->irq < 0)
> + return priv->irq;
>
> priv->clk = devm_clk_get(dev, NULL);
> if (IS_ERR(priv->clk))
> @@ -347,21 +357,18 @@ static int mtk_disp_rdma_probe(struct platform_device
> *pdev)
> if (ret && (ret != -EINVAL))
> return dev_err_probe(dev, ret, "Failed to get rdma fifo
> size\n");
>
> - /* Disable and clear pending interrupts */
> - writel(0x0, priv->regs + DISP_REG_RDMA_INT_ENABLE);
> - writel(0x0, priv->regs + DISP_REG_RDMA_INT_STATUS);
> -
> - ret = devm_request_irq(dev, irq, mtk_disp_rdma_irq_handler,
> - IRQF_TRIGGER_NONE, dev_name(dev), priv);
> - if (ret < 0)
> - return dev_err_probe(dev, ret, "Failed to request irq %d\n",
> irq);
> -
> priv->data = of_device_get_match_data(dev);
>
> platform_set_drvdata(pdev, priv);
>
> pm_runtime_enable(dev);
>
> + irq_set_status_flags(priv->irq, IRQ_NOAUTOEN);
> + ret = devm_request_irq(dev, priv->irq, mtk_disp_rdma_irq_handler,
> + IRQF_TRIGGER_NONE, dev_name(dev), priv);
Same comment as OVL driver change.
ChenYu
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Failed to request irq %d\n",
> priv->irq);
> +
> ret = component_add(dev, &mtk_disp_rdma_component_ops);
> if (ret) {
> pm_runtime_disable(dev);
> --
> 2.48.1
>
