On Fri, Mar 30, 2018 at 03:23:28PM -0700, José Roberto de Souza wrote:
> The disable and exit sequence are very similar with a lot common
> code between both, so here sharing the code.
I don't believe that we should do this.
Disable as is has some extra wait/timeouts that could slow things down.
Or at least not do this while we don't have a proper robuts test in place
and to test also performance impacts...
>
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_psr.c | 84 ++++++++++++++++++----------------------
> 2 files changed, 38 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a8d300280a2c..cb72ee27422f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -617,7 +617,7 @@ struct i915_psr {
> void (*enable_sink)(struct intel_dp *);
> void (*activate)(struct intel_dp *);
> void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
> - void (*exit)(struct intel_dp *intel_dp);
> + void (*exit)(struct intel_dp *intel_dp, bool disabling);
> };
>
> enum intel_pch {
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index bcaac9e69f8c..d3451afeb8bb 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -672,23 +672,9 @@ static void vlv_psr_disable(struct intel_dp *intel_dp,
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
> - uint32_t val;
>
> if (dev_priv->psr.active) {
> - /* Put VLV PSR back to PSR_state 0 (disabled). */
> - if (intel_wait_for_register(dev_priv,
> - VLV_PSRSTAT(crtc->pipe),
> - VLV_EDP_PSR_IN_TRANS,
> - 0,
> - 1))
> - WARN(1, "PSR transition took longer than expected\n");
> -
> - val = I915_READ(VLV_PSRCTL(crtc->pipe));
> - val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
> - val &= ~VLV_EDP_PSR_ENABLE;
> - val &= ~VLV_EDP_PSR_MODE_MASK;
> - I915_WRITE(VLV_PSRCTL(crtc->pipe), val);
> -
> + dev_priv->psr.exit(intel_dp, true);
> dev_priv->psr.active = false;
> } else {
> WARN_ON(vlv_is_psr_active_on_pipe(dev, crtc->pipe));
> @@ -703,31 +689,7 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
> struct drm_i915_private *dev_priv = to_i915(dev);
>
> if (dev_priv->psr.active) {
> - i915_reg_t psr_status;
> - u32 psr_status_mask;
> -
> - if (dev_priv->psr.psr2_enabled) {
> - psr_status = EDP_PSR2_STATUS;
> - psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> -
> - I915_WRITE(EDP_PSR2_CTL,
> - I915_READ(EDP_PSR2_CTL) &
> - ~(EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE));
> -
> - } else {
> - psr_status = EDP_PSR_STATUS;
> - psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> -
> - I915_WRITE(EDP_PSR_CTL,
> - I915_READ(EDP_PSR_CTL) & ~EDP_PSR_ENABLE);
> - }
> -
> - /* Wait till PSR is idle */
> - if (intel_wait_for_register(dev_priv,
> - psr_status, psr_status_mask, 0,
> - 2000))
> - DRM_ERROR("Timed out waiting for PSR Idle State\n");
> -
> + dev_priv->psr.exit(intel_dp, true);
> dev_priv->psr.active = false;
> } else {
> if (dev_priv->psr.psr2_enabled)
> @@ -838,25 +800,41 @@ static void intel_psr_work(struct work_struct *work)
> mutex_unlock(&dev_priv->psr.lock);
> }
>
> -static void hsw_psr_exit(struct intel_dp *intel_dp)
> +static void hsw_psr_exit(struct intel_dp *intel_dp, bool disabling)
> {
> struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> + i915_reg_t psr_status;
> + u32 psr_status_mask;
> u32 val;
>
> if (dev_priv->psr.psr2_enabled) {
> + psr_status = EDP_PSR2_STATUS;
> + psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
> +
> val = I915_READ(EDP_PSR2_CTL);
> - WARN_ON(!(val & EDP_PSR2_ENABLE));
> - I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
> + WARN_ON(!disabling && !(val & EDP_PSR2_ENABLE));
> + val &= ~EDP_PSR2_ENABLE;
> + if (disabling)
> + val &= ~EDP_SU_TRACK_ENABLE;
> + I915_WRITE(EDP_PSR2_CTL, val);
> } else {
> + psr_status = EDP_PSR_STATUS;
> + psr_status_mask = EDP_PSR_STATUS_STATE_MASK;
> +
> val = I915_READ(EDP_PSR_CTL);
> - WARN_ON(!(val & EDP_PSR_ENABLE));
> + WARN_ON(!disabling && !(val & EDP_PSR_ENABLE));
> I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
> }
> +
> + /* When disabling wait till PSR is idle */
> + if (disabling && intel_wait_for_register(dev_priv, psr_status,
> + psr_status_mask, 0, 2000))
> + DRM_ERROR("Timed out waiting for PSR Idle State\n");
> }
>
> -static void vlv_psr_exit(struct intel_dp *intel_dp)
> +static void vlv_psr_exit(struct intel_dp *intel_dp, bool disabling)
> {
> struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = dig_port->base.base.dev;
> @@ -865,10 +843,14 @@ static void vlv_psr_exit(struct intel_dp *intel_dp)
> enum pipe pipe = to_intel_crtc(crtc)->pipe;
> u32 val;
>
> + if (disabling && intel_wait_for_register(dev_priv, VLV_PSRSTAT(pipe),
> + VLV_EDP_PSR_IN_TRANS, 0, 1))
> + DRM_WARN("PSR transition took longer than expected\n");
> +
> val = I915_READ(VLV_PSRCTL(pipe));
>
> /*
> - * Here we do the transition drirectly from
> + * Here we do the transition directly from
> * PSR_state 3 (active - no Remote Frame Buffer (RFB) update) to
> * PSR_state 5 (exit).
> * PSR State 4 (active with single frame update) can be skipped.
> @@ -877,8 +859,16 @@ static void vlv_psr_exit(struct intel_dp *intel_dp)
> * Now we are at Same state after vlv_psr_enable_source.
> */
> val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
> + if (disabling) {
> + /* Put VLV PSR back to PSR_state 0 (disabled). */
> + val &= ~VLV_EDP_PSR_ENABLE;
> + val &= ~VLV_EDP_PSR_MODE_MASK;
> + }
> I915_WRITE(VLV_PSRCTL(pipe), val);
>
> + if (disabling)
> + return;
> +
> /*
> * Send AUX wake up - Spec says after transitioning to PSR
> * active we have to send AUX wake up by writing 01h in DPCD
> @@ -898,7 +888,7 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
> if (!dev_priv->psr.active)
> return;
>
> - dev_priv->psr.exit(intel_dp);
> + dev_priv->psr.exit(intel_dp, false);
> dev_priv->psr.active = false;
> }
>
> --
> 2.16.3
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
