> -----Original Message----- > From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] > Sent: Wednesday, July 27, 2016 3:28 PM > To: R, Durgadoss <durgadoss.r@xxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Conselvan De Oliveira, Ander > <ander.conselvan.de.oliveira@xxxxxxxxx>; Bride, Jim <jim.bride@xxxxxxxxx>; > Navare, Manasi D <manasi.d.navare@xxxxxxxxx> > Subject: Re: [PATCHv7 5/5] drm/i915/dp: Enable Upfront link training for > typeC DP support on BXT > > On Wed, Jul 13, 2016 at 04:15:44PM +0530, Durgadoss R 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, the display driver will > > start link training with max lanes, and if that fails, the driver > > falls back to x2 lanes; and repeats this procedure for all > > bandwidth/lane configurations. > > > > * Since link training is done before modeset only the port > > (and not pipe/planes) and its associated PLLs are enabled. > > * On DP hotplug: Directly start link training on the DP encoder. > > * On Connected boot scenarios: When booted with an LFP and a DP, > > sometimes BIOS brings up DP. In these cases, we disable the > > crtc and then do upfront link training; and bring it back up. > > * All local changes made for upfront link training are reset > > to their previous values once it is done; so that the > > subsequent modeset is not aware of these changes. > > > > Changes since v6: > > * Fix some initialization bugs on link_rate (Jim Bride) > > * Use link_rate (and not link_bw) for upfront (Ville) > > * Make intel_dp_upfront*() as a vfunc (Ander) > > * The train_set_valid variable in intel_dp was removed due to > > issues in fast link training. So, to indicate the link train > > status, move the channel_eq inside intel_dp. > > Changes since v5: > > * Moved retry logic in upfront to intel_dp.c so that it > > can be used for all platforms. > > Changes since v4: > > * Removed usage of crtc_state in upfront link training; > > Hence no need to find free crtc to do upfront now. > > * Re-enable crtc if it was disabled for upfront. > > * Use separate variables to track max lane count > > and link rate found by upfront, without modifying > > the original DPCD read from panel. > > Changes since v3: > > * Fixed a return value on BXT check > > * Reworked on top of bxt_ddi_pll_select split from Ander > > * Renamed from ddi_upfront to bxt_upfront since the > > upfront logic includes BXT specific functions for now. > > Changes since v2: > > * Rebased on top of latest dpll_mgr.c code and > > latest HPD related clean ups. > > * Corrected return values from upfront (Ander) > > * Corrected atomic locking for upfront in intel_dp.c (Ville) > > Changes since v1: > > * all pll related functions inside ddi.c > > > > Signed-off-by: Durgadoss R <durgadoss.r@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 45 ++++++++ > > drivers/gpu/drm/i915/intel_dp.c | 155 > +++++++++++++++++++++++++- > > drivers/gpu/drm/i915/intel_dp_link_training.c | 7 +- > > drivers/gpu/drm/i915/intel_drv.h | 15 +++ > > 4 files changed, 213 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > > index ab1745a..3bad01b 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -2356,6 +2356,51 @@ intel_ddi_init_hdmi_connector(struct > intel_digital_port *intel_dig_port) > > return connector; > > } > > > > +bool intel_bxt_upfront_link_train(struct intel_dp *intel_dp, > > + int clock, uint8_t lane_count) > > +{ > > + struct intel_connector *connector = intel_dp->attached_connector; > > + struct intel_encoder *encoder = connector->encoder; > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > + struct intel_shared_dpll *pll; > > + struct intel_shared_dpll_config tmp_pll_config; > > + struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp); > > + enum intel_dpll_id dpll_id = (enum intel_dpll_id)dig_port->port; > > + > > + /* > > + * FIXME: Works only for BXT. > > + * Select the required PLL. This works for platforms where > > + * there is no shared DPLL. > > + */ > > + pll = &dev_priv->shared_dplls[dpll_id]; > > + if (WARN_ON(pll->active_mask)) { > > + DRM_ERROR("Shared DPLL already in use. > active_mask:%x\n", pll->active_mask); > > + return false; > > + } > > + > > + tmp_pll_config = pll->config; > > + > > + if (!bxt_ddi_dp_set_dpll_hw_state(clock, &pll->config.hw_state)) { > > + DRM_ERROR("Could not setup DPLL\n"); > > + pll->config = tmp_pll_config; > > + return false; > > + } > > + > > + /* Enable PLL followed by port */ > > + pll->funcs.enable(dev_priv, pll); > > + intel_ddi_pre_enable_dp(encoder, clock, lane_count, pll); > > + > > + DRM_DEBUG_KMS("Upfront link train %s: link_clock:%d > lanes:%d\n", > > + intel_dp->channel_eq_status ? "Passed" : "Failed", clock, > lane_count); > > + > > + /* Disable port followed by PLL for next retry/clean up */ > > + intel_ddi_post_disable(encoder); > > + pll->funcs.disable(dev_priv, pll); > > + > > + pll->config = tmp_pll_config; > > + return intel_dp->channel_eq_status; > > +} > > + > > void intel_ddi_init(struct drm_device *dev, enum port port) > > { > > struct drm_i915_private *dev_priv = to_i915(dev); > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > b/drivers/gpu/drm/i915/intel_dp.c > > index 817897c..394e62d 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; > > } > > > > /* > > @@ -1380,6 +1389,15 @@ 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, > > @@ -4208,6 +4226,114 @@ intel_dp_unset_edid(struct intel_dp *intel_dp) > > intel_dp->has_audio = false; > > } > > > > +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 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; > > + int ret; > > + bool done = false; > > + uint8_t lane_count, max_lanes = 4; > > + int common_rates[DP_MAX_SUPPORTED_RATES] = {}; > > + int clock, common_len; > > + > > + common_len = intel_dp_common_rates(intel_dp, common_rates); > > + 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); > > + ret = intel_dp_upfront_crtc_disable(intel_crtc, &ctx, false); > > Still seems wrong to just disable things here. And if we can't disable > things, then what can we do? > > I believe there are people working on a proper USB type-C framework, > and once that materializes, I'm hoping it could just signal us whenever > the cable mode gets changed. I assume the biggest mess for us would > be how to detect that we're dealing with a type-C port, so that we'd > know when to register for these notifications. > > Anyway, once there's a proper framework for this stuff, I don't think > we'd need any upfront link training snymore. I guess we could use it > as a temporary thing, and garbage collect it if/when the proper USB > stuff happens? Sure, I am fine with this as a temporary stuff.. Thanks, Durga > > > + if (ret) > > + goto exit_fail; > > + } > > + > > + mutex_lock(&dev_priv->dpll_lock); > > + > > + for (lane_count = max_lanes; lane_count >= 1 && !done; lane_count > >>= 1) { > > + for (clock = common_len - 1; clock >= 0 && !done; clock--) { > > + done = intel_dp->upfront_link_train(intel_dp, > > + common_rates[clock], lane_count); > > + if (done) { > > + intel_dp->max_lanes_upfront = lane_count; > > + intel_dp->max_link_rate_upfront = > common_rates[clock]; > > + break; > > + } > > + } > > + } > > + > > + 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); > > + return done; > > +} > > + > > static void > > intel_dp_long_pulse(struct intel_connector *intel_connector) > > { > > @@ -4218,7 +4344,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; > > - bool ret; > > + bool ret, do_upfront_link_train; > > u8 sink_irq_vector; > > > > power_domain = > intel_display_port_aux_power_domain(intel_encoder); > > @@ -4303,10 +4429,23 @@ intel_dp_long_pulse(struct intel_connector > *intel_connector) > > DRM_DEBUG_DRIVER("CP or sink specific irq > unhandled\n"); > > } > > > > + /* Do not do upfront link train, if it is a compliance request */ > > + do_upfront_link_train = intel_dp->upfront_link_train && > > + !intel_dp->upfront_done && > > + intel_encoder->type == INTEL_OUTPUT_DP > && > > + 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); > > + if (!intel_dp->upfront_done) > > + status = connector_status_disconnected; > > + } > > + > > out: > > - if ((status != connector_status_connected) && > > - (intel_dp->is_mst == false)) > > + if (status != connector_status_connected && !intel_dp->is_mst) { > > intel_dp_unset_edid(intel_dp); > > + intel_dp->upfront_done = false; > > + } > > > > intel_display_power_put(to_i915(dev), power_domain); > > return; > > @@ -5562,6 +5701,12 @@ 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)) > > + intel_dp->upfront_link_train = > intel_bxt_upfront_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 60fb39c..e58442a 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c > > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c > > @@ -232,7 +232,6 @@ static u32 intel_dp_training_pattern(struct intel_dp > *intel_dp) > > static void > > intel_dp_link_training_channel_equalization(struct intel_dp *intel_dp) > > { > > - bool channel_eq = false; > > int tries, cr_tries; > > u32 training_pattern; > > > > @@ -248,7 +247,7 @@ intel_dp_link_training_channel_equalization(struct > intel_dp *intel_dp) > > > > tries = 0; > > cr_tries = 0; > > - channel_eq = false; > > + intel_dp->channel_eq_status = false; > > for (;;) { > > uint8_t link_status[DP_LINK_STATUS_SIZE]; > > > > @@ -276,7 +275,7 @@ intel_dp_link_training_channel_equalization(struct > intel_dp *intel_dp) > > > > if (drm_dp_channel_eq_ok(link_status, > > intel_dp->lane_count)) { > > - channel_eq = true; > > + intel_dp->channel_eq_status = true; > > break; > > } > > > > @@ -302,7 +301,7 @@ intel_dp_link_training_channel_equalization(struct > intel_dp *intel_dp) > > > > intel_dp_set_idle_link_train(intel_dp); > > > > - if (channel_eq) > > + if (intel_dp->channel_eq_status) > > DRM_DEBUG_KMS("Channel EQ done. DP Training > successful\n"); > > } > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > > index 16f70d4..88306fc 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -851,6 +851,15 @@ struct intel_dp { > > enum hdmi_force_audio force_audio; > > bool limited_color_range; > > bool color_range_auto; > > + > > + /* Whether Channel Equalization was achieved during Link training */ > > + bool channel_eq_status; > > + > > + /* 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]; > > @@ -908,6 +917,10 @@ 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); > > + > > /* Displayport compliance testing */ > > unsigned long compliance_test_type; > > unsigned long compliance_test_data; > > @@ -1127,6 +1140,8 @@ void intel_ddi_clock_get(struct intel_encoder > *encoder, > > struct intel_crtc_state *pipe_config); > > 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_bxt_upfront_link_train(struct intel_dp *intel_dp, > > + int clock, uint8_t lane_count); > > > > /* intel_frontbuffer.c */ > > void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj, > > -- > > 1.9.1 > > -- > Ville Syrjälä > Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
