Re: [PATCH 19/19] drm/i915: only enable HWSTAM interrupts on postinstall on ILK+

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Apr 02, 2014 at 05:08:34PM +0100, Damien Lespiau wrote:
> On Wed, Apr 02, 2014 at 04:28:06PM +0100, Chris Wilson wrote:
> > On Wed, Apr 02, 2014 at 11:14:58AM -0300, Paulo Zanoni wrote:
> > > 2014-04-02 8:27 GMT-03:00 Chris Wilson <chris@chris-wilson.co.uk>:
> > > > On Wed, Apr 02, 2014 at 12:23:51PM +0100, Chris Wilson wrote:
> > > >> On Wed, Apr 02, 2014 at 12:21:45PM +0100, Damien Lespiau wrote:
> > > >> > On Tue, Apr 01, 2014 at 03:37:27PM -0300, Paulo Zanoni wrote:
> > > >> > > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > >> > >
> > > >> > > We should only enable interrupts at postinstall.
> > > >> > >
> > > >> > > And now on ILK/SNB/IVB/HSW the irq_preinstall and irq_postinstall
> > > >> > > functions leave the hardware in the same state.
> > > >> > >
> > > >> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > >> >
> > > >> > Orthogonal note to this patch, I'm wondering why we enable any interrupt
> > > >> > reporting in the HW status page, I don't see anywhere we read that
> > > >> > information back? Any idea?
> > > >>
> > > >> Back in the SNB days, interrupts broke without this piece of magic.
> > > >
> > > > To be more precise, iirc, it was a step towards getting coherent
> > > > breadcrumb writes into the HWS. As it turns out, there were more steps
> > > > required. Considering that it can now probably be dropped again? Any
> > > > takers?
> > > 
> > > I noticed the HWSTAM code looks weird, but I don't really know much
> > > about why it's there, so I decided to not add any regressions. I wrote
> > > this patch a long time ago, and gave up on upstreaming it, but in case
> > > anybody wants, I can send it to the list:
> > > http://cgit.freedesktop.org/~pzanoni/linux/commit/?h=c8-wip&id=7ebe245a2c55379ddc7b36f1fb440215c23f1570
> > > . Look at the commit message, it may be an interesting starting point
> > > if anybody wants to dig on the issue...
> > 
> > Yup, that is very interesting. That may be the minimal piece of HWSTAM
> > magic we need, and be an interim step to removing HWSTAM entirely.
> 
> Would you mind resending:
>   - a patch with Chris' comment above, that's alone is useful for the
>     next guy looking at the code,
>   - a patch with the change (Maybe reusing GT_RENDER_USER_INTERRUPT as
>     HWSTAM uses the same definitions as the other interrupts registers)?
> 
> I'm sure we can get some reviews on the first, hopefully some testing on
> the second.

Seconded. Documenting what we're doing is always good ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch