vc4_irq_disable() masks the V3D interrupt sources and then calls
synchronize_irq() before the V3D is powered down. However, by itself,
this is not enough to quiesce the interrupt handler.
synchronize_irq() waits for handlers that have already set
IRQD_IRQ_INPROGRESS, and for irqchips reporting IRQCHIP_STATE_ACTIVE.
A GIC interrupt chip is able to mark an interrupt active as soon as a
CPU acknowledges it, so there these checks cover the whole dispatch path.
On RPi 0-3, however, the interrupt controller is ARMCTRL, which has no
active state. A CPU that has read the hwirq out of the pending register
but has not yet reached handle_level_irq() stays invisible to
synchronize_irq().
During a power transition, vc4_irq() may therefore run after the power
domain is off, where every V3D register read will return 0xdeadbeef.
0xdeadbeef has FLDONE, FRDONE and OUTOMEM set. This problem doesn't
trigger NULL pointer dereference issues only because the functions
vc4_irq_finish_bin_job() and vc4_irq_finish_render_job() return early
when there is no job pointer, but OUTOMEM is still able to schedule
vc4_overflow_mem_work() after vc4_irq_disable() has already cancelled it.
Therefore, fix the spurious interrupts by disabling the interrupt line
across the PM transition. To preserve the enable/disable balance, move
vc4_irq_install() and request the IRQ line disabled before the first
runtime resume.
Fixes: 9b6f461582e6 ("drm/vc4: v3d: Stop disabling interrupts")
Signed-off-by: Maíra Canal <[email protected]>
---
drivers/gpu/drm/vc4/vc4_irq.c | 18 +++---------------
drivers/gpu/drm/vc4/vc4_v3d.c | 13 +++++++------
2 files changed, 10 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_irq.c b/drivers/gpu/drm/vc4/vc4_irq.c
index 7877d493d80e..98a8db720091 100644
--- a/drivers/gpu/drm/vc4/vc4_irq.c
+++ b/drivers/gpu/drm/vc4/vc4_irq.c
@@ -276,7 +276,7 @@ vc4_irq_disable(struct drm_device *dev)
V3D_WRITE(V3D_INTCTL, V3D_DRIVER_IRQS);
/* Finish any interrupt handler still in flight. */
- synchronize_irq(vc4->irq);
+ disable_irq(vc4->irq);
cancel_work_sync(&vc4->overflow_mem_work);
}
@@ -284,7 +284,6 @@ vc4_irq_disable(struct drm_device *dev)
int vc4_irq_install(struct drm_device *dev, int irq)
{
struct vc4_dev *vc4 = to_vc4_dev(dev);
- int ret;
if (WARN_ON_ONCE(vc4->gen > VC4_GEN_4))
return -ENODEV;
@@ -298,19 +297,8 @@ int vc4_irq_install(struct drm_device *dev, int irq)
init_waitqueue_head(&vc4->job_wait_queue);
INIT_WORK(&vc4->overflow_mem_work, vc4_overflow_mem_work);
- /* Clear any pending interrupts someone might have left around
- * for us.
- */
- V3D_WRITE(V3D_INTCTL, V3D_DRIVER_IRQS);
-
- ret = devm_request_irq(dev->dev, irq, vc4_irq, 0,
- dev_name(dev->dev), dev);
- if (ret)
- return ret;
-
- vc4_irq_enable(dev);
-
- return 0;
+ return devm_request_irq(dev->dev, irq, vc4_irq, IRQF_NO_AUTOEN,
+ dev_name(dev->dev), dev);
}
void vc4_irq_uninstall(struct drm_device *dev)
diff --git a/drivers/gpu/drm/vc4/vc4_v3d.c b/drivers/gpu/drm/vc4/vc4_v3d.c
index f32410420d3e..ca4b9d1a0e80 100644
--- a/drivers/gpu/drm/vc4/vc4_v3d.c
+++ b/drivers/gpu/drm/vc4/vc4_v3d.c
@@ -396,6 +396,7 @@ static int vc4_v3d_runtime_resume(struct device *dev)
vc4_v3d_init_hw(&vc4->base);
vc4_irq_enable(&vc4->base);
+ enable_irq(vc4->irq);
return 0;
}
@@ -452,6 +453,12 @@ static int vc4_v3d_bind(struct device *dev, struct device
*master, void *data)
return ret;
vc4->irq = ret;
+ ret = vc4_irq_install(drm, vc4->irq);
+ if (ret) {
+ drm_err(drm, "Failed to install IRQ handler\n");
+ return ret;
+ }
+
ret = devm_pm_runtime_enable(dev);
if (ret)
return ret;
@@ -473,12 +480,6 @@ static int vc4_v3d_bind(struct device *dev, struct device
*master, void *data)
V3D_WRITE(V3D_BPOA, 0);
V3D_WRITE(V3D_BPOS, 0);
- ret = vc4_irq_install(drm, vc4->irq);
- if (ret) {
- drm_err(drm, "Failed to install IRQ handler\n");
- goto err_put_runtime_pm;
- }
-
pm_runtime_use_autosuspend(dev);
pm_runtime_set_autosuspend_delay(dev, 40); /* a little over 2 frames. */
pm_runtime_put_autosuspend(dev);
--
2.55.0
