Re: [PATCH] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Nov 21, 2014 at 10:00:53PM +0000, Vivi, Rodrigo wrote:
> Yeah, I'm glad you skipped this one. It was something old I was just carrying for too long... 
> 
> Just tested on -nightly and everything is working fine. Even after suspend-resume! :)
> 
> Still said that I cannot use sink_crc when psr is enabled...

Don't we still need a patch to obey the minimal psr/non-psr times as
specified in the vbt like on hsw/bdw? Or is this a case where the vbt
doesn't quite mean the same thing on vlv/chv than on big core platforms?

Please chase the vbt/vbios/bangalore folks a bit to make sure we don't
have a gap here.

Thanks, Daniel
> 
> Thank you very much.
> 
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Friday, November 21, 2014 10:46 AM
> To: Vivi, Rodrigo
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.
> 
> On Wed, Nov 19, 2014 at 07:38:51AM -0800, 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: s/psr/PSR on comment (by Durgadoss)
> > 
> > Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index dd0e6e0..57bf6d4 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -597,6 +597,11 @@ 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 and let the user experience smooth enough.
> > +	 */
> > +	int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000);
> 
> I'd like to know how we arrived at this number? And the justification imo doesn't make a lot of sense, whether it's hw or sw entering psr state doesn't take a lot of cpu cycles. Two things:
> 
> - Should we compute this delay according to the minimal psr exit frames we
>   already get from vbt and use for hsw/bdw?
> - Do we need to adjust igt testcases to wait longer with this 5s delay?
> 
> I'll punt on this patch for now until this is resolved.
> -Daniel
> 
> >  
> >  	mutex_lock(&dev_priv->psr.lock);
> >  	if (!dev_priv->psr.enabled) {
> > @@ -631,7 +636,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

-- 
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