[PATCH 0/9] PSR stability patches for HSW/BDW and VLV/CHV.

[PATCH 0/9] PSR stability patches for HSW/BDW and VLV/CHV.

[Rodrigo Vivi]

Here goes my latest work on PSR for better stability.

Unfortunatelly I'm not enabling it by default yet because it still needs to be
intensivelly validated and there is one still pending flicker when I force link standby on
HSW & BDW.

Since I'm going to start the bug maintainance team now I'm sending this partial work that
I believe that is ready to be merged upstream right now.

The remaining issues will be fixed and it will be enabled soon.

Thanks,
Rodrigo.

Rodrigo Vivi (9):
  drm/i915: VLV/CHV PSR needs to exit PSR on every flush.
  drm/i915: PSR VLV/CHV: Remove condition checks that only applies to
    Haswell.
  drm/i915: PSR HSW/BDW: Fix inverted logic at sink main_link_active
    bit.
  drm/i915: Add missing vbt check.
  drm/i915: group link_standby setup and let this info visible
    everywhere.
  drm/i915: PSR link standby at debugfs
  drm/i915: PSR VLV/CHV: let's respect link_standby here as well.
  drm/i915: PSR: respect vbt time for link trains when available.
  drm/i915: PSR: Expose wakeup time.

 drivers/gpu/drm/i915/i915_debugfs.c |  5 +++
 drivers/gpu/drm/i915/i915_drv.h     |  2 ++
 drivers/gpu/drm/i915/intel_psr.c    | 65 +++++++++++++++++++++----------------
 3 files changed, 44 insertions(+), 28 deletions(-)

-- 
2.1.0

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

[PATCH 1/9] drm/i915: VLV/CHV PSR needs to exit PSR on every flush.

[Rodrigo Vivi]

ON these platforms we don't have hardware tracking working for any case.
So we need to fake this on software by forcing psr to exit on every
flush.

Manual tests indicated this was needed.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index dd0e6e0..afb8b8c 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -620,13 +620,11 @@ void intel_psr_flush(struct drm_device *dev,
 
 	/*
 	 * On Valleyview and Cherryview we don't use hardware tracking so
-	 * sprite plane updates or cursor moves don't result in a PSR
+	 * any plane updates or cursor moves don't result in a PSR
 	 * invalidating. Which means we need to manually fake this in
 	 * software for all flushes, not just when we've seen a preceding
 	 * invalidation through frontbuffer rendering. */
-	if (!HAS_DDI(dev) &&
-	    ((frontbuffer_bits & INTEL_FRONTBUFFER_SPRITE(pipe)) ||
-	     (frontbuffer_bits & INTEL_FRONTBUFFER_CURSOR(pipe))))
+	if (!HAS_DDI(dev))
 		intel_psr_exit(dev);
 
 	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
-- 
2.1.0

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

[PATCH 2/9] drm/i915: PSR VLV/CHV: Remove condition checks that only applies to Haswell.

[Rodrigo Vivi]

These conditions applies only to Haswell and we were also checking for them
on Valleyview/Cherryview.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index afb8b8c..3dd8886 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -270,22 +270,19 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
 		return false;
 	}
 
-	/* Below limitations aren't valid for Broadwell */
-	if (IS_BROADWELL(dev))
-		goto out;
-
-	if (I915_READ(HSW_STEREO_3D_CTL(intel_crtc->config.cpu_transcoder)) &
-	    S3D_ENABLE) {
+	if (IS_HASWELL(dev) &&
+	    I915_READ(HSW_STEREO_3D_CTL(intel_crtc->config.cpu_transcoder)) &
+		      S3D_ENABLE) {
 		DRM_DEBUG_KMS("PSR condition failed: Stereo 3D is Enabled\n");
 		return false;
 	}
 
-	if (intel_crtc->config.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
+	if (IS_HASWELL(dev) &&
+	    intel_crtc->config.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
 		DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
 		return false;
 	}
 
- out:
 	dev_priv->psr.source_ok = true;
 	return true;
 }
-- 
2.1.0

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

[PATCH 3/9] drm/i915: PSR HSW/BDW: Fix inverted logic at sink main_link_active bit.

[Rodrigo Vivi]

We have only two possible states with so many names and combinations that
might be confusing.

1 - Main link active / enabled / stand by / on
2 - Main link disabled / off / full off

Let's start organizing it by fixing a inverted logic when setting the sink bit.

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 3dd8886..0af89db 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -163,10 +163,10 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 	/* Enable PSR in sink */
 	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT || only_standby)
 		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
-				   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
+				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
 	else
 		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
-				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
+				   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
 
 	/* Setup AUX registers */
 	for (i = 0; i < sizeof(aux_msg); i += 4)
-- 
2.1.0

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

[PATCH 4/9] drm/i915: Add missing vbt check.

[Rodrigo Vivi]

Let's respect vbt full_link (link_standby) on source side as well.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0af89db..20db835 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -226,7 +226,7 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
 			       dev_priv->vbt.psr.idle_frames + 1 : 2;
 	uint32_t val = 0x0;
 	const uint32_t link_entry_time = EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
-	bool only_standby = false;
+	bool only_standby = dev_priv->vbt.psr.full_link;
 
 	if (IS_BROADWELL(dev) && dig_port->port != PORT_A)
 		only_standby = true;
-- 
2.1.0

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

[PATCH 5/9] drm/i915: group link_standby setup and let this info visible everywhere.

[Rodrigo Vivi]

No functional changes on this patch. Just grouping the link_standy decision
to avoid miss any change and also making this info available everywhere
what will help to decide when to use vbt's tp time on following patch.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_psr.c | 19 +++++++++----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e008fa0..b4f01b4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -789,6 +789,7 @@ struct i915_psr {
 	bool active;
 	struct delayed_work work;
 	unsigned busy_frontbuffer_bits;
+	bool link_standby;
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 20db835..5ae193e 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -143,7 +143,6 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t aux_clock_divider;
 	int precharge = 0x3;
-	bool only_standby = dev_priv->vbt.psr.full_link;
 	static const uint8_t aux_msg[] = {
 		[0] = DP_AUX_NATIVE_WRITE << 4,
 		[1] = DP_SET_POWER >> 8,
@@ -157,11 +156,8 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
 
 	aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0);
 
-	if (IS_BROADWELL(dev) && dig_port->port != PORT_A)
-		only_standby = true;
-
 	/* Enable PSR in sink */
-	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT || only_standby)
+	if (dev_priv->psr.link_standby)
 		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
 				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
 	else
@@ -226,12 +222,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
 			       dev_priv->vbt.psr.idle_frames + 1 : 2;
 	uint32_t val = 0x0;
 	const uint32_t link_entry_time = EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
-	bool only_standby = dev_priv->vbt.psr.full_link;
-
-	if (IS_BROADWELL(dev) && dig_port->port != PORT_A)
-		only_standby = true;
 
-	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT || only_standby) {
+	if (dev_priv->psr.link_standby) {
 		val |= EDP_PSR_LINK_STANDBY;
 		val |= EDP_PSR_TP2_TP3_TIME_0us;
 		val |= EDP_PSR_TP1_TIME_0us;
@@ -341,6 +333,13 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 	if (!intel_psr_match_conditions(intel_dp))
 		goto unlock;
 
+	/* First we check VBT, but we must respect sink and source
+	 * known restrictions */
+	dev_priv->psr.link_standby = dev_priv->vbt.psr.full_link;
+	if ((intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) ||
+	    (IS_BROADWELL(dev) && intel_dig_port->port != PORT_A))
+		dev_priv->psr.link_standby = true;
+
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 
 	if (HAS_DDI(dev)) {
-- 
2.1.0

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

[PATCH 6/9] drm/i915: PSR link standby at debugfs

[Rodrigo Vivi]

It is useful to know at debug time if we are keeping main link on.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e515aad..0d11cbe 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2248,6 +2248,9 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
 		}
 	seq_puts(m, "\n");
 
+	seq_printf(m, "Link standby: %s\n",
+		   yesno((bool)dev_priv->psr.link_standby));
+
 	/* CHV PSR has no kind of performance counter */
 	if (HAS_PSR(dev) && HAS_DDI(dev)) {
 		psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) &
-- 
2.1.0

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

[PATCH 7/9] drm/i915: PSR VLV/CHV: let's respect link_standby here as well.

[Rodrigo Vivi]

Let's respect vbt and panel for link_standby/on x link_disabled

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 5ae193e..313347a 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -132,8 +132,17 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
 
 static void vlv_psr_enable_sink(struct intel_dp *intel_dp)
 {
-	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
-			   DP_PSR_ENABLE);
+	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 = dev->dev_private;
+
+	/* Enable PSR in sink */
+	if (dev_priv->psr.link_standby)
+		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
+	else
+		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
+				   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
 }
 
 static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
@@ -183,11 +192,12 @@ static void vlv_psr_enable_source(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc = dig_port->base.base.crtc;
 	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+	bool standby = dev_priv->psr.link_standby;
 
 	/* Transition from PSR_state 0 to PSR_state 1, i.e. PSR Inactive */
 	I915_WRITE(VLV_PSRCTL(pipe),
 		   VLV_EDP_PSR_MODE_SW_TIMER |
-		   VLV_EDP_PSR_SRC_TRANSMITTER_STATE |
+		   standby ? VLV_EDP_PSR_SRC_TRANSMITTER_STATE : 0 |
 		   VLV_EDP_PSR_ENABLE);
 }
 
-- 
2.1.0

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

[PATCH 8/9] drm/i915: PSR: respect vbt time for link trains when available.

[Rodrigo Vivi]

If link is off we need to give enough time for source to do link train.
This time is usually set at VBT.

VBT tp time comse in multiple of hundreds.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 313347a..d1c2c31 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -603,6 +603,10 @@ void intel_psr_flush(struct drm_device *dev,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc;
 	enum pipe pipe;
+	bool link_on = dev_priv->psr.link_standby;
+	int tp = 100 * dev_priv->vbt.psr.tp2_tp3_wakeup_time +
+		 100 * dev_priv->vbt.psr.tp1_wakeup_time;
+	int delay = tp && !link_on ? tp : 100;
 
 	mutex_lock(&dev_priv->psr.lock);
 	if (!dev_priv->psr.enabled) {
@@ -635,7 +639,7 @@ void intel_psr_flush(struct drm_device *dev,
 
 	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
 		schedule_delayed_work(&dev_priv->psr.work,
-				      msecs_to_jiffies(100));
+				      msecs_to_jiffies(delay));
 	mutex_unlock(&dev_priv->psr.lock);
 }
 
-- 
2.1.0

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

[PATCH 9/9] drm/i915: PSR: Expose wakeup time.

[Rodrigo Vivi]

This will be useful for automated test to know for how long to wait for PSR
to comeback before time out.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
 drivers/gpu/drm/i915/i915_drv.h     | 1 +
 drivers/gpu/drm/i915/intel_psr.c    | 5 +++--
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0d11cbe..8823867 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2250,6 +2250,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
 
 	seq_printf(m, "Link standby: %s\n",
 		   yesno((bool)dev_priv->psr.link_standby));
+	seq_printf(m, "Wakeup time: %d ms\n",
+		   dev_priv->psr.wakeup_time);
 
 	/* CHV PSR has no kind of performance counter */
 	if (HAS_PSR(dev) && HAS_DDI(dev)) {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b4f01b4..4b1d07d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -790,6 +790,7 @@ struct i915_psr {
 	struct delayed_work work;
 	unsigned busy_frontbuffer_bits;
 	bool link_standby;
+	int wakeup_time;
 };
 
 enum intel_pch {
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index d1c2c31..3838921 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -606,7 +606,8 @@ void intel_psr_flush(struct drm_device *dev,
 	bool link_on = dev_priv->psr.link_standby;
 	int tp = 100 * dev_priv->vbt.psr.tp2_tp3_wakeup_time +
 		 100 * dev_priv->vbt.psr.tp1_wakeup_time;
-	int delay = tp && !link_on ? tp : 100;
+
+	dev_priv->psr.wakeup_time = tp && !link_on ? tp : 100;
 
 	mutex_lock(&dev_priv->psr.lock);
 	if (!dev_priv->psr.enabled) {
@@ -639,7 +640,7 @@ void intel_psr_flush(struct drm_device *dev,
 
 	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
 		schedule_delayed_work(&dev_priv->psr.work,
-				      msecs_to_jiffies(delay));
+				      msecs_to_jiffies(dev_priv->psr.wakeup_time));
 	mutex_unlock(&dev_priv->psr.lock);
 }
 
-- 
2.1.0

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

Re: [PATCH 9/9] drm/i915: PSR: Expose wakeup time.

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  354/354              354/354
ILK                                  354/354              354/354
SNB              +1-1              401/424              401/424
IVB                                  488/488              488/488
BYT                                  278/278              278/278
HSW                 -41              529/529              488/529
BDW                 -1              405/405              404/405
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 SNB  igt_kms_flip_flip-vs-dpms-off-vs-modeset-interruptible      NSPT(1, M35)PASS(6, M35M22)      PASS(1, M22)
*SNB  igt_gem_concurrent_blit_gtt-rcs-early-read-interruptible      PASS(7, M35M22)      DMESG_WARN(1, M22)
 HSW  igt_kms_cursor_crc_cursor-size-change      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_kms_fence_pin_leak      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_kms_flip_event_leak      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_kms_mmio_vs_cs_flip_setcrtc_vs_cs_flip      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_lpsp_non-edp      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_cursor      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_cursor-dpms      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_dpms-mode-unset-non-lpsp      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_dpms-non-lpsp      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_drm-resources-equal      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_fences      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_fences-dpms      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_gem-execbuf      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_gem-mmap-cpu      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_gem-mmap-gtt      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_gem-pread      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_i2c      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_modeset-non-lpsp      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_modeset-non-lpsp-stress-no-wait      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_pci-d3-state      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_pm_rpm_rte      NSPT(3, M40M19)PASS(1, M20)      NSPT(1, M40)
 HSW  igt_gem_concurrent_blit_gtt-bcs-early-read-forked      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gtt-bcs-early-read-interruptible      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-forked      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-interruptible      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gtt-bcs-overwrite-source-forked      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gtt-bcs-overwrite-source-interruptible      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gtt-rcs-early-read-forked      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gtt-rcs-early-read-interruptible      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gtt-rcs-gpu-read-after-write-forked      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gtt-rcs-gpu-read-after-write-interruptible      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gtt-rcs-overwrite-source-forked      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gtt-rcs-overwrite-source-interruptible      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gttX-bcs-early-read-interruptible      DMESG_WARN(2, M40)PASS(2, M20M19)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gttX-bcs-gpu-read-after-write-interruptible      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gttX-bcs-overwrite-source-forked      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gttX-bcs-overwrite-source-interruptible      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gttX-rcs-early-read-interruptible      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gttX-rcs-gpu-read-after-write-interruptible      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
 HSW  igt_gem_concurrent_blit_gttX-rcs-overwrite-source-forked      DMESG_WARN(3, M40M19)PASS(1, M20)      DMESG_WARN(1, M40)
*BDW  igt_gem_concurrent_blit_gtt-bcs-gpu-read-after-write-interruptible      PASS(6, M30M28)      DMESG_WARN(1, M28)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/9] drm/i915: PSR HSW/BDW: Fix inverted logic at sink main_link_active bit.

[R, Durgadoss]

Hi Rodrigo,

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Rodrigo Vivi
>Sent: Monday, January 12, 2015 11:45 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo
>Subject: [Intel-gfx] [PATCH 3/9] drm/i915: PSR HSW/BDW: Fix inverted logic at sink main_link_active bit.
>
>We have only two possible states with so many names and combinations that
>might be confusing.
>
>1 - Main link active / enabled / stand by / on
>2 - Main link disabled / off / full off

In this case, I think we should use 'link_active' instead of 'link_standby'
since 'active' is what is used by the eDP spec and most of our PSR
macros. But, I believe we can have this as a separate patch.

For patches 1-3,
Reviewed-by: Durgadoss R <durgadoss.r@intel.com>

Thanks,
Durga

>
>Let's start organizing it by fixing a inverted logic when setting the sink bit.
>
>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 3dd8886..0af89db 100644
>--- a/drivers/gpu/drm/i915/intel_psr.c
>+++ b/drivers/gpu/drm/i915/intel_psr.c
>@@ -163,10 +163,10 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
> 	/* Enable PSR in sink */
> 	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT || only_standby)
> 		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>-				   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
>+				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
> 	else
> 		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>-				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
>+				   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
>
> 	/* Setup AUX registers */
> 	for (i = 0; i < sizeof(aux_msg); i += 4)
>--
>2.1.0
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/9] drm/i915: PSR link standby at debugfs

[R, Durgadoss]

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Rodrigo Vivi
>Sent: Monday, January 12, 2015 11:45 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo
>Subject: [Intel-gfx] [PATCH 6/9] drm/i915: PSR link standby at debugfs
>
>It is useful to know at debug time if we are keeping main link on.
>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

For patches, 4-6,
Reviewed-by: Durgadoss R <durgadoss.r@intel.com>

Thanks,
Durga

>---
> drivers/gpu/drm/i915/i915_debugfs.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>index e515aad..0d11cbe 100644
>--- a/drivers/gpu/drm/i915/i915_debugfs.c
>+++ b/drivers/gpu/drm/i915/i915_debugfs.c
>@@ -2248,6 +2248,9 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
> 		}
> 	seq_puts(m, "\n");
>
>+	seq_printf(m, "Link standby: %s\n",
>+		   yesno((bool)dev_priv->psr.link_standby));
>+
> 	/* CHV PSR has no kind of performance counter */
> 	if (HAS_PSR(dev) && HAS_DDI(dev)) {
> 		psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) &
>--
>2.1.0
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/9] drm/i915: PSR VLV/CHV: let's respect link_standby here as well.

[R, Durgadoss]

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Rodrigo Vivi
>Sent: Monday, January 12, 2015 11:45 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo
>Subject: [Intel-gfx] [PATCH 7/9] drm/i915: PSR VLV/CHV: let's respect link_standby here as well.
>
>Let's respect vbt and panel for link_standby/on x link_disabled
>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>---
> drivers/gpu/drm/i915/intel_psr.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>index 5ae193e..313347a 100644
>--- a/drivers/gpu/drm/i915/intel_psr.c
>+++ b/drivers/gpu/drm/i915/intel_psr.c
>@@ -132,8 +132,17 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
>
> static void vlv_psr_enable_sink(struct intel_dp *intel_dp)
> {
>-	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>-			   DP_PSR_ENABLE);
>+	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 = dev->dev_private;
>+
>+	/* Enable PSR in sink */
>+	if (dev_priv->psr.link_standby)
>+		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>+				   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
>+	else
>+		drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>+				   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
> }
>
> static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>@@ -183,11 +192,12 @@ static void vlv_psr_enable_source(struct intel_dp *intel_dp)
> 	struct drm_i915_private *dev_priv = dev->dev_private;
> 	struct drm_crtc *crtc = dig_port->base.base.crtc;
> 	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>+	bool standby = dev_priv->psr.link_standby;
>
> 	/* Transition from PSR_state 0 to PSR_state 1, i.e. PSR Inactive */
> 	I915_WRITE(VLV_PSRCTL(pipe),
> 		   VLV_EDP_PSR_MODE_SW_TIMER |
>-		   VLV_EDP_PSR_SRC_TRANSMITTER_STATE |
>+		   standby ? VLV_EDP_PSR_SRC_TRANSMITTER_STATE : 0 |

Apart from VBT, this also depends on the Panel DPCD 71h.
If bit 0 of 71h is 0, we may need Link training to exit PSR,
If main link is off.

So, unless we try it out on various panels and
are confident about it, I suggest we keep this
TRANSMITTER_STATE to 1.

Thanks,
Durga

> 		   VLV_EDP_PSR_ENABLE);
> }
>
>--
>2.1.0
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/9] drm/i915: PSR: respect vbt time for link trains when available.

[R, Durgadoss]

>-----Original Message-----
>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Rodrigo Vivi
>Sent: Monday, January 12, 2015 11:45 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo
>Subject: [Intel-gfx] [PATCH 8/9] drm/i915: PSR: respect vbt time for link trains when available.
>
>If link is off we need to give enough time for source to do link train.
>This time is usually set at VBT.
>
>VBT tp time comse in multiple of hundreds.

s/comes/comes

>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>---
> drivers/gpu/drm/i915/intel_psr.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>index 313347a..d1c2c31 100644
>--- a/drivers/gpu/drm/i915/intel_psr.c
>+++ b/drivers/gpu/drm/i915/intel_psr.c
>@@ -603,6 +603,10 @@ void intel_psr_flush(struct drm_device *dev,
> 	struct drm_i915_private *dev_priv = dev->dev_private;
> 	struct drm_crtc *crtc;
> 	enum pipe pipe;
>+	bool link_on = dev_priv->psr.link_standby;
>+	int tp = 100 * dev_priv->vbt.psr.tp2_tp3_wakeup_time +
>+		 100 * dev_priv->vbt.psr.tp1_wakeup_time;
>+	int delay = tp && !link_on ? tp : 100;

May be we should WARN/DRM_ERR on link_on is false but still tp is 0 ?

With this, for patches 8-9,

Reviewed-by: Durgadoss R <durgadoss.r@intel.com>

Thanks,
Durga

>
> 	mutex_lock(&dev_priv->psr.lock);
> 	if (!dev_priv->psr.enabled) {
>@@ -635,7 +639,7 @@ void intel_psr_flush(struct drm_device *dev,
>
> 	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
> 		schedule_delayed_work(&dev_priv->psr.work,
>-				      msecs_to_jiffies(100));
>+				      msecs_to_jiffies(delay));
> 	mutex_unlock(&dev_priv->psr.lock);
> }
>
>--
>2.1.0
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/9] drm/i915: PSR HSW/BDW: Fix inverted logic at sink main_link_active bit.

[Daniel Vetter]

On Tue, Jan 13, 2015 at 02:24:54PM +0000, R, Durgadoss wrote:
> Hi Rodrigo,
>
> >-----Original Message-----
> >From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Rodrigo Vivi
> >Sent: Monday, January 12, 2015 11:45 PM
> >To: intel-gfx@lists.freedesktop.org
> >Cc: Vivi, Rodrigo
> >Subject: [Intel-gfx] [PATCH 3/9] drm/i915: PSR HSW/BDW: Fix inverted logic at sink main_link_active bit.
> >
> >We have only two possible states with so many names and combinations that
> >might be confusing.
> >
> >1 - Main link active / enabled / stand by / on
> >2 - Main link disabled / off / full off
>
> In this case, I think we should use 'link_active' instead of 'link_standby'
> since 'active' is what is used by the eDP spec and most of our PSR
> macros. But, I believe we can have this as a separate patch.

I haven't read the patches in detail, but consistent naming is imo very
important. Also since this seems to be confusing some extension to the psr
kerneldoc overview comment is imo asked for here.

Rodrigo, can you please update the patches or do a follow up? Either is
fine with me.
-Daniel

>
> For patches 1-3,
> Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
>
> Thanks,
> Durga
>
> >
> >Let's start organizing it by fixing a inverted logic when setting the sink bit.
> >
> >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 3dd8886..0af89db 100644
> >--- a/drivers/gpu/drm/i915/intel_psr.c
> >+++ b/drivers/gpu/drm/i915/intel_psr.c
> >@@ -163,10 +163,10 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
> > /* Enable PSR in sink */
> > if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT || only_standby)
> > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
> >-   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
> >+   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
> > else
> > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
> >-   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
> >+   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
> >
> > /* Setup AUX registers */
> > for (i = 0; i < sizeof(aux_msg); i += 4)
> >--
> >2.1.0
> >
> >_______________________________________________
> >Intel-gfx mailing list
> >Intel-gfx@lists.freedesktop.org
> >http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/9] drm/i915: PSR HSW/BDW: Fix inverted logic at sink main_link_active bit.

[Rodrigo Vivi]

On Tue, Jan 13, 2015 at 2:26 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Jan 13, 2015 at 02:24:54PM +0000, R, Durgadoss wrote:
>> Hi Rodrigo,
>>
>> >-----Original Message-----
>> >From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Rodrigo Vivi
>> >Sent: Monday, January 12, 2015 11:45 PM
>> >To: intel-gfx@lists.freedesktop.org
>> >Cc: Vivi, Rodrigo
>> >Subject: [Intel-gfx] [PATCH 3/9] drm/i915: PSR HSW/BDW: Fix inverted logic at sink main_link_active bit.
>> >
>> >We have only two possible states with so many names and combinations that
>> >might be confusing.
>> >
>> >1 - Main link active / enabled / stand by / on
>> >2 - Main link disabled / off / full off
>>
>> In this case, I think we should use 'link_active' instead of 'link_standby'
>> since 'active' is what is used by the eDP spec and most of our PSR
>> macros. But, I believe we can have this as a separate patch.
>
> I haven't read the patches in detail, but consistent naming is imo very
> important. Also since this seems to be confusing some extension to the psr
> kerneldoc overview comment is imo asked for here.

I also agree that consistent naming is good. This name caused the
confusion of inverted logic somewhere here.
But I believe active name can be counfused with active/exit state we
have on source side.

Also standby is the name used on the source. Actually we have many
names for same think

* full_link_on at VBT
* link_active on DP
* transmitter state on VLV/CHV
* link_standby on core implementation

I prefer the link_standby.

>
> Rodrigo, can you please update the patches or do a follow up? Either is
> fine with me.

I prefer a follow-up so we can continue discussion to see if I change
my mind on the better name ;)

> -Daniel

Thank you both,
Rodrigo.

>
>>
>> For patches 1-3,
>> Reviewed-by: Durgadoss R <durgadoss.r@intel.com>
>>
>> Thanks,
>> Durga
>>
>> >
>> >Let's start organizing it by fixing a inverted logic when setting the sink bit.
>> >
>> >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 3dd8886..0af89db 100644
>> >--- a/drivers/gpu/drm/i915/intel_psr.c
>> >+++ b/drivers/gpu/drm/i915/intel_psr.c
>> >@@ -163,10 +163,10 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>> > /* Enable PSR in sink */
>> > if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT || only_standby)
>> > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>> >-   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
>> >+   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
>> > else
>> > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>> >-   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
>> >+   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
>> >
>> > /* Setup AUX registers */
>> > for (i = 0; i < sizeof(aux_msg); i += 4)
>> >--
>> >2.1.0
>> >
>> >_______________________________________________
>> >Intel-gfx mailing list
>> >Intel-gfx@lists.freedesktop.org
>> >http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH 7/9] drm/i915: PSR VLV/CHV: let's respect link_standby here as well.

[Rodrigo Vivi]

Sure, please ignore this patch for now...

Also this one was a wrong version without a needed parenthesis on
transmitter line, so it was actually disabling PSR on CHV.

On Tue, Jan 13, 2015 at 6:46 AM, R, Durgadoss <durgadoss.r@intel.com> wrote:
>>-----Original Message-----
>>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Rodrigo Vivi
>>Sent: Monday, January 12, 2015 11:45 PM
>>To: intel-gfx@lists.freedesktop.org
>>Cc: Vivi, Rodrigo
>>Subject: [Intel-gfx] [PATCH 7/9] drm/i915: PSR VLV/CHV: let's respect link_standby here as well.
>>
>>Let's respect vbt and panel for link_standby/on x link_disabled
>>
>>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>---
>> drivers/gpu/drm/i915/intel_psr.c | 16 +++++++++++++---
>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>>index 5ae193e..313347a 100644
>>--- a/drivers/gpu/drm/i915/intel_psr.c
>>+++ b/drivers/gpu/drm/i915/intel_psr.c
>>@@ -132,8 +132,17 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp)
>>
>> static void vlv_psr_enable_sink(struct intel_dp *intel_dp)
>> {
>>-      drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>>-                         DP_PSR_ENABLE);
>>+      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 = dev->dev_private;
>>+
>>+      /* Enable PSR in sink */
>>+      if (dev_priv->psr.link_standby)
>>+              drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>>+                                 DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
>>+      else
>>+              drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
>>+                                 DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
>> }
>>
>> static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
>>@@ -183,11 +192,12 @@ static void vlv_psr_enable_source(struct intel_dp *intel_dp)
>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>       struct drm_crtc *crtc = dig_port->base.base.crtc;
>>       enum pipe pipe = to_intel_crtc(crtc)->pipe;
>>+      bool standby = dev_priv->psr.link_standby;
>>
>>       /* Transition from PSR_state 0 to PSR_state 1, i.e. PSR Inactive */
>>       I915_WRITE(VLV_PSRCTL(pipe),
>>                  VLV_EDP_PSR_MODE_SW_TIMER |
>>-                 VLV_EDP_PSR_SRC_TRANSMITTER_STATE |
>>+                 standby ? VLV_EDP_PSR_SRC_TRANSMITTER_STATE : 0 |
>
> Apart from VBT, this also depends on the Panel DPCD 71h.
> If bit 0 of 71h is 0, we may need Link training to exit PSR,
> If main link is off.
>
> So, unless we try it out on various panels and
> are confident about it, I suggest we keep this
> TRANSMITTER_STATE to 1.
>
> Thanks,
> Durga
>
>>                  VLV_EDP_PSR_ENABLE);
>> }
>>
>>--
>>2.1.0
>>
>>_______________________________________________
>>Intel-gfx mailing list
>>Intel-gfx@lists.freedesktop.org
>>http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH 9/9] drm/i915: PSR: Expose wakeup time.

[Daniel Vetter]

On Mon, Jan 12, 2015 at 10:14:36AM -0800, Rodrigo Vivi wrote:
> This will be useful for automated test to know for how long to wait for PSR
> to comeback before time out.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
>  drivers/gpu/drm/i915/i915_drv.h     | 1 +
>  drivers/gpu/drm/i915/intel_psr.c    | 5 +++--
>  3 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 0d11cbe..8823867 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2250,6 +2250,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
>
>   seq_printf(m, "Link standby: %s\n",
>     yesno((bool)dev_priv->psr.link_standby));
> + seq_printf(m, "Wakeup time: %d ms\n",
> +   dev_priv->psr.wakeup_time);
>
>   /* CHV PSR has no kind of performance counter */
>   if (HAS_PSR(dev) && HAS_DDI(dev)) {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b4f01b4..4b1d07d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -790,6 +790,7 @@ struct i915_psr {
>   struct delayed_work work;
>   unsigned busy_frontbuffer_bits;
>   bool link_standby;
> + int wakeup_time;
>  };
>
>  enum intel_pch {
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index d1c2c31..3838921 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -606,7 +606,8 @@ void intel_psr_flush(struct drm_device *dev,
>   bool link_on = dev_priv->psr.link_standby;
>   int tp = 100 * dev_priv->vbt.psr.tp2_tp3_wakeup_time +
>   100 * dev_priv->vbt.psr.tp1_wakeup_time;
> - int delay = tp && !link_on ? tp : 100;
> +
> + dev_priv->psr.wakeup_time = tp && !link_on ? tp : 100;

Recomputing this on every flush looks a bit strange. Especially since it
means we have this kind of one-time computations at different places. Imo
that should all be consolidated (probably best in the psr_enable
funcions). I'll wait for a resend for this and patch 8 (and the naming
unconfusion+kerneldoc patch), all others merged.

Thanks, Daniel
>
>   mutex_lock(&dev_priv->psr.lock);
>   if (!dev_priv->psr.enabled) {
> @@ -639,7 +640,7 @@ void intel_psr_flush(struct drm_device *dev,
>
>   if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
>   schedule_delayed_work(&dev_priv->psr.work,
> -      msecs_to_jiffies(delay));
> +      msecs_to_jiffies(dev_priv->psr.wakeup_time));
>   mutex_unlock(&dev_priv->psr.lock);
>  }
>
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx