diff mbox

[17/68] Revert "drm/i915/bdw: Use timeout mode for RC6 on bdw"

Message ID 1408677155-1840-18-git-send-email-benjamin.widawsky@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Widawsky Aug. 22, 2014, 3:11 a.m. UTC 
This reverts commit 0d68b25e9ceb344fe2f93373b1c0311d33814265.

At one time I bisected reset breakage to this patch by using a mesa that is
guaranteed to generate a hang when using the fs, and then running the following
test case:

./bin/shader_runner  tests/shaders/glsl-algebraic-add-zero.shader_test -auto
./bin/shader_runner  tests/shaders/glsl-fs-texture2d.shader_test -auto
./bin/shader_runner  tests/shaders/glsl-algebraic-add-zero.shader_test -auto

The symptom is that after the first GPU hang, all subsequent commands
will hang the GPU.

Oddly at some point I believe this revert stopped fixing the issue, but I am
leaving it in the series to minimize variables.

---
 drivers/gpu/drm/i915/intel_pm.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

Comments

Rodrigo Vivi Oct. 31, 2014, 7:45 p.m. UTC | #1 
I'm wondering how many hangs we could have fixed with this patch.

Althougth it can end up in worst RC6 residency this is what spec shows.
From our experience with SNB it is always better to stay with spec, mainly
on threshold values.

So,

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


On Thu, Aug 21, 2014 at 8:11 PM, Ben Widawsky
<benjamin.widawsky@intel.com> wrote:
> This reverts commit 0d68b25e9ceb344fe2f93373b1c0311d33814265.
>
> At one time I bisected reset breakage to this patch by using a mesa that is
> guaranteed to generate a hang when using the fs, and then running the following
> test case:
>
> ./bin/shader_runner  tests/shaders/glsl-algebraic-add-zero.shader_test -auto
> ./bin/shader_runner  tests/shaders/glsl-fs-texture2d.shader_test -auto
> ./bin/shader_runner  tests/shaders/glsl-algebraic-add-zero.shader_test -auto
>
> The symptom is that after the first GPU hang, all subsequent commands
> will hang the GPU.
>
> Oddly at some point I believe this revert stopped fixing the issue, but I am
> leaving it in the series to minimize variables.
>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 41de760..3255c10 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3658,23 +3658,15 @@ static void gen8_enable_rps(struct drm_device *dev)
>         for_each_ring(ring, dev_priv, unused)
>                 I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
>         I915_WRITE(GEN6_RC_SLEEP, 0);
> -       if (IS_BROADWELL(dev))
> -               I915_WRITE(GEN6_RC6_THRESHOLD, 625); /* 800us/1.28 for TO */
> -       else
> -               I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
> +       I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
>
>         /* 3: Enable RC6 */
>         if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE)
>                 rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
>         intel_print_rc6_info(dev, rc6_mask);
> -       if (IS_BROADWELL(dev))
> -               I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> -                               GEN7_RC_CTL_TO_MODE |
> -                               rc6_mask);
> -       else
> -               I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> -                               GEN6_RC_CTL_EI_MODE(1) |
> -                               rc6_mask);
> +       I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> +                                   GEN6_RC_CTL_EI_MODE(1) |
> +                                   rc6_mask);
>
>         /* 4 Program defaults and thresholds for RPS*/
>         I915_WRITE(GEN6_RPNSWREQ,
> --
> 2.0.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Rodrigo Vivi Oct. 31, 2014, 9:10 p.m. UTC | #2 
Please ignore my command and this revert. It seems original version
that is currently applied is a needed Workaround.

On Fri, Oct 31, 2014 at 12:45 PM, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote:
> I'm wondering how many hangs we could have fixed with this patch.
>
> Althougth it can end up in worst RC6 residency this is what spec shows.
> From our experience with SNB it is always better to stay with spec, mainly
> on threshold values.
>
> So,
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
>
> On Thu, Aug 21, 2014 at 8:11 PM, Ben Widawsky
> <benjamin.widawsky@intel.com> wrote:
>> This reverts commit 0d68b25e9ceb344fe2f93373b1c0311d33814265.
>>
>> At one time I bisected reset breakage to this patch by using a mesa that is
>> guaranteed to generate a hang when using the fs, and then running the following
>> test case:
>>
>> ./bin/shader_runner  tests/shaders/glsl-algebraic-add-zero.shader_test -auto
>> ./bin/shader_runner  tests/shaders/glsl-fs-texture2d.shader_test -auto
>> ./bin/shader_runner  tests/shaders/glsl-algebraic-add-zero.shader_test -auto
>>
>> The symptom is that after the first GPU hang, all subsequent commands
>> will hang the GPU.
>>
>> Oddly at some point I believe this revert stopped fixing the issue, but I am
>> leaving it in the series to minimize variables.
>>
>> ---
>>  drivers/gpu/drm/i915/intel_pm.c | 16 ++++------------
>>  1 file changed, 4 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 41de760..3255c10 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -3658,23 +3658,15 @@ static void gen8_enable_rps(struct drm_device *dev)
>>         for_each_ring(ring, dev_priv, unused)
>>                 I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
>>         I915_WRITE(GEN6_RC_SLEEP, 0);
>> -       if (IS_BROADWELL(dev))
>> -               I915_WRITE(GEN6_RC6_THRESHOLD, 625); /* 800us/1.28 for TO */
>> -       else
>> -               I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
>> +       I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
>>
>>         /* 3: Enable RC6 */
>>         if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE)
>>                 rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
>>         intel_print_rc6_info(dev, rc6_mask);
>> -       if (IS_BROADWELL(dev))
>> -               I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
>> -                               GEN7_RC_CTL_TO_MODE |
>> -                               rc6_mask);
>> -       else
>> -               I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
>> -                               GEN6_RC_CTL_EI_MODE(1) |
>> -                               rc6_mask);
>> +       I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
>> +                                   GEN6_RC_CTL_EI_MODE(1) |
>> +                                   rc6_mask);
>>
>>         /* 4 Program defaults and thresholds for RPS*/
>>         I915_WRITE(GEN6_RPNSWREQ,
>> --
>> 2.0.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 41de760..3255c10 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3658,23 +3658,15 @@  static void gen8_enable_rps(struct drm_device *dev)
 	for_each_ring(ring, dev_priv, unused)
 		I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
 	I915_WRITE(GEN6_RC_SLEEP, 0);
-	if (IS_BROADWELL(dev))
-		I915_WRITE(GEN6_RC6_THRESHOLD, 625); /* 800us/1.28 for TO */
-	else
-		I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
+	I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
 
 	/* 3: Enable RC6 */
 	if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE)
 		rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
 	intel_print_rc6_info(dev, rc6_mask);
-	if (IS_BROADWELL(dev))
-		I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
-				GEN7_RC_CTL_TO_MODE |
-				rc6_mask);
-	else
-		I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
-				GEN6_RC_CTL_EI_MODE(1) |
-				rc6_mask);
+	I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
+				    GEN6_RC_CTL_EI_MODE(1) |
+				    rc6_mask);
 
 	/* 4 Program defaults and thresholds for RPS*/
 	I915_WRITE(GEN6_RPNSWREQ,