[Intel-gfx] [PATCH 19/24] drm/i915: don't enable PM_VEBOX_CS_ERROR_INTERRUPT

[Ben Widawsky]

On Wed, Jun 12, 2013 at 08:32:44PM +0200, Daniel Vetter wrote:
> On Wed, Jun 12, 2013 at 8:19 PM, Ben Widawsky <ben at bwidawsk.net> wrote:
> > On Wed, Jun 12, 2013 at 07:18:38PM +0200, Daniel Vetter wrote:
> >> On Wed, Jun 12, 2013 at 10:13:41AM -0700, Ben Widawsky wrote:
> >> > On Wed, Jun 12, 2013 at 01:37:21PM +0200, Daniel Vetter wrote:
> >> > > The code to handle it is broken - there's simply no code to clear CS
> >> > > parser errors on gen5+. And behold, for all the other rings we also
> >> > > don't enable it!
> >> > >
> >> > > Leave the handling code itself in place just to be consistent with the
> >> > > existing mess though. And in case someone feels like fixing it all up.
> >> > >
> >> > > This has been errornously enabled in
> >> > >
> >> > > commit 12638c57f31952127c734c26315e1348fa1334c2
> >> > > Author: Ben Widawsky <ben at bwidawsk.net>
> >> > > Date:   Tue May 28 19:22:31 2013 -0700
> >> > >
> >> > >     drm/i915: Enable vebox interrupts
> >> > >
> >> > > Cc: Damien Lespiau <damien.lespiau at intel.com>
> >> > > Cc: Ben Widawsky <ben at bwidawsk.net>
> >> > > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >> > >
> >> > Why not fix the problem instead of just disabling the interrupt? Then
> >> > the "existing mess" is justified. It really shouldn't be terribly
> >> > difficult to fix it.
> >>
> >> Haven't seen it proven useful, and I don't want to blow through time just
> >> for fun on it. Hangcheck seems to be able to deal with it just fine.
> >> -Daniel
> > I don't get it. Isn't it just clearing a bit in the handler, vs.
> > clearing the flag here?
> 
> On a quick Bspec read we'd need to set up per-ring error interrupt
> registers, put them to sensible value in them, wire up the interrupt
> handling for each ring since ilk+ and then also test it. I really
> don't see the point in doing that. The only upshot of all that work is
> that if there's an instruction error we'd get an interrupt instead of
> waiting for the hangcheck to kick in.
> 
> If you insist I can follow-up with a patch to just rip out the
> bare-bones handler we have now. Everything else really feels like
> wasted effort to me.
> -Daniel

No. I don't insist. I was just thinking these kind of error interrupts
should be always on, like L3 parity. But that is indeed more than just
clearing a bit (though not THAT much).

-- 
Ben Widawsky, Intel Open Source Technology Center