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

2015-09-09 Thread Jani Nikula
On Tue, 08 Sep 2015, Daniel Vetter  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: russianneuroman...@ya.ru
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91906
>> Signed-off-by: Chris Wilson 
>> Cc: Michał Winiarski 
>> Cc: Daniel Vetter 
>> Cc: Jani Nikula 
>> Cc: stable@vger.kernel.org
>
> Still Reviewed-by: Daniel Vetter 

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
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-09-08 Thread Daniel Vetter
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: russianneuroman...@ya.ru
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91906
> Signed-off-by: Chris Wilson 
> Cc: Michał Winiarski 
> Cc: Daniel Vetter 
> Cc: Jani Nikula 
> Cc: stable@vger.kernel.org

Still Reviewed-by: Daniel Vetter 

> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-09-08 Thread Chris Wilson
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: russianneuroman...@ya.ru
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91906
Signed-off-by: Chris Wilson 
Cc: Michał Winiarski 
Cc: Daniel Vetter 
Cc: Jani Nikula 
Cc: stable@vger.kernel.org
---
 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

--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html