On Fri, Sep 9, 2016 at 4:29 PM, Manasi Navare <manasi.d.navare@xxxxxxxxx> 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@xxxxxxxxx> > > 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@xxxxxxxxx> > Signed-off-by: Jim Bride <jim.bride@xxxxxxxxxxxxxxx> > Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx