[Intel-gfx] [PATCHv7 5/5] drm/i915/dp: Enable Upfront link training for typeC DP support on BXT
R, Durgadoss
durgadoss.r at intel.com
Wed Jul 27 11:20:18 UTC 2016
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala at linux.intel.com]
> Sent: Wednesday, July 27, 2016 3:28 PM
> To: R, Durgadoss <durgadoss.r at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; Conselvan De Oliveira, Ander
> <ander.conselvan.de.oliveira at intel.com>; Bride, Jim <jim.bride at intel.com>;
> Navare, Manasi D <manasi.d.navare at intel.com>
> 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 at intel.com>
> > ---
> > 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
More information about the Intel-gfx
mailing list