Skip to contentSkip to conversation

drm/i915: don't send DP "idle" pattern before "normal" on HSW PORT_A

PATCH Applied Paulo Zanoni 3 participants 6 messages Jan 29, 2013 – Jan 31, 2013

Subsystems (2)
DRM drivers Intel i915
lore
Share & tools
lore

Paulo Zanoni

author
#0 lore raw

PATCH 1/3 drm/i915: don't send DP "idle" pattern before "normal" on HSW PORT_A

From: Paulo Zanoni <[email protected]>

The DP_TP_STATUS register for PORT_A doesn't exist. Our documentation will be fixed soon, so the code does not match it for now.

This solves "Timed out waiting for DP idle patterns" and "unclaimed register" messages on eDP.

V1: Was called "drm/i915: don't read DP_TP_STATUS(PORT_A)"
V2: Was called "drm/i915: don't send DP idle pattern before normal pattern on HSW"
V3: Only change the code that touches PORT_A.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/intel_dp.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/intel_dp.c +10 -6
index 51fd797..1b76b04 100644
@@ -1785,14 +1785,18 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 		temp &= ~DP_TP_CTL_LINK_TRAIN_MASK;
 		switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
 		case DP_TRAINING_PATTERN_DISABLE:
-			temp |= DP_TP_CTL_LINK_TRAIN_IDLE;
-			I915_WRITE(DP_TP_CTL(port), temp);
 
-			if (wait_for((I915_READ(DP_TP_STATUS(port)) &
-				      DP_TP_STATUS_IDLE_DONE), 1))
-				DRM_ERROR("Timed out waiting for DP idle patterns\n");
+			if (port != PORT_A) {
+				temp |= DP_TP_CTL_LINK_TRAIN_IDLE;
+				I915_WRITE(DP_TP_CTL(port), temp);
+
+				if (wait_for((I915_READ(DP_TP_STATUS(port)) &
+					      DP_TP_STATUS_IDLE_DONE), 1))
+					DRM_ERROR("Timed out waiting for DP idle patterns\n");
+
+				temp &= ~DP_TP_CTL_LINK_TRAIN_MASK;
+			}
 
-			temp &= ~DP_TP_CTL_LINK_TRAIN_MASK;
 			temp |= DP_TP_CTL_LINK_TRAIN_NORMAL;
 
 			break;
Signature · 1 line
1.7.10.4
author
#1 lore raw

PATCH 2/3 drm/i915: check the power down well on assert_pipe()

From: Paulo Zanoni <[email protected]>

If the power well is disabled, we should not try to read its registers, otherwise we'll get "unclaimed register" messages.

V2: Don't check whether the power well is enabled or not, just check whether we asked it to be enabled or not: if we asked to disable the power well, don't use the registers on it, even if it's still enabled.

V3: Fix bug that breaks all non-Haswell machines.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/intel_display.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
drivers/gpu/drm/i915/intel_display.c +9 -3
index 56c51dd..de3efdd 100644
@@ -1214,9 +1214,15 @@ void assert_pipe(struct drm_i915_private *dev_priv,
 	if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE)
 		state = true;
 
-	reg = PIPECONF(cpu_transcoder);
-	val = I915_READ(reg);
-	cur_state = !!(val & PIPECONF_ENABLE);
+	if (IS_HASWELL(dev_priv->dev) && cpu_transcoder != TRANSCODER_EDP &&
+	    !(I915_READ(HSW_PWR_WELL_DRIVER) & HSW_PWR_WELL_ENABLE)) {
+		cur_state = false;
+	} else {
+		reg = PIPECONF(cpu_transcoder);
+		val = I915_READ(reg);
+		cur_state = !!(val & PIPECONF_ENABLE);
+	}
+
 	WARN(cur_state != state,
 	     "pipe %c assertion failure (expected %s, current %s)\n",
 	     pipe_name(pipe), state_string(state), state_string(cur_state));
Signature · 1 line
1.7.10.4
author
#2 lore raw

PATCH 3/3 drm/i915: dynamic Haswell display power well support

In reply to Paulo Zanoni

From: Daniel Vetter <[email protected]>

We can disable (almost) all the display hw if we only use pipe A, with the integrated edp transcoder on port A. Because we don't set the cpu transcoder that early (yet), we need to help us with a trick to simply check for any edp encoders.

v2: Paulo Zanoni pointed out that we also need to configure the eDP cpu transcoder correctly.

v3: Made by Paulo Zanoni
- Rebase patch to be on top of "fix intel_init_power_wells" patch
- Fix typos
- Fix a small bug by adding a "connectors_active" check
- Restore the initial code that unconditionally enables the power well when taking over from the BIOS

v4: Made by Paulo Zanoni
- One more typo spotted by Jani Nikula

v5: Made by Paulo Zanoni
- Rebase

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/intel_ddi.c     |    8 +++++++-
 drivers/gpu/drm/i915/intel_display.c |   31 +++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

I know we might not want to apply this right now, but at least there's an updated version on the list :)

drivers/gpu/drm/i915/intel_ddi.c +7 -1
index 33b9112..cedf4ab 100644
@@ -988,7 +988,13 @@ void intel_ddi_enable_pipe_func(struct drm_crtc *crtc)
 	if (cpu_transcoder == TRANSCODER_EDP) {
 		switch (pipe) {
 		case PIPE_A:
-			temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
+			/* Can only use the always-on power well for eDP when
+			 * not using the panel fitter, and when not using motion
+			  * blur mitigation (which we don't support). */
+			if (dev_priv->pch_pf_size)
+				temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
+			else
+				temp |= TRANS_DDI_EDP_INPUT_A_ON;
 			break;
 		case PIPE_B:
 			temp |= TRANS_DDI_EDP_INPUT_B_ONOFF;
drivers/gpu/drm/i915/intel_display.c +31 -0
index de3efdd..d204ed2 100644
@@ -5607,6 +5607,35 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	return fdi_config_ok ? ret : -EINVAL;
 }
 
+static void haswell_modeset_global_resources(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	bool enable = false;
+	struct intel_crtc *crtc;
+	struct intel_encoder *encoder;
+
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
+		if (crtc->pipe != PIPE_A && crtc->base.enabled)
+			enable = true;
+		/* XXX: Should check for edp transcoder here, but thanks to init
+		 * sequence that's not yet available. Just in case desktop eDP
+		 * on PORT D is possible on haswell, too. */
+	}
+
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
+			    base.head) {
+		if (encoder->type != INTEL_OUTPUT_EDP &&
+		    encoder->connectors_active)
+			enable = true;
+	}
+
+	/* Even the eDP panel fitter is outside the always-on well. */
+	if (dev_priv->pch_pf_size)
+		enable = true;
+
+	intel_set_power_well(dev, enable);
+}
+
 static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 				 struct drm_display_mode *mode,
 				 struct drm_display_mode *adjusted_mode,

@@ -8486,6 +8515,8 @@ static void intel_init_display(struct drm_device *dev)
 		} else if (IS_HASWELL(dev)) {
 			dev_priv->display.fdi_link_train = hsw_fdi_link_train;
 			dev_priv->display.write_eld = haswell_write_eld;
+			dev_priv->display.modeset_global_resources =
+				haswell_modeset_global_resources;
 		}
 	} else if (IS_G4X(dev)) {
 		dev_priv->display.write_eld = g4x_write_eld;
Signature · 1 line
1.7.10.4
maintainer Reviewed-by on v1 1/3
#3 lore raw
In reply to Paulo Zanoni

On the series,

Reviewed-by Jani Nikula <[email protected]>
Quoted 54 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

The DP_TP_STATUS register for PORT_A doesn't exist. Our documentation will be fixed soon, so the code does not match it for now.

This solves "Timed out waiting for DP idle patterns" and "unclaimed register" messages on eDP.

V1: Was called "drm/i915: don't read DP_TP_STATUS(PORT_A)"
V2: Was called "drm/i915: don't send DP idle pattern before normal pattern on HSW"
V3: Only change the code that touches PORT_A.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/intel_dp.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/intel_dp.c +10 -6
index 51fd797..1b76b04 100644
@@ -1785,14 +1785,18 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 		temp &= ~DP_TP_CTL_LINK_TRAIN_MASK;
 		switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
 		case DP_TRAINING_PATTERN_DISABLE:
-			temp |= DP_TP_CTL_LINK_TRAIN_IDLE;
-			I915_WRITE(DP_TP_CTL(port), temp);
 
-			if (wait_for((I915_READ(DP_TP_STATUS(port)) &
-				      DP_TP_STATUS_IDLE_DONE), 1))
-				DRM_ERROR("Timed out waiting for DP idle patterns\n");
+			if (port != PORT_A) {
+				temp |= DP_TP_CTL_LINK_TRAIN_IDLE;
+				I915_WRITE(DP_TP_CTL(port), temp);
+
+				if (wait_for((I915_READ(DP_TP_STATUS(port)) &
+					      DP_TP_STATUS_IDLE_DONE), 1))
+					DRM_ERROR("Timed out waiting for DP idle patterns\n");
+
+				temp &= ~DP_TP_CTL_LINK_TRAIN_MASK;
+			}
 
-			temp &= ~DP_TP_CTL_LINK_TRAIN_MASK;
 			temp |= DP_TP_CTL_LINK_TRAIN_NORMAL;
 
 			break;
Signature · 6 lines
1.7.10.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#4 lore raw
Quoted 4 lines · Jani Nikula

On the series,

Reviewed-by Jani Nikula <[email protected]>

Merged the series to dinq, thanks patches and review. -Daniel

Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Applied
#5 lore raw
In reply to Paulo Zanoni
Quoted 29 lines · Paulo Zanoni

From: Paulo Zanoni <[email protected]>

The DP_TP_STATUS register for PORT_A doesn't exist. Our documentation will be fixed soon, so the code does not match it for now.

This solves "Timed out waiting for DP idle patterns" and "unclaimed register" messages on eDP.

V1: Was called "drm/i915: don't read DP_TP_STATUS(PORT_A)"
V2: Was called "drm/i915: don't send DP idle pattern before normal pattern on HSW"
V3: Only change the code that touches PORT_A.

Signed-off-by Paulo Zanoni <[email protected]>
---
 drivers/gpu/drm/i915/intel_dp.c |   16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/gpu/drm/i915/intel_dp.c +0 -2
index 51fd797..1b76b04 100644
@@ -1785,14 +1785,18 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 		temp &= ~DP_TP_CTL_LINK_TRAIN_MASK;
 		switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
 		case DP_TRAINING_PATTERN_DISABLE:
-			temp |= DP_TP_CTL_LINK_TRAIN_IDLE;
-			I915_WRITE(DP_TP_CTL(port), temp);

drivers/gpu/drm/i915/intel_dp.c intel_dp_set_link_train(struct intel_dp *intel_dp, · line 1795

Quoted 10 lines
drivers/gpu/drm/i915/intel_dp.c +7 -3
-			if (wait_for((I915_READ(DP_TP_STATUS(port)) &
-				      DP_TP_STATUS_IDLE_DONE), 1))
-				DRM_ERROR("Timed out waiting for DP idle patterns\n");
+			if (port != PORT_A) {
+				temp |= DP_TP_CTL_LINK_TRAIN_IDLE;
+				I915_WRITE(DP_TP_CTL(port), temp);
+
+				if (wait_for((I915_READ(DP_TP_STATUS(port)) &
+					      DP_TP_STATUS_IDLE_DONE), 1))
+					DRM_ERROR("Timed out waiting for DP idle patterns\n");

I checkpatch complained about the long line, and I count about 5 levels of indentation. The function is also growing a bit long in lines. Can you please cut out per-platform helpers or something like that as a follow-up?

Queued for -next, thanks for the patch.
-Daniel

Quoted 15 lines
drivers/gpu/drm/i915/intel_dp.c +3 -1
+
+				temp &= ~DP_TP_CTL_LINK_TRAIN_MASK;
+			}
 
-			temp &= ~DP_TP_CTL_LINK_TRAIN_MASK;
 			temp |= DP_TP_CTL_LINK_TRAIN_NORMAL;
 
 			break;
Signature · 6 lines
1.7.10.4

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch