On Wed, 2014-04-16 at 15:22 +0300, Ville Syrjälä wrote:
> On Mon, Apr 14, 2014 at 08:24:31PM +0300, Imre Deak wrote:
> > While checking the error capture path I noticed that this register is
> > read twice for GEN2, so fix this and also move the read where it's done
> > for other platforms.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gpu_error.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 4865ade..ba79b59 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -1053,9 +1053,6 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
> >  		error->gfx_mode = I915_READ(GFX_MODE);
> >  	}
> >  
> > -	if (IS_GEN2(dev))
> > -		error->ier = I915_READ16(IER);
> > -
> >  	/* 2: Registers which belong to multiple generations */
> >  	if (INTEL_INFO(dev)->gen >= 7)
> >  		error->forcewake = I915_READ(FORCEWAKE_MT);
> > @@ -1079,7 +1076,10 @@ static void i915_capture_reg_state(struct drm_i915_private *dev_priv,
> >  	if (HAS_PCH_SPLIT(dev))
> >  		error->ier = I915_READ(DEIER) | I915_READ(GTIER);
> >  	else {
> > -		error->ier = I915_READ(IER);
> > +		if (IS_GEN2(dev))
> > +			error->ier = I915_READ16(IER);
> > +		else
> > +			error->ier = I915_READ(IER);
> >  		for_each_pipe(pipe)
> >  			error->pipestat[pipe] = I915_READ(PIPESTAT(pipe));
> >  	}
> 
> The IER handling seems fairly bogus all around. On VLV and PCH platforms
> we smash both the display and GT IER into the same u32. So probably no
> one can make any sense of the result.
> 
> Also I don't know why we try to dump only these two interrupt registers
> but not the others.
> 
> So seems like there's more work that needs to be done here.

Right, haven't noticed those.

This patch could be still applied as it's just one step towards fixing
the other issues you mentioned, but I think it should be anyway kept
separate (being just a cleanup patch).

--Imre