[3/3] drm/i915: Support DDI lane reversal for DP
diff mbox

Message ID 1438099409-25456-4-git-send-email-benjamin.tissoires@redhat.com
State New
Headers show

Commit Message

Benjamin Tissoires July 28, 2015, 4:03 p.m. UTC
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(+)

Comments

Sivakumar Thulasimani July 29, 2015, 8:26 a.m. UTC | #1
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.

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);
Benjamin Tissoires July 29, 2015, 3:22 p.m. UTC | #2
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
>

Patch
diff mbox

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);