Re: [PATCH] drm/i915: Check for driver readyness before handling an underrun interrupt

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	intel-gfx@lists.freedesktop.org, daniel.vetter@ffwll.ch,
	rclark@redhat.com, stable@vger.kernel.org
Subject: Re: [PATCH] drm/i915: Check for driver readyness before handling an underrun interrupt
Date: Fri, 27 Feb 2015 15:24:22 +0100	[thread overview]
Message-ID: <20150227142422.GJ24485@phenom.ffwll.local> (raw)
In-Reply-To: <20150227092704.GI20682@nuc-i3427.alporthouse.com>

On Fri, Feb 27, 2015 at 09:27:04AM +0000, Chris Wilson wrote:
> On Fri, Feb 27, 2015 at 10:20:05AM +0200, Jani Nikula wrote:
> > On Fri, 27 Feb 2015, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > > On Thu, 26 Feb 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > >> When we takeover from the BIOS and install our interrupt handler, the
> > >> BIOS may have left us a few surprises in the form of spontaneous
> > >> interrupts. (This is especially likely on hardware like 965gm where
> > >> display fifo underruns are continuous and the GMCH cannot filter that
> > >> interrupt souce.) As we enable our IRQ early so that we can use it
> > >> during hardware probing, our interrupt handler must be prepared to
> > >> handle a few sources prior to being fully configured. As such, we need
> > >> to add a simple is-ready check prior to dereferencing our KMS state for
> > >> reporting underruns.
> > >>
> > >> Reported-by: Rob Clark <rclark@redhat.com>
> > >> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1193972
> > >
> > > "You are not authorized to access bug #1193972. To see this bug, you
> > > must first log in to an account with the appropriate permissions."
> > >
> > > Meh.
> > 
> > Pushed to drm-intel-fixes, thanks for the patch and review. If someone
> > both cares and has access to the bugzilla, please update it.
> 
> Jani, I goofed...
> 
> > >>  	/* GMCH can't disable fifo underruns, filter them. */
> > >>  	if (HAS_GMCH_DISPLAY(dev_priv->dev) &&
> > >> -	    !__cpu_fifo_underrun_reporting_enabled(dev_priv, pipe))
> > >> +	    !to_intel_crtc(crtc)->cpu_fifo_underrun_disabled)
> 
> Should be without the '!':
> if (HAS_GMCH_DISPLAY(dev_priv->dev) &&
>     to_intel_crtc(crtc)->cpu_fifo_underrun_disabled)

Somewhere on my todo is to invert the sense of our underrun reporting.
I've screwed this up countless times, but I fear the patch to switch will
be buggy as well ;-)

/me is chicken

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx