| Message ID | 1431111069-11847-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6366
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 302/302 302/302
SNB 316/316 316/316
IVB 342/342 342/342
BYT 286/286 286/286
BDW 321/321 321/321
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
Note: You need to pay more attention to line start with '*'
On 05/08/2015 11:51 AM, Daniel Vetter wrote: > Like with every other feature that's not enabled by default we break > runtime pm support way too often by accident because the overall test > coverage isn't great. And it's been almost 2 years since we enabled > the power well code by default > > commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41 > Author: Paulo Zanoni <paulo.r.zanoni@intel.com> > Date: Wed Jul 3 17:12:13 2013 -0300 > > drm/i915: switch disable_power_well default value to 1 > > It's really more than overdue for runtime pm itself to follow! > > Note that in practice this wont do a hole lot yet, since we're still > gated on snd-hda-intel doing proper runtime pm. But I've discussed > this with Liam and we agreed that this needs to be done. And the audio > team is working to hold up their end of this bargain. > > And the justification for updating the autosuspend delay to 100ms: > Quick measurment shows that we can do a full rpm cycle in about 5ms, > which means the delay should still be really conservative from a power > conservation pov. The only workload that would suffer from ping-pong > is also only gpu/compute with all screens off. 100ms should cover any > kind of latency with submitting follow-up batches. > > Cc: Takashi Iwai <tiwai@suse.de> > Cc: Liam Girdwood <liam.r.girdwood@intel.com> > Cc: Yang, Libin <libin.yang@intel.com> > Cc: Lin, Mengdong <mengdong.lin@intel.com> > Cc: Li, Jocelyn <jocelyn.li@intel.com> > Cc: Kaskinen, Tanu <tanu.kaskinen@intel.com> > Cc: Zanoni, Paulo R <paulo.r.zanoni@intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 317b9b43d1c1..36ecbe275dd9 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -1919,9 +1919,10 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv) > return; > } > > - pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */ > + pm_runtime_set_autosuspend_delay(device, 100); > pm_runtime_mark_last_busy(device); > pm_runtime_use_autosuspend(device); > + pm_runtime_allow(device); > > pm_runtime_put_autosuspend(device); > } > This also changes the timeout... it shouldn't break anything, but it would be good to get some numbers from Eero's team on this. It might be better to use a lower or higher number based on a typical desktop workload by default (though I guess the user or distro can do their own tuning and override the value too). Jesse
Can I get an ack on this please? The audio folks already move ahead http://permalink.gmane.org/gmane.linux.alsa.devel/139831 Would be embarassing if we are late to the party we organized ourselves ... -Daniel On Fri, May 8, 2015 at 8:51 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > Like with every other feature that's not enabled by default we break > runtime pm support way too often by accident because the overall test > coverage isn't great. And it's been almost 2 years since we enabled > the power well code by default > > commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41 > Author: Paulo Zanoni <paulo.r.zanoni@intel.com> > Date: Wed Jul 3 17:12:13 2013 -0300 > > drm/i915: switch disable_power_well default value to 1 > > It's really more than overdue for runtime pm itself to follow! > > Note that in practice this wont do a hole lot yet, since we're still > gated on snd-hda-intel doing proper runtime pm. But I've discussed > this with Liam and we agreed that this needs to be done. And the audio > team is working to hold up their end of this bargain. > > And the justification for updating the autosuspend delay to 100ms: > Quick measurment shows that we can do a full rpm cycle in about 5ms, > which means the delay should still be really conservative from a power > conservation pov. The only workload that would suffer from ping-pong > is also only gpu/compute with all screens off. 100ms should cover any > kind of latency with submitting follow-up batches. > > Cc: Takashi Iwai <tiwai@suse.de> > Cc: Liam Girdwood <liam.r.girdwood@intel.com> > Cc: Yang, Libin <libin.yang@intel.com> > Cc: Lin, Mengdong <mengdong.lin@intel.com> > Cc: Li, Jocelyn <jocelyn.li@intel.com> > Cc: Kaskinen, Tanu <tanu.kaskinen@intel.com> > Cc: Zanoni, Paulo R <paulo.r.zanoni@intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 317b9b43d1c1..36ecbe275dd9 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -1919,9 +1919,10 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv) > return; > } > > - pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */ > + pm_runtime_set_autosuspend_delay(device, 100); > pm_runtime_mark_last_busy(device); > pm_runtime_use_autosuspend(device); > + pm_runtime_allow(device); > > pm_runtime_put_autosuspend(device); > } > -- > 2.1.0 >
On Tue, 2015-06-16 at 10:34 +0200, Daniel Vetter wrote: > Can I get an ack on this please? The audio folks already move ahead > > http://permalink.gmane.org/gmane.linux.alsa.devel/139831 > > Would be embarassing if we are late to the party we organized ourselves ... > -Daniel > > Mengdong, Libin - I assume someone has tested this ? could someone Ack. Thanks Liam > On Fri, May 8, 2015 at 8:51 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > Like with every other feature that's not enabled by default we break > > runtime pm support way too often by accident because the overall test > > coverage isn't great. And it's been almost 2 years since we enabled > > the power well code by default > > > > commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41 > > Author: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Date: Wed Jul 3 17:12:13 2013 -0300 > > > > drm/i915: switch disable_power_well default value to 1 > > > > It's really more than overdue for runtime pm itself to follow! > > > > Note that in practice this wont do a hole lot yet, since we're still > > gated on snd-hda-intel doing proper runtime pm. But I've discussed > > this with Liam and we agreed that this needs to be done. And the audio > > team is working to hold up their end of this bargain. > > > > And the justification for updating the autosuspend delay to 100ms: > > Quick measurment shows that we can do a full rpm cycle in about 5ms, > > which means the delay should still be really conservative from a power > > conservation pov. The only workload that would suffer from ping-pong > > is also only gpu/compute with all screens off. 100ms should cover any > > kind of latency with submitting follow-up batches. > > > > Cc: Takashi Iwai <tiwai@suse.de> > > Cc: Liam Girdwood <liam.r.girdwood@intel.com> > > Cc: Yang, Libin <libin.yang@intel.com> > > Cc: Lin, Mengdong <mengdong.lin@intel.com> > > Cc: Li, Jocelyn <jocelyn.li@intel.com> > > Cc: Kaskinen, Tanu <tanu.kaskinen@intel.com> > > Cc: Zanoni, Paulo R <paulo.r.zanoni@intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 317b9b43d1c1..36ecbe275dd9 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -1919,9 +1919,10 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv) > > return; > > } > > > > - pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */ > > + pm_runtime_set_autosuspend_delay(device, 100); > > pm_runtime_mark_last_busy(device); > > pm_runtime_use_autosuspend(device); > > + pm_runtime_allow(device); > > > > pm_runtime_put_autosuspend(device); > > } > > -- > > 2.1.0 > > > > > --------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
2015-06-16 5:34 GMT-03:00 Daniel Vetter <daniel.vetter@ffwll.ch>: > Can I get an ack on this please? The audio folks already move ahead > > http://permalink.gmane.org/gmane.linux.alsa.devel/139831 > > Would be embarassing if we are late to the party we organized ourselves ... The situation is already embarassing for us :) pzanoni@nhoque:~/nfs/intel-gpu-tools/tests$ sudo ./pm_rpm --quick 2>&1 | tee pm_rpm.txt (lots of stuff) pzanoni@nhoque:~/nfs/intel-gpu-tools/tests$ ./sumario.sh pm_rpm.txt Subtest drm-resources-equal: FAIL (5.897s) Subtest dpms-lpsp: FAIL (10.045s) Subtest dpms-non-lpsp: FAIL (10.027s) Subtest cursor-dpms: FAIL (12.693s) Subtest legacy-planes-dpms: FAIL (12.636s) Subtest universal-planes-dpms: FAIL (12.719s) Subtest dpms-mode-unset-lpsp: FAIL (12.588s) Subtest dpms-mode-unset-non-lpsp: FAIL (10.234s) Subtest fences-dpms: FAIL (11.726s) Total: 41 SUCCESS: 32 FAIL: 9 SKIP: 0 pzanoni@nhoque:~/nfs/intel-gpu-tools/tests$ edmesg [ 166.007957] WARNING: CPU: 1 PID: 1648 at drivers/gpu/drm/i915/intel_uncore.c:620 hsw_unclaimed_reg_debug+0x75/0xa0 [i915]() [ 174.944917] WARNING: CPU: 1 PID: 1648 at drivers/gpu/drm/i915/intel_uncore.c:620 hsw_unclaimed_reg_debug+0x75/0xa0 [i915]() [ 327.095531] WARNING: CPU: 0 PID: 1648 at drivers/gpu/drm/i915/i915_gem.c:5316 i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() [ 327.096026] WARNING: CPU: 0 PID: 1648 at drivers/gpu/drm/i915/i915_gem.c:5316 i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() [ 327.096597] WARNING: CPU: 0 PID: 1648 at drivers/gpu/drm/i915/i915_gem.c:5316 i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() [ 327.096866] WARNING: CPU: 0 PID: 1648 at drivers/gpu/drm/i915/i915_gem.c:5316 i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() [ 327.097089] WARNING: CPU: 0 PID: 1648 at drivers/gpu/drm/i915/i915_gem.c:5316 i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() [ 327.097323] WARNING: CPU: 0 PID: 1648 at drivers/gpu/drm/i915/i915_gem.c:5316 i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() [ 327.097534] WARNING: CPU: 0 PID: 1648 at drivers/gpu/drm/i915/i915_gem.c:5316 i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() [ 327.097767] WARNING: CPU: 0 PID: 1648 at drivers/gpu/drm/i915/i915_gem.c:5316 i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() pzanoni@nhoque:~/nfs/intel-gpu-tools/tests$ lspci -nn | grep VGA 00:02.0 VGA compatible controller [0300]: Intel Corporation Broadwell-U Integrated Graphics [8086:1616] (rev 09) Please notice that, if I remember correctly, just a few weeks ago we were passing everything, although we were having some dmesg WARNs. Now we're not even passing the tests anymore :( And I only see 2 pm_rpm bugs on bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90863 https://bugs.freedesktop.org/show_bug.cgi?id=90546 Why isn't PRTS/QA cathing stuff!? I'm sorry, but for now, I just can't give the ACK. > -Daniel > > > On Fri, May 8, 2015 at 8:51 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >> Like with every other feature that's not enabled by default we break >> runtime pm support way too often by accident because the overall test >> coverage isn't great. And it's been almost 2 years since we enabled >> the power well code by default >> >> commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41 >> Author: Paulo Zanoni <paulo.r.zanoni@intel.com> >> Date: Wed Jul 3 17:12:13 2013 -0300 >> >> drm/i915: switch disable_power_well default value to 1 >> >> It's really more than overdue for runtime pm itself to follow! >> >> Note that in practice this wont do a hole lot yet, since we're still >> gated on snd-hda-intel doing proper runtime pm. But I've discussed >> this with Liam and we agreed that this needs to be done. And the audio >> team is working to hold up their end of this bargain. >> >> And the justification for updating the autosuspend delay to 100ms: >> Quick measurment shows that we can do a full rpm cycle in about 5ms, >> which means the delay should still be really conservative from a power >> conservation pov. The only workload that would suffer from ping-pong >> is also only gpu/compute with all screens off. 100ms should cover any >> kind of latency with submitting follow-up batches. >> >> Cc: Takashi Iwai <tiwai@suse.de> >> Cc: Liam Girdwood <liam.r.girdwood@intel.com> >> Cc: Yang, Libin <libin.yang@intel.com> >> Cc: Lin, Mengdong <mengdong.lin@intel.com> >> Cc: Li, Jocelyn <jocelyn.li@intel.com> >> Cc: Kaskinen, Tanu <tanu.kaskinen@intel.com> >> Cc: Zanoni, Paulo R <paulo.r.zanoni@intel.com> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >> --- >> drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c >> index 317b9b43d1c1..36ecbe275dd9 100644 >> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c >> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c >> @@ -1919,9 +1919,10 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv) >> return; >> } >> >> - pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */ >> + pm_runtime_set_autosuspend_delay(device, 100); >> pm_runtime_mark_last_busy(device); >> pm_runtime_use_autosuspend(device); >> + pm_runtime_allow(device); >> >> pm_runtime_put_autosuspend(device); >> } >> -- >> 2.1.0 >> > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
2015-06-16 9:23 GMT-03:00 Paulo Zanoni <przanoni@gmail.com>: > 2015-06-16 5:34 GMT-03:00 Daniel Vetter <daniel.vetter@ffwll.ch>: >> Can I get an ack on this please? The audio folks already move ahead >> >> http://permalink.gmane.org/gmane.linux.alsa.devel/139831 >> >> Would be embarassing if we are late to the party we organized ourselves ... > > The situation is already embarassing for us :) > > pzanoni@nhoque:~/nfs/intel-gpu-tools/tests$ sudo ./pm_rpm --quick 2>&1 > | tee pm_rpm.txt > (lots of stuff) > pzanoni@nhoque:~/nfs/intel-gpu-tools/tests$ ./sumario.sh pm_rpm.txt For details, please see: http://people.freedesktop.org/~pzanoni/pm_rpm.txt http://people.freedesktop.org/~pzanoni/dmesg.txt > Subtest drm-resources-equal: FAIL (5.897s) > Subtest dpms-lpsp: FAIL (10.045s) > Subtest dpms-non-lpsp: FAIL (10.027s) > Subtest cursor-dpms: FAIL (12.693s) > Subtest legacy-planes-dpms: FAIL (12.636s) > Subtest universal-planes-dpms: FAIL (12.719s) > Subtest dpms-mode-unset-lpsp: FAIL (12.588s) > Subtest dpms-mode-unset-non-lpsp: FAIL (10.234s) > Subtest fences-dpms: FAIL (11.726s) > > Total: 41 > SUCCESS: 32 > FAIL: 9 > SKIP: 0 > pzanoni@nhoque:~/nfs/intel-gpu-tools/tests$ edmesg > [ 166.007957] WARNING: CPU: 1 PID: 1648 at > drivers/gpu/drm/i915/intel_uncore.c:620 > hsw_unclaimed_reg_debug+0x75/0xa0 [i915]() > [ 174.944917] WARNING: CPU: 1 PID: 1648 at > drivers/gpu/drm/i915/intel_uncore.c:620 > hsw_unclaimed_reg_debug+0x75/0xa0 [i915]() > [ 327.095531] WARNING: CPU: 0 PID: 1648 at > drivers/gpu/drm/i915/i915_gem.c:5316 > i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() > [ 327.096026] WARNING: CPU: 0 PID: 1648 at > drivers/gpu/drm/i915/i915_gem.c:5316 > i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() > [ 327.096597] WARNING: CPU: 0 PID: 1648 at > drivers/gpu/drm/i915/i915_gem.c:5316 > i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() > [ 327.096866] WARNING: CPU: 0 PID: 1648 at > drivers/gpu/drm/i915/i915_gem.c:5316 > i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() > [ 327.097089] WARNING: CPU: 0 PID: 1648 at > drivers/gpu/drm/i915/i915_gem.c:5316 > i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() > [ 327.097323] WARNING: CPU: 0 PID: 1648 at > drivers/gpu/drm/i915/i915_gem.c:5316 > i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() > [ 327.097534] WARNING: CPU: 0 PID: 1648 at > drivers/gpu/drm/i915/i915_gem.c:5316 > i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() > [ 327.097767] WARNING: CPU: 0 PID: 1648 at > drivers/gpu/drm/i915/i915_gem.c:5316 > i915_gem_obj_ggtt_offset_view+0xad/0x100 [i915]() > pzanoni@nhoque:~/nfs/intel-gpu-tools/tests$ lspci -nn | grep VGA > 00:02.0 VGA compatible controller [0300]: Intel Corporation > Broadwell-U Integrated Graphics [8086:1616] (rev 09) > > Please notice that, if I remember correctly, just a few weeks ago we > were passing everything, although we were having some dmesg WARNs. Now > we're not even passing the tests anymore :( > > And I only see 2 pm_rpm bugs on bugzilla: > https://bugs.freedesktop.org/show_bug.cgi?id=90863 > https://bugs.freedesktop.org/show_bug.cgi?id=90546 > > Why isn't PRTS/QA cathing stuff!? > > I'm sorry, but for now, I just can't give the ACK. > >> -Daniel >> >> >> On Fri, May 8, 2015 at 8:51 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: >>> Like with every other feature that's not enabled by default we break >>> runtime pm support way too often by accident because the overall test >>> coverage isn't great. And it's been almost 2 years since we enabled >>> the power well code by default >>> >>> commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41 >>> Author: Paulo Zanoni <paulo.r.zanoni@intel.com> >>> Date: Wed Jul 3 17:12:13 2013 -0300 >>> >>> drm/i915: switch disable_power_well default value to 1 >>> >>> It's really more than overdue for runtime pm itself to follow! >>> >>> Note that in practice this wont do a hole lot yet, since we're still >>> gated on snd-hda-intel doing proper runtime pm. But I've discussed >>> this with Liam and we agreed that this needs to be done. And the audio >>> team is working to hold up their end of this bargain. >>> >>> And the justification for updating the autosuspend delay to 100ms: >>> Quick measurment shows that we can do a full rpm cycle in about 5ms, >>> which means the delay should still be really conservative from a power >>> conservation pov. The only workload that would suffer from ping-pong >>> is also only gpu/compute with all screens off. 100ms should cover any >>> kind of latency with submitting follow-up batches. >>> >>> Cc: Takashi Iwai <tiwai@suse.de> >>> Cc: Liam Girdwood <liam.r.girdwood@intel.com> >>> Cc: Yang, Libin <libin.yang@intel.com> >>> Cc: Lin, Mengdong <mengdong.lin@intel.com> >>> Cc: Li, Jocelyn <jocelyn.li@intel.com> >>> Cc: Kaskinen, Tanu <tanu.kaskinen@intel.com> >>> Cc: Zanoni, Paulo R <paulo.r.zanoni@intel.com> >>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c >>> index 317b9b43d1c1..36ecbe275dd9 100644 >>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c >>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c >>> @@ -1919,9 +1919,10 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv) >>> return; >>> } >>> >>> - pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */ >>> + pm_runtime_set_autosuspend_delay(device, 100); >>> pm_runtime_mark_last_busy(device); >>> pm_runtime_use_autosuspend(device); >>> + pm_runtime_allow(device); >>> >>> pm_runtime_put_autosuspend(device); >>> } >>> -- >>> 2.1.0 >>> >> >> >> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni
On 06/16/2015 05:26 AM, Paulo Zanoni wrote: > 2015-06-16 9:23 GMT-03:00 Paulo Zanoni <przanoni@gmail.com>: >> 2015-06-16 5:34 GMT-03:00 Daniel Vetter <daniel.vetter@ffwll.ch>: >>> Can I get an ack on this please? The audio folks already move ahead >>> >>> http://permalink.gmane.org/gmane.linux.alsa.devel/139831 >>> >>> Would be embarassing if we are late to the party we organized ourselves ... >> >> The situation is already embarassing for us :) >> >> pzanoni@nhoque:~/nfs/intel-gpu-tools/tests$ sudo ./pm_rpm --quick 2>&1 >> | tee pm_rpm.txt >> (lots of stuff) >> pzanoni@nhoque:~/nfs/intel-gpu-tools/tests$ ./sumario.sh pm_rpm.txt > > For details, please see: > http://people.freedesktop.org/~pzanoni/pm_rpm.txt > http://people.freedesktop.org/~pzanoni/dmesg.txt > >> Subtest drm-resources-equal: FAIL (5.897s) >> Subtest dpms-lpsp: FAIL (10.045s) >> Subtest dpms-non-lpsp: FAIL (10.027s) >> Subtest cursor-dpms: FAIL (12.693s) >> Subtest legacy-planes-dpms: FAIL (12.636s) >> Subtest universal-planes-dpms: FAIL (12.719s) >> Subtest dpms-mode-unset-lpsp: FAIL (12.588s) >> Subtest dpms-mode-unset-non-lpsp: FAIL (10.234s) >> Subtest fences-dpms: FAIL (11.726s) Ugg... I'd like to make sure we have bugs on these and at least some kind of plan to fix them before enabling by default, otherwise we're just asking for another 'disable by default' patch just before the next release... Jesse
Hi Liam, > -----Original Message----- > From: Girdwood, Liam R > Sent: Tuesday, June 16, 2015 6:42 PM > To: Daniel Vetter; Lin, Mengdong; Yang, Libin > Cc: Intel Graphics Development; Takashi Iwai; Yang, Libin; Lin, > Mengdong; Li, Jocelyn; Kaskinen, Tanu; Zanoni, Paulo R; Vetter, Daniel > Subject: Re: [PATCH] drm/i915: Enable runtime pm > > On Tue, 2015-06-16 at 10:34 +0200, Daniel Vetter wrote: > > Can I get an ack on this please? The audio folks already move ahead > > > > http://permalink.gmane.org/gmane.linux.alsa.devel/139831 > > > > Would be embarassing if we are late to the party we organized > ourselves ... > > -Daniel > > > > > > Mengdong, Libin - I assume someone has tested this ? could someone > Ack. Our patches are ready for this feature. Suppose Keqiao has test this feature. > > Thanks > > Liam > > > On Fri, May 8, 2015 at 8:51 PM, Daniel Vetter <daniel.vetter@ffwll.ch> > wrote: > > > Like with every other feature that's not enabled by default we > break > > > runtime pm support way too often by accident because the overall > test > > > coverage isn't great. And it's been almost 2 years since we enabled > > > the power well code by default > > > > > > commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41 > > > Author: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > Date: Wed Jul 3 17:12:13 2013 -0300 > > > > > > drm/i915: switch disable_power_well default value to 1 > > > > > > It's really more than overdue for runtime pm itself to follow! > > > > > > Note that in practice this wont do a hole lot yet, since we're still > > > gated on snd-hda-intel doing proper runtime pm. But I've > discussed > > > this with Liam and we agreed that this needs to be done. And the > audio > > > team is working to hold up their end of this bargain. > > > > > > And the justification for updating the autosuspend delay to 100ms: > > > Quick measurment shows that we can do a full rpm cycle in about > 5ms, > > > which means the delay should still be really conservative from a > power > > > conservation pov. The only workload that would suffer from ping- > pong > > > is also only gpu/compute with all screens off. 100ms should cover > any > > > kind of latency with submitting follow-up batches. > > > > > > Cc: Takashi Iwai <tiwai@suse.de> > > > Cc: Liam Girdwood <liam.r.girdwood@intel.com> > > > Cc: Yang, Libin <libin.yang@intel.com> > > > Cc: Lin, Mengdong <mengdong.lin@intel.com> > > > Cc: Li, Jocelyn <jocelyn.li@intel.com> > > > Cc: Kaskinen, Tanu <tanu.kaskinen@intel.com> > > > Cc: Zanoni, Paulo R <paulo.r.zanoni@intel.com> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > index 317b9b43d1c1..36ecbe275dd9 100644 > > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > > @@ -1919,9 +1919,10 @@ void intel_runtime_pm_enable(struct > drm_i915_private *dev_priv) > > > return; > > > } > > > > > > - pm_runtime_set_autosuspend_delay(device, 10000); /* 10s > */ > > > + pm_runtime_set_autosuspend_delay(device, 100); > > > pm_runtime_mark_last_busy(device); > > > pm_runtime_use_autosuspend(device); > > > + pm_runtime_allow(device); > > > > > > pm_runtime_put_autosuspend(device); > > > } > > > -- > > > 2.1.0 > > > > > > > > > >
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 317b9b43d1c1..36ecbe275dd9 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -1919,9 +1919,10 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv) return; } - pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */ + pm_runtime_set_autosuspend_delay(device, 100); pm_runtime_mark_last_busy(device); pm_runtime_use_autosuspend(device); + pm_runtime_allow(device); pm_runtime_put_autosuspend(device); }
Like with every other feature that's not enabled by default we break runtime pm support way too often by accident because the overall test coverage isn't great. And it's been almost 2 years since we enabled the power well code by default commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41 Author: Paulo Zanoni <paulo.r.zanoni@intel.com> Date: Wed Jul 3 17:12:13 2013 -0300 drm/i915: switch disable_power_well default value to 1 It's really more than overdue for runtime pm itself to follow! Note that in practice this wont do a hole lot yet, since we're still gated on snd-hda-intel doing proper runtime pm. But I've discussed this with Liam and we agreed that this needs to be done. And the audio team is working to hold up their end of this bargain. And the justification for updating the autosuspend delay to 100ms: Quick measurment shows that we can do a full rpm cycle in about 5ms, which means the delay should still be really conservative from a power conservation pov. The only workload that would suffer from ping-pong is also only gpu/compute with all screens off. 100ms should cover any kind of latency with submitting follow-up batches. Cc: Takashi Iwai <tiwai@suse.de> Cc: Liam Girdwood <liam.r.girdwood@intel.com> Cc: Yang, Libin <libin.yang@intel.com> Cc: Lin, Mengdong <mengdong.lin@intel.com> Cc: Li, Jocelyn <jocelyn.li@intel.com> Cc: Kaskinen, Tanu <tanu.kaskinen@intel.com> Cc: Zanoni, Paulo R <paulo.r.zanoni@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)