| Message ID | 1440118544-26282-2-git-send-email-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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()? 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. 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. > > 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)) & > EDP_PSR_ENABLE);
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)) &
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)) & EDP_PSR_ENABLE);
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. Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/intel_psr.c | 10 ++++++++++ 1 file changed, 10 insertions(+)