| Message ID | 1463662236-18192-1-git-send-email-david.weinehall@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 19, 2016 at 01:23:53PM -0000, Patchwork wrote: > == Series Details == > > Series: drm/i915: only disable memory self-refresh on GMCH > URL : https://patchwork.freedesktop.org/series/7406/ > State : failure > > == Summary == > > Series 7406v1 drm/i915: only disable memory self-refresh on GMCH > http://patchwork.freedesktop.org/api/1.0/series/7406/revisions/1/mbox > > Test drv_hangman: > Subgroup error-state-basic: > fail -> PASS (ro-ilk1-i5-650) Sadly unlikely to be the result of my patch... > Test gem_exec_flush: > Subgroup basic-batch-kernel-default-cmd: > pass -> FAIL (ro-byt-n2820) This testcase fails on my N2820 even without my patch. Kind regards, David Weinehall
On Thu, May 19, 2016 at 03:50:36PM +0300, David Weinehall wrote: > The atomic version of intel_pre_plane_update did not check > for HAS_GMCH_DISPLAY before calling intel_set_memory_cxsr(). > While this doesn't cause any issues on its own (it will > return without doing anything if the hardware doesn't > have the required feature), the drm_wait_one_vblank() that > is needed if memory self-refresh is disabled introduces > an unnecessary delay in the suspend path. > > In cases where i915 is on the critical path it means that > we slow down suspend by 16.8ms on platforms that don't > need to disable memory self-refresh. > > Signed-off-by: David Weinehall <david.weinehall@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 94d28c795e22..542806056cd9 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4644,7 +4644,7 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state) > intel_pre_disable_primary(&crtc->base); > } > > - if (pipe_config->disable_cxsr) { > + if (pipe_config->disable_cxsr && HAS_GMCH_DISPLAY(dev)) { Might be even better to track the state of cxsr and only wait when it really changed state. But this looks OK for now. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > crtc->wm.cxsr_allowed = false; > > /* > -- > 2.8.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, May 19, 2016 at 06:09:11PM +0300, David Weinehall wrote: > On Thu, May 19, 2016 at 01:23:53PM -0000, Patchwork wrote: > > == Series Details == > > > > Series: drm/i915: only disable memory self-refresh on GMCH > > URL : https://patchwork.freedesktop.org/series/7406/ > > State : failure > > > > == Summary == > > > > Series 7406v1 drm/i915: only disable memory self-refresh on GMCH > > http://patchwork.freedesktop.org/api/1.0/series/7406/revisions/1/mbox > > > > Test drv_hangman: > > Subgroup error-state-basic: > > fail -> PASS (ro-ilk1-i5-650) > > Sadly unlikely to be the result of my patch... > > > Test gem_exec_flush: > > Subgroup basic-batch-kernel-default-cmd: > > pass -> FAIL (ro-byt-n2820) > > This testcase fails on my N2820 even without my patch. I would appreaciate a bit more detail in these analyses. (gem_exec_flush:6039) CRITICAL: Test assertion failure function batch, file gem_exec_flush.c:456: (gem_exec_flush:6039) CRITICAL: Failed assertion: map[i] == cycles + i (gem_exec_flush:6039) CRITICAL: error: 0xabcdabcd != 0x2f5 Subtest basic-batch-kernel-default-cmd failed. https://bugs.freedesktop.org/show_bug.cgi?id=95372 Patch pushed to dinq. Thanks.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 94d28c795e22..542806056cd9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4644,7 +4644,7 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state) intel_pre_disable_primary(&crtc->base); } - if (pipe_config->disable_cxsr) { + if (pipe_config->disable_cxsr && HAS_GMCH_DISPLAY(dev)) { crtc->wm.cxsr_allowed = false; /*
The atomic version of intel_pre_plane_update did not check for HAS_GMCH_DISPLAY before calling intel_set_memory_cxsr(). While this doesn't cause any issues on its own (it will return without doing anything if the hardware doesn't have the required feature), the drm_wait_one_vblank() that is needed if memory self-refresh is disabled introduces an unnecessary delay in the suspend path. In cases where i915 is on the critical path it means that we slow down suspend by 16.8ms on platforms that don't need to disable memory self-refresh. Signed-off-by: David Weinehall <david.weinehall@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)