| Message ID | 1438099409-25456-4-git-send-email-benjamin.tissoires@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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);
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 >
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);
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(+)