Re: [PATCH 3/3] drm/i915: Support DDI lane reversal for DP

[Benjamin Tissoires <benjamin.tissoires@redhat.com>]

On Jul 29 2015 or thereabouts, Sivakumar Thulasimani wrote:
> why not detect reverse in intel_dp_detect/intel_hpd_pulse ? that way you can
> identify both lane count and reversal state without touching anything in the
> link training code. i am yet to upstream my changes for CHT that i can share
> if required that does the same in intel_dp_detect without touching any line
> in link training path.

With my current limited knowledge of the dp hotplug (and i915 driver) I
am not sure we could detect the reversed state without trying to train 1
lane only. I'd be glad to look at your changes and test them on my
system if you think that could help having a cleaner solution.

Cheers,
Benjamin

> 
> On 7/28/2015 9:33 PM, Benjamin Tissoires wrote:
> >The DP outputs connected through a USB Type-C port can have inverted
> >lanes. To detect that case, we implement autodetection by training only
> >the first lane if it doesn't work, we assume that we need to invert
> >the lanes.
> >
> >Tested on a Chromebook Pixel 2015 (samus) with a USB Type-C to HDMI
> >adapter and a Dell 4K and some various regular monitors.
> >
> >Based on 2 patches from the ChromeOS tree by:
> >Stéphane Marchesin <marcheu@chromium.org>
> >Todd Broch <tbroch@chromium.org>
> >
> >Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >---
> >  drivers/gpu/drm/i915/intel_ddi.c | 13 +++++++++++++
> >  drivers/gpu/drm/i915/intel_dp.c  | 36 ++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h |  1 +
> >  3 files changed, 50 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >index 9a40bfb..0b0c1ec 100644
> >--- a/drivers/gpu/drm/i915/intel_ddi.c
> >+++ b/drivers/gpu/drm/i915/intel_ddi.c
> >@@ -2249,6 +2249,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
> >  	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> >  	int type = intel_encoder->type;
> >  	int hdmi_level;
> >+	bool reversed = false;
> >  	if (type == INTEL_OUTPUT_EDP) {
> >  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> >@@ -2295,8 +2296,20 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
> >  	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
> >  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> >+		if (IS_BROADWELL(dev) && type == INTEL_OUTPUT_DISPLAYPORT) {
> >+			intel_ddi_init_dp_buf_reg(intel_encoder);
> >+			reversed = intel_dp_is_reversed(intel_dp);
> >+		}
> >+
> >  		intel_ddi_init_dp_buf_reg(intel_encoder);
> >+		if (IS_BROADWELL(dev)) {
> >+			if (reversed)
> >+				intel_dp->DP |= DDI_BUF_PORT_REVERSAL;
> >+			else
> >+				intel_dp->DP &= ~DDI_BUF_PORT_REVERSAL;
> >+		}
> >+
> >  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> >  		intel_dp_start_link_train(intel_dp);
> >  		intel_dp_complete_link_train(intel_dp);
> >diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >index b740987..18280cc 100644
> >--- a/drivers/gpu/drm/i915/intel_dp.c
> >+++ b/drivers/gpu/drm/i915/intel_dp.c
> >@@ -3820,6 +3820,42 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
> >  		intel_dp->DP = DP;
> >  }
> >+bool intel_dp_is_reversed(struct intel_dp *intel_dp)
> >+{
> >+	struct drm_encoder *encoder = &dp_to_dig_port(intel_dp)->base.base;
> >+	struct drm_device *dev = encoder->dev;
> >+	struct drm_i915_private *dev_priv = dev->dev_private;
> >+	uint32_t DP = intel_dp->DP;
> >+
> >+	/*
> >+	 * Train with 1 lane. There is no guarantee that the monitor supports
> >+	 * 2 or 4 lanes, and we wouldn't see any asymetricity with 4 lanes.
> >+	 */
> >+	const uint8_t lane_count = 1;
> >+	bool reversed;
> >+
> >+	if (!HAS_DDI(dev))
> >+		return false;
> >+
> >+	DP &= ~(DDI_BUF_PORT_REVERSAL | DDI_PORT_WIDTH(4));
> >+	DP |= DDI_PORT_WIDTH(lane_count);
> >+
> >+	I915_WRITE(intel_dp->output_reg, DP);
> >+	POSTING_READ(intel_dp->output_reg);
> >+	udelay(600);
> >+
> >+	if (!_intel_dp_start_link_train(intel_dp, lane_count, &DP, true))
> >+		return true;
> >+
> >+	reversed = !_intel_dp_complete_link_train(intel_dp, lane_count, &DP, true);
> >+
> >+	/* clear training, we had only one lane */
> >+	intel_dp->train_set_valid = false;
> >+
> >+	return reversed;
> >+
> >+}
> >+
> >  void intel_dp_stop_link_train(struct intel_dp *intel_dp)
> >  {
> >  	intel_dp_set_link_train(intel_dp, &intel_dp->DP,
> >diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >index 320c9e6..cba00c6 100644
> >--- a/drivers/gpu/drm/i915/intel_drv.h
> >+++ b/drivers/gpu/drm/i915/intel_drv.h
> >@@ -1169,6 +1169,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc);
> >  bool intel_dp_compute_config(struct intel_encoder *encoder,
> >  			     struct intel_crtc_state *pipe_config);
> >  bool intel_dp_is_edp(struct drm_device *dev, enum port port);
> >+bool intel_dp_is_reversed(struct intel_dp *intel_dp);
> >  enum irqreturn intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port,
> >  				  bool long_hpd);
> >  void intel_edp_backlight_on(struct intel_dp *intel_dp);
> 
> -- 
> regards,
> Sivakumar
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx