On Mon, Dec 8, 2014 at 1:35 AM, Daniel Vetter <daniel@xxxxxxxx> 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@xxxxxxxxx> >> Reviewed-by: Durgadoss R <durgadoss.r@xxxxxxxxx> >> --- >> 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@xxxxxxxxxxxxxxxxxxxxx >> 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx