From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stable-owner@vger.kernel.org>
Received: from gloria.sntech.de ([95.129.55.99]:52442 "EHLO gloria.sntech.de"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1753772AbeFRJis (ORCPT <rfc822;stable@vger.kernel.org>);
        Mon, 18 Jun 2018 05:38:48 -0400
From: Heiko Stuebner <heiko@sntech.de>
To: Tomasz Figa <tfiga@chromium.org>
Cc: dri-devel <dri-devel@lists.freedesktop.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@lists.infradead.org>,
        Ezequiel Garcia <ezequiel@collabora.com>,
        Robin Murphy <robin.murphy@arm.com>, marc.zyngier@arm.com,
        Jeffy <jeffy.chen@rock-chips.com>,
        Sandy Huang <hjc@rock-chips.com>,
        enric.balletbo@collabora.co.uk,
        Tomeu Vizoso <tomeu.vizoso@collabora.co.uk>,
        stable@vger.kernel.org
Subject: Re: [PATCH v3 2/2] drm/rockchip: vop: fix irq disabled after vop driver probed
Date: Mon, 18 Jun 2018 11:38:32 +0200
Message-ID: <84967793.QqIqAsAVEt@phil>
In-Reply-To: <CAAFQd5D77W8qQm9YmOtr_bpD7WpA9VokjQ+jvTo05F4BFdyrTw@mail.gmail.com>
References: <20180612121537.31223-1-heiko@sntech.de> <20180612121537.31223-3-heiko@sntech.de> <CAAFQd5D77W8qQm9YmOtr_bpD7WpA9VokjQ+jvTo05F4BFdyrTw@mail.gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Sender: stable-owner@vger.kernel.org
List-ID: <stable.vger.kernel.org>

Am Montag, 18. Juni 2018, 10:44:58 CEST schrieb Tomasz Figa:
> Hi Heiko,
> 
> On Tue, Jun 12, 2018 at 9:15 PM Heiko Stuebner <heiko@sntech.de> wrote:
> >
> > From: Sandy Huang <hjc@rock-chips.com>
> >
> > 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
> [snip]
> > @@ -1209,8 +1215,11 @@ static irqreturn_t vop_isr(int irq, void *data)
> >         spin_unlock(&vop->irq_lock);
> >
> >         /* This is expected for vop iommu irqs, since the irq is shared */
> > -       if (!active_irqs)
> > -               return IRQ_NONE;
> > +       if (!active_irqs) {
> > +               ret = IRQ_NONE;
> > +               vop_core_clks_disable(vop);
> 
> nit: If we're adding "out:", couldn't we also add "out_clks:" and move
> the call to vop_core_clks_disable() there?
> 
> > +               goto out;
> > +       }
> >
> >         if (active_irqs & DSP_HOLD_VALID_INTR) {
> >                 complete(&vop->dsp_hold_completion);
> > @@ -1236,6 +1245,10 @@ static irqreturn_t vop_isr(int irq, void *data)
> >                 DRM_DEV_ERROR(vop->dev, "Unknown VOP IRQs: %#02x\n",
> >                               active_irqs);
> >
> > +       vop_core_clks_disable(vop);
> > +
> > +out:
> > +       pm_runtime_put(vop->dev);
> >         return ret;
> >  }
> 
> Other than that:
> 
> Reviewed-by: Tomasz Figa <tfiga@chromium.org>

That's similar to what Marc suggested and thus already part of v4
posted last tuesday, so I'll just carry over your Reviewed-by.

Could you possibly also give patch1 a nod of approval? So I can honor
the strong suggestion in the drm-misc documentation? ;-)

Thanks
Heiko