* [PATCH 1/2] drm/i915: VLV/CHV PSR needs to exit PSR on every flush. @ 2014-12-06 1:40 Rodrigo Vivi 2014-12-06 1:40 ` [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR Rodrigo Vivi 0 siblings, 1 reply; 6+ messages in thread From: Rodrigo Vivi @ 2014-12-06 1:40 UTC (permalink / raw) To: intel-gfx; +Cc: Rodrigo Vivi ON these platforms we don't have hardware tracking working for any case. So we need to fake this on software by forcing psr to exit on every flush. Manual tests indicated this was needed. Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/intel_psr.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index dd0e6e0..afb8b8c 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -620,13 +620,11 @@ void intel_psr_flush(struct drm_device *dev, /* * On Valleyview and Cherryview we don't use hardware tracking so - * sprite plane updates or cursor moves don't result in a PSR + * any plane updates or cursor moves don't result in a PSR * invalidating. Which means we need to manually fake this in * software for all flushes, not just when we've seen a preceding * invalidation through frontbuffer rendering. */ - if (!HAS_DDI(dev) && - ((frontbuffer_bits & INTEL_FRONTBUFFER_SPRITE(pipe)) || - (frontbuffer_bits & INTEL_FRONTBUFFER_CURSOR(pipe)))) + if (!HAS_DDI(dev)) intel_psr_exit(dev); if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits) -- 1.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR. 2014-12-06 1:40 [PATCH 1/2] drm/i915: VLV/CHV PSR needs to exit PSR on every flush Rodrigo Vivi @ 2014-12-06 1:40 ` Rodrigo Vivi 2014-12-06 5:08 ` shuang.he 2014-12-08 9:35 ` Daniel Vetter 0 siblings, 2 replies; 6+ messages in thread From: Rodrigo Vivi @ 2014-12-06 1:40 UTC (permalink / raw) To: intel-gfx; +Cc: Rodrigo Vivi Since active function on VLV immediately activate PSR let's give more time for idleness. v2: Rebase over intel_psr.c and fix typo. v3: Revival: Manual tests indicated that this is needed. With a short delay there is a huge risk of getting blank screens when planes are being enabled. Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Reviewed-by: Durgadoss R <durgadoss.r@intel.com> --- drivers/gpu/drm/i915/intel_psr.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index afb8b8c..a6028b6 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -597,6 +597,12 @@ void intel_psr_flush(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *crtc; enum pipe pipe; + /* On HSW/BDW Hardware controls idle_frames to go to PSR entry state + * However on VLV we go to PSR active state with psr work. So let's + * wait more time. The main reason is to give more time when primary + * plane is getting enabled avoiding blank screens. + */ + int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000); mutex_lock(&dev_priv->psr.lock); if (!dev_priv->psr.enabled) { @@ -629,7 +635,7 @@ void intel_psr_flush(struct drm_device *dev, if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits) schedule_delayed_work(&dev_priv->psr.work, - msecs_to_jiffies(100)); + msecs_to_jiffies(delay)); mutex_unlock(&dev_priv->psr.lock); } -- 1.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR. 2014-12-06 1:40 ` [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR Rodrigo Vivi @ 2014-12-06 5:08 ` shuang.he 2014-12-08 9:35 ` Daniel Vetter 1 sibling, 0 replies; 6+ messages in thread From: shuang.he @ 2014-12-06 5:08 UTC (permalink / raw) To: shuang.he, intel-gfx, rodrigo.vivi Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) -------------------------------------Summary------------------------------------- Platform Delta drm-intel-nightly Series Applied PNV 364/364 364/364 ILK -1 366/366 365/366 SNB -1 450/450 449/450 IVB +17 481/498 498/498 BYT 289/289 289/289 HSW -1 564/564 563/564 BDW 417/417 417/417 -------------------------------------Detailed------------------------------------- Platform Test drm-intel-nightly Series Applied *ILK igt_gem_fenced_exec_thrash_no-spare-fences-busy-interruptible PASS(1, M37) DMESG_WARN(1, M37) SNB igt_kms_force_connector NRUN(5, M35M22)PASS(1, M35) NRUN(1, M35) IVB igt_kms_3d DMESG_WARN(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-128x128-onscreen NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-128x128-random NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-128x128-sliding NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-256x256-offscreen NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-256x256-onscreen NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-256x256-sliding NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-offscreen NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-onscreen NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-random NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-64x64-sliding NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_cursor_crc_cursor-size-change NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_fence_pin_leak NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_rotation_crc_primary-rotation NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) IVB igt_kms_rotation_crc_sprite-rotation NSPT(1, M34)PASS(15, M4M34M21) PASS(1, M34) HSW igt_kms_force_connector NRUN(5, M40M19M20)PASS(1, M40) NRUN(1, M19) Note: You need to pay more attention to line start with '*' _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR. 2014-12-06 1:40 ` [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR Rodrigo Vivi 2014-12-06 5:08 ` shuang.he @ 2014-12-08 9:35 ` Daniel Vetter 2014-12-13 3:16 ` Rodrigo Vivi 1 sibling, 1 reply; 6+ messages in thread From: Daniel Vetter @ 2014-12-08 9:35 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: intel-gfx On Fri, Dec 05, 2014 at 08:40:42PM -0500, Rodrigo Vivi wrote: > Since active function on VLV immediately activate PSR let's give more > time for idleness. > > v2: Rebase over intel_psr.c and fix typo. > v3: Revival: Manual tests indicated that this is needed. With a short delay there is a huge > risk of getting blank screens when planes are being enabled. > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > Reviewed-by: Durgadoss R <durgadoss.r@intel.com> > --- > drivers/gpu/drm/i915/intel_psr.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > index afb8b8c..a6028b6 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -597,6 +597,12 @@ void intel_psr_flush(struct drm_device *dev, > struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_crtc *crtc; > enum pipe pipe; > + /* On HSW/BDW Hardware controls idle_frames to go to PSR entry state > + * However on VLV we go to PSR active state with psr work. So let's > + * wait more time. The main reason is to give more time when primary > + * plane is getting enabled avoiding blank screens. > + */ > + int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000); Same question as before: Shouldn't we look at vbt perhaps like on ddi platforms to compute a reasonable delay? 5s is kinda not reasonable I think ;-) -Daniel > > mutex_lock(&dev_priv->psr.lock); > if (!dev_priv->psr.enabled) { > @@ -629,7 +635,7 @@ void intel_psr_flush(struct drm_device *dev, > > if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits) > schedule_delayed_work(&dev_priv->psr.work, > - msecs_to_jiffies(100)); > + msecs_to_jiffies(delay)); > mutex_unlock(&dev_priv->psr.lock); > } > > -- > 1.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR. 2014-12-08 9:35 ` Daniel Vetter @ 2014-12-13 3:16 ` Rodrigo Vivi 2014-12-15 8:23 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Rodrigo Vivi @ 2014-12-13 3:16 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx, Rodrigo Vivi On Mon, Dec 8, 2014 at 1:35 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, Dec 05, 2014 at 08:40:42PM -0500, Rodrigo Vivi wrote: >> Since active function on VLV immediately activate PSR let's give more >> time for idleness. >> >> v2: Rebase over intel_psr.c and fix typo. >> v3: Revival: Manual tests indicated that this is needed. With a short delay there is a huge >> risk of getting blank screens when planes are being enabled. >> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> >> Reviewed-by: Durgadoss R <durgadoss.r@intel.com> >> --- >> drivers/gpu/drm/i915/intel_psr.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c >> index afb8b8c..a6028b6 100644 >> --- a/drivers/gpu/drm/i915/intel_psr.c >> +++ b/drivers/gpu/drm/i915/intel_psr.c >> @@ -597,6 +597,12 @@ void intel_psr_flush(struct drm_device *dev, >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct drm_crtc *crtc; >> enum pipe pipe; >> + /* On HSW/BDW Hardware controls idle_frames to go to PSR entry state >> + * However on VLV we go to PSR active state with psr work. So let's >> + * wait more time. The main reason is to give more time when primary >> + * plane is getting enabled avoiding blank screens. >> + */ >> + int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000); > > Same question as before: Shouldn't we look at vbt perhaps like on ddi > platforms to compute a reasonable delay? 5s is kinda not reasonable I > think ;-) I agree 5s is an eternity here... to be honest I was using 1 sec here but when sending the patch I reused the one I had sent before without noticing it was 5. Anyway, what kind of value at vbt do you suggest? If we use the tp1 wake for instance that is 50 here in multiple of 100 it will end up in 5 sec again. Another thing that I notice today is that I also got that sink crc issue on BDW. So it isn't only on VLV/CHV that we are unable to use sink crc for test. I still believe that is the link fully off that is making us to be unable to get panel crc when in psr. But there is hope. I tested different values here and if we use 400 ms I could use automated kms_psr_sink_crc here... with 300 and bellow it fails. The issue is that I'm not sure if it depends on different panel so I'll test this value on other platforms and different panels. Do you think 500ms reasonable to apply for all platforms in order to keep automated tests and everything working well? What do you think? Thanks, Rodrigo. > -Daniel > >> >> mutex_lock(&dev_priv->psr.lock); >> if (!dev_priv->psr.enabled) { >> @@ -629,7 +635,7 @@ void intel_psr_flush(struct drm_device *dev, >> >> if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits) >> schedule_delayed_work(&dev_priv->psr.work, >> - msecs_to_jiffies(100)); >> + msecs_to_jiffies(delay)); >> mutex_unlock(&dev_priv->psr.lock); >> } >> >> -- >> 1.9.3 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > 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 -- Rodrigo Vivi Blog: http://blog.vivi.eng.br _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR. 2014-12-13 3:16 ` Rodrigo Vivi @ 2014-12-15 8:23 ` Daniel Vetter 0 siblings, 0 replies; 6+ messages in thread From: Daniel Vetter @ 2014-12-15 8:23 UTC (permalink / raw) To: Rodrigo Vivi; +Cc: intel-gfx, Rodrigo Vivi On Fri, Dec 12, 2014 at 07:16:34PM -0800, Rodrigo Vivi wrote: > On Mon, Dec 8, 2014 at 1:35 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Fri, Dec 05, 2014 at 08:40:42PM -0500, Rodrigo Vivi wrote: > >> Since active function on VLV immediately activate PSR let's give more > >> time for idleness. > >> > >> v2: Rebase over intel_psr.c and fix typo. > >> v3: Revival: Manual tests indicated that this is needed. With a short delay there is a huge > >> risk of getting blank screens when planes are being enabled. > >> > >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > >> Reviewed-by: Durgadoss R <durgadoss.r@intel.com> > >> --- > >> drivers/gpu/drm/i915/intel_psr.c | 8 +++++++- > >> 1 file changed, 7 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > >> index afb8b8c..a6028b6 100644 > >> --- a/drivers/gpu/drm/i915/intel_psr.c > >> +++ b/drivers/gpu/drm/i915/intel_psr.c > >> @@ -597,6 +597,12 @@ void intel_psr_flush(struct drm_device *dev, > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> struct drm_crtc *crtc; > >> enum pipe pipe; > >> + /* On HSW/BDW Hardware controls idle_frames to go to PSR entry state > >> + * However on VLV we go to PSR active state with psr work. So let's > >> + * wait more time. The main reason is to give more time when primary > >> + * plane is getting enabled avoiding blank screens. > >> + */ > >> + int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000); > > > > Same question as before: Shouldn't we look at vbt perhaps like on ddi > > platforms to compute a reasonable delay? 5s is kinda not reasonable I > > think ;-) > > I agree 5s is an eternity here... to be honest I was using 1 sec here > but when sending the patch I reused the one I had sent before without > noticing it was 5. > Anyway, what kind of value at vbt do you suggest? If we use the tp1 > wake for instance that is 50 here in multiple of 100 it will end up in > 5 sec again. Yeah that's the one I was thinking off. So maybe 5s is indeed the right value, but we should compute it from vbt? > Another thing that I notice today is that I also got that sink crc > issue on BDW. So it isn't only on VLV/CHV that we are unable to use > sink crc for test. I still believe that is the link fully off that is > making us to be unable to get panel crc when in psr. Hm, maybe we need a special mode to disallow link shutdown for the psr test? Then we could run all the functional correctness tests in that mode (psr still works the same wrt frontbuffer tracking). Ofc that means we need a new set of tests to exercise full psr (including link shutdown) against e.g. suspend/resume and stuff to make sure that still works. But those are then only doing modesets, s/r and won't check crcs. So additional tests on top of what we have. > But there is hope. I tested different values here and if we use 400 ms > I could use automated kms_psr_sink_crc here... with 300 and bellow it > fails. > The issue is that I'm not sure if it depends on different panel so > I'll test this value on other platforms and different panels. > > Do you think 500ms reasonable to apply for all platforms in order to > keep automated tests and everything working well? > > What do you think? Imo if vbt provides a delay, we should follow that. Even when it's horrible, who knows what kind of broken panels we'll see ... -Daniel -- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-12-15 8:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-12-06 1:40 [PATCH 1/2] drm/i915: VLV/CHV PSR needs to exit PSR on every flush Rodrigo Vivi 2014-12-06 1:40 ` [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR Rodrigo Vivi 2014-12-06 5:08 ` shuang.he 2014-12-08 9:35 ` Daniel Vetter 2014-12-13 3:16 ` Rodrigo Vivi 2014-12-15 8:23 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox