Skip to content

Commit

Permalink
drm/rockchip: vop: fix irq disabled after vop driver probed
Browse files Browse the repository at this point in the history
The vop irq is shared between vop and iommu and irq probing in the
iommu driver moved to the probe function recently. This can in some
cases lead to a stall if the irq is triggered while the vop driver
still has it disabled, but the vop irq handler gets called.

But there is no real need to disable the irq, as the vop can simply
also track its enabled state and ignore irqs in that case.
For this we can simply check the power-domain state of the vop,
similar to how the iommu driver does it.

So remove the enable/disable handling and add appropriate condition
to the irq handler.

changes in v2:
- move to just check the power-domain state
- add clock handling
changes in v3:
- clarify comment to speak of runtime-pm not power-domain
changes in v4:
- address Marc's comments (clk-enable WARN_ON and style improvement)

Fixes: d0b912b ("iommu/rockchip: Request irqs in rk_iommu_probe()")
Cc: stable@vger.kernel.org
Signed-off-by: Sandy Huang <hjc@rock-chips.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Tested-by: Ezequiel Garcia <ezequiel@collabora.com>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180612132028.27490-3-heiko@sntech.de
  • Loading branch information
Sandy Huang authored and Heiko Stuebner committed Jun 18, 2018
1 parent e2810a7 commit 6456314
Showing 1 changed file with 17 additions and 8 deletions.
25 changes: 17 additions & 8 deletions drivers/gpu/drm/rockchip/rockchip_drm_vop.c
Original file line number Diff line number Diff line change
Expand Up @@ -573,8 +573,6 @@ static int vop_enable(struct drm_crtc *crtc)

spin_unlock(&vop->reg_lock);

enable_irq(vop->irq);

drm_crtc_vblank_on(crtc);

return 0;
Expand Down Expand Up @@ -618,8 +616,6 @@ static void vop_crtc_atomic_disable(struct drm_crtc *crtc,

vop_dsp_hold_valid_irq_disable(vop);

disable_irq(vop->irq);

vop->is_enabled = false;

/*
Expand Down Expand Up @@ -1195,6 +1191,18 @@ static irqreturn_t vop_isr(int irq, void *data)
uint32_t active_irqs;
int ret = IRQ_NONE;

/*
* The irq is shared with the iommu. If the runtime-pm state of the
* vop-device is disabled the irq has to be targeted at the iommu.
*/
if (!pm_runtime_get_if_in_use(vop->dev))
return IRQ_NONE;

if (vop_core_clks_enable(vop)) {
DRM_DEV_ERROR_RATELIMITED(vop->dev, "couldn't enable clocks\n");
goto out;
}

/*
* interrupt register has interrupt status, enable and clear bits, we
* must hold irq_lock to avoid a race with enable/disable_vblank().
Expand All @@ -1210,7 +1218,7 @@ static irqreturn_t vop_isr(int irq, void *data)

/* This is expected for vop iommu irqs, since the irq is shared */
if (!active_irqs)
return IRQ_NONE;
goto out_disable;

if (active_irqs & DSP_HOLD_VALID_INTR) {
complete(&vop->dsp_hold_completion);
Expand All @@ -1236,6 +1244,10 @@ static irqreturn_t vop_isr(int irq, void *data)
DRM_DEV_ERROR(vop->dev, "Unknown VOP IRQs: %#02x\n",
active_irqs);

out_disable:
vop_core_clks_disable(vop);
out:
pm_runtime_put(vop->dev);
return ret;
}

Expand Down Expand Up @@ -1614,9 +1626,6 @@ static int vop_bind(struct device *dev, struct device *master, void *data)
if (ret)
goto err_disable_pm_runtime;

/* IRQ is initially disabled; it gets enabled in power_on */
disable_irq(vop->irq);

return 0;

err_disable_pm_runtime:
Expand Down

0 comments on commit 6456314

Please sign in to comment.