[Intel-gfx] [PATCH v16 13-2/14] drm/i915/dp: Enable Upfront link training on HSW/BDW/SKL/BXT

Rodrigo Vivi rodrigo.vivi at gmail.com
Tue Sep 13 00:22:54 UTC 2016


On Fri, Sep 9, 2016 at 4:29 PM, Manasi Navare <manasi.d.navare at intel.com> wrote:
> To support USB type C alternate DP mode, the display driver needs to
> know the number of lanes required by the 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 we need to train the link before
> modeset. This upfront link training caches the values of max link rate
> and max lane count that get used later during modeset. Upfront link
> training does not change any HW state, the link is disabled and PLL
> values are reset to previous values after upfront link tarining so
> that subsequent modeset is not aware of these changes.
>
> This patch is based on prior work done by
> R,Durgadoss <durgadoss.r at intel.com>
>
> Changes since v15:
> * Split this patch into two patches - one with functional
> changes to enable upfront and other with moving the existing
> functions around so that they can be used for upfront (Jani Nikula)
> * Cleaned up the commit message
>
> Signed-off-by: Durgadoss R <durgadoss.r at intel.com>
> Signed-off-by: Jim Bride <jim.bride at linux.intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c              |  21 ++-
>  drivers/gpu/drm/i915/intel_dp.c               | 196 +++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_dp_link_training.c |   1 -
>  drivers/gpu/drm/i915/intel_drv.h              |  14 +-
>  4 files changed, 221 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 2c13d0c..2eee5b5 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1673,7 +1673,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>         pll->config.crtc_mask = 0;
>
>         /* If Link Training fails, send a uevent to generate a hotplug */
> -       if (!intel_ddi_link_train(intel_dp, link_rate, lane_count, link_mst))
> +       if (!intel_ddi_link_train(intel_dp, link_rate, lane_count, link_mst,
> +                                 false))
>                 drm_kms_helper_hotplug_event(encoder->base.dev);
>         pll->config = tmp_pll_config;
>  }
> @@ -2460,7 +2461,7 @@ intel_ddi_get_link_dpll(struct intel_dp *intel_dp, int clock)
>
>  bool
>  intel_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate,
> -                    uint8_t max_lane_count, bool link_mst)
> +                    uint8_t max_lane_count, bool link_mst, bool is_upfront)
>  {
>         struct intel_connector *connector = intel_dp->attached_connector;
>         struct intel_encoder *encoder = connector->encoder;
> @@ -2509,6 +2510,7 @@ intel_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate,
>                         pll->funcs.disable(dev_priv, pll);
>                         pll->config = tmp_pll_config;
>                 }
> +
>                 if (ret) {
>                         DRM_DEBUG_KMS("Link Training successful at link rate: "
>                                       "%d lane:%d\n", link_rate, lane_count);
> @@ -2517,6 +2519,21 @@ intel_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate,
>         }
>         intel_dp_stop_link_train(intel_dp);
>
> +       if (is_upfront) {
> +               DRM_DEBUG_KMS("Upfront link train %s: link_clock:%d lanes:%d\n",
> +                             ret ? "Passed" : "Failed",
> +                             link_rate, lane_count);
> +               /* Disable port followed by PLL for next retry/clean up */
> +               intel_ddi_post_disable(encoder, NULL, NULL);
> +               pll->funcs.disable(dev_priv, pll);
> +               pll->config = tmp_pll_config;
> +               if (ret) {
> +                       /* Save the upfront values */
> +                       intel_dp->max_lanes_upfront = lane_count;
> +                       intel_dp->max_link_rate_upfront = link_rate;
> +               }
> +       }
> +
>         if (!lane_count)
>                 DRM_ERROR("Link Training Failed\n");
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 429ecf8..18c663d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -153,12 +153,21 @@ intel_dp_max_link_bw(struct intel_dp  *intel_dp)
>  static u8 intel_dp_max_lane_count(struct intel_dp *intel_dp)
>  {
>         struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> -       u8 source_max, sink_max;
> +       u8 temp, source_max, sink_max;
>
>         source_max = intel_dig_port->max_lanes;
>         sink_max = drm_dp_max_lane_count(intel_dp->dpcd);
>
> -       return min(source_max, sink_max);
> +       temp = min(source_max, sink_max);
> +
> +       /*
> +        * Limit max lanes w.r.t to the max value found
> +        * using Upfront link training also.
> +        */
> +       if (intel_dp->max_lanes_upfront)
> +               return min(temp, intel_dp->max_lanes_upfront);
> +       else
> +               return temp;
>  }
>
>  /*
> @@ -190,6 +199,42 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
>         return (max_link_clock * max_lanes * 8) / 10;
>  }
>
> +static int intel_dp_upfront_crtc_disable(struct intel_crtc *crtc,
> +                                        struct drm_modeset_acquire_ctx *ctx,
> +                                        bool enable)
> +{
> +       int ret;
> +       struct drm_atomic_state *state;
> +       struct intel_crtc_state *crtc_state;
> +       struct drm_device *dev = crtc->base.dev;
> +       enum pipe pipe = crtc->pipe;
> +
> +       state = drm_atomic_state_alloc(dev);
> +       if (!state)
> +               return -ENOMEM;
> +
> +       state->acquire_ctx = ctx;
> +
> +       crtc_state = intel_atomic_get_crtc_state(state, crtc);
> +       if (IS_ERR(crtc_state)) {
> +               ret = PTR_ERR(crtc_state);
> +               drm_atomic_state_free(state);
> +               return ret;
> +       }
> +
> +       DRM_DEBUG_KMS("%sabling crtc %c %s upfront link train\n",
> +                       enable ? "En" : "Dis",
> +                       pipe_name(pipe),
> +                       enable ? "after" : "before");
> +
> +       crtc_state->base.active = enable;
> +       ret = drm_atomic_commit(state);
> +       if (ret)
> +               drm_atomic_state_free(state);
> +
> +       return ret;
> +}
> +
>  static int
>  intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
>  {
> @@ -274,6 +319,17 @@ static int intel_dp_common_rates(struct intel_dp *intel_dp,
>         int source_len, sink_len;
>
>         sink_len = intel_dp_sink_rates(intel_dp, &sink_rates);
> +
> +       /* Cap sink rates w.r.t upfront values */
> +       if (intel_dp->max_link_rate_upfront) {
> +               int len = sink_len - 1;
> +
> +               while (len > 0 && sink_rates[len] >
> +                      intel_dp->max_link_rate_upfront)
> +                       len--;
> +               sink_len = len + 1;
> +       }
> +
>         source_len = intel_dp_source_rates(intel_dp, &source_rates);
>
>         return intersect_rates(source_rates, source_len,
> @@ -281,6 +337,92 @@ static int intel_dp_common_rates(struct intel_dp *intel_dp,
>                                common_rates);
>  }
>
> +static bool intel_dp_upfront_link_train(struct intel_dp *intel_dp)
> +{
> +       struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> +       struct intel_encoder *intel_encoder = &intel_dig_port->base;
> +       struct drm_device *dev = intel_encoder->base.dev;
> +       struct drm_i915_private *dev_priv = to_i915(dev);
> +       struct drm_mode_config *config = &dev->mode_config;
> +       struct drm_modeset_acquire_ctx ctx;
> +       struct intel_crtc *intel_crtc;
> +       struct drm_crtc *crtc = NULL;
> +       struct intel_shared_dpll *pll;
> +       struct intel_shared_dpll_config tmp_pll_config;
> +       bool disable_dpll = false;
> +       int ret;
> +       bool done = false, has_mst = false;
> +       uint8_t max_lanes;
> +       int common_rates[DP_MAX_SUPPORTED_RATES] = {};
> +       int common_len;
> +       enum intel_display_power_domain power_domain;
> +
> +       power_domain = intel_display_port_power_domain(intel_encoder);
> +       intel_display_power_get(dev_priv, power_domain);
> +
> +       common_len = intel_dp_common_rates(intel_dp, common_rates);
> +       max_lanes = intel_dp_max_lane_count(intel_dp);
> +       if (WARN_ON(common_len <= 0))
> +               return true;
> +
> +       drm_modeset_acquire_init(&ctx, 0);
> +retry:
> +       ret = drm_modeset_lock(&config->connection_mutex, &ctx);
> +       if (ret)
> +               goto exit_fail;
> +
> +       if (intel_encoder->base.crtc) {
> +               crtc = intel_encoder->base.crtc;
> +
> +               ret = drm_modeset_lock(&crtc->mutex, &ctx);
> +               if (ret)
> +                       goto exit_fail;
> +
> +               ret = drm_modeset_lock(&crtc->primary->mutex, &ctx);
> +               if (ret)
> +                       goto exit_fail;
> +
> +               intel_crtc = to_intel_crtc(crtc);
> +               pll = intel_crtc->config->shared_dpll;
> +               disable_dpll = true;
> +               has_mst = intel_crtc_has_type(intel_crtc->config,
> +                                             INTEL_OUTPUT_DP_MST);
> +               ret = intel_dp_upfront_crtc_disable(intel_crtc, &ctx, false);
> +               if (ret)
> +                       goto exit_fail;
> +       }
> +
> +       mutex_lock(&dev_priv->dpll_lock);
> +       if (disable_dpll) {
> +               /* Clear the PLL config state */
> +               tmp_pll_config = pll->config;
> +               pll->config.crtc_mask = 0;
> +       }
> +
> +       done = intel_dp->upfront_link_train(intel_dp,
> +                                           common_rates[common_len-1],
> +                                           max_lanes,
> +                                           has_mst,
> +                                           true);
> +       if (disable_dpll)
> +               pll->config = tmp_pll_config;
> +
> +       mutex_unlock(&dev_priv->dpll_lock);
> +
> +       if (crtc)
> +               ret = intel_dp_upfront_crtc_disable(intel_crtc, &ctx, true);
> +
> +exit_fail:
> +       if (ret == -EDEADLK) {
> +               drm_modeset_backoff(&ctx);
> +               goto retry;
> +       }
> +       drm_modeset_drop_locks(&ctx);
> +       drm_modeset_acquire_fini(&ctx);
> +       intel_display_power_put(dev_priv, power_domain);
> +       return done;
> +}
> +
>  static enum drm_mode_status
>  intel_dp_mode_valid(struct drm_connector *connector,
>                     struct drm_display_mode *mode)
> @@ -302,6 +444,19 @@ intel_dp_mode_valid(struct drm_connector *connector,
>                 target_clock = fixed_mode->clock;
>         }
>
> +       if (intel_dp->upfront_link_train && !intel_dp->upfront_done) {
> +               bool do_upfront_link_train;
> +               /* Do not do upfront link train, if it is a compliance
> +                * request
> +                */
> +               do_upfront_link_train = !intel_dp->upfront_done &&
> +                       (intel_dp->compliance_test_type !=
> +                        DP_TEST_LINK_TRAINING);
> +
> +               if (do_upfront_link_train)
> +                       intel_dp->upfront_done = intel_dp_upfront_link_train(intel_dp);
> +       }
> +
>         max_link_clock = intel_dp_max_link_rate(intel_dp);
>         max_lanes = intel_dp_max_lane_count(intel_dp);
>
> @@ -1436,6 +1591,9 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp)
>         int rates[DP_MAX_SUPPORTED_RATES] = {};
>         int len;
>
> +       if (intel_dp->max_link_rate_upfront)
> +               return intel_dp->max_link_rate_upfront;
> +
>         len = intel_dp_common_rates(intel_dp, rates);
>         if (WARN_ON(len <= 0))
>                 return 162000;
> @@ -1488,7 +1646,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>         enum port port = dp_to_dig_port(intel_dp)->port;
>         struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>         struct intel_connector *intel_connector = intel_dp->attached_connector;
> -       int lane_count, clock;
> +       int lane_count, clock = 0;

minor, but I believe you don't need this.

>         int min_lane_count = 1;
>         int max_lane_count = intel_dp_max_lane_count(intel_dp);
>         /* Conveniently, the link BW constants become indices with a shift...*/
> @@ -1567,6 +1725,21 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>         for (; bpp >= 6*3; bpp -= 2*3) {
>                 mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
>                                                    bpp);
> +
> +               if (!is_edp(intel_dp) && intel_dp->upfront_done) {
> +                       clock = max_clock;
> +                       lane_count = intel_dp->max_lanes_upfront;
> +                       link_clock = intel_dp->max_link_rate_upfront;
> +                       link_avail = intel_dp_max_data_rate(link_clock,
> +                                                           lane_count);
> +                       mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
> +                                                          bpp);
> +                       if (mode_rate <= link_avail)
> +                               goto found;
> +                       else
> +                               continue;
> +               }
> +
>                 clock = max_clock;
>                 lane_count = max_lane_count;
>                 link_clock = common_rates[clock];
> @@ -1595,7 +1768,6 @@ found:
>         }
>
>         pipe_config->lane_count = lane_count;
> -
>         pipe_config->pipe_bpp = bpp;
>         pipe_config->port_clock = common_rates[clock];
>
> @@ -4279,7 +4451,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>         struct drm_device *dev = connector->dev;
>         enum drm_connector_status status;
>         enum intel_display_power_domain power_domain;
> -       u8 sink_irq_vector = 0;
> +       u8 sink_irq_vector;

out of context... Why do you need this on this patch?
or separated patch?

>
>         power_domain = intel_display_port_aux_power_domain(intel_encoder);
>         intel_display_power_get(to_i915(dev), power_domain);
> @@ -4372,9 +4544,12 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>         }
>
>  out:
> -       if ((status != connector_status_connected) &&
> -           (intel_dp->is_mst == false))

This should be in a separated patch I believe. There is no explanation
why you don't need this for MST

> +       if (status != connector_status_connected) {
>                 intel_dp_unset_edid(intel_dp);
> +               intel_dp->upfront_done = false;
> +               intel_dp->max_lanes_upfront = 0;
> +               intel_dp->max_link_rate_upfront = 0;
> +       }
>
>         intel_display_power_put(to_i915(dev), power_domain);
>         return;
> @@ -5618,6 +5793,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>         if (type == DRM_MODE_CONNECTOR_eDP)
>                 intel_encoder->type = INTEL_OUTPUT_EDP;
>
> +       /* Initialize upfront link training vfunc for DP */
> +       if (intel_encoder->type != INTEL_OUTPUT_EDP) {
> +               if (IS_BROXTON(dev_priv) || IS_SKYLAKE(dev_priv) ||
> +                   IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))

This should be HAS_DDI(dev_priv), otherwise we have the risk to forget
a platform, like we are leaving Kabylake behind.

> +                       intel_dp->upfront_link_train = intel_ddi_link_train;
> +       }
> +
>         /* eDP only on port B and/or C on vlv/chv */
>         if (WARN_ON((IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) &&
>                     is_edp(intel_dp) && port != PORT_B && port != PORT_C))
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index f1e08f0..b6f380b 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -304,7 +304,6 @@ intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp)
>         intel_dp_set_idle_link_train(intel_dp);
>
>         return intel_dp->channel_eq_status;
> -
>  }
>
>  void intel_dp_stop_link_train(struct intel_dp *intel_dp)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5b97a7d4..e5ab375 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -882,6 +882,12 @@ struct intel_dp {
>         enum hdmi_force_audio force_audio;
>         bool limited_color_range;
>         bool color_range_auto;
> +
> +       /* Upfront link train parameters */
> +       int max_link_rate_upfront;
> +       uint8_t max_lanes_upfront;
> +       bool upfront_done;
> +
>         uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
>         uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
>         uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> @@ -939,6 +945,11 @@ struct intel_dp {
>         /* This is called before a link training is starterd */
>         void (*prepare_link_retrain)(struct intel_dp *intel_dp);
>
> +       /* For Upfront link training */
> +       bool (*upfront_link_train)(struct intel_dp *intel_dp, int clock,
> +                                  uint8_t lane_count, bool link_mst,
> +                                  bool is_upfront);
> +
>         /* Displayport compliance testing */
>         unsigned long compliance_test_type;
>         unsigned long compliance_test_data;
> @@ -1161,7 +1172,8 @@ void intel_ddi_clock_get(struct intel_encoder *encoder,
>  void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state);
>  uint32_t ddi_signal_levels(struct intel_dp *intel_dp);
>  bool intel_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate,
> -                         uint8_t max_lane_count, bool link_mst);
> +                         uint8_t max_lane_count, bool link_mst,
> +                         bool is_upfront);
>  struct intel_shared_dpll *intel_ddi_get_link_dpll(struct intel_dp *intel_dp,
>                                                   int clock);
>  unsigned int intel_fb_align_height(struct drm_device *dev,
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br


More information about the Intel-gfx mailing list