diff mbox

drm/i915: PSR: Fix DP_PSR_NO_TRAIN_ON_EXIT logic

Message ID 1428689418-2501-1-git-send-email-rodrigo.vivi@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rodrigo Vivi April 10, 2015, 6:10 p.m. UTC
Since the beginning there is a missunderstanding on the meaning of this
dpcd bit.
This bit shouldn't indicate whether to use link standby or not, but just
be used to configure TP1, TP2 and TP3 times and tell hw aux should be skiped
since HW is the responsible one.

Even with help of frontbuffer tracking, HW is still fully responsible for
PSR exit logic with/without DP training.

DP_PSR_NO_TRAIN_ON_EXIT means the source doesn't need to do the training, but
it doesn't tell to avoid TP patterns, so we will send minimal TP1 and avoid
TP2. It also means that sink itself can take up to 5 idle frames for training.
6 in our case since we might be off by 1. So we also increment idle_frames by 4
here.

v2: Fix and improve commit message (Durga).
v3: Use minimal TP1 time avoiding TP2 and increase idle frame.

Cc: Durgadoss R <durgadoss.r@intel.com>
Cc: Arthur Runyan <arthur.j.runyan@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Shuang He April 11, 2015, 1:22 a.m. UTC | #1
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6179
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -2              276/276              274/276
ILK                                  301/301              301/301
SNB                 -1              316/316              315/316
IVB                                  328/328              328/328
BYT                                  285/285              285/285
HSW                                  394/394              394/394
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt@gen3_render_mixed_blits      FAIL(3)PASS(4)      FAIL(2)
 PNV  igt@gen3_render_tiledx_blits      FAIL(4)PASS(4)      FAIL(2)
*SNB  igt@kms_flip@dpms-vs-vblank-race      PASS(5)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:intel_dp_start_link_train[i915]]*ERROR*too_many_voltage_retries,give_up@too many voltage .* give up
drm:intel_dp_complete_link_train[i915]]*ERROR*failed_to_train_DP,aborting@failed to train .* aborting
Note: You need to pay more attention to line start with '*'
durgadoss.r@intel.com April 14, 2015, 1:18 p.m. UTC | #2
>-----Original Message-----
>From: Vivi, Rodrigo
>Sent: Friday, April 10, 2015 11:40 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: Vivi, Rodrigo; R, Durgadoss; Runyan, Arthur J
>Subject: [PATCH] drm/i915: PSR: Fix DP_PSR_NO_TRAIN_ON_EXIT logic
>
>Since the beginning there is a missunderstanding on the meaning of this
>dpcd bit.
>This bit shouldn't indicate whether to use link standby or not, but just
>be used to configure TP1, TP2 and TP3 times and tell hw aux should be skiped
>since HW is the responsible one.
>
>Even with help of frontbuffer tracking, HW is still fully responsible for
>PSR exit logic with/without DP training.
>
>DP_PSR_NO_TRAIN_ON_EXIT means the source doesn't need to do the training, but
>it doesn't tell to avoid TP patterns, so we will send minimal TP1 and avoid
>TP2. It also means that sink itself can take up to 5 idle frames for training.
>6 in our case since we might be off by 1. So we also increment idle_frames by 4
>here.
>
>v2: Fix and improve commit message (Durga).
>v3: Use minimal TP1 time avoiding TP2 and increase idle frame.
>
>Cc: Durgadoss R <durgadoss.r@intel.com>
>Cc: Arthur Runyan <arthur.j.runyan@intel.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

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

Thanks,
Durga

>---
> drivers/gpu/drm/i915/intel_psr.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>index db95b39..0e3b652 100644
>--- a/drivers/gpu/drm/i915/intel_psr.c
>+++ b/drivers/gpu/drm/i915/intel_psr.c
>@@ -264,11 +264,17 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
> 	uint32_t val = 0x0;
> 	const uint32_t link_entry_time = EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
>
>-	if (dev_priv->psr.link_standby) {
>+	if (dev_priv->psr.link_standby)
> 		val |= EDP_PSR_LINK_STANDBY;
>+
>+	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) {
>+		/* It doesn't mean we shouldn't send TPS patters, so let's
>+		   send the minimal TP1 possible and skip TP2. */
>+		val |= EDP_PSR_TP1_TIME_100us;
> 		val |= EDP_PSR_TP2_TP3_TIME_0us;
>-		val |= EDP_PSR_TP1_TIME_0us;
> 		val |= EDP_PSR_SKIP_AUX_EXIT;
>+		/* Sink should be able to train with the 5 or 6 idle patterns */
>+		idle_frames += 4;
> 	}
>
> 	I915_WRITE(EDP_PSR_CTL(dev), val |
>@@ -381,8 +387,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
> 	/* 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))
>+	if (IS_BROADWELL(dev) && intel_dig_port->port != PORT_A)
> 		dev_priv->psr.link_standby = true;
>
> 	dev_priv->psr.busy_frontbuffer_bits = 0;
>--
>2.1.0
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index db95b39..0e3b652 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -264,11 +264,17 @@  static void hsw_psr_enable_source(struct intel_dp *intel_dp)
 	uint32_t val = 0x0;
 	const uint32_t link_entry_time = EDP_PSR_MIN_LINK_ENTRY_TIME_8_LINES;
 
-	if (dev_priv->psr.link_standby) {
+	if (dev_priv->psr.link_standby)
 		val |= EDP_PSR_LINK_STANDBY;
+
+	if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT) {
+		/* It doesn't mean we shouldn't send TPS patters, so let's
+		   send the minimal TP1 possible and skip TP2. */
+		val |= EDP_PSR_TP1_TIME_100us;
 		val |= EDP_PSR_TP2_TP3_TIME_0us;
-		val |= EDP_PSR_TP1_TIME_0us;
 		val |= EDP_PSR_SKIP_AUX_EXIT;
+		/* Sink should be able to train with the 5 or 6 idle patterns */
+		idle_frames += 4;
 	}
 
 	I915_WRITE(EDP_PSR_CTL(dev), val |
@@ -381,8 +387,7 @@  void intel_psr_enable(struct intel_dp *intel_dp)
 	/* 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))
+	if (IS_BROADWELL(dev) && intel_dig_port->port != PORT_A)
 		dev_priv->psr.link_standby = true;
 
 	dev_priv->psr.busy_frontbuffer_bits = 0;