Although there are more work to be done I don't see any issue or
damage by putting it already to the correct place.
So, feel free to use: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
On Wed, Apr 16, 2014 at 9:57 AM, Imre Deak <imre.deak@xxxxxxxxx> wrote:
> 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@xxxxxxxxx>
>> > ---
>> > 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
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
