Re: [PATCH 2/7] drm/i915: Fix PSR disable sequence on core platforms.

From: "Vivi, Rodrigo" <rodrigo.vivi@intel.com>
To: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Zanoni, Paulo R" <paulo.r.zanoni@intel.com>
Subject: Re: [PATCH 2/7] drm/i915: Fix PSR disable sequence on core platforms.
Date: Mon, 24 Aug 2015 22:18:16 +0000	[thread overview]
Message-ID: <1440454764.3879.95.camel@intel.com> (raw)
In-Reply-To: <1440425675.2526.16.camel@intel.com>

On Mon, 2015-08-24 at 14:14 +0000, Zanoni, Paulo R wrote:
> Em Qui, 2015-08-20 às 17:55 -0700, Rodrigo Vivi escreveu:
> > According to spec the disable sequence is:
> > Driver will do the following on PSR Disable.
> > 1. Disable PSR in PSR control register, SRD_CTL[bit 31].
> > 2. Poll on PSR idle
> > 3. Wait for VBlank
> > 4. Disable VSC DIP.
> 
> Shouldn't this be done at intel_psr_exit() instead  of
> intel_psr_disable()?

don't think this is a good idea...

> 
> In case it's yes (which is my guess), then the wait_for_vblank() is
> probably going to slow down everything, so we may need to use some 
> sort
> of delayed work.

delayed work wouldn't help because locks would still block the exit...

> 
> In case it's no, then I don't think we need the wait_for_vblank() 
> since
> the encoder disable only happens after the pipe disable.

hm indeed... so I believe just ignore this patch for now since it works
without this...

> 
> > 
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 51f0514..92e2b467 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -459,6 +459,10 @@ static void hsw_psr_disable(struct intel_dp 
> > *intel_dp)
> >  	struct intel_digital_port *intel_dig_port = 
> > dp_to_dig_port(intel_dp);
> >  	struct drm_device *dev = intel_dig_port->base.base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> > +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> > +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > +	u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config
> > ->cpu_transcoder);
> >  
> >  	if (dev_priv->psr.active) {
> >  		I915_WRITE(EDP_PSR_CTL(dev),
> > @@ -469,6 +473,12 @@ static void hsw_psr_disable(struct intel_dp 
> > *intel_dp)
> >  			       EDP_PSR_STATUS_STATE_MASK) == 0, 
> > 2000, 10))
> >  			DRM_ERROR("Timed out waiting for PSR Idle 
> > State\n");
> >  
> > +		intel_wait_for_vblank(dev, pipe);
> > +
> > +		I915_WRITE(ctl_reg, I915_READ(ctl_reg)
> > +			   & ~VIDEO_DIP_ENABLE_VSC_HSW);
> > +		POSTING_READ(ctl_reg);
> > +
> >  		dev_priv->psr.active = false;
> >  	} else {
> >  		WARN_ON(I915_READ(EDP_PSR_CTL(dev)) & 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx