All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] i915: pm: Be less agressive with clockfreq changes on Bay Trail
@ 2018-01-06 19:53 Hans de Goede
  2018-01-06 19:53 ` [PATCH] " Hans de Goede
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Hans de Goede @ 2018-01-06 19:53 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Mika Kuoppala
  Cc: Hans de Goede, intel-gfx, dri-devel

Hi All,

So this patch, which I already submitted a while ago and back then it
got one comment from Chris Wilson:

> Basically you want a limit on the frequency of ... pcode access?
> As presented, you ultimately do not trust any write and the only
> solution is to disable all writes. No RPS whatsoever, run at max and
> hope rc6 works (maybe even decrease the rc6 threshold).
> 
> One of the ideas we floated was moving the pcode access to a worker and
> ratelimiting the updates.

Has finally seen some testing by users affected by the infamous
"intel_idle.max_cstate=1 required to prevent crashes" bug:
https://bugzilla.kernel.org/show_bug.cgi?id=109051

And so far the reports are that it does help to make the users stable for
2/3 users and it not being effective for 1/3 users.

Now that we've some test results,  I believe that it is worthwhile to get
this simple patch mainlined, hence this re-submission.

Regards,

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] i915: pm: Be less agressive with clockfreq changes on Bay Trail
  2018-01-06 19:53 [PATCH 0/1] i915: pm: Be less agressive with clockfreq changes on Bay Trail Hans de Goede
@ 2018-01-06 19:53 ` Hans de Goede
  2018-01-06 20:14 ` ✓ Fi.CI.BAT: success for i915: pm: Be less agressive with clockfreq changes on Bay Trail (rev2) Patchwork
  2018-01-08 15:33 ` ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2018-01-06 19:53 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Ville Syrjälä,
	Mika Kuoppala
  Cc: Hans de Goede, intel-gfx, dri-devel

Bay Trail devices are known to hang when changing the frequency often,
this is discussed in great length in:
https://bugzilla.kernel.org/show_bug.cgi?id=109051

Commit 6067a27d1f01 ("drm/i915: Avoid tweaking evaluation thresholds
on Baytrail v3") is an attempt to workaround this. Several users in
bko109051 report that an earlier version of this patch, v1:
https://bugzilla.kernel.org/attachment.cgi?id=251471

Works better for them and they still see hangs with the merged v3.

Comparing the 2 versions shows that they are indeed not equivalent,
v1 not only skips writing the GEN6_RP* registers from valleyview_set_rps,
as v3 does. It also contained these modifications to i915_irq.c:

     if (pm_iir & GEN6_PM_RP_DOWN_EI_EXPIRED) {
         if (!vlv_c0_above(dev_priv,
                   &dev_priv->rps.down_ei, &now,
-                  dev_priv->rps.down_threshold))
+                  VLV_RP_DOWN_EI_THRESHOLD))
             events |= GEN6_PM_RP_DOWN_THRESHOLD;
         dev_priv->rps.down_ei = now;
     }

     if (pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) {
         if (vlv_c0_above(dev_priv,
                  &dev_priv->rps.up_ei, &now,
-                 dev_priv->rps.up_threshold))
+                 VLV_RP_UP_EI_THRESHOLD))
             events |= GEN6_PM_RP_UP_THRESHOLD;
         dev_priv->rps.up_ei = now;
     }

Which use less aggressive up/down thresholds, which results in less
GEN6_PM_RP_*_THRESHOLD events and thus in less calls to intel_set_rps() ->
valleyview_set_rps() -> vlv_punit_write(PUNIT_REG_GPU_FREQ_REQ).
With the last call being the likely cause of the hang.

This commit hardcodes the threshold_up and _down values for Bay Trail to
less aggressive values, reducing the amount of clock frequency changes,
thus avoiding the hangs some people are still seeing with the merged fix.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=109051
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 3 +++
 drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 505c605eff98..acafc8408e43 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1390,6 +1390,9 @@ enum i915_power_well_id {
 #define 	VLV_BIAS_CPU_125_SOC_875 (6 << 2)
 #define 	CHV_BIAS_CPU_50_SOC_50 (3 << 2)
 
+#define VLV_RP_UP_EI_THRESHOLD			90
+#define VLV_RP_DOWN_EI_THRESHOLD		70
+
 /* vlv2 north clock has */
 #define CCK_FUSE_REG				0x8
 #define  CCK_FUSE_HPLL_FREQ_MASK		0x3
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1db79a860b96..b06379622301 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6116,8 +6116,11 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
 	/* When byt can survive without system hang with dynamic
 	 * sw freq adjustments, this restriction can be lifted.
 	 */
-	if (IS_VALLEYVIEW(dev_priv))
+	if (IS_VALLEYVIEW(dev_priv)) {
+		threshold_up = VLV_RP_UP_EI_THRESHOLD;
+		threshold_down = VLV_RP_DOWN_EI_THRESHOLD;
 		goto skip_hw_write;
+	}
 
 	I915_WRITE(GEN6_RP_UP_EI,
 		   GT_INTERVAL_FROM_US(dev_priv, ei_up));
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.BAT: success for i915: pm: Be less agressive with clockfreq changes on Bay Trail (rev2)
  2018-01-06 19:53 [PATCH 0/1] i915: pm: Be less agressive with clockfreq changes on Bay Trail Hans de Goede
  2018-01-06 19:53 ` [PATCH] " Hans de Goede
@ 2018-01-06 20:14 ` Patchwork
  2018-01-08 15:33 ` ✓ Fi.CI.IGT: " Patchwork
  2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-01-06 20:14 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx

== Series Details ==

Series: i915: pm: Be less agressive with clockfreq changes on Bay Trail (rev2)
URL   : https://patchwork.freedesktop.org/series/33541/
State : success

== Summary ==

Series 33541v2 i915: pm: Be less agressive with clockfreq changes on Bay Trail
https://patchwork.freedesktop.org/api/1.0/series/33541/revisions/2/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-kbl-r) fdo#104172 +1
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-bdw-5557u) fdo#104162

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172
fdo#104162 https://bugs.freedesktop.org/show_bug.cgi?id=104162

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:418s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:424s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:369s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:483s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:275s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:480s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:478s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:462s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:449s
fi-elk-e7500     total:224  pass:168  dwarn:10  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:264s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:514s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:389s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:400s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:412s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:454s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:410s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:465s
fi-kbl-7560u     total:288  pass:268  dwarn:1   dfail:0   fail:0   skip:19  time:498s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:451s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:500s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:573s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:429s
fi-skl-6600u     total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:507s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:525s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:497s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:490s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:430s
fi-snb-2520m     total:3    pass:2    dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:393s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:557s
fi-cnl-y         total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:24 
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:470s

0cb68b015fa3b8a40c65109737aed36d1985796b drm-tip: 2018y-01m-05d-17h-47m-51s UTC integration manifest
b24dead8dba9 i915: pm: Be less agressive with clockfreq changes on Bay Trail

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7622/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.IGT: success for i915: pm: Be less agressive with clockfreq changes on Bay Trail (rev2)
  2018-01-06 19:53 [PATCH 0/1] i915: pm: Be less agressive with clockfreq changes on Bay Trail Hans de Goede
  2018-01-06 19:53 ` [PATCH] " Hans de Goede
  2018-01-06 20:14 ` ✓ Fi.CI.BAT: success for i915: pm: Be less agressive with clockfreq changes on Bay Trail (rev2) Patchwork
@ 2018-01-08 15:33 ` Patchwork
  2 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2018-01-08 15:33 UTC (permalink / raw)
  To: Hans de Goede; +Cc: intel-gfx

== Series Details ==

Series: i915: pm: Be less agressive with clockfreq changes on Bay Trail (rev2)
URL   : https://patchwork.freedesktop.org/series/33541/
State : success

== Summary ==

Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
                pass       -> FAIL       (shard-snb) fdo#101623 +1
        Subgroup fbc-2p-primscrn-pri-shrfb-draw-pwrite:
                incomplete -> SKIP       (shard-hsw) fdo#104218
Test kms_flip:
        Subgroup vblank-vs-modeset-suspend-interruptible:
                pass       -> SKIP       (shard-hsw) fdo#103540
Test gem_softpin:
        Subgroup noreloc-s4:
                skip       -> FAIL       (shard-snb) fdo#103375 +1

fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#104218 https://bugs.freedesktop.org/show_bug.cgi?id=104218
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375

shard-hsw        total:2677 pass:1514 dwarn:1   dfail:0   fail:10  skip:1151 time:8588s
shard-snb        total:2713 pass:1310 dwarn:1   dfail:0   fail:11  skip:1391 time:7824s
Blacklisted hosts:
shard-apl        total:2713 pass:1687 dwarn:1   dfail:0   fail:24  skip:1001 time:13335s
shard-kbl        total:2695 pass:1788 dwarn:1   dfail:0   fail:27  skip:878 time:10230s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7622/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] i915: pm: Be less agressive with clockfreq changes on Bay Trail
  2017-11-10 14:56 ` Chris Wilson
@ 2017-11-10 14:59   ` Hans de Goede
  0 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2017-11-10 14:59 UTC (permalink / raw)
  To: Chris Wilson, Hans de Goede, Daniel Vetter, Jani Nikula,
	Rodrigo Vivi, Mika Kuoppala
  Cc: intel-gfx, dri-devel

Hi,

On 10-11-17 15:56, Chris Wilson wrote:
> Quoting Hans de Goede (2017-11-09 18:51:01)
>> Bay Trail devices are known to hang when changing the frequency often,
>> this is discussed in great length in:
>> https://bugzilla.kernel.org/show_bug.cgi?id=109051
>>
>> Commit 6067a27d1f01 ("drm/i915: Avoid tweaking evaluation thresholds
>> on Baytrail v3") is an attempt to workaround this. Several users in
>> bko109051 report that an earlier version of this patch, v1:
>> https://bugzilla.kernel.org/attachment.cgi?id=251471
>>
>> Works better for them and they still see hangs with the merged v3.
>>
>> Comparing the 2 versions shows that they are indeed not equivalent,
>> v1 not only skips writing the GEN6_RP* registers from valleyview_set_rps,
>> as v3 does. It also contained these modifications to i915_irq.c:
>>
>>       if (pm_iir & GEN6_PM_RP_DOWN_EI_EXPIRED) {
>>           if (!vlv_c0_above(dev_priv,
>>                     &dev_priv->rps.down_ei, &now,
>> -                  dev_priv->rps.down_threshold))
>> +                  VLV_RP_DOWN_EI_THRESHOLD))
>>               events |= GEN6_PM_RP_DOWN_THRESHOLD;
>>           dev_priv->rps.down_ei = now;
>>       }
>>
>>       if (pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) {
>>           if (vlv_c0_above(dev_priv,
>>                    &dev_priv->rps.up_ei, &now,
>> -                 dev_priv->rps.up_threshold))
>> +                 VLV_RP_UP_EI_THRESHOLD))
>>               events |= GEN6_PM_RP_UP_THRESHOLD;
>>           dev_priv->rps.up_ei = now;
>>       }
>>
>> Which use less aggressive up/down thresholds, which results in less
>> GEN6_PM_RP_*_THRESHOLD events and thus in less calls to intel_set_rps() ->
>> valleyview_set_rps() -> vlv_punit_write(PUNIT_REG_GPU_FREQ_REQ).
>> With the last call being the likely cause of the hang.
>>
>> This commit hardcodes the threshold_up and _down values for Bay Trail to
>> less aggressive values, reducing the amount of clock frequency changes,
>> thus avoiding the hangs some people are still seeing with the merged fix.
>>
>> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=109051
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h | 3 +++
>>   drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
>>   2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 68a58cce6ab1..2561af075ebb 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -1355,6 +1355,9 @@ enum i915_power_well_id {
>>   #define        VLV_BIAS_CPU_125_SOC_875 (6 << 2)
>>   #define        CHV_BIAS_CPU_50_SOC_50 (3 << 2)
>>   
>> +#define VLV_RP_UP_EI_THRESHOLD                 90
>> +#define VLV_RP_DOWN_EI_THRESHOLD               70
>> +
>>   /* vlv2 north clock has */
>>   #define CCK_FUSE_REG                           0x8
>>   #define  CCK_FUSE_HPLL_FREQ_MASK               0x3
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 01966b89be14..177b6caa0a38 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -6096,8 +6096,11 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
>>          /* When byt can survive without system hang with dynamic
>>           * sw freq adjustments, this restriction can be lifted.
>>           */
>> -       if (IS_VALLEYVIEW(dev_priv))
>> +       if (IS_VALLEYVIEW(dev_priv)) {
>> +               threshold_up = VLV_RP_UP_EI_THRESHOLD;
>> +               threshold_down = VLV_RP_DOWN_EI_THRESHOLD;
> 
> Basically you want a limit on the frequency of ... pcode access?
> As presented, you ultimately do not trust any write and the only
> solution is to disable all writes. No RPS whatsoever, run at max and
> hope rc6 works (maybe even decrease the rc6 threshold).
> 
> One of the ideas we floated was moving the pcode access to a worker and
> ratelimiting the updates.

Yes ratelimiting the amount of vlv_punit_write(PUNIT_REG_GPU_FREQ_REQ)
calls is also something which came to my mind as an alternative solution.

As I tried to explain in the commit message my main inspiration for this
patch was that people were reporting different success rates with v1
and v3 of the Commit 6067a27d1f01 ("drm/i915: Avoid tweaking evaluation thresholds
on Baytrail v3") patch.

Regards,

Hans



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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] i915: pm: Be less agressive with clockfreq changes on Bay Trail
  2017-11-09 18:51 [PATCH] i915: pm: Be less agressive with clockfreq changes on Bay Trail Hans de Goede
@ 2017-11-10 14:56 ` Chris Wilson
  2017-11-10 14:59   ` Hans de Goede
  0 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2017-11-10 14:56 UTC (permalink / raw)
  To: Hans de Goede, Daniel Vetter, Jani Nikula, Rodrigo Vivi, Mika Kuoppala
  Cc: Hans de Goede, intel-gfx, dri-devel

Quoting Hans de Goede (2017-11-09 18:51:01)
> Bay Trail devices are known to hang when changing the frequency often,
> this is discussed in great length in:
> https://bugzilla.kernel.org/show_bug.cgi?id=109051
> 
> Commit 6067a27d1f01 ("drm/i915: Avoid tweaking evaluation thresholds
> on Baytrail v3") is an attempt to workaround this. Several users in
> bko109051 report that an earlier version of this patch, v1:
> https://bugzilla.kernel.org/attachment.cgi?id=251471
> 
> Works better for them and they still see hangs with the merged v3.
> 
> Comparing the 2 versions shows that they are indeed not equivalent,
> v1 not only skips writing the GEN6_RP* registers from valleyview_set_rps,
> as v3 does. It also contained these modifications to i915_irq.c:
> 
>      if (pm_iir & GEN6_PM_RP_DOWN_EI_EXPIRED) {
>          if (!vlv_c0_above(dev_priv,
>                    &dev_priv->rps.down_ei, &now,
> -                  dev_priv->rps.down_threshold))
> +                  VLV_RP_DOWN_EI_THRESHOLD))
>              events |= GEN6_PM_RP_DOWN_THRESHOLD;
>          dev_priv->rps.down_ei = now;
>      }
> 
>      if (pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) {
>          if (vlv_c0_above(dev_priv,
>                   &dev_priv->rps.up_ei, &now,
> -                 dev_priv->rps.up_threshold))
> +                 VLV_RP_UP_EI_THRESHOLD))
>              events |= GEN6_PM_RP_UP_THRESHOLD;
>          dev_priv->rps.up_ei = now;
>      }
> 
> Which use less aggressive up/down thresholds, which results in less
> GEN6_PM_RP_*_THRESHOLD events and thus in less calls to intel_set_rps() ->
> valleyview_set_rps() -> vlv_punit_write(PUNIT_REG_GPU_FREQ_REQ).
> With the last call being the likely cause of the hang.
> 
> This commit hardcodes the threshold_up and _down values for Bay Trail to
> less aggressive values, reducing the amount of clock frequency changes,
> thus avoiding the hangs some people are still seeing with the merged fix.
> 
> Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=109051
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 3 +++
>  drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 68a58cce6ab1..2561af075ebb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1355,6 +1355,9 @@ enum i915_power_well_id {
>  #define        VLV_BIAS_CPU_125_SOC_875 (6 << 2)
>  #define        CHV_BIAS_CPU_50_SOC_50 (3 << 2)
>  
> +#define VLV_RP_UP_EI_THRESHOLD                 90
> +#define VLV_RP_DOWN_EI_THRESHOLD               70
> +
>  /* vlv2 north clock has */
>  #define CCK_FUSE_REG                           0x8
>  #define  CCK_FUSE_HPLL_FREQ_MASK               0x3
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 01966b89be14..177b6caa0a38 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6096,8 +6096,11 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
>         /* When byt can survive without system hang with dynamic
>          * sw freq adjustments, this restriction can be lifted.
>          */
> -       if (IS_VALLEYVIEW(dev_priv))
> +       if (IS_VALLEYVIEW(dev_priv)) {
> +               threshold_up = VLV_RP_UP_EI_THRESHOLD;
> +               threshold_down = VLV_RP_DOWN_EI_THRESHOLD;

Basically you want a limit on the frequency of ... pcode access?
As presented, you ultimately do not trust any write and the only
solution is to disable all writes. No RPS whatsoever, run at max and
hope rc6 works (maybe even decrease the rc6 threshold).

One of the ideas we floated was moving the pcode access to a worker and
ratelimiting the updates.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] i915: pm: Be less agressive with clockfreq changes on Bay Trail
@ 2017-11-09 18:51 Hans de Goede
  2017-11-10 14:56 ` Chris Wilson
  0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2017-11-09 18:51 UTC (permalink / raw)
  To: Daniel Vetter, Jani Nikula, Rodrigo Vivi, Mika Kuoppala
  Cc: Hans de Goede, intel-gfx, dri-devel

Bay Trail devices are known to hang when changing the frequency often,
this is discussed in great length in:
https://bugzilla.kernel.org/show_bug.cgi?id=109051

Commit 6067a27d1f01 ("drm/i915: Avoid tweaking evaluation thresholds
on Baytrail v3") is an attempt to workaround this. Several users in
bko109051 report that an earlier version of this patch, v1:
https://bugzilla.kernel.org/attachment.cgi?id=251471

Works better for them and they still see hangs with the merged v3.

Comparing the 2 versions shows that they are indeed not equivalent,
v1 not only skips writing the GEN6_RP* registers from valleyview_set_rps,
as v3 does. It also contained these modifications to i915_irq.c:

     if (pm_iir & GEN6_PM_RP_DOWN_EI_EXPIRED) {
         if (!vlv_c0_above(dev_priv,
                   &dev_priv->rps.down_ei, &now,
-                  dev_priv->rps.down_threshold))
+                  VLV_RP_DOWN_EI_THRESHOLD))
             events |= GEN6_PM_RP_DOWN_THRESHOLD;
         dev_priv->rps.down_ei = now;
     }

     if (pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) {
         if (vlv_c0_above(dev_priv,
                  &dev_priv->rps.up_ei, &now,
-                 dev_priv->rps.up_threshold))
+                 VLV_RP_UP_EI_THRESHOLD))
             events |= GEN6_PM_RP_UP_THRESHOLD;
         dev_priv->rps.up_ei = now;
     }

Which use less aggressive up/down thresholds, which results in less
GEN6_PM_RP_*_THRESHOLD events and thus in less calls to intel_set_rps() ->
valleyview_set_rps() -> vlv_punit_write(PUNIT_REG_GPU_FREQ_REQ).
With the last call being the likely cause of the hang.

This commit hardcodes the threshold_up and _down values for Bay Trail to
less aggressive values, reducing the amount of clock frequency changes,
thus avoiding the hangs some people are still seeing with the merged fix.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=109051
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 3 +++
 drivers/gpu/drm/i915/intel_pm.c | 5 ++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 68a58cce6ab1..2561af075ebb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1355,6 +1355,9 @@ enum i915_power_well_id {
 #define 	VLV_BIAS_CPU_125_SOC_875 (6 << 2)
 #define 	CHV_BIAS_CPU_50_SOC_50 (3 << 2)
 
+#define VLV_RP_UP_EI_THRESHOLD			90
+#define VLV_RP_DOWN_EI_THRESHOLD		70
+
 /* vlv2 north clock has */
 #define CCK_FUSE_REG				0x8
 #define  CCK_FUSE_HPLL_FREQ_MASK		0x3
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 01966b89be14..177b6caa0a38 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6096,8 +6096,11 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val)
 	/* When byt can survive without system hang with dynamic
 	 * sw freq adjustments, this restriction can be lifted.
 	 */
-	if (IS_VALLEYVIEW(dev_priv))
+	if (IS_VALLEYVIEW(dev_priv)) {
+		threshold_up = VLV_RP_UP_EI_THRESHOLD;
+		threshold_down = VLV_RP_DOWN_EI_THRESHOLD;
 		goto skip_hw_write;
+	}
 
 	I915_WRITE(GEN6_RP_UP_EI,
 		   GT_INTERVAL_FROM_US(dev_priv, ei_up));
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-01-08 15:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-06 19:53 [PATCH 0/1] i915: pm: Be less agressive with clockfreq changes on Bay Trail Hans de Goede
2018-01-06 19:53 ` [PATCH] " Hans de Goede
2018-01-06 20:14 ` ✓ Fi.CI.BAT: success for i915: pm: Be less agressive with clockfreq changes on Bay Trail (rev2) Patchwork
2018-01-08 15:33 ` ✓ Fi.CI.IGT: " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2017-11-09 18:51 [PATCH] i915: pm: Be less agressive with clockfreq changes on Bay Trail Hans de Goede
2017-11-10 14:56 ` Chris Wilson
2017-11-10 14:59   ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.