Re: [PATCH v2] drm/i915: Limit the number of loops for reading a split 64bit register

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 08 Sep 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Sep 08, 2015 at 02:17:13PM +0100, Chris Wilson wrote:
>> In I915_READ64_2x32 we attempt to read a 64bit register using 2 32bit
>> reads. Due to the nature of the registers we try to read in this manner,
>> they may increment between the two instruction (e.g. a timestamp
>> counter). To keep the result accurate, we repeat the read if we detect
>> an overflow (i.e. the upper value varies). However, some harware is just
>> plain flaky and may endless loop as the the upper 32bits are not stable.
>> Just give up after a couple of tries and report whatever we read last.
>> 
>> v2: Use the most recent values when erring out on an unstable register.
>> 
>> Reported-by: russianneuromancer@ya.ru
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91906
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: stable@vger.kernel.org
>
> Still Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Pushed to drm-intel-next-fixes, thanks for the patch and review.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 12870073d58f..51a88e70a6f7 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3402,13 +3402,13 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
>>  #define I915_READ64(reg)	dev_priv->uncore.funcs.mmio_readq(dev_priv, (reg), true)
>>  
>>  #define I915_READ64_2x32(lower_reg, upper_reg) ({			\
>> -	u32 upper, lower, tmp;						\
>> -	tmp = I915_READ(upper_reg);					\
>> +	u32 upper, lower, old_upper, loop = 0;				\
>> +	upper = I915_READ(upper_reg);					\
>>  	do {								\
>> -		upper = tmp;						\
>> +		old_upper = upper;					\
>>  		lower = I915_READ(lower_reg);				\
>> -		tmp = I915_READ(upper_reg);				\
>> -	} while (upper != tmp);						\
>> +		upper = I915_READ(upper_reg);				\
>> +	} while (upper != old_upper && loop++ < 2);			\
>>  	(u64)upper << 32 | lower; })
>>  
>>  #define POSTING_READ(reg)	(void)I915_READ_NOTRACE(reg)
>> -- 
>> 2.5.1
>> 
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx