[Intel-gfx] [PATCH 2/4] drm/i915/gen9: Verify and enforce dc6 state writes

2016-02-18 Thread Mika Kuoppala
It has been observed that sometimes disabling the dc6 fails
and dc6 state pops back up, brief moment after disabling. This
has to be dmc save/restore timing issue or other bug in the
way dc states are handled.

Try to work around this issue as we don't have firmware fix
yet available. Verify that the value we wrote for the dmc sticks,
and also enforce it by rewriting it, if it didn't.

v2: Zero rereads on rewrite for extra paranoia (Imre)

Testcase: kms_flip/basic-flip-vs-dpms
References: https://bugs.freedesktop.org/show_bug.cgi?id=93768
Cc: Patrik Jakobsson 
Cc: Rodrigo Vivi 
Cc: Imre Deak 
Signed-off-by: Mika Kuoppala 
Reviewed-by: Imre Deak 
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 41 +++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 8b9290fdb3b2..814cf5ac1ef0 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -470,6 +470,43 @@ static void gen9_set_dc_state_debugmask_memory_up(
}
 }
 
+static void gen9_write_dc_state(struct drm_i915_private *dev_priv,
+   u32 state)
+{
+   int rewrites = 0;
+   int rereads = 0;
+   u32 v;
+
+   I915_WRITE(DC_STATE_EN, state);
+
+   /* It has been observed that disabling the dc6 state sometimes
+* doesn't stick and dmc keeps returning old value. Make sure
+* the write really sticks enough times and also force rewrite until
+* we are confident that state is exactly what we want.
+*/
+   do  {
+   v = I915_READ(DC_STATE_EN);
+
+   if (v != state) {
+   I915_WRITE(DC_STATE_EN, state);
+   rewrites++;
+   rereads = 0;
+   } else if (rereads++ > 5) {
+   break;
+   }
+
+   } while (rewrites < 100);
+
+   if (v != state)
+   DRM_ERROR("Writing dc state to 0x%x failed, now 0x%x\n",
+ state, v);
+
+   /* Most of the times we need one retry, avoid spam */
+   if (rewrites > 1)
+   DRM_DEBUG_KMS("Rewrote dc state to 0x%x %d times\n",
+ state, rewrites);
+}
+
 static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t 
state)
 {
uint32_t val;
@@ -502,8 +539,8 @@ static void gen9_set_dc_state(struct drm_i915_private 
*dev_priv, uint32_t state)
 
val &= ~mask;
val |= state;
-   I915_WRITE(DC_STATE_EN, val);
-   POSTING_READ(DC_STATE_EN);
+
+   gen9_write_dc_state(dev_priv, val);
 
dev_priv->csr.dc_state = val & mask;
 }
-- 
2.5.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/4] drm/i915/gen9: Verify and enforce dc6 state writes

2016-02-18 Thread Mika Kuoppala
Imre Deak  writes:

> On to, 2016-02-18 at 17:21 +0200, Mika Kuoppala wrote:
>> It has been observed that sometimes disabling the dc6 fails
>> and dc6 state pops back up, brief moment after disabling. This
>> has to be dmc save/restore timing issue or other bug in the
>> way dc states are handled.
>> 
>> Try to work around this issue as we don't have firmware fix
>> yet available. Verify that the value we wrote for the dmc sticks,
>> and also enforce it by rewriting it, if it didn't.
>> 
>> Testcase: kms_flip/basic-flip-vs-dpms
>> References: https://bugs.freedesktop.org/show_bug.cgi?id=93768
>> Cc: Patrik Jakobsson 
>> Cc: Rodrigo Vivi 
>> Cc: Imre Deak 
>> Signed-off-by: Mika Kuoppala 
>> ---
>>  drivers/gpu/drm/i915/intel_runtime_pm.c | 40
>> +++--
>>  1 file changed, 38 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> index 8b9290fdb3b2..cb91540cfbad 100644
>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> @@ -470,6 +470,42 @@ static void
>> gen9_set_dc_state_debugmask_memory_up(
>>  }
>>  }
>>  
>> +static void gen9_write_dc_state(struct drm_i915_private *dev_priv,
>> +u32 state)
>> +{
>> +int rewrites = 0;
>> +int rereads = 0;
>> +u32 v;
>> +
>> +I915_WRITE(DC_STATE_EN, state);
>> +
>> +/* It has been observed that disabling the dc6 state
>> sometimes
>> + * doesn't stick and dmc keeps returning old value. Make
>> sure
>> + * the write really sticks enough times and also force
>> rewrite until
>> + * we are confident that state is exactly what we want.
>> + */
>> +do  {
>> +v = I915_READ(DC_STATE_EN);
>> +
>> +if (v != state) {
>> +I915_WRITE(DC_STATE_EN, state);
>> +rewrites++;
>
> Could be rereads = 0; for extra paranoia. Either way:

Oh yes, extra paranoia in here is warranted. I will
add that.

> Reviewed-by: Imre Deak 

Thanks,
-Mika

>
>> +} else if (rereads++ > 5) {
>> +break;
>> +}
>> +
>> +} while (rewrites < 100);
>> +
>> +if (v != state)
>> +DRM_ERROR("Writing dc state to 0x%x failed, now
>> 0x%x\n",
>> +  state, v);
>> +
>> +/* Most of the times we need one retry, avoid spam */
>> +if (rewrites > 1)
>> +DRM_DEBUG_KMS("Rewrote dc state to 0x%x %d times\n",
>> +  state, rewrites);
>> +}
>> +
>>  static void gen9_set_dc_state(struct drm_i915_private *dev_priv,
>> uint32_t state)
>>  {
>>  uint32_t val;
>> @@ -502,8 +538,8 @@ static void gen9_set_dc_state(struct
>> drm_i915_private *dev_priv, uint32_t state)
>>  
>>  val &= ~mask;
>>  val |= state;
>> -I915_WRITE(DC_STATE_EN, val);
>> -POSTING_READ(DC_STATE_EN);
>> +
>> +gen9_write_dc_state(dev_priv, val);
>>  
>>  dev_priv->csr.dc_state = val & mask;
>>  }
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/4] drm/i915/gen9: Verify and enforce dc6 state writes

2016-02-18 Thread Imre Deak
On to, 2016-02-18 at 17:21 +0200, Mika Kuoppala wrote:
> It has been observed that sometimes disabling the dc6 fails
> and dc6 state pops back up, brief moment after disabling. This
> has to be dmc save/restore timing issue or other bug in the
> way dc states are handled.
> 
> Try to work around this issue as we don't have firmware fix
> yet available. Verify that the value we wrote for the dmc sticks,
> and also enforce it by rewriting it, if it didn't.
> 
> Testcase: kms_flip/basic-flip-vs-dpms
> References: https://bugs.freedesktop.org/show_bug.cgi?id=93768
> Cc: Patrik Jakobsson 
> Cc: Rodrigo Vivi 
> Cc: Imre Deak 
> Signed-off-by: Mika Kuoppala 
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 40
> +++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 8b9290fdb3b2..cb91540cfbad 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -470,6 +470,42 @@ static void
> gen9_set_dc_state_debugmask_memory_up(
>   }
>  }
>  
> +static void gen9_write_dc_state(struct drm_i915_private *dev_priv,
> + u32 state)
> +{
> + int rewrites = 0;
> + int rereads = 0;
> + u32 v;
> +
> + I915_WRITE(DC_STATE_EN, state);
> +
> + /* It has been observed that disabling the dc6 state
> sometimes
> +  * doesn't stick and dmc keeps returning old value. Make
> sure
> +  * the write really sticks enough times and also force
> rewrite until
> +  * we are confident that state is exactly what we want.
> +  */
> + do  {
> + v = I915_READ(DC_STATE_EN);
> +
> + if (v != state) {
> + I915_WRITE(DC_STATE_EN, state);
> + rewrites++;

Could be rereads = 0; for extra paranoia. Either way:
Reviewed-by: Imre Deak 

> + } else if (rereads++ > 5) {
> + break;
> + }
> +
> + } while (rewrites < 100);
> +
> + if (v != state)
> + DRM_ERROR("Writing dc state to 0x%x failed, now
> 0x%x\n",
> +   state, v);
> +
> + /* Most of the times we need one retry, avoid spam */
> + if (rewrites > 1)
> + DRM_DEBUG_KMS("Rewrote dc state to 0x%x %d times\n",
> +   state, rewrites);
> +}
> +
>  static void gen9_set_dc_state(struct drm_i915_private *dev_priv,
> uint32_t state)
>  {
>   uint32_t val;
> @@ -502,8 +538,8 @@ static void gen9_set_dc_state(struct
> drm_i915_private *dev_priv, uint32_t state)
>  
>   val &= ~mask;
>   val |= state;
> - I915_WRITE(DC_STATE_EN, val);
> - POSTING_READ(DC_STATE_EN);
> +
> + gen9_write_dc_state(dev_priv, val);
>  
>   dev_priv->csr.dc_state = val & mask;
>  }
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 2/4] drm/i915/gen9: Verify and enforce dc6 state writes

2016-02-18 Thread Mika Kuoppala
It has been observed that sometimes disabling the dc6 fails
and dc6 state pops back up, brief moment after disabling. This
has to be dmc save/restore timing issue or other bug in the
way dc states are handled.

Try to work around this issue as we don't have firmware fix
yet available. Verify that the value we wrote for the dmc sticks,
and also enforce it by rewriting it, if it didn't.

Testcase: kms_flip/basic-flip-vs-dpms
References: https://bugs.freedesktop.org/show_bug.cgi?id=93768
Cc: Patrik Jakobsson 
Cc: Rodrigo Vivi 
Cc: Imre Deak 
Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 40 +++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 8b9290fdb3b2..cb91540cfbad 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -470,6 +470,42 @@ static void gen9_set_dc_state_debugmask_memory_up(
}
 }
 
+static void gen9_write_dc_state(struct drm_i915_private *dev_priv,
+   u32 state)
+{
+   int rewrites = 0;
+   int rereads = 0;
+   u32 v;
+
+   I915_WRITE(DC_STATE_EN, state);
+
+   /* It has been observed that disabling the dc6 state sometimes
+* doesn't stick and dmc keeps returning old value. Make sure
+* the write really sticks enough times and also force rewrite until
+* we are confident that state is exactly what we want.
+*/
+   do  {
+   v = I915_READ(DC_STATE_EN);
+
+   if (v != state) {
+   I915_WRITE(DC_STATE_EN, state);
+   rewrites++;
+   } else if (rereads++ > 5) {
+   break;
+   }
+
+   } while (rewrites < 100);
+
+   if (v != state)
+   DRM_ERROR("Writing dc state to 0x%x failed, now 0x%x\n",
+ state, v);
+
+   /* Most of the times we need one retry, avoid spam */
+   if (rewrites > 1)
+   DRM_DEBUG_KMS("Rewrote dc state to 0x%x %d times\n",
+ state, rewrites);
+}
+
 static void gen9_set_dc_state(struct drm_i915_private *dev_priv, uint32_t 
state)
 {
uint32_t val;
@@ -502,8 +538,8 @@ static void gen9_set_dc_state(struct drm_i915_private 
*dev_priv, uint32_t state)
 
val &= ~mask;
val |= state;
-   I915_WRITE(DC_STATE_EN, val);
-   POSTING_READ(DC_STATE_EN);
+
+   gen9_write_dc_state(dev_priv, val);
 
dev_priv->csr.dc_state = val & mask;
 }
-- 
2.5.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx