[PATCH 00/11] PSR clean-up and vfuncs for clear split between different psr implementations

[PATCH 00/11] PSR clean-up and vfuncs for clear split between different psr implementations

[Rodrigo Vivi]

The ultimate goal is to be able to use more HW tracking on the PSR
implementation where that is possible, i.e. all other platforms but VLV/CHV.

But before doing that, let's organize PSR a bit more so it will be really
clear the platforms where HW tracking is possible.

This series is not addressing the more usage of HW tracking itself since
that needs more test and more carefulness, but let's move with this clean-up
before another rework impacts this again requiring another rebase.

In summary this v2:
- Remove nacked: drm/i915/psr: Remove vlv_is_active function.
- Include v2 of some patches based on Jani's comment
- Adds drm/i915/psr: Move hsw_enable_source after enabling sink.
- Include typo fixes pointed by DK.
- Remove for now  drm/i915/psr: Use more PSR HW tracking.

Thanks,
Rodrigo.

Rodrigo Vivi (11):
  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: Move hsw_enable_source after enabling sink.
  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.

 drivers/gpu/drm/i915/i915_drv.h  |   8 ++
 drivers/gpu/drm/i915/intel_psr.c | 235 ++++++++++++++++++++-------------------
 2 files changed, 127 insertions(+), 116 deletions(-)

-- 
2.13.2

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

[PATCH 01/11] 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: 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 f62ab05d3d62..60a1ece6d4c9 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -496,10 +496,8 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 	enum transcoder cpu_transcoder = crtc_state->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");
@@ -678,6 +676,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);
@@ -829,6 +830,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.
@@ -875,6 +879,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);
@@ -912,6 +919,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);
@@ -944,6 +954,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 02/11] 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.

v2: Rebased on top of commit d2419ffc10e4 ("drm/i915: Plumb
    crtc_state to PSR enable/disable")

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@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  |  3 +++
 drivers/gpu/drm/i915/intel_psr.c | 12 +++++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 63ca2ffcafef..d24793cdf949 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1180,6 +1180,9 @@ struct i915_psr {
 	bool y_cord_support;
 	bool colorimetry_support;
 	bool alpm;
+
+	void (*disable_source)(struct intel_dp *,
+			       const struct intel_crtc_state *);
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 60a1ece6d4c9..1d2b52e6a6d4 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -685,11 +685,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, old_crtc_state);
-	else
-		vlv_psr_disable(intel_dp, old_crtc_state);
+	dev_priv->psr.disable_source(intel_dp, old_crtc_state);
 
 	/* Disable PSR on Sink */
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0);
@@ -987,4 +983,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 03/11] drm/i915/psr: hsw_psr_activate.

[Rodrigo Vivi]

On 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.

v2: Fix typo on commit message (DK).

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@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 1d2b52e6a6d4..92b3db3aa830 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -263,7 +263,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;
@@ -317,7 +317,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;
@@ -353,17 +353,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)
@@ -469,11 +474,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 04/11] 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: 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 d24793cdf949..fa279f3cc838 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1183,6 +1183,7 @@ struct i915_psr {
 
 	void (*disable_source)(struct intel_dp *,
 			       const struct intel_crtc_state *);
+	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 92b3db3aa830..b3c1e601a8b2 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -472,12 +472,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;
 }
 
@@ -987,7 +982,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 05/11] 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.

v2: Rebased on top of commit d2419ffc10e4 ("drm/i915: Plumb
    crtc_state to PSR enable/disable")

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@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 | 61 ++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index b3c1e601a8b2..eca6170becdc 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -117,46 +117,41 @@ static void vlv_psr_setup_vsc(struct intel_dp *intel_dp,
 	I915_WRITE(VLV_VSCSDP(crtc->pipe), val);
 }
 
-static void skl_psr_setup_su_vsc(struct intel_dp *intel_dp,
-				 const struct intel_crtc_state *crtc_state)
+static void hsw_psr_setup_vsc(struct intel_dp *intel_dp,
+			      const struct intel_crtc_state *crtc_state)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
 	struct edp_vsc_psr psr_vsc;
 
-	/* 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;
+	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,
-			      const struct intel_crtc_state *crtc_state)
-{
-	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,
@@ -512,9 +507,10 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 
 	if (HAS_DDI(dev_priv)) {
-		if (dev_priv->psr.psr2_support) {
-			skl_psr_setup_su_vsc(intel_dp, crtc_state);
 
+		hsw_psr_setup_vsc(intel_dp, crtc_state);
+
+		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;
@@ -527,9 +523,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, crtc_state);
-
 			/*
 			 * 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 06/11] 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.
v3: Rebased on top of commit d2419ffc10e4 ("drm/i915: Plumb
    crtc_state to PSR enable/disable")

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@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 | 70 +++++++++++++++++++++-------------------
 1 file changed, 37 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index eca6170becdc..575c8b93e96f 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -471,6 +471,42 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
 	dev_priv->psr.active = true;
 }
 
+static void hsw_psr_enable_source(struct intel_dp *intel_dp,
+				  const struct intel_crtc_state *crtc_state)
+{
+	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);
+	enum transcoder cpu_transcoder = crtc_state->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);
+	}
+}
+
 /**
  * intel_psr_enable - Enable PSR
  * @intel_dp: Intel DP
@@ -484,8 +520,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);
-	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
-	u32 chicken;
 
 	if (!HAS_PSR(dev_priv))
 		return;
@@ -510,31 +544,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 
 		hsw_psr_setup_vsc(intel_dp, crtc_state);
 
-		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, crtc_state);
 
 		/* Enable PSR on the panel */
 		hsw_psr_enable_sink(intel_dp);
@@ -547,12 +557,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, crtc_state);
 	}
 
-- 
2.13.2

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

[PATCH 07/11] 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: 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 575c8b93e96f..245cf3ee979f 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -544,11 +544,11 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 
 		hsw_psr_setup_vsc(intel_dp, crtc_state);
 
-		hsw_psr_enable_source(intel_dp, crtc_state);
-
 		/* Enable PSR on the panel */
 		hsw_psr_enable_sink(intel_dp);
 
+		hsw_psr_enable_source(intel_dp, crtc_state);
+
 		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

[PATCH 08/11] 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.
v3: Rebased on top of commit d2419ffc10e4 ("drm/i915: Plumb
    crtc_state to PSR enable/disable")

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@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, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 245cf3ee979f..7031dfd50ae9 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -549,8 +549,6 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 
 		hsw_psr_enable_source(intel_dp, crtc_state);
 
-		if (INTEL_GEN(dev_priv) >= 9)
-			intel_psr_activate(intel_dp);
 	} else {
 		vlv_psr_setup_vsc(intel_dp, crtc_state);
 
@@ -560,20 +558,25 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 		vlv_psr_enable_source(intel_dp, crtc_state);
 	}
 
-	/*
-	 * 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/11] 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.

v2: Rebased on top of commit d2419ffc10e4 ("drm/i915: Plumb
    crtc_state to PSR enable/disable")

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@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 fa279f3cc838..54786bc73263 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1184,6 +1184,7 @@ struct i915_psr {
 	void (*disable_source)(struct intel_dp *,
 			       const struct intel_crtc_state *);
 	void (*activate)(struct intel_dp *);
+	void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 7031dfd50ae9..2516d2a50022 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -540,18 +540,15 @@ 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, crtc_state);
+	dev_priv->psr.setup_vsc(intel_dp, crtc_state);
 
+	if (HAS_DDI(dev_priv)) {
 		/* Enable PSR on the panel */
 		hsw_psr_enable_sink(intel_dp);
 
 		hsw_psr_enable_source(intel_dp, crtc_state);
 
 	} else {
-		vlv_psr_setup_vsc(intel_dp, crtc_state);
-
 		/* Enable PSR on the panel */
 		vlv_psr_enable_sink(intel_dp);
 
@@ -983,8 +980,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/11] drm/i915/psr: Add enable_sink vfunc.

[Rodrigo Vivi]

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

v2: Fix typo on commit message (DK).

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@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 54786bc73263..4efc2b591f6f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1183,6 +1183,7 @@ struct i915_psr {
 
 	void (*disable_source)(struct intel_dp *,
 			       const struct intel_crtc_state *);
+	void (*enable_sink)(struct intel_dp *);
 	void (*activate)(struct intel_dp *);
 	void (*setup_vsc)(struct intel_dp *, const struct intel_crtc_state *);
 };
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2516d2a50022..02c32cc38648 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -541,17 +541,12 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 
 	dev_priv->psr.setup_vsc(intel_dp, crtc_state);
+	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, crtc_state);
 
 	} else {
-		/* Enable PSR on the panel */
-		vlv_psr_enable_sink(intel_dp);
-
 		vlv_psr_enable_source(intel_dp, crtc_state);
 	}
 
@@ -979,10 +974,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/11] drm/i915/psr: Add enable_source vfunc.

[Rodrigo Vivi]

Continue on VLV 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.

v2: Fix typo on commit message (DK).

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@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 | 14 ++++----------
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4efc2b591f6f..ec70121410e4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1181,6 +1181,8 @@ struct i915_psr {
 	bool colorimetry_support;
 	bool alpm;
 
+	void (*enable_source)(struct intel_dp *,
+			      const struct intel_crtc_state *);
 	void (*disable_source)(struct intel_dp *,
 			       const struct intel_crtc_state *);
 	void (*enable_sink)(struct intel_dp *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 02c32cc38648..fdd9e3d95efb 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -542,14 +542,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 
 	dev_priv->psr.setup_vsc(intel_dp, crtc_state);
 	dev_priv->psr.enable_sink(intel_dp);
-
-	if (HAS_DDI(dev_priv)) {
-		hsw_psr_enable_source(intel_dp, crtc_state);
-
-	} else {
-		vlv_psr_enable_source(intel_dp, crtc_state);
-	}
-
+	dev_priv->psr.enable_source(intel_dp, crtc_state);
 	dev_priv->psr.enabled = intel_dp;
 
 	if (INTEL_GEN(dev_priv) >= 9) {
@@ -777,8 +770,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);
@@ -973,11 +965,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

✓ Fi.CI.BAT: success for PSR clean-up and vfuncs for clear split between different psr implementations

[Patchwork]

== Series Details ==

Series: PSR clean-up and vfuncs for clear split between different psr implementations
URL   : https://patchwork.freedesktop.org/series/29980/
State : success

== Summary ==

Series 29980v1 PSR clean-up and vfuncs for clear split between different psr implementations
https://patchwork.freedesktop.org/api/1.0/series/29980/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-ivb-3770) fdo#100215
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> SKIP       (fi-skl-x1585l) fdo#101781
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                skip       -> PASS       (fi-cfl-s) fdo#102294

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

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:460s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:445s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:367s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:555s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:254s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:523s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:523s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:524s
fi-cfl-s         total:289  pass:250  dwarn:4   dfail:0   fail:0   skip:35  time:459s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:436s
fi-glk-2a        total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:612s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:443s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:427s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:422s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:505s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:1   skip:28  time:475s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:513s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:601s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:599s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:525s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:475s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:531s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:517s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:447s
fi-skl-x1585l    total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:491s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:559s
fi-snb-2600      total:289  pass:248  dwarn:0   dfail:0   fail:2   skip:39  time:405s

0680081b3eade6a0ea25a012e470780c89e7440a drm-tip: 2017y-09m-07d-21h-01m-55s UTC integration manifest
793599be12c2 drm/i915/psr: Add enable_source vfunc.
e1898d5acaab drm/i915/psr: Add enable_sink vfunc.
1c6ea64defee drm/i915/psr: Add setup VSC vfunc.
ee72f725b21e drm/i915/psr: Re-org Activate after enable
1497256651fc drm/i915/psr: Move hsw_enable_source after enabling sink.
b629b43b4194 drm/i915/psr: Re-create a hsw_psr_enable_source.
1d465aa15931 drm/i915/psr: Unify VSC setup functions.
42fdb9c0363f drm/i915/psr: Add activate vfunc.
213d2060e48d drm/i915/psr: hsw_psr_activate.
e0788d1dc3eb drm/i915/psr: vfunc for disabling source.
e246bbf07460 drm/i915/psr: Avoid any PSR stuff on platforms without support.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5612/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for PSR clean-up and vfuncs for clear split between different psr implementations

[Patchwork]

== Series Details ==

Series: PSR clean-up and vfuncs for clear split between different psr implementations
URL   : https://patchwork.freedesktop.org/series/29980/
State : failure

== Summary ==

Test kms_flip:
        Subgroup wf_vblank-vs-dpms-interruptible:
                pass       -> DMESG-WARN (shard-hsw)
Test gem_eio:
        Subgroup in-flight:
                pass       -> FAIL       (shard-hsw)
Test kms_fbc_crc:
        Subgroup context:
                pass       -> FAIL       (shard-hsw)
Test gem_busy:
        Subgroup extended-vebox:
                pass       -> SKIP       (shard-hsw)
Test drv_missed_irq:
                pass       -> SKIP       (shard-hsw)

shard-hsw        total:2302 pass:1230 dwarn:1   dfail:0   fail:17  skip:1054 time:9616s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5612/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Pandiyan, Dhinakaran]

On Thu, 2017-09-07 at 16:00 -0700, Rodrigo Vivi wrote:
> Continue on VLV 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.
> 
> v2: Fix typo on commit message (DK).
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@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 | 14 ++++----------
>  2 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4efc2b591f6f..ec70121410e4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1181,6 +1181,8 @@ struct i915_psr {
>  	bool colorimetry_support;
>  	bool alpm;
>  
> +	void (*enable_source)(struct intel_dp *,
> +			      const struct intel_crtc_state *);
>  	void (*disable_source)(struct intel_dp *,
>  			       const struct intel_crtc_state *);
>  	void (*enable_sink)(struct intel_dp *);
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 02c32cc38648..fdd9e3d95efb 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -542,14 +542,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
>  
>  	dev_priv->psr.setup_vsc(intel_dp, crtc_state);
>  	dev_priv->psr.enable_sink(intel_dp);
> -
> -	if (HAS_DDI(dev_priv)) {
> -		hsw_psr_enable_source(intel_dp, crtc_state);
> -
> -	} else {
> -		vlv_psr_enable_source(intel_dp, crtc_state);
> -	}
> -
> +	dev_priv->psr.enable_source(intel_dp, crtc_state);
>  	dev_priv->psr.enabled = intel_dp;
>  
>  	if (INTEL_GEN(dev_priv) >= 9) {
> @@ -777,8 +770,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.

I find this comment hard to understand, everything else looks good.

Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> for the
series.

>  		 */
>  		val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
>  		I915_WRITE(VLV_PSRCTL(pipe), val);
> @@ -973,11 +965,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

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

[Vivi, Rodrigo]

On Fri, 2017-09-08 at 01:43 +0000, Pandiyan, Dhinakaran wrote:
> On Thu, 2017-09-07 at 16:00 -0700, Rodrigo Vivi wrote:
> > Continue on VLV 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.
> > 
> > v2: Fix typo on commit message (DK).
> > 
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@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 | 14 ++++----------
> >  2 files changed, 6 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 4efc2b591f6f..ec70121410e4 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1181,6 +1181,8 @@ struct i915_psr {
> >  	bool colorimetry_support;
> >  	bool alpm;
> >  
> > +	void (*enable_source)(struct intel_dp *,
> > +			      const struct intel_crtc_state *);
> >  	void (*disable_source)(struct intel_dp *,
> >  			       const struct intel_crtc_state *);
> >  	void (*enable_sink)(struct intel_dp *);
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 02c32cc38648..fdd9e3d95efb 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -542,14 +542,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> >  
> >  	dev_priv->psr.setup_vsc(intel_dp, crtc_state);
> >  	dev_priv->psr.enable_sink(intel_dp);
> > -
> > -	if (HAS_DDI(dev_priv)) {
> > -		hsw_psr_enable_source(intel_dp, crtc_state);
> > -
> > -	} else {
> > -		vlv_psr_enable_source(intel_dp, crtc_state);
> > -	}
> > -
> > +	dev_priv->psr.enable_source(intel_dp, crtc_state);
> >  	dev_priv->psr.enabled = intel_dp;
> >  
> >  	if (INTEL_GEN(dev_priv) >= 9) {
> > @@ -777,8 +770,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.
> 
> I find this comment hard to understand, everything else looks good.

really hard to understand... I will try to re-phrase the entire comment
in a separated patch...

> 
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> for the
> series.

merged to dinq. thanks

> 
> >  		 */
> >  		val &= ~VLV_EDP_PSR_ACTIVE_ENTRY;
> >  		I915_WRITE(VLV_PSRCTL(pipe), val);
> > @@ -973,11 +965,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