[PATCH 00/12] PSR clean-up, new vfuncs and more use of HW tracking.

[PATCH 00/12] PSR clean-up, new vfuncs and more use of HW tracking.

[Rodrigo Vivi]

Most of patches on this series is only a clean up with
no functional changes. The goal is to allow split VLV PSR
implementation better. In a way that would get more cleaner
when we use or not the hw tracking.

The ones with actual functional changes are:
0002-drm-i915-psr-Avoid-any-PSR-stuff-on-platforms-withou.patch
that protects PSR of running anything on the platforms without support.

and
0012-drm-i915-psr-Use-more-PSR-HW-tracking.patch
that allows us to use more HW tracking and consequently
a proper PSR2 with partial screen updates.

Rodrigo Vivi (12):
  drm/i915/psr: Remove vlv_is_active function.
  drm/i915/psr: Avoid any PSR stuff on platforms without support.
  drm/i915/psr: vfunc for disabling source.
  drm/i915/psr: hsw_psr_activate.
  drm/i915/psr: Add activate vfunc.
  drm/i915/psr: Unify VSC setup functions.
  drm/i915/psr: Re-create a hsw_psr_enable_source.
  drm/i915/psr: Re-org Activate after enable
  drm/i915/psr: Add setup VSC vfunc.
  drm/i915/psr: Add enable_sink vfunc.
  drm/i915/psr: Add enable_source vfunc.
  drm/i915/psr: Use more PSR HW tracking.

 drivers/gpu/drm/i915/i915_drv.h          |   7 +
 drivers/gpu/drm/i915/intel_drv.h         |   3 +-
 drivers/gpu/drm/i915/intel_frontbuffer.c |   2 +-
 drivers/gpu/drm/i915/intel_psr.c         | 267 ++++++++++++++++---------------
 4 files changed, 150 insertions(+), 129 deletions(-)

-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/12] drm/i915/psr: Remove vlv_is_active function.

[Rodrigo Vivi]

Let's start the clean-up and re-org of VLV PSR functions by
removing an useless one.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 559f1ab42bfc..1af4438a6095 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -61,17 +61,6 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
 	return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED;
 }
 
-static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
-{
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	uint32_t val;
-
-	val = I915_READ(VLV_PSRSTAT(pipe)) &
-	      VLV_EDP_PSR_CURR_STATE_MASK;
-	return (val == VLV_EDP_PSR_ACTIVE_NORFB_UP) ||
-	       (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);
-}
-
 static void intel_psr_write_vsc(struct intel_dp *intel_dp,
 				const struct edp_vsc_psr *vsc_psr)
 {
@@ -610,7 +599,10 @@ static void vlv_psr_disable(struct intel_dp *intel_dp)
 
 		dev_priv->psr.active = false;
 	} else {
-		WARN_ON(vlv_is_psr_active_on_pipe(dev, intel_crtc->pipe));
+		val = I915_READ(VLV_PSRSTAT(intel_crtc->pipe)) &
+			VLV_EDP_PSR_CURR_STATE_MASK;
+		WARN_ON(val == VLV_EDP_PSR_ACTIVE_NORFB_UP ||
+			val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);
 	}
 }
 
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/12] drm/i915/psr: Avoid any PSR stuff on platforms without support.

[Rodrigo Vivi]

We really don't want to setup vfuncs and lock mutexes on
platforms that has no support to PSR.

Also we know what platforms they are so let's do it quietly.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 1af4438a6095..71d65e166277 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -485,10 +485,8 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
 	u32 chicken;
 
-	if (!HAS_PSR(dev_priv)) {
-		DRM_DEBUG_KMS("PSR not supported on this platform\n");
+	if (!HAS_PSR(dev_priv))
 		return;
-	}
 
 	if (!is_edp_psr(intel_dp)) {
 		DRM_DEBUG_KMS("PSR not supported by this panel\n");
@@ -664,6 +662,9 @@ void intel_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);
 
+	if (!HAS_PSR(dev_priv))
+		return;
+
 	mutex_lock(&dev_priv->psr.lock);
 	if (!dev_priv->psr.enabled) {
 		mutex_unlock(&dev_priv->psr.lock);
@@ -815,6 +816,9 @@ void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
 	enum pipe pipe;
 	u32 val;
 
+	if (!HAS_PSR(dev_priv))
+		return;
+
 	/*
 	 * Single frame update is already supported on BDW+ but it requires
 	 * many W/A and it isn't really needed.
@@ -861,6 +865,9 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
 	struct drm_crtc *crtc;
 	enum pipe pipe;
 
+	if (!HAS_PSR(dev_priv))
+		return;
+
 	mutex_lock(&dev_priv->psr.lock);
 	if (!dev_priv->psr.enabled) {
 		mutex_unlock(&dev_priv->psr.lock);
@@ -898,6 +905,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 	struct drm_crtc *crtc;
 	enum pipe pipe;
 
+	if (!HAS_PSR(dev_priv))
+		return;
+
 	mutex_lock(&dev_priv->psr.lock);
 	if (!dev_priv->psr.enabled) {
 		mutex_unlock(&dev_priv->psr.lock);
@@ -930,6 +940,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
  */
 void intel_psr_init(struct drm_i915_private *dev_priv)
 {
+	if (!HAS_PSR(dev_priv))
+		return;
+
 	dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ?
 		HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE;
 
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 03/12] drm/i915/psr: vfunc for disabling source.

[Rodrigo Vivi]

VLV/CHV has a total different PSR implementation than the
other platforms, so let's start moving that to vfuncs.
Let's start with disable_src one.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  2 ++
 drivers/gpu/drm/i915/intel_psr.c | 12 +++++++-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 81cd21ecfa7d..b8f14fc41dd4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1145,6 +1145,8 @@ struct i915_psr {
 	bool y_cord_support;
 	bool colorimetry_support;
 	bool alpm;
+
+	void (*disable_source)(struct intel_dp *);
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 71d65e166277..535ea0b7ec20 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -671,11 +671,7 @@ void intel_psr_disable(struct intel_dp *intel_dp)
 		return;
 	}
 
-	/* Disable PSR on Source */
-	if (HAS_DDI(dev_priv))
-		hsw_psr_disable(intel_dp);
-	else
-		vlv_psr_disable(intel_dp);
+	dev_priv->psr.disable_source(intel_dp);
 
 	/* Disable PSR on Sink */
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
@@ -973,4 +969,10 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 
 	INIT_DELAYED_WORK(&dev_priv->psr.work, intel_psr_work);
 	mutex_init(&dev_priv->psr.lock);
+
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+		dev_priv->psr.disable_source = vlv_psr_disable;
+	} else {
+		dev_priv->psr.disable_source = hsw_psr_disable;
+	}
 }
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 04/12] drm/i915/psr: hsw_psr_activate.

[Rodrigo Vivi]

Oh HSW the real activate of PSR is decided by the source
after certain amount of configured idle frames.

However for the driver perspective where we track psr.active
variable this function here is the actual activate one. So
let's rename it before moving to vfunc with that.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 535ea0b7ec20..45c640989a46 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -254,7 +254,7 @@ static void vlv_psr_activate(struct intel_dp *intel_dp)
 		   VLV_EDP_PSR_ACTIVE_ENTRY);
 }
 
-static void intel_enable_source_psr1(struct intel_dp *intel_dp)
+static void hsw_activate_psr1(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = dig_port->base.base.dev;
@@ -307,7 +307,7 @@ static void intel_enable_source_psr1(struct intel_dp *intel_dp)
 	I915_WRITE(EDP_PSR_CTL, val);
 }
 
-static void intel_enable_source_psr2(struct intel_dp *intel_dp)
+static void hsw_activate_psr2(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = dig_port->base.base.dev;
@@ -343,17 +343,22 @@ static void intel_enable_source_psr2(struct intel_dp *intel_dp)
 	I915_WRITE(EDP_PSR2_CTL, val);
 }
 
-static void hsw_psr_enable_source(struct intel_dp *intel_dp)
+static void hsw_psr_activate(struct intel_dp *intel_dp)
 {
 	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);
 
+	/* On HSW+ after we enable PSR on source it will activate it
+	 * as soon as it match configure idle_frame count. So
+	 * we just actually enable it here on activation time.
+	 */
+
 	/* psr1 and psr2 are mutually exclusive.*/
 	if (dev_priv->psr.psr2_support)
-		intel_enable_source_psr2(intel_dp);
+		hsw_activate_psr2(intel_dp);
 	else
-		intel_enable_source_psr1(intel_dp);
+		hsw_activate_psr1(intel_dp);
 }
 
 static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
@@ -459,11 +464,7 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
 
 	/* Enable/Re-enable PSR on the host */
 	if (HAS_DDI(dev_priv))
-		/* On HSW+ after we enable PSR on source it will activate it
-		 * as soon as it match configure idle_frame count. So
-		 * we just actually enable it here on activation time.
-		 */
-		hsw_psr_enable_source(intel_dp);
+		hsw_psr_activate(intel_dp);
 	else
 		vlv_psr_activate(intel_dp);
 
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 05/12] drm/i915/psr: Add activate vfunc.

[Rodrigo Vivi]

Continue on VLV PSR split with vfunc, let's move activate
function there.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  | 1 +
 drivers/gpu/drm/i915/intel_psr.c | 9 +++------
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b8f14fc41dd4..ac33f0c89ec5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1147,6 +1147,7 @@ struct i915_psr {
 	bool alpm;
 
 	void (*disable_source)(struct intel_dp *);
+	void (*activate)(struct intel_dp *);
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 45c640989a46..8880d2cbe7fb 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -462,12 +462,7 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
 	WARN_ON(dev_priv->psr.active);
 	lockdep_assert_held(&dev_priv->psr.lock);
 
-	/* Enable/Re-enable PSR on the host */
-	if (HAS_DDI(dev_priv))
-		hsw_psr_activate(intel_dp);
-	else
-		vlv_psr_activate(intel_dp);
-
+	dev_priv->psr.activate(intel_dp);
 	dev_priv->psr.active = true;
 }
 
@@ -973,7 +968,9 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		dev_priv->psr.disable_source = vlv_psr_disable;
+		dev_priv->psr.activate = vlv_psr_activate;
 	} else {
 		dev_priv->psr.disable_source = hsw_psr_disable;
+		dev_priv->psr.activate = hsw_psr_activate;
 	}
 }
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 06/12] drm/i915/psr: Unify VSC setup functions.

[Rodrigo Vivi]

VSC package is decided per eDP spec for psr1 or psr2,
and not per platform, so let's unify it and kill "skl"
func.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 59 +++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 8880d2cbe7fb..2fbf803f0667 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -108,45 +108,42 @@ static void vlv_psr_setup_vsc(struct intel_dp *intel_dp)
 	I915_WRITE(VLV_VSCSDP(pipe), val);
 }
 
-static void skl_psr_setup_su_vsc(struct intel_dp *intel_dp)
+static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
 {
-	struct edp_vsc_psr psr_vsc;
 	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 = to_i915(dev);
 
-	/* Prepare VSC Header for SU as per EDP 1.4 spec, Table 6.11 */
-	memset(&psr_vsc, 0, sizeof(psr_vsc));
-	psr_vsc.sdp_header.HB0 = 0;
-	psr_vsc.sdp_header.HB1 = 0x7;
-	if (dev_priv->psr.colorimetry_support &&
-		dev_priv->psr.y_cord_support) {
-		psr_vsc.sdp_header.HB2 = 0x5;
-		psr_vsc.sdp_header.HB3 = 0x13;
-	} else if (dev_priv->psr.y_cord_support) {
-		psr_vsc.sdp_header.HB2 = 0x4;
-		psr_vsc.sdp_header.HB3 = 0xe;
+	struct edp_vsc_psr psr_vsc;
+
+	if (dev_priv->psr.psr2_support) {
+		/* Prepare VSC Header for SU as per EDP 1.4 spec, Table 6.11 */
+		memset(&psr_vsc, 0, sizeof(psr_vsc));
+		psr_vsc.sdp_header.HB0 = 0;
+		psr_vsc.sdp_header.HB1 = 0x7;
+		if (dev_priv->psr.colorimetry_support &&
+		    dev_priv->psr.y_cord_support) {
+			psr_vsc.sdp_header.HB2 = 0x5;
+			psr_vsc.sdp_header.HB3 = 0x13;
+		} else if (dev_priv->psr.y_cord_support) {
+			psr_vsc.sdp_header.HB2 = 0x4;
+			psr_vsc.sdp_header.HB3 = 0xe;
+		} else {
+			psr_vsc.sdp_header.HB2 = 0x3;
+			psr_vsc.sdp_header.HB3 = 0xc;
+		}
 	} else {
-		psr_vsc.sdp_header.HB2 = 0x3;
-		psr_vsc.sdp_header.HB3 = 0xc;
+		/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
+		memset(&psr_vsc, 0, sizeof(psr_vsc));
+		psr_vsc.sdp_header.HB0 = 0;
+		psr_vsc.sdp_header.HB1 = 0x7;
+		psr_vsc.sdp_header.HB2 = 0x2;
+		psr_vsc.sdp_header.HB3 = 0x8;
 	}
 
 	intel_psr_write_vsc(intel_dp, &psr_vsc);
 }
 
-static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
-{
-	struct edp_vsc_psr psr_vsc;
-
-	/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
-	memset(&psr_vsc, 0, sizeof(psr_vsc));
-	psr_vsc.sdp_header.HB0 = 0;
-	psr_vsc.sdp_header.HB1 = 0x7;
-	psr_vsc.sdp_header.HB2 = 0x2;
-	psr_vsc.sdp_header.HB3 = 0x8;
-	intel_psr_write_vsc(intel_dp, &psr_vsc);
-}
-
 static void vlv_psr_enable_sink(struct intel_dp *intel_dp)
 {
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
@@ -501,8 +498,10 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 
 	if (HAS_DDI(dev_priv)) {
+
+		hsw_psr_setup_vsc(intel_dp);
+
 		if (dev_priv->psr.psr2_support) {
-			skl_psr_setup_su_vsc(intel_dp);
 			chicken = PSR2_VSC_ENABLE_PROG_HEADER;
 			if (dev_priv->psr.y_cord_support)
 				chicken |= PSR2_ADD_VERTICAL_LINE_COUNT;
@@ -514,8 +513,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 				   EDP_PSR_DEBUG_MASK_MAX_SLEEP |
 				   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
 		} else {
-			/* set up vsc header for psr1 */
-			hsw_psr_setup_vsc(intel_dp);
 			/*
 			 * Per Spec: Avoid continuous PSR exit by masking MEMUP
 			 * and HPD. also mask LPSP to avoid dependency on other
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 07/12] drm/i915/psr: Re-create a hsw_psr_enable_source.

[Rodrigo Vivi]

This sequence is part of enable source anyways, but they
only need to be executed once and not on every activation,
So let's re-create hsw_enable_source.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 77 ++++++++++++++++++++++------------------
 1 file changed, 43 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2fbf803f0667..422033a6f9f6 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -219,6 +219,47 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 	I915_WRITE(aux_ctl_reg, aux_ctl);
 }
 
+static void hsw_psr_enable_source(struct intel_dp *intel_dp)
+{
+	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);
+	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
+	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
+
+	u32 chicken;
+
+	if (dev_priv->psr.psr2_support) {
+		chicken = PSR2_VSC_ENABLE_PROG_HEADER;
+		if (dev_priv->psr.y_cord_support)
+			chicken |= PSR2_ADD_VERTICAL_LINE_COUNT;
+		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
+		I915_WRITE(EDP_PSR_DEBUG_CTL,
+			   EDP_PSR_DEBUG_MASK_MEMUP |
+			   EDP_PSR_DEBUG_MASK_HPD |
+			   EDP_PSR_DEBUG_MASK_LPSP |
+			   EDP_PSR_DEBUG_MASK_MAX_SLEEP |
+			   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
+	} else {
+		/*
+		 * Per Spec: Avoid continuous PSR exit by masking MEMUP
+		 * and HPD. also mask LPSP to avoid dependency on other
+		 * drivers that might block runtime_pm besides
+		 * preventing  other hw tracking issues now we can rely
+		 * on frontbuffer tracking.
+		 */
+		I915_WRITE(EDP_PSR_DEBUG_CTL,
+			   EDP_PSR_DEBUG_MASK_MEMUP |
+			   EDP_PSR_DEBUG_MASK_HPD |
+			   EDP_PSR_DEBUG_MASK_LPSP);
+	}
+
+	/*
+	 * The rest of enable source sequence that should be here is actually
+	 * executed during activation time. So check hsw_psr_activate().
+	 */
+}
+
 static void vlv_psr_enable_source(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
@@ -474,9 +515,6 @@ void intel_psr_enable(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 = to_i915(dev);
-	struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
-	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
-	u32 chicken;
 
 	if (!HAS_PSR(dev_priv))
 		return;
@@ -501,34 +539,11 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 
 		hsw_psr_setup_vsc(intel_dp);
 
-		if (dev_priv->psr.psr2_support) {
-			chicken = PSR2_VSC_ENABLE_PROG_HEADER;
-			if (dev_priv->psr.y_cord_support)
-				chicken |= PSR2_ADD_VERTICAL_LINE_COUNT;
-			I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
-			I915_WRITE(EDP_PSR_DEBUG_CTL,
-				   EDP_PSR_DEBUG_MASK_MEMUP |
-				   EDP_PSR_DEBUG_MASK_HPD |
-				   EDP_PSR_DEBUG_MASK_LPSP |
-				   EDP_PSR_DEBUG_MASK_MAX_SLEEP |
-				   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
-		} else {
-			/*
-			 * Per Spec: Avoid continuous PSR exit by masking MEMUP
-			 * and HPD. also mask LPSP to avoid dependency on other
-			 * drivers that might block runtime_pm besides
-			 * preventing  other hw tracking issues now we can rely
-			 * on frontbuffer tracking.
-			 */
-			I915_WRITE(EDP_PSR_DEBUG_CTL,
-				   EDP_PSR_DEBUG_MASK_MEMUP |
-				   EDP_PSR_DEBUG_MASK_HPD |
-				   EDP_PSR_DEBUG_MASK_LPSP);
-		}
-
 		/* Enable PSR on the panel */
 		hsw_psr_enable_sink(intel_dp);
 
+		hsw_psr_enable_source(intel_dp);
+
 		if (INTEL_GEN(dev_priv) >= 9)
 			intel_psr_activate(intel_dp);
 	} else {
@@ -537,12 +552,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 		/* Enable PSR on the panel */
 		vlv_psr_enable_sink(intel_dp);
 
-		/* On HSW+ enable_source also means go to PSR entry/active
-		 * state as soon as idle_frame achieved and here would be
-		 * to soon. However on VLV enable_source just enable PSR
-		 * but let it on inactive state. So we might do this prior
-		 * to active transition, i.e. here.
-		 */
 		vlv_psr_enable_source(intel_dp);
 	}
 
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 08/12] drm/i915/psr: Re-org Activate after enable

[Rodrigo Vivi]

Let's move the activation calls together after enable is done.

No real functional change should be expected here. Just an attempt
to get it clear when we are really activating PSR after enabling it.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 422033a6f9f6..750df0172e8b 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -543,9 +543,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 		hsw_psr_enable_sink(intel_dp);
 
 		hsw_psr_enable_source(intel_dp);
-
-		if (INTEL_GEN(dev_priv) >= 9)
-			intel_psr_activate(intel_dp);
 	} else {
 		vlv_psr_setup_vsc(intel_dp);
 
@@ -555,20 +552,24 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 		vlv_psr_enable_source(intel_dp);
 	}
 
-	/*
-	 * FIXME: Activation should happen immediately since this function
-	 * is just called after pipe is fully trained and enabled.
-	 * However on every platform we face issues when first activation
-	 * follows a modeset so quickly.
-	 *     - On VLV/CHV we get bank screen on first activation
-	 *     - On HSW/BDW we get a recoverable frozen screen until next
-	 *       exit-activate sequence.
-	 */
-	if (INTEL_GEN(dev_priv) < 9)
+	dev_priv->psr.enabled = intel_dp;
+
+	if (INTEL_GEN(dev_priv) >= 9)
+		intel_psr_activate(intel_dp);
+	else
+		/*
+		 * FIXME: Activation should happen immediately since this
+		 * function is just called after pipe is fully trained and
+		 * enabled.
+		 * However on some platforms we face issues when first
+		 * activation follows a modeset so quickly.
+		 *     - On VLV/CHV we get bank screen on first activation
+		 *     - On HSW/BDW we get a recoverable frozen screen until
+		 *       next exit-activate sequence.
+		 */
 		schedule_delayed_work(&dev_priv->psr.work,
 				      msecs_to_jiffies(intel_dp->panel_power_cycle_delay * 5));
 
-	dev_priv->psr.enabled = intel_dp;
 unlock:
 	mutex_unlock(&dev_priv->psr.lock);
 }
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/12] drm/i915/psr: Add setup VSC vfunc.

[Rodrigo Vivi]

Continue on VLV PSR split with vfunc, let's also create
one for setting up VSC.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  | 1 +
 drivers/gpu/drm/i915/intel_psr.c | 9 ++++-----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ac33f0c89ec5..326a0ef645c2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1148,6 +1148,7 @@ struct i915_psr {
 
 	void (*disable_source)(struct intel_dp *);
 	void (*activate)(struct intel_dp *);
+	void (*setup_vsc)(struct intel_dp *);
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 750df0172e8b..b52e4da8a151 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -535,17 +535,14 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 
-	if (HAS_DDI(dev_priv)) {
-
-		hsw_psr_setup_vsc(intel_dp);
+	dev_priv->psr.setup_vsc(intel_dp);
 
+	if (HAS_DDI(dev_priv)) {
 		/* Enable PSR on the panel */
 		hsw_psr_enable_sink(intel_dp);
 
 		hsw_psr_enable_source(intel_dp);
 	} else {
-		vlv_psr_setup_vsc(intel_dp);
-
 		/* Enable PSR on the panel */
 		vlv_psr_enable_sink(intel_dp);
 
@@ -976,8 +973,10 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		dev_priv->psr.disable_source = vlv_psr_disable;
 		dev_priv->psr.activate = vlv_psr_activate;
+		dev_priv->psr.setup_vsc = vlv_psr_setup_vsc;
 	} else {
 		dev_priv->psr.disable_source = hsw_psr_disable;
 		dev_priv->psr.activate = hsw_psr_activate;
+		dev_priv->psr.setup_vsc = hsw_psr_setup_vsc;
 	}
 }
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 10/12] drm/i915/psr: Add enable_sink vfunc.

[Rodrigo Vivi]

Continue on VPV PSR split with vfunc, let's also create one
for enabling sink.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  | 1 +
 drivers/gpu/drm/i915/intel_psr.c | 9 +++------
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 326a0ef645c2..50b577b5e4d1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1147,6 +1147,7 @@ struct i915_psr {
 	bool alpm;
 
 	void (*disable_source)(struct intel_dp *);
+	void (*enable_sink)(struct intel_dp *);
 	void (*activate)(struct intel_dp *);
 	void (*setup_vsc)(struct intel_dp *);
 };
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index b52e4da8a151..73f7ba78f4d2 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -536,16 +536,11 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 
 	dev_priv->psr.setup_vsc(intel_dp);
+	dev_priv->psr.enable_sink(intel_dp);
 
 	if (HAS_DDI(dev_priv)) {
-		/* Enable PSR on the panel */
-		hsw_psr_enable_sink(intel_dp);
-
 		hsw_psr_enable_source(intel_dp);
 	} else {
-		/* Enable PSR on the panel */
-		vlv_psr_enable_sink(intel_dp);
-
 		vlv_psr_enable_source(intel_dp);
 	}
 
@@ -972,10 +967,12 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		dev_priv->psr.disable_source = vlv_psr_disable;
+		dev_priv->psr.enable_sink = vlv_psr_enable_sink;
 		dev_priv->psr.activate = vlv_psr_activate;
 		dev_priv->psr.setup_vsc = vlv_psr_setup_vsc;
 	} else {
 		dev_priv->psr.disable_source = hsw_psr_disable;
+		dev_priv->psr.enable_sink = hsw_psr_enable_sink;
 		dev_priv->psr.activate = hsw_psr_activate;
 		dev_priv->psr.setup_vsc = hsw_psr_setup_vsc;
 	}
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 11/12] drm/i915/psr: Add enable_source vfunc.

[Rodrigo Vivi]

Continue on VPV PSR split with vfunc, let's also create one
for enabling source.

Also since we are touching *_enable_source functions let's
fix a comment with wrong name for vlv's one.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_psr.c | 13 ++++---------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 50b577b5e4d1..6a4d973e7fe9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1146,6 +1146,7 @@ struct i915_psr {
 	bool colorimetry_support;
 	bool alpm;
 
+	void (*enable_source)(struct intel_dp *);
 	void (*disable_source)(struct intel_dp *);
 	void (*enable_sink)(struct intel_dp *);
 	void (*activate)(struct intel_dp *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 73f7ba78f4d2..f4ceda6d6fd3 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -537,13 +537,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 
 	dev_priv->psr.setup_vsc(intel_dp);
 	dev_priv->psr.enable_sink(intel_dp);
-
-	if (HAS_DDI(dev_priv)) {
-		hsw_psr_enable_source(intel_dp);
-	} else {
-		vlv_psr_enable_source(intel_dp);
-	}
-
+	dev_priv->psr.enable_source(intel_dp);
 	dev_priv->psr.enabled = intel_dp;
 
 	if (INTEL_GEN(dev_priv) >= 9)
@@ -770,8 +764,7 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
 		 * directly once PSR State 4 that is active with single frame
 		 * update can be skipped. PSR_state 5 that is PSR exit then
 		 * Hardware is responsible to transition back to PSR_state 1
-		 * that is PSR inactive. Same state after
-		 * vlv_edp_psr_enable_source.
+		 * that is PSR inactive. Same state after vlv_psr_enable_source.
 		 */
 		val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
 		I915_WRITE(VLV_PSRCTL(pipe), val);
@@ -966,11 +959,13 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 	mutex_init(&dev_priv->psr.lock);
 
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+		dev_priv->psr.enable_source = vlv_psr_enable_source;
 		dev_priv->psr.disable_source = vlv_psr_disable;
 		dev_priv->psr.enable_sink = vlv_psr_enable_sink;
 		dev_priv->psr.activate = vlv_psr_activate;
 		dev_priv->psr.setup_vsc = vlv_psr_setup_vsc;
 	} else {
+		dev_priv->psr.enable_source = hsw_psr_enable_source;
 		dev_priv->psr.disable_source = hsw_psr_disable;
 		dev_priv->psr.enable_sink = hsw_psr_enable_sink;
 		dev_priv->psr.activate = hsw_psr_activate;
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 12/12] drm/i915/psr: Use more PSR HW tracking.

[Rodrigo Vivi]

So far we are using frontbuffer tracking for everything
and ignoring that PSR has a HW capable HW tracking for many
modern usages of GPU on Core platforms and newer Atom ones.

One reason for that is that we were trying to keep same
infrastructure in place for VLV/CHV than the rest of platforms.
But also because when this infrastructure was created
the front-buffer-tracking origin wasn't that good and stable
how it is today after Paulo reworked it to attend FBC cases.

However this PSR implementation without HW tracking died
on gen8LP. And newer platforms are starting to demand more HW
tracking specially with PSR2 cases in mind.

By disabling and re-enabling PSR totally every time we believe
someone is going to change the front buffer content we don't
allow PSR HW tracking to do this job and specially compromising
the whole idea of PSR2 case where the HW tracking detect only
the damaged area and do a partial screen update.

So, from now on, on the platforms that has hw_tracking let's
rely more on HW tracking.

This also is the case in used by other drivers and more validated
by SV teams. So I hope that this will lead us to less misterious
bugs.

v2: Only do this for platform that actually has hw tracking.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          |  1 +
 drivers/gpu/drm/i915/intel_drv.h         |  3 ++-
 drivers/gpu/drm/i915/intel_frontbuffer.c |  2 +-
 drivers/gpu/drm/i915/intel_psr.c         | 12 +++++++++++-
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6a4d973e7fe9..a8c3a3151275 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1145,6 +1145,7 @@ struct i915_psr {
 	bool y_cord_support;
 	bool colorimetry_support;
 	bool alpm;
+	bool has_hw_tracking;
 
 	void (*enable_source)(struct intel_dp *);
 	void (*disable_source)(struct intel_dp *);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d17a32437f07..0ff5b1741b98 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1738,7 +1738,8 @@ static inline void intel_backlight_device_unregister(struct intel_connector *con
 void intel_psr_enable(struct intel_dp *intel_dp);
 void intel_psr_disable(struct intel_dp *intel_dp);
 void intel_psr_invalidate(struct drm_i915_private *dev_priv,
-			  unsigned frontbuffer_bits);
+			  unsigned frontbuffer_bits,
+			  enum fb_op_origin origin);
 void intel_psr_flush(struct drm_i915_private *dev_priv,
 		     unsigned frontbuffer_bits,
 		     enum fb_op_origin origin);
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
index fcfc217e754e..efda1af9a5b3 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -79,7 +79,7 @@ void __intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
 		spin_unlock(&dev_priv->fb_tracking.lock);
 	}
 
-	intel_psr_invalidate(dev_priv, frontbuffer_bits);
+	intel_psr_invalidate(dev_priv, frontbuffer_bits, origin);
 	intel_edp_drrs_invalidate(dev_priv, frontbuffer_bits);
 	intel_fbc_invalidate(dev_priv, frontbuffer_bits, origin);
 }
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index f4ceda6d6fd3..50035f24c63f 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -835,6 +835,7 @@ void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
  * intel_psr_invalidate - Invalidade PSR
  * @dev_priv: i915 device
  * @frontbuffer_bits: frontbuffer plane tracking bits
+ * @origin: which operation caused the invalidated
  *
  * Since the hardware frontbuffer tracking has gaps we need to integrate
  * with the software frontbuffer tracking. This function gets called every
@@ -844,7 +845,7 @@ void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
  * Dirty frontbuffers relevant to PSR are tracked in busy_frontbuffer_bits."
  */
 void intel_psr_invalidate(struct drm_i915_private *dev_priv,
-			  unsigned frontbuffer_bits)
+			  unsigned frontbuffer_bits, enum fb_op_origin origin)
 {
 	struct drm_crtc *crtc;
 	enum pipe pipe;
@@ -852,6 +853,10 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
 	if (!HAS_PSR(dev_priv))
 		return;
 
+	if (dev_priv->psr.has_hw_tracking &&
+	    (origin == ORIGIN_FLIP || origin == ORIGIN_CS))
+		return;
+
 	mutex_lock(&dev_priv->psr.lock);
 	if (!dev_priv->psr.enabled) {
 		mutex_unlock(&dev_priv->psr.lock);
@@ -892,6 +897,10 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 	if (!HAS_PSR(dev_priv))
 		return;
 
+	if (dev_priv->psr.has_hw_tracking &&
+	    (origin == ORIGIN_FLIP || origin == ORIGIN_CS))
+		return;
+
 	mutex_lock(&dev_priv->psr.lock);
 	if (!dev_priv->psr.enabled) {
 		mutex_unlock(&dev_priv->psr.lock);
@@ -965,6 +974,7 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 		dev_priv->psr.activate = vlv_psr_activate;
 		dev_priv->psr.setup_vsc = vlv_psr_setup_vsc;
 	} else {
+		dev_priv->psr.has_hw_tracking = true;
 		dev_priv->psr.enable_source = hsw_psr_enable_source;
 		dev_priv->psr.disable_source = hsw_psr_disable;
 		dev_priv->psr.enable_sink = hsw_psr_enable_sink;
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for PSR clean-up, new vfuncs and more use of HW tracking.

[Patchwork]

== Series Details ==

Series: PSR clean-up, new vfuncs and more use of HW tracking.
URL   : https://patchwork.freedesktop.org/series/27194/
State : warning

== Summary ==

Series 27194v1 PSR clean-up, new vfuncs and more use of HW tracking.
https://patchwork.freedesktop.org/api/1.0/series/27194/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> SKIP       (fi-skl-x1585l)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-pnv-d510) fdo#101597
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-j1900) fdo#101705

fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:442s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:429s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:358s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:526s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:509s
fi-byt-j1900     total:279  pass:255  dwarn:0   dfail:0   fail:0   skip:24  time:490s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:483s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:592s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:438s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:412s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:422s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:487s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:468s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:464s
fi-kbl-7560u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:566s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:581s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:565s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:456s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:586s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:469s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:487s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:434s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:477s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:553s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:412s

8ad9e19aafea47c272163c2cbf554e06ff7f9857 drm-tip: 2017y-07m-11d-19h-08m-20s UTC integration manifest
576cd6a drm/i915/psr: Use more PSR HW tracking.
5f6296f drm/i915/psr: Add enable_source vfunc.
1323ed3 drm/i915/psr: Add enable_sink vfunc.
c3f23d2 drm/i915/psr: Add setup VSC vfunc.
0f41575 drm/i915/psr: Re-org Activate after enable
a00ce09 drm/i915/psr: Re-create a hsw_psr_enable_source.
322f67f drm/i915/psr: Unify VSC setup functions.
64ad7ca drm/i915/psr: Add activate vfunc.
a3309b7 drm/i915/psr: hsw_psr_activate.
2e3fd28 drm/i915/psr: vfunc for disabling source.
ef935e6 drm/i915/psr: Avoid any PSR stuff on platforms without support.
d5fcfc8 drm/i915/psr: Remove vlv_is_active function.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5175/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/12] drm/i915/psr: Remove vlv_is_active function.

[Chris Wilson]

Quoting Rodrigo Vivi (2017-07-12 20:20:31)
> Let's start the clean-up and re-org of VLV PSR functions by
> removing an useless one.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 559f1ab42bfc..1af4438a6095 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -61,17 +61,6 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
>         return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED;
>  }
>  
> -static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> -{
> -       struct drm_i915_private *dev_priv = to_i915(dev);
> -       uint32_t val;
> -
> -       val = I915_READ(VLV_PSRSTAT(pipe)) &
> -             VLV_EDP_PSR_CURR_STATE_MASK;
> -       return (val == VLV_EDP_PSR_ACTIVE_NORFB_UP) ||
> -              (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);
> -}
> -
>  static void intel_psr_write_vsc(struct intel_dp *intel_dp,
>                                 const struct edp_vsc_psr *vsc_psr)
>  {
> @@ -610,7 +599,10 @@ static void vlv_psr_disable(struct intel_dp *intel_dp)
>  
>                 dev_priv->psr.active = false;
>         } else {
> -               WARN_ON(vlv_is_psr_active_on_pipe(dev, intel_crtc->pipe));
> +               val = I915_READ(VLV_PSRSTAT(intel_crtc->pipe)) &
> +                       VLV_EDP_PSR_CURR_STATE_MASK;
> +               WARN_ON(val == VLV_EDP_PSR_ACTIVE_NORFB_UP ||
> +                       val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);

The value here is in the warning message if it ever fails. Which is
clearer

	WARNING vlv_is_psr_active_on_pipe(dev, intel_crtc->pipe)

or

	WARNING val == VLV_EDP_PSR_ACTIVE_NORFB_UP || vall == VLV_EDP_ACTIVE_SF_UPDATE

followed by the stacktrace starting with vlv_psr_disable()? (And the
former is smaller .data!)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/12] drm/i915/psr: Remove vlv_is_active function.

[Rodrigo Vivi]

On Wed, Jul 12, 2017 at 12:56 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Rodrigo Vivi (2017-07-12 20:20:31)
>> Let's start the clean-up and re-org of VLV PSR functions by
>> removing an useless one.
>>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> Cc: Jim Bride <jim.bride@linux.intel.com>
>> Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_psr.c | 16 ++++------------
>>  1 file changed, 4 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>> index 559f1ab42bfc..1af4438a6095 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -61,17 +61,6 @@ static bool is_edp_psr(struct intel_dp *intel_dp)
>>         return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED;
>>  }
>>
>> -static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
>> -{
>> -       struct drm_i915_private *dev_priv = to_i915(dev);
>> -       uint32_t val;
>> -
>> -       val = I915_READ(VLV_PSRSTAT(pipe)) &
>> -             VLV_EDP_PSR_CURR_STATE_MASK;
>> -       return (val == VLV_EDP_PSR_ACTIVE_NORFB_UP) ||
>> -              (val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);
>> -}
>> -
>>  static void intel_psr_write_vsc(struct intel_dp *intel_dp,
>>                                 const struct edp_vsc_psr *vsc_psr)
>>  {
>> @@ -610,7 +599,10 @@ static void vlv_psr_disable(struct intel_dp *intel_dp)
>>
>>                 dev_priv->psr.active = false;
>>         } else {
>> -               WARN_ON(vlv_is_psr_active_on_pipe(dev, intel_crtc->pipe));
>> +               val = I915_READ(VLV_PSRSTAT(intel_crtc->pipe)) &
>> +                       VLV_EDP_PSR_CURR_STATE_MASK;
>> +               WARN_ON(val == VLV_EDP_PSR_ACTIVE_NORFB_UP ||
>> +                       val == VLV_EDP_PSR_ACTIVE_SF_UPDATE);
>
> The value here is in the warning message if it ever fails. Which is
> clearer
>
>         WARNING vlv_is_psr_active_on_pipe(dev, intel_crtc->pipe)
>
> or
>
>         WARNING val == VLV_EDP_PSR_ACTIVE_NORFB_UP || vall == VLV_EDP_ACTIVE_SF_UPDATE
>
> followed by the stacktrace starting with vlv_psr_disable()? (And the
> former is smaller .data!)

good points...

so let's just discard this patch.

Thanks

> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 07/12] drm/i915/psr: Re-create a hsw_psr_enable_source.

[Jani Nikula]

On Wed, 12 Jul 2017, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> This sequence is part of enable source anyways, but they
> only need to be executed once and not on every activation,
> So let's re-create hsw_enable_source.
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 77 ++++++++++++++++++++++------------------
>  1 file changed, 43 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 2fbf803f0667..422033a6f9f6 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -219,6 +219,47 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>  	I915_WRITE(aux_ctl_reg, aux_ctl);
>  }
>  
> +static void hsw_psr_enable_source(struct intel_dp *intel_dp)
> +{
> +	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);
> +	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
> +	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
> +
> +	u32 chicken;
> +
> +	if (dev_priv->psr.psr2_support) {
> +		chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> +		if (dev_priv->psr.y_cord_support)
> +			chicken |= PSR2_ADD_VERTICAL_LINE_COUNT;
> +		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
> +		I915_WRITE(EDP_PSR_DEBUG_CTL,
> +			   EDP_PSR_DEBUG_MASK_MEMUP |
> +			   EDP_PSR_DEBUG_MASK_HPD |
> +			   EDP_PSR_DEBUG_MASK_LPSP |
> +			   EDP_PSR_DEBUG_MASK_MAX_SLEEP |
> +			   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
> +	} else {
> +		/*
> +		 * Per Spec: Avoid continuous PSR exit by masking MEMUP
> +		 * and HPD. also mask LPSP to avoid dependency on other
> +		 * drivers that might block runtime_pm besides
> +		 * preventing  other hw tracking issues now we can rely
> +		 * on frontbuffer tracking.
> +		 */
> +		I915_WRITE(EDP_PSR_DEBUG_CTL,
> +			   EDP_PSR_DEBUG_MASK_MEMUP |
> +			   EDP_PSR_DEBUG_MASK_HPD |
> +			   EDP_PSR_DEBUG_MASK_LPSP);
> +	}
> +
> +	/*
> +	 * The rest of enable source sequence that should be here is actually
> +	 * executed during activation time. So check hsw_psr_activate().
> +	 */
> +}
> +
>  static void vlv_psr_enable_source(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> @@ -474,9 +515,6 @@ void intel_psr_enable(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 = to_i915(dev);
> -	struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> -	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
> -	u32 chicken;
>  
>  	if (!HAS_PSR(dev_priv))
>  		return;
> @@ -501,34 +539,11 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>  
>  		hsw_psr_setup_vsc(intel_dp);
>  
> -		if (dev_priv->psr.psr2_support) {
> -			chicken = PSR2_VSC_ENABLE_PROG_HEADER;
> -			if (dev_priv->psr.y_cord_support)
> -				chicken |= PSR2_ADD_VERTICAL_LINE_COUNT;
> -			I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
> -			I915_WRITE(EDP_PSR_DEBUG_CTL,
> -				   EDP_PSR_DEBUG_MASK_MEMUP |
> -				   EDP_PSR_DEBUG_MASK_HPD |
> -				   EDP_PSR_DEBUG_MASK_LPSP |
> -				   EDP_PSR_DEBUG_MASK_MAX_SLEEP |
> -				   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
> -		} else {
> -			/*
> -			 * Per Spec: Avoid continuous PSR exit by masking MEMUP
> -			 * and HPD. also mask LPSP to avoid dependency on other
> -			 * drivers that might block runtime_pm besides
> -			 * preventing  other hw tracking issues now we can rely
> -			 * on frontbuffer tracking.
> -			 */
> -			I915_WRITE(EDP_PSR_DEBUG_CTL,
> -				   EDP_PSR_DEBUG_MASK_MEMUP |
> -				   EDP_PSR_DEBUG_MASK_HPD |
> -				   EDP_PSR_DEBUG_MASK_LPSP);
> -		}
> -
>  		/* Enable PSR on the panel */
>  		hsw_psr_enable_sink(intel_dp);
>  
> +		hsw_psr_enable_source(intel_dp);
> +

Wouldn't it be better to keep the order? Or have the order changed in a
separate patch if you need the order changed?

BR,
Jani.

>  		if (INTEL_GEN(dev_priv) >= 9)
>  			intel_psr_activate(intel_dp);
>  	} else {
> @@ -537,12 +552,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>  		/* Enable PSR on the panel */
>  		vlv_psr_enable_sink(intel_dp);
>  
> -		/* On HSW+ enable_source also means go to PSR entry/active
> -		 * state as soon as idle_frame achieved and here would be
> -		 * to soon. However on VLV enable_source just enable PSR
> -		 * but let it on inactive state. So we might do this prior
> -		 * to active transition, i.e. here.
> -		 */
>  		vlv_psr_enable_source(intel_dp);
>  	}

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/12] drm/i915/psr: Re-org Activate after enable

[Jani Nikula]

On Wed, 12 Jul 2017, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> Let's move the activation calls together after enable is done.
>
> No real functional change should be expected here. Just an attempt
> to get it clear when we are really activating PSR after enabling it.
>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 422033a6f9f6..750df0172e8b 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -543,9 +543,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>  		hsw_psr_enable_sink(intel_dp);
>  
>  		hsw_psr_enable_source(intel_dp);
> -
> -		if (INTEL_GEN(dev_priv) >= 9)
> -			intel_psr_activate(intel_dp);
>  	} else {
>  		vlv_psr_setup_vsc(intel_dp);
>  
> @@ -555,20 +552,24 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>  		vlv_psr_enable_source(intel_dp);
>  	}
>  
> -	/*
> -	 * FIXME: Activation should happen immediately since this function
> -	 * is just called after pipe is fully trained and enabled.
> -	 * However on every platform we face issues when first activation
> -	 * follows a modeset so quickly.
> -	 *     - On VLV/CHV we get bank screen on first activation
> -	 *     - On HSW/BDW we get a recoverable frozen screen until next
> -	 *       exit-activate sequence.
> -	 */
> -	if (INTEL_GEN(dev_priv) < 9)
> +	dev_priv->psr.enabled = intel_dp;
> +
> +	if (INTEL_GEN(dev_priv) >= 9)
> +		intel_psr_activate(intel_dp);
> +	else
> +		/*
> +		 * FIXME: Activation should happen immediately since this
> +		 * function is just called after pipe is fully trained and
> +		 * enabled.
> +		 * However on some platforms we face issues when first
> +		 * activation follows a modeset so quickly.
> +		 *     - On VLV/CHV we get bank screen on first activation
> +		 *     - On HSW/BDW we get a recoverable frozen screen until
> +		 *       next exit-activate sequence.
> +		 */
>  		schedule_delayed_work(&dev_priv->psr.work,
>  				      msecs_to_jiffies(intel_dp->panel_power_cycle_delay * 5));

While technically not required, I'd put curly braces around the branches
here for readability, because the comment is so long.

BR,
Jani.

>  
> -	dev_priv->psr.enabled = intel_dp;
>  unlock:
>  	mutex_unlock(&dev_priv->psr.lock);
>  }

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915/psr: Re-create a hsw_psr_enable_source.

[Rodrigo Vivi]

This sequence is part of enable source anyways, but they
only need to be executed once and not on every activation,
So let's re-create hsw_enable_source.

v2: Avoid changing order here to avoid changing behaviour
    as suggested by Jani.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 75 ++++++++++++++++++++++------------------
 1 file changed, 42 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index febaca1dec4b..3caf330017da 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -230,6 +230,47 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 	I915_WRITE(aux_ctl_reg, aux_ctl);
 }
 
+static void hsw_psr_enable_source(struct intel_dp *intel_dp)
+{
+	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);
+	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
+	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
+
+	u32 chicken;
+
+	if (dev_priv->psr.psr2_support) {
+		chicken = PSR2_VSC_ENABLE_PROG_HEADER;
+		if (dev_priv->psr.y_cord_support)
+			chicken |= PSR2_ADD_VERTICAL_LINE_COUNT;
+		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
+		I915_WRITE(EDP_PSR_DEBUG_CTL,
+			   EDP_PSR_DEBUG_MASK_MEMUP |
+			   EDP_PSR_DEBUG_MASK_HPD |
+			   EDP_PSR_DEBUG_MASK_LPSP |
+			   EDP_PSR_DEBUG_MASK_MAX_SLEEP |
+			   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
+	} else {
+		/*
+		 * Per Spec: Avoid continuous PSR exit by masking MEMUP
+		 * and HPD. also mask LPSP to avoid dependency on other
+		 * drivers that might block runtime_pm besides
+		 * preventing  other hw tracking issues now we can rely
+		 * on frontbuffer tracking.
+		 */
+		I915_WRITE(EDP_PSR_DEBUG_CTL,
+			   EDP_PSR_DEBUG_MASK_MEMUP |
+			   EDP_PSR_DEBUG_MASK_HPD |
+			   EDP_PSR_DEBUG_MASK_LPSP);
+	}
+
+	/*
+	 * The rest of enable source sequence that should be here is actually
+	 * executed during activation time. So check hsw_psr_activate().
+	 */
+}
+
 static void vlv_psr_enable_source(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
@@ -485,9 +526,6 @@ void intel_psr_enable(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 = to_i915(dev);
-	struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
-	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
-	u32 chicken;
 
 	if (!HAS_PSR(dev_priv))
 		return;
@@ -512,30 +550,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 
 		hsw_psr_setup_vsc(intel_dp);
 
-		if (dev_priv->psr.psr2_support) {
-			chicken = PSR2_VSC_ENABLE_PROG_HEADER;
-			if (dev_priv->psr.y_cord_support)
-				chicken |= PSR2_ADD_VERTICAL_LINE_COUNT;
-			I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
-			I915_WRITE(EDP_PSR_DEBUG_CTL,
-				   EDP_PSR_DEBUG_MASK_MEMUP |
-				   EDP_PSR_DEBUG_MASK_HPD |
-				   EDP_PSR_DEBUG_MASK_LPSP |
-				   EDP_PSR_DEBUG_MASK_MAX_SLEEP |
-				   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
-		} else {
-			/*
-			 * Per Spec: Avoid continuous PSR exit by masking MEMUP
-			 * and HPD. also mask LPSP to avoid dependency on other
-			 * drivers that might block runtime_pm besides
-			 * preventing  other hw tracking issues now we can rely
-			 * on frontbuffer tracking.
-			 */
-			I915_WRITE(EDP_PSR_DEBUG_CTL,
-				   EDP_PSR_DEBUG_MASK_MEMUP |
-				   EDP_PSR_DEBUG_MASK_HPD |
-				   EDP_PSR_DEBUG_MASK_LPSP);
-		}
+		hsw_psr_enable_source(intel_dp);
 
 		/* Enable PSR on the panel */
 		hsw_psr_enable_sink(intel_dp);
@@ -548,12 +563,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 		/* Enable PSR on the panel */
 		vlv_psr_enable_sink(intel_dp);
 
-		/* On HSW+ enable_source also means go to PSR entry/active
-		 * state as soon as idle_frame achieved and here would be
-		 * to soon. However on VLV enable_source just enable PSR
-		 * but let it on inactive state. So we might do this prior
-		 * to active transition, i.e. here.
-		 */
 		vlv_psr_enable_source(intel_dp);
 	}
 
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915/psr: Move hsw_enable_source after enabling sink.

[Rodrigo Vivi]

No functional change is expected here since at this point
PSR is not allowed to go to any active state. In other
words, not really enabled.

However let's do in a separated patch so it gets clear
on what is change and specially it can helps on bisect
case if we figure something has caused changes in behaviour.

But this needs to be done before we make the vfunc to
enable source to be in parity with VLV implementation.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 3caf330017da..c2c7afe6a902 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -550,11 +550,11 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 
 		hsw_psr_setup_vsc(intel_dp);
 
-		hsw_psr_enable_source(intel_dp);
-
 		/* Enable PSR on the panel */
 		hsw_psr_enable_sink(intel_dp);
 
+		hsw_psr_enable_source(intel_dp);
+
 		if (INTEL_GEN(dev_priv) >= 9)
 			intel_psr_activate(intel_dp);
 	} else {
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for PSR clean-up, new vfuncs and more use of HW tracking. (rev3)

[Patchwork]

== Series Details ==

Series: PSR clean-up, new vfuncs and more use of HW tracking. (rev3)
URL   : https://patchwork.freedesktop.org/series/27194/
State : failure

== Summary ==

  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CHK     include/generated/bounds.h
  CHK     include/generated/timeconst.h
  CHK     include/generated/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  CHK     scripts/mod/devicetable-offsets.h
  CHK     include/generated/compile.h
  CHK     kernel/config_data.h
  CC [M]  drivers/gpu/drm/i915/intel_psr.o
drivers/gpu/drm/i915/intel_psr.c: In function ‘intel_psr_enable’:
drivers/gpu/drm/i915/intel_psr.c:547:1: error: expected expression before ‘<<’ token
 <<<<<<< 9e8f83846114215a62362801b9fb8a7908c35b6d
 ^
drivers/gpu/drm/i915/intel_psr.c:547:9: error: invalid suffix "f83846114215a62362801b9fb8a7908c35b6d" on floating constant
 <<<<<<< 9e8f83846114215a62362801b9fb8a7908c35b6d
         ^
drivers/gpu/drm/i915/intel_psr.c:550:1: error: expected expression before ‘==’ token
 =======
 ^
drivers/gpu/drm/i915/intel_psr.c:552:1: error: expected expression before ‘>>’ token
 >>>>>>> drm/i915/psr: Re-org Activate after enable
 ^
scripts/Makefile.build:302: recipe for target 'drivers/gpu/drm/i915/intel_psr.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_psr.o] Error 1
scripts/Makefile.build:561: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:561: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:561: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1016: recipe for target 'drivers' failed
make: *** [drivers] Error 2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/psr: Re-org Activate after enable

[Rodrigo Vivi]

Let's move the activation calls together after enable is done.

No real functional change should be expected here. Just an attempt
to get it clear when we are really activating PSR after enabling it.

v2: Add braces on if/else because commit message there is too long
    as suggested by Jani.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index c2c7afe6a902..09e16768dacc 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -554,9 +554,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 		hsw_psr_enable_sink(intel_dp);
 
 		hsw_psr_enable_source(intel_dp);
-
-		if (INTEL_GEN(dev_priv) >= 9)
-			intel_psr_activate(intel_dp);
 	} else {
 		vlv_psr_setup_vsc(intel_dp);
 
@@ -566,20 +563,25 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 		vlv_psr_enable_source(intel_dp);
 	}
 
-	/*
-	 * FIXME: Activation should happen immediately since this function
-	 * is just called after pipe is fully trained and enabled.
-	 * However on every platform we face issues when first activation
-	 * follows a modeset so quickly.
-	 *     - On VLV/CHV we get bank screen on first activation
-	 *     - On HSW/BDW we get a recoverable frozen screen until next
-	 *       exit-activate sequence.
-	 */
-	if (INTEL_GEN(dev_priv) < 9)
+	dev_priv->psr.enabled = intel_dp;
+
+	if (INTEL_GEN(dev_priv) >= 9) {
+		intel_psr_activate(intel_dp);
+	} else {
+		/*
+		 * FIXME: Activation should happen immediately since this
+		 * function is just called after pipe is fully trained and
+		 * enabled.
+		 * However on some platforms we face issues when first
+		 * activation follows a modeset so quickly.
+		 *     - On VLV/CHV we get bank screen on first activation
+		 *     - On HSW/BDW we get a recoverable frozen screen until
+		 *       next exit-activate sequence.
+		 */
 		schedule_delayed_work(&dev_priv->psr.work,
 				      msecs_to_jiffies(intel_dp->panel_power_cycle_delay * 5));
+	}
 
-	dev_priv->psr.enabled = intel_dp;
 unlock:
 	mutex_unlock(&dev_priv->psr.lock);
 }
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for PSR clean-up, new vfuncs and more use of HW tracking. (rev4)

[Patchwork]

== Series Details ==

Series: PSR clean-up, new vfuncs and more use of HW tracking. (rev4)
URL   : https://patchwork.freedesktop.org/series/27194/
State : success

== Summary ==

Series 27194v4 PSR clean-up, new vfuncs and more use of HW tracking.
https://patchwork.freedesktop.org/api/1.0/series/27194/revisions/4/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-snb-2600) fdo#100215
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                skip       -> PASS       (fi-skl-x1585l) fdo#101781

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101781 https://bugs.freedesktop.org/show_bug.cgi?id=101781

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:444s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:431s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:355s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:527s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:511s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:491s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:486s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:595s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:443s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:411s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:423s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:502s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:472s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:468s
fi-kbl-7560u     total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  time:572s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:576s
fi-pnv-d510      total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  time:567s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:454s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:589s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:466s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:482s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:441s
fi-skl-x1585l    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:500s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:546s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:408s

e1306e0d65a3a850838534019a9a61e73bf1efcc drm-tip: 2017y-07m-14d-13h-56m-32s UTC integration manifest
ee28d85 drm/i915/psr: Re-create a hsw_psr_enable_source.
8d81dca drm/i915/psr: Unify VSC setup functions.
fc23fed drm/i915/psr: Add activate vfunc.
bcf4fb1 drm/i915/psr: hsw_psr_activate.
621fdb1 drm/i915/psr: vfunc for disabling source.
bf733f4 drm/i915/psr: Avoid any PSR stuff on platforms without support.
61776f5 drm/i915/psr: Remove vlv_is_active function.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5194/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/12] drm/i915/psr: hsw_psr_activate.

[Pandiyan, Dhinakaran]

On Wed, 2017-07-12 at 12:20 -0700, Rodrigo Vivi wrote:
> Oh HSW the real activate of PSR is decided by the source

Typos: On HSW+

> after certain amount of configured idle frames.
> 
> However for the driver perspective where we track psr.active
> variable this function here is the actual activate one. So
> let's rename it before moving to vfunc with that.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 535ea0b7ec20..45c640989a46 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -254,7 +254,7 @@ static void vlv_psr_activate(struct intel_dp *intel_dp)
>  		   VLV_EDP_PSR_ACTIVE_ENTRY);
>  }
>  
> -static void intel_enable_source_psr1(struct intel_dp *intel_dp)
> +static void hsw_activate_psr1(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = dig_port->base.base.dev;
> @@ -307,7 +307,7 @@ static void intel_enable_source_psr1(struct intel_dp *intel_dp)
>  	I915_WRITE(EDP_PSR_CTL, val);
>  }
>  
> -static void intel_enable_source_psr2(struct intel_dp *intel_dp)
> +static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = dig_port->base.base.dev;
> @@ -343,17 +343,22 @@ static void intel_enable_source_psr2(struct intel_dp *intel_dp)
>  	I915_WRITE(EDP_PSR2_CTL, val);
>  }
>  
> -static void hsw_psr_enable_source(struct intel_dp *intel_dp)
> +static void hsw_psr_activate(struct intel_dp *intel_dp)
>  {
>  	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);
>  
> +	/* On HSW+ after we enable PSR on source it will activate it
> +	 * as soon as it match configure idle_frame count. So
> +	 * we just actually enable it here on activation time.
> +	 */
> +
>  	/* psr1 and psr2 are mutually exclusive.*/
>  	if (dev_priv->psr.psr2_support)
> -		intel_enable_source_psr2(intel_dp);
> +		hsw_activate_psr2(intel_dp);
>  	else
> -		intel_enable_source_psr1(intel_dp);
> +		hsw_activate_psr1(intel_dp);
>  }
>  
>  static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
> @@ -459,11 +464,7 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
>  
>  	/* Enable/Re-enable PSR on the host */
>  	if (HAS_DDI(dev_priv))
> -		/* On HSW+ after we enable PSR on source it will activate it
> -		 * as soon as it match configure idle_frame count. So
> -		 * we just actually enable it here on activation time.
> -		 */
> -		hsw_psr_enable_source(intel_dp);
> +		hsw_psr_activate(intel_dp);
>  	else
>  		vlv_psr_activate(intel_dp);
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/12] PSR clean-up, new vfuncs and more use of HW tracking.

[Pandiyan, Dhinakaran]

Sorry for the delayed review, this series needs to rebased as we are
passing crtc_state to a lot of these functions now. I don't see any
issues based on my cursory review.


On Wed, 2017-07-12 at 12:20 -0700, Rodrigo Vivi wrote:
> Most of patches on this series is only a clean up with
> no functional changes. The goal is to allow split VLV PSR
> implementation better. In a way that would get more cleaner
> when we use or not the hw tracking.
> 
> The ones with actual functional changes are:
> 0002-drm-i915-psr-Avoid-any-PSR-stuff-on-platforms-withou.patch
> that protects PSR of running anything on the platforms without support.
> 
> and
> 0012-drm-i915-psr-Use-more-PSR-HW-tracking.patch
> that allows us to use more HW tracking and consequently
> a proper PSR2 with partial screen updates.
> 
> Rodrigo Vivi (12):
>   drm/i915/psr: Remove vlv_is_active function.
>   drm/i915/psr: Avoid any PSR stuff on platforms without support.
>   drm/i915/psr: vfunc for disabling source.
>   drm/i915/psr: hsw_psr_activate.
>   drm/i915/psr: Add activate vfunc.
>   drm/i915/psr: Unify VSC setup functions.
>   drm/i915/psr: Re-create a hsw_psr_enable_source.
>   drm/i915/psr: Re-org Activate after enable
>   drm/i915/psr: Add setup VSC vfunc.
>   drm/i915/psr: Add enable_sink vfunc.
>   drm/i915/psr: Add enable_source vfunc.
>   drm/i915/psr: Use more PSR HW tracking.
> 
>  drivers/gpu/drm/i915/i915_drv.h          |   7 +
>  drivers/gpu/drm/i915/intel_drv.h         |   3 +-
>  drivers/gpu/drm/i915/intel_frontbuffer.c |   2 +-
>  drivers/gpu/drm/i915/intel_psr.c         | 267 ++++++++++++++++---------------
>  4 files changed, 150 insertions(+), 129 deletions(-)
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/12] drm/i915/psr: Add enable_sink vfunc.

[Pandiyan, Dhinakaran]

On Wed, 2017-07-12 at 12:20 -0700, Rodrigo Vivi wrote:
> Continue on VPV PSR split with vfunc, let's also create one

Typo s/VPV/VLV

> for enabling sink.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  | 1 +
>  drivers/gpu/drm/i915/intel_psr.c | 9 +++------
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 326a0ef645c2..50b577b5e4d1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1147,6 +1147,7 @@ struct i915_psr {
>  	bool alpm;
>  
>  	void (*disable_source)(struct intel_dp *);
> +	void (*enable_sink)(struct intel_dp *);
>  	void (*activate)(struct intel_dp *);
>  	void (*setup_vsc)(struct intel_dp *);
>  };
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index b52e4da8a151..73f7ba78f4d2 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -536,16 +536,11 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>  	dev_priv->psr.busy_frontbuffer_bits = 0;
>  
>  	dev_priv->psr.setup_vsc(intel_dp);
> +	dev_priv->psr.enable_sink(intel_dp);
>  
>  	if (HAS_DDI(dev_priv)) {
> -		/* Enable PSR on the panel */
> -		hsw_psr_enable_sink(intel_dp);
> -
>  		hsw_psr_enable_source(intel_dp);
>  	} else {
> -		/* Enable PSR on the panel */
> -		vlv_psr_enable_sink(intel_dp);
> -
>  		vlv_psr_enable_source(intel_dp);
>  	}
>  
> @@ -972,10 +967,12 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
>  
>  	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
>  		dev_priv->psr.disable_source = vlv_psr_disable;
> +		dev_priv->psr.enable_sink = vlv_psr_enable_sink;
>  		dev_priv->psr.activate = vlv_psr_activate;
>  		dev_priv->psr.setup_vsc = vlv_psr_setup_vsc;
>  	} else {
>  		dev_priv->psr.disable_source = hsw_psr_disable;
> +		dev_priv->psr.enable_sink = hsw_psr_enable_sink;
>  		dev_priv->psr.activate = hsw_psr_activate;
>  		dev_priv->psr.setup_vsc = hsw_psr_setup_vsc;
>  	}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 11/12] drm/i915/psr: Add enable_source vfunc.

[Pandiyan, Dhinakaran]

On Wed, 2017-07-12 at 12:20 -0700, Rodrigo Vivi wrote:
> Continue on VPV PSR split with vfunc, let's also create one

Typo: s/VPV/VLV

> for enabling source.
> 
> Also since we are touching *_enable_source functions let's
> fix a comment with wrong name for vlv's one.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Cc: Vathsala NAgaraju <vathsala.nagaraju@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_psr.c | 13 ++++---------
>  2 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 50b577b5e4d1..6a4d973e7fe9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1146,6 +1146,7 @@ struct i915_psr {
>  	bool colorimetry_support;
>  	bool alpm;
>  
> +	void (*enable_source)(struct intel_dp *);
>  	void (*disable_source)(struct intel_dp *);
>  	void (*enable_sink)(struct intel_dp *);
>  	void (*activate)(struct intel_dp *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 73f7ba78f4d2..f4ceda6d6fd3 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -537,13 +537,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>  
>  	dev_priv->psr.setup_vsc(intel_dp);
>  	dev_priv->psr.enable_sink(intel_dp);
> -
> -	if (HAS_DDI(dev_priv)) {
> -		hsw_psr_enable_source(intel_dp);
> -	} else {
> -		vlv_psr_enable_source(intel_dp);
> -	}
> -
> +	dev_priv->psr.enable_source(intel_dp);
>  	dev_priv->psr.enabled = intel_dp;
>  
>  	if (INTEL_GEN(dev_priv) >= 9)
> @@ -770,8 +764,7 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
>  		 * directly once PSR State 4 that is active with single frame
>  		 * update can be skipped. PSR_state 5 that is PSR exit then
>  		 * Hardware is responsible to transition back to PSR_state 1
> -		 * that is PSR inactive. Same state after
> -		 * vlv_edp_psr_enable_source.
> +		 * that is PSR inactive. Same state after vlv_psr_enable_source.
>  		 */
>  		val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
>  		I915_WRITE(VLV_PSRCTL(pipe), val);
> @@ -966,11 +959,13 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
>  	mutex_init(&dev_priv->psr.lock);
>  
>  	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> +		dev_priv->psr.enable_source = vlv_psr_enable_source;
>  		dev_priv->psr.disable_source = vlv_psr_disable;
>  		dev_priv->psr.enable_sink = vlv_psr_enable_sink;
>  		dev_priv->psr.activate = vlv_psr_activate;
>  		dev_priv->psr.setup_vsc = vlv_psr_setup_vsc;
>  	} else {
> +		dev_priv->psr.enable_source = hsw_psr_enable_source;
>  		dev_priv->psr.disable_source = hsw_psr_disable;
>  		dev_priv->psr.enable_sink = hsw_psr_enable_sink;
>  		dev_priv->psr.activate = hsw_psr_activate;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx