[03/11] drm/i915/psr: Share code between disable and exit

Message ID	20180330222336.5262-3-jose.souza@intel.com (mailing list archive)
State	New, archived
Headers	show Return-Path: <intel-gfx-bounces@lists.freedesktop.org> From: =?UTF-8?q?Jos=C3=A9=20Roberto=20de=20Souza?= <jose.souza@intel.com> To: intel-gfx@lists.freedesktop.org Date: Fri, 30 Mar 2018 15:23:28 -0700 Message-Id: <20180330222336.5262-3-jose.souza@intel.com> In-Reply-To: <20180330222336.5262-1-jose.souza@intel.com> References: <20180330222336.5262-1-jose.souza@intel.com> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH 03/11] drm/i915/psr: Share code between disable and exit Precedence: list Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>, Rodrigo Vivi <rodrigo.vivi@intel.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Message ID

20180330222336.5262-3-jose.souza@intel.com (mailing list archive)

State

New, archived

Headers

From: =?UTF-8?q?Jos=C3=A9=20Roberto=20de=20Souza?= <jose.souza@intel.com>
To: intel-gfx@lists.freedesktop.org
Date: Fri, 30 Mar 2018 15:23:28 -0700
Message-Id: <20180330222336.5262-3-jose.souza@intel.com>
In-Reply-To: <20180330222336.5262-1-jose.souza@intel.com>
References: <20180330222336.5262-1-jose.souza@intel.com>
MIME-Version: 1.0
Subject: [Intel-gfx] [PATCH 03/11] drm/i915/psr: Share code between disable
	and exit
Precedence: list
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
Errors-To: intel-gfx-bounces@lists.freedesktop.org
Sender: "Intel-gfx" <intel-gfx-bounces@lists.freedesktop.org>

Commit Message

Souza, Jose March 30, 2018, 10:23 p.m. UTC

The disable and exit sequence are very similar with a lot common
code between both, so here sharing the code.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  2 +-
 drivers/gpu/drm/i915/intel_psr.c | 84 ++++++++++++++++++----------------------
 2 files changed, 38 insertions(+), 48 deletions(-)

Comments

Rodrigo Vivi April 2, 2018, 6:11 p.m. UTC | #1

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@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  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
>

Souza, Jose April 2, 2018, 9:15 p.m. UTC | #2

On Mon, 2018-04-02 at 11:11 -0700, Rodrigo Vivi wrote:
> 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...


When the disabling argument is false it don't have the disable
wait/timeouts, I kept the same behavior to both just sharing the common
part.

> 

> > 

> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

> > ---

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

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

> >

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

[03/11] drm/i915/psr: Share code between disable and exit

Commit Message

Comments

Patch