[Intel-gfx] [PATCH 3/3] drm/i915: Support DDI lane reversal for DP
Benjamin Tissoires
benjamin.tissoires at redhat.com
Thu Jul 30 07:44:52 PDT 2015
On Jul 30 2015 or thereabouts, Sivakumar Thulasimani wrote:
>
>
> On 7/29/2015 8:52 PM, Benjamin Tissoires wrote:
> >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
> No, what i recommended was to do link training but in intel_dp_detect. Since
> USB Type C cable
> also has its own lane count restriction (it can have different lane count
> than the one supported
> by panel) you might have to figure that out as well. so both reversal and
> lane count detection
> can be done outside the modeset path and keep the code free of type C
> changes outside
> detection path.
Thanks for the detailed explanations. Now I see what you mean and
definitively understand why this is better.
>
> Please find below the code to do the same. Do not waste time trying to apply
> this directly on
> nightly since this is based on a local tree and because this is pre- atomic
> changes code, so you
> might have to modify chv_upfront_link_train to work on top of the latest
> nightly code. we
> are supposed to upstream this and is in my todo list.
Thanks for this patch. I'll try to adapt it to test on the Broadwell
laptop I have and get the reversed state detected here.
Cheers,
Benjamin
>
> ---------------------------------------------------------------------------------------------------------------------------
>
> Author: Durgadoss R <durgadoss.r at intel.com>
> Date: Fri May 22 14:30:07 2015 +0530
>
> drm/i915: Enable Upfront link training for type-C DP support
>
> To support USB type C alternate DP mode, the display driver needs to
> know the
> number of lanes required by DP panel as well as number of lanes that can
> be
> supported by the type-C cable. Sometimes, the type-C cable may limit the
> bandwidth even if Panel can support more lanes. To address these
> scenarios,
> the display driver will start link training with max lanes, and if the
> link
> training fails, the driver then falls back to x2 lanes.
>
> * Since link training is done before modeset, planes are not enabled.
> Only
> encoder and the its associated PLLs are enabled.
> * Once link training is done, the encoder and its PLLs are disabled; so
> that
> the subsequent modeset is not aware of these changes.
> * As of now, this is tested only on CHV.
>
> Signed-off-by: Durgadoss R <durgadoss.r at intel.com>
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 0c8ae2a..c72dcaa 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14793,3 +14793,121 @@ intel_display_print_error_state(struct
> drm_i915_error_state_buf *m,
> err_printf(m, " VSYNC: %08x\n", error->transcoder[i].vsync);
> }
> }
> +
> +bool chv_upfront_link_train(struct drm_device *dev,
> + struct intel_dp *intel_dp, struct intel_crtc *crtc)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_connector *connector = intel_dp->attached_connector;
> + struct intel_encoder *encoder = connector->encoder;
> + bool found = false;
> + bool valid_crtc = false;
> +
> + if (!connector || !encoder) {
> + DRM_DEBUG_KMS("dp connector/encoder is NULL\n");
> + return false;
> + }
> +
> + /* If we already have a crtc, start link training directly */
> + if (crtc) {
> + valid_crtc = true;
> + goto start_link_train;
> + }
> +
> + /* Find an unused crtc and use it for link training */
> + for_each_intel_crtc(dev, crtc) {
> + if (intel_crtc_active(&crtc->base))
> + continue;
> +
> + connector->new_encoder = encoder;
> + encoder->new_crtc = crtc;
> + encoder->base.crtc = &crtc->base;
> +
> + /* Make sure the new crtc will work with the encoder */
> + if (drm_encoder_crtc_ok(&encoder->base,
> + &crtc->base)) {
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found) {
> + DRM_ERROR("Could not find crtc for upfront link training\n");
> + return false;
> + }
> +
> +start_link_train:
> +
> + DRM_DEBUG_KMS("upfront link training on pipe:%c\n",
> + pipe_name(crtc->pipe));
> + found = false;
> +
> + /* Initialize with Max Link rate & lane count supported by panel */
> + intel_dp->link_bw = intel_dp->dpcd[DP_MAX_LINK_RATE];
> + intel_dp->lane_count = intel_dp->dpcd[DP_MAX_LANE_COUNT] &
> + DP_MAX_LANE_COUNT_MASK;
> +
> + do {
> + /* Find port clock from link_bw */
> + crtc->config.port_clock =
> + drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
> +
> + /* Enable PLL followed by port */
> + intel_dp_set_clock(encoder, &crtc->config, intel_dp->link_bw);
> + chv_update_pll(crtc);
> + encoder->pre_pll_enable(encoder);
> + chv_enable_pll(crtc);
> + encoder->pre_enable(encoder);
> +
> + /* Check if link training passed; if so update lane count */
> + if (intel_dp->train_set_valid) {
> + intel_dp->dpcd[DP_MAX_LANE_COUNT] &=
> + ~DP_MAX_LANE_COUNT_MASK;
> + intel_dp->dpcd[DP_MAX_LANE_COUNT] |=
> + intel_dp->lane_count & DP_MAX_LANE_COUNT_MASK;
> +
> + found = true;
> + }
> +
> + /* Reset encoder for next retry or for clean up */
> + encoder->disable(encoder);
> + encoder->post_disable(encoder);
> + chv_disable_pll(dev_priv, crtc->pipe);
> +
> + if (found)
> + goto exit;
> +
> + DRM_DEBUG_KMS("upfront link training failed. lanes:%d bw:%d\n",
> + intel_dp->lane_count, intel_dp->link_bw);
> +
> + /* Go down to the next level and retry link training */
> + if (intel_dp->lane_count == 4) {
> + intel_dp->lane_count = 2;
> + } else if (intel_dp->lane_count == 2) {
> + intel_dp->lane_count = 1;
> + } else if (intel_dp->link_bw == DP_LINK_BW_5_4) {
> + intel_dp->link_bw = DP_LINK_BW_2_7;
> + intel_dp->lane_count = 4;
> + } else if (intel_dp->link_bw == DP_LINK_BW_2_7) {
> + intel_dp->link_bw = DP_LINK_BW_1_62;
> + intel_dp->lane_count = 4;
> + } else {
> + /* Tried all combinations, so exit */
> + break;
> + }
> +
> + } while (1);
> +
> +exit:
> + /* Clear local associations made */
> + if (!valid_crtc) {
> + connector->new_encoder = NULL;
> + encoder->new_crtc = NULL;
> + encoder->base.crtc = NULL;
> + }
> +
> + if (found)
> + DRM_DEBUG_KMS("upfront link training passed. lanes:%d bw:%d\n",
> + intel_dp->lane_count, intel_dp->link_bw);
> + return found;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 97f03bf..394a7a5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -878,7 +878,7 @@ intel_dp_connector_unregister(struct intel_connector
> *intel_connector)
> intel_connector_unregister(intel_connector);
> }
>
> -static void
> +void
> intel_dp_set_clock(struct intel_encoder *encoder,
> struct intel_crtc_config *pipe_config, int link_bw)
> {
> @@ -1010,7 +1010,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> link_clock = drm_dp_bw_code_to_link_rate(bws[clock]);
> link_avail = intel_dp_max_data_rate(link_clock,
> lane_count);
> -
> if (mode_rate <= link_avail) {
> goto found;
> }
> @@ -4522,6 +4521,11 @@ g4x_dp_detect(struct intel_dp *intel_dp)
> if ((I915_READ(PORT_HOTPLUG_STAT) & bit) == 0)
> return connector_status_disconnected;
>
> + /* Avoid DPCD opertations if status is same */
> + if (intel_dp->attached_connector->base.status ==
> + connector_status_connected)
> + return connector_status_connected;
> +
> return intel_dp_detect_dpcd(intel_dp);
> }
>
> @@ -4566,11 +4570,13 @@ intel_dp_detect(struct drm_connector *connector,
> bool force)
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> struct intel_encoder *intel_encoder = &intel_dig_port->base;
> + struct drm_crtc *crtc = intel_dig_port->base.base.crtc;
> struct drm_device *dev = connector->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> enum drm_connector_status status;
> enum intel_display_power_domain power_domain;
> struct edid *edid = NULL;
> + struct intel_crtc *intel_crtc = NULL;
>
> intel_runtime_pm_get(dev_priv);
>
> @@ -4590,6 +4596,11 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
> if (status != connector_status_connected)
> goto out;
>
> + if (connector->status == connector_status_connected) {
> + DRM_DEBUG_KMS("Connector status is already connected\n");
> + goto out;
> + }
> +
> intel_dp_probe_oui(intel_dp);
>
> if (intel_dp->force_audio != HDMI_AUDIO_AUTO) {
> @@ -4604,6 +4615,22 @@ intel_dp_detect(struct drm_connector *connector, bool
> force)
>
> if (intel_encoder->type != INTEL_OUTPUT_EDP)
> intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
> +
> + if (IS_CHERRYVIEW(dev) &&
> + intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT) {
> + /* Handle connected boot scenario where panel is enabled
> + * by GOP/VBIOS.
> + */
> + if (intel_encoder->connectors_active &&
> + crtc && crtc->enabled) {
> + intel_crtc = to_intel_crtc(crtc);
> + DRM_DEBUG_KMS("Disabling crtc %c for upfront link training\n",
> + pipe_name(intel_crtc->pipe));
> + intel_crtc_control(crtc, false);
> + }
> + chv_upfront_link_train(dev, intel_dp, intel_crtc);
> + }
> +
> status = connector_status_connected;
>
> out:
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index fc266da..15eef94d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -991,6 +991,10 @@ void intel_dp_start_link_train(struct intel_dp
> *intel_dp);
> void intel_dp_complete_link_train(struct intel_dp *intel_dp);
> void intel_dp_stop_link_train(struct intel_dp *intel_dp);
> bool intel_dp_fast_link_train(struct intel_dp *intel_dp);
> +bool chv_upfront_link_train(struct drm_device *dev,
> + struct intel_dp *intel_dp, struct intel_crtc *crtc);
> +void intel_dp_set_clock(struct intel_encoder *encoder,
> + struct intel_crtc_config *pipe_config, int link_bw);
> void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
> void intel_dp_set_m2_n2(struct intel_crtc *crtc, struct intel_link_m_n
> *m_n);
> void intel_dp_encoder_destroy(struct drm_encoder *encoder);
>
> --
> regards,
> Sivakumar
>
More information about the Intel-gfx
mailing list