| Message ID | 1479179603-28476-6-git-send-email-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Jani/Ville could you please review this patch? This has been ACKed by DRM and is good from DRM point fo view. But I need r-b from either of you for this to get merged. Regards Manasi On Mon, Nov 14, 2016 at 07:13:23PM -0800, Manasi Navare wrote: > If link training at a link rate optimal for a particular > mode fails during modeset's atomic commit phase, then we > let the modeset complete and then retry. We save the link rate > value at which link training failed, update the link status property > to "BAD" and use a lower link rate to prune the modes. It will redo > the modeset on the current mode at lower link rate or if the current > mode gets pruned due to lower link constraints then, it will send a > hotplug uevent for userspace to handle it. > > This is also required to pass DP CTS tests 4.3.1.3, 4.3.1.4, > 4.3.1.6. > > v5: > * Move set link status to drm core (Daniel Vetter, Jani Nikula) > v4: > * Add fallback support for non DDI platforms too > * Set connector->link status inside set_link_status function > (Jani Nikula) > v3: > * Set link status property to BAd unconditionally (Jani Nikula) > * Dont use two separate variables link_train_failed and link_status > to indicate same thing (Jani Nikula) > v2: > * Squashed a few patches (Jani Nikula) > > Acked-by: Tony Cheng <tony.cheng@amd.com> > Acked-by: Harry Wentland <Harry.wentland@amd.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 21 +++++- > drivers/gpu/drm/i915/intel_dp.c | 102 +++++++++++++++++++++++++- > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > drivers/gpu/drm/i915/intel_drv.h | 6 +- > 4 files changed, 131 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 0ad4e16..57b7ee9 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1684,6 +1684,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > enum port port = intel_ddi_get_encoder_port(encoder); > + struct intel_connector *intel_connector = intel_dp->attached_connector; > + struct drm_connector *connector = &intel_connector->base; > > intel_dp_set_link_params(intel_dp, link_rate, lane_count, > link_mst); > @@ -1694,7 +1696,24 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > intel_prepare_dp_ddi_buffers(encoder); > intel_ddi_init_dp_buf_reg(encoder); > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > - intel_dp_start_link_train(intel_dp); > + if (!intel_dp_start_link_train(intel_dp)) { > + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", > + link_rate, lane_count); > + if (!intel_dp_get_link_train_fallback_values(intel_dp, > + link_rate, > + lane_count)) > + /* Schedule a Hotplug Uevent to userspace to start modeset */ > + schedule_work(&intel_connector->modeset_retry_work); > + } else { > + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", > + link_rate, lane_count); > + intel_dp->fallback_link_rate_index = -1; > + intel_dp->fallback_link_rate = 0; > + intel_dp->fallback_lane_count = 0; > + drm_mode_connector_set_link_status_property(connector, > + DRM_MODE_LINK_STATUS_GOOD); > + } > + > if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > intel_dp_stop_link_train(intel_dp); > } > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index e87b451..af2f8b2 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -353,8 +353,14 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp, > target_clock = fixed_mode->clock; > } > > - max_link_clock = intel_dp_max_link_rate(intel_dp); > - max_lanes = intel_dp_max_lane_count(intel_dp); > + /* Prune the modes using the fallback link rate/lane count */ > + if (connector->link_status == DRM_MODE_LINK_STATUS_BAD) { > + max_link_clock = intel_dp->fallback_link_rate; > + max_lanes = intel_dp->fallback_lane_count; > + } else { > + max_link_clock = intel_dp_max_link_rate(intel_dp); > + max_lanes = intel_dp_max_lane_count(intel_dp); > + } > > max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes); > mode_rate = intel_dp_link_required(target_clock, 18); > @@ -1591,6 +1597,7 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp, > 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; > + struct drm_connector *connector = &intel_connector->base; > int lane_count, clock; > int min_lane_count = 1; > int max_lane_count = intel_dp_max_lane_count(intel_dp); > @@ -1638,6 +1645,12 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp, > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) > return false; > > + /* Fall back to lower link rate in case of failure in previous modeset */ > + if (connector->link_status == DRM_MODE_LINK_STATUS_BAD) { > + min_lane_count = max_lane_count = intel_dp->fallback_lane_count; > + min_clock = max_clock = intel_dp->fallback_link_rate_index; > + } > + > DRM_DEBUG_KMS("DP link computation with max lane count %i " > "max bw %d pixel clock %iKHz\n", > max_lane_count, common_rates[max_clock], > @@ -2781,6 +2794,8 @@ static void intel_enable_dp(struct intel_encoder *encoder, > struct drm_device *dev = encoder->base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc); > + struct intel_connector *intel_connector = intel_dp->attached_connector; > + struct drm_connector *connector = &intel_connector->base; > uint32_t dp_reg = I915_READ(intel_dp->output_reg); > enum pipe pipe = crtc->pipe; > > @@ -2811,7 +2826,23 @@ static void intel_enable_dp(struct intel_encoder *encoder, > } > > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > - intel_dp_start_link_train(intel_dp); > + if (!intel_dp_start_link_train(intel_dp)) { > + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", > + intel_dp->link_rate, intel_dp->lane_count); > + if (!intel_dp_get_link_train_fallback_values(intel_dp, > + intel_dp->link_rate, > + intel_dp->lane_count)) > + /* Schedule a Hotplug Uevent to userspace to start modeset */ > + schedule_work(&intel_connector->modeset_retry_work); > + } else { > + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", > + intel_dp->link_rate, intel_dp->lane_count); > + intel_dp->fallback_link_rate_index = -1; > + intel_dp->fallback_link_rate = 0; > + intel_dp->fallback_lane_count = 0; > + drm_mode_connector_set_link_status_property(connector, > + DRM_MODE_LINK_STATUS_GOOD); > + } > intel_dp_stop_link_train(intel_dp); > > if (pipe_config->has_audio) { > @@ -4018,6 +4049,8 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc); > + struct intel_connector *intel_connector = intel_dp->attached_connector; > + struct drm_connector *connector = &intel_connector->base; > > /* Suppress underruns caused by re-training */ > intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); > @@ -4025,7 +4058,23 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) > intel_set_pch_fifo_underrun_reporting(dev_priv, > intel_crtc_pch_transcoder(crtc), false); > > - intel_dp_start_link_train(intel_dp); > + if (!intel_dp_start_link_train(intel_dp)) { > + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", > + intel_dp->link_rate, intel_dp->lane_count); > + if (!intel_dp_get_link_train_fallback_values(intel_dp, > + intel_dp->link_rate, > + intel_dp->lane_count)) > + /* Schedule a Hotplug Uevent to userspace to start modeset */ > + schedule_work(&intel_connector->modeset_retry_work); > + } else { > + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", > + intel_dp->link_rate, intel_dp->lane_count); > + intel_dp->fallback_link_rate_index = -1; > + intel_dp->fallback_link_rate = 0; > + intel_dp->fallback_lane_count = 0; > + drm_mode_connector_set_link_status_property(connector, > + DRM_MODE_LINK_STATUS_GOOD); > + } > intel_dp_stop_link_train(intel_dp); > > /* Keep underrun reporting disabled until things are stable */ > @@ -4419,6 +4468,11 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > intel_dp->compliance_test_active = 0; > intel_dp->compliance_test_type = 0; > intel_dp->compliance_test_data = 0; > + intel_dp->fallback_link_rate_index = -1; > + intel_dp->fallback_link_rate = 0; > + intel_dp->fallback_lane_count = 0; > + drm_mode_connector_set_link_status_property(connector, > + DRM_MODE_LINK_STATUS_GOOD); > > if (intel_dp->is_mst) { > DRM_DEBUG_KMS("MST device may have disappeared %d vs %d\n", > @@ -4510,6 +4564,11 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > connector->base.id, connector->name); > > + /* If this is a retry due to link trianing failure */ > + if (status == connector_status_connected && > + connector->link_status == DRM_MODE_LINK_STATUS_BAD) > + return status; > + > /* If full detect is not performed yet, do a full detect */ > if (!intel_dp->detect_done) > status = intel_dp_long_pulse(intel_dp->attached_connector); > @@ -5677,6 +5736,37 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > return false; > } > > +static void intel_dp_modeset_retry_work_fn(struct work_struct *work) > +{ > + struct intel_connector *intel_connector; > + struct drm_connector *connector; > + struct drm_display_mode *mode; > + bool verbose_prune = true; > + > + intel_connector = container_of(work, typeof(*intel_connector), > + modeset_retry_work); > + connector = &intel_connector->base; > + > + /* Grab the locks before changing connector property*/ > + mutex_lock(&connector->dev->mode_config.mutex); > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, > + connector->name); > + list_for_each_entry(mode, &connector->modes, head) { > + mode->status = intel_dp_mode_valid(connector, > + mode); > + } > + drm_mode_prune_invalid(connector->dev, &connector->modes, > + verbose_prune); > + /* Set connector link status to BAD and send a Uevent to notify > + * userspace to do a modeset. > + */ > + drm_mode_connector_set_link_status_property(connector, > + DRM_MODE_LINK_STATUS_BAD); > + mutex_unlock(&connector->dev->mode_config.mutex); > + /* Send Hotplug uevent so userspace can reprobe */ > + drm_kms_helper_hotplug_event(connector->dev); > +} > + > bool > intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > struct intel_connector *intel_connector) > @@ -5689,6 +5779,10 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > enum port port = intel_dig_port->port; > int type; > > + /* Initialize the work for modeset in case of link train failure */ > + INIT_WORK(&intel_connector->modeset_retry_work, > + intel_dp_modeset_retry_work_fn); > + > if (WARN(intel_dig_port->max_lanes < 1, > "Not enough lanes (%d) for DP on port %c\n", > intel_dig_port->max_lanes, port_name(port))) > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > index 0048b52..10f81ab 100644 > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c > @@ -310,9 +310,15 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp) > DP_TRAINING_PATTERN_DISABLE); > } > > -void > +bool > intel_dp_start_link_train(struct intel_dp *intel_dp) > { > - intel_dp_link_training_clock_recovery(intel_dp); > - intel_dp_link_training_channel_equalization(intel_dp); > + bool ret; > + > + if (intel_dp_link_training_clock_recovery(intel_dp)) { > + ret = intel_dp_link_training_channel_equalization(intel_dp); > + if (ret) > + return true; > + } > + return false; > } > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 55ceb44..88918f4 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -312,6 +312,9 @@ struct intel_connector { > void *port; /* store this opaque as its illegal to dereference it */ > > struct intel_dp *mst_port; > + > + /* Work struct to schedule a uevent on link train failure */ > + struct work_struct modeset_retry_work; > }; > > struct dpll { > @@ -890,7 +893,6 @@ struct intel_dp { > int fallback_link_rate; > uint8_t fallback_lane_count; > int fallback_link_rate_index; > - bool link_train_failed; > uint8_t sink_count; > bool link_mst; > bool has_audio; > @@ -1389,7 +1391,7 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp, > bool link_mst); > int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp, > int link_rate, uint8_t lane_count); > -void intel_dp_start_link_train(struct intel_dp *intel_dp); > +bool intel_dp_start_link_train(struct intel_dp *intel_dp); > void intel_dp_stop_link_train(struct intel_dp *intel_dp); > void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode); > void intel_dp_encoder_reset(struct drm_encoder *encoder); > -- > 1.9.1 >
On Tue, 15 Nov 2016, Manasi Navare <manasi.d.navare@intel.com> wrote: > If link training at a link rate optimal for a particular > mode fails during modeset's atomic commit phase, then we > let the modeset complete and then retry. We save the link rate > value at which link training failed, update the link status property > to "BAD" and use a lower link rate to prune the modes. It will redo > the modeset on the current mode at lower link rate or if the current > mode gets pruned due to lower link constraints then, it will send a > hotplug uevent for userspace to handle it. > > This is also required to pass DP CTS tests 4.3.1.3, 4.3.1.4, > 4.3.1.6. > > v5: > * Move set link status to drm core (Daniel Vetter, Jani Nikula) > v4: > * Add fallback support for non DDI platforms too > * Set connector->link status inside set_link_status function > (Jani Nikula) > v3: > * Set link status property to BAd unconditionally (Jani Nikula) > * Dont use two separate variables link_train_failed and link_status > to indicate same thing (Jani Nikula) > v2: > * Squashed a few patches (Jani Nikula) > > Acked-by: Tony Cheng <tony.cheng@amd.com> > Acked-by: Harry Wentland <Harry.wentland@amd.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 21 +++++- > drivers/gpu/drm/i915/intel_dp.c | 102 +++++++++++++++++++++++++- > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > drivers/gpu/drm/i915/intel_drv.h | 6 +- > 4 files changed, 131 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 0ad4e16..57b7ee9 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1684,6 +1684,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > enum port port = intel_ddi_get_encoder_port(encoder); > + struct intel_connector *intel_connector = intel_dp->attached_connector; > + struct drm_connector *connector = &intel_connector->base; > > intel_dp_set_link_params(intel_dp, link_rate, lane_count, > link_mst); > @@ -1694,7 +1696,24 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > intel_prepare_dp_ddi_buffers(encoder); > intel_ddi_init_dp_buf_reg(encoder); > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > - intel_dp_start_link_train(intel_dp); > + if (!intel_dp_start_link_train(intel_dp)) { > + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", > + link_rate, lane_count); > + if (!intel_dp_get_link_train_fallback_values(intel_dp, > + link_rate, > + lane_count)) > + /* Schedule a Hotplug Uevent to userspace to start modeset */ > + schedule_work(&intel_connector->modeset_retry_work); > + } else { > + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", > + link_rate, lane_count); > + intel_dp->fallback_link_rate_index = -1; > + intel_dp->fallback_link_rate = 0; > + intel_dp->fallback_lane_count = 0; > + drm_mode_connector_set_link_status_property(connector, > + DRM_MODE_LINK_STATUS_GOOD); > + } Code block with intel_dp_start_link_train() failure handling. Okay. > + > if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > intel_dp_stop_link_train(intel_dp); > } > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index e87b451..af2f8b2 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -353,8 +353,14 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp, > target_clock = fixed_mode->clock; > } > > - max_link_clock = intel_dp_max_link_rate(intel_dp); > - max_lanes = intel_dp_max_lane_count(intel_dp); > + /* Prune the modes using the fallback link rate/lane count */ > + if (connector->link_status == DRM_MODE_LINK_STATUS_BAD) { > + max_link_clock = intel_dp->fallback_link_rate; > + max_lanes = intel_dp->fallback_lane_count; > + } else { > + max_link_clock = intel_dp_max_link_rate(intel_dp); > + max_lanes = intel_dp_max_lane_count(intel_dp); > + } > > max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes); > mode_rate = intel_dp_link_required(target_clock, 18); > @@ -1591,6 +1597,7 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp, > 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; > + struct drm_connector *connector = &intel_connector->base; > int lane_count, clock; > int min_lane_count = 1; > int max_lane_count = intel_dp_max_lane_count(intel_dp); > @@ -1638,6 +1645,12 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp, > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) > return false; > > + /* Fall back to lower link rate in case of failure in previous modeset */ > + if (connector->link_status == DRM_MODE_LINK_STATUS_BAD) { > + min_lane_count = max_lane_count = intel_dp->fallback_lane_count; > + min_clock = max_clock = intel_dp->fallback_link_rate_index; > + } > + > DRM_DEBUG_KMS("DP link computation with max lane count %i " > "max bw %d pixel clock %iKHz\n", > max_lane_count, common_rates[max_clock], > @@ -2781,6 +2794,8 @@ static void intel_enable_dp(struct intel_encoder *encoder, > struct drm_device *dev = encoder->base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc); > + struct intel_connector *intel_connector = intel_dp->attached_connector; > + struct drm_connector *connector = &intel_connector->base; > uint32_t dp_reg = I915_READ(intel_dp->output_reg); > enum pipe pipe = crtc->pipe; > > @@ -2811,7 +2826,23 @@ static void intel_enable_dp(struct intel_encoder *encoder, > } > > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > - intel_dp_start_link_train(intel_dp); > + if (!intel_dp_start_link_train(intel_dp)) { > + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", > + intel_dp->link_rate, intel_dp->lane_count); > + if (!intel_dp_get_link_train_fallback_values(intel_dp, > + intel_dp->link_rate, > + intel_dp->lane_count)) > + /* Schedule a Hotplug Uevent to userspace to start modeset */ > + schedule_work(&intel_connector->modeset_retry_work); > + } else { > + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", > + intel_dp->link_rate, intel_dp->lane_count); > + intel_dp->fallback_link_rate_index = -1; > + intel_dp->fallback_link_rate = 0; > + intel_dp->fallback_lane_count = 0; > + drm_mode_connector_set_link_status_property(connector, > + DRM_MODE_LINK_STATUS_GOOD); > + } Second identical copy of intel_dp_start_link_train() failure handling. > intel_dp_stop_link_train(intel_dp); > > if (pipe_config->has_audio) { > @@ -4018,6 +4049,8 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc); > + struct intel_connector *intel_connector = intel_dp->attached_connector; > + struct drm_connector *connector = &intel_connector->base; > > /* Suppress underruns caused by re-training */ > intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); > @@ -4025,7 +4058,23 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) > intel_set_pch_fifo_underrun_reporting(dev_priv, > intel_crtc_pch_transcoder(crtc), false); > > - intel_dp_start_link_train(intel_dp); > + if (!intel_dp_start_link_train(intel_dp)) { > + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", > + intel_dp->link_rate, intel_dp->lane_count); > + if (!intel_dp_get_link_train_fallback_values(intel_dp, > + intel_dp->link_rate, > + intel_dp->lane_count)) > + /* Schedule a Hotplug Uevent to userspace to start modeset */ > + schedule_work(&intel_connector->modeset_retry_work); > + } else { > + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", > + intel_dp->link_rate, intel_dp->lane_count); > + intel_dp->fallback_link_rate_index = -1; > + intel_dp->fallback_link_rate = 0; > + intel_dp->fallback_lane_count = 0; > + drm_mode_connector_set_link_status_property(connector, > + DRM_MODE_LINK_STATUS_GOOD); > + } Third identical copy of intel_dp_start_link_train() failure handling. Please abstract. Why don't all intel_dp_start_link_train() calls need this? Could the above code be *within* intel_dp_start_link_train()? > intel_dp_stop_link_train(intel_dp); > > /* Keep underrun reporting disabled until things are stable */ > @@ -4419,6 +4468,11 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > intel_dp->compliance_test_active = 0; > intel_dp->compliance_test_type = 0; > intel_dp->compliance_test_data = 0; > + intel_dp->fallback_link_rate_index = -1; > + intel_dp->fallback_link_rate = 0; > + intel_dp->fallback_lane_count = 0; > + drm_mode_connector_set_link_status_property(connector, > + DRM_MODE_LINK_STATUS_GOOD); > > if (intel_dp->is_mst) { > DRM_DEBUG_KMS("MST device may have disappeared %d vs %d\n", > @@ -4510,6 +4564,11 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > connector->base.id, connector->name); > > + /* If this is a retry due to link trianing failure */ > + if (status == connector_status_connected && > + connector->link_status == DRM_MODE_LINK_STATUS_BAD) > + return status; > + > /* If full detect is not performed yet, do a full detect */ > if (!intel_dp->detect_done) > status = intel_dp_long_pulse(intel_dp->attached_connector); > @@ -5677,6 +5736,37 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > return false; > } > > +static void intel_dp_modeset_retry_work_fn(struct work_struct *work) > +{ > + struct intel_connector *intel_connector; > + struct drm_connector *connector; > + struct drm_display_mode *mode; > + bool verbose_prune = true; > + > + intel_connector = container_of(work, typeof(*intel_connector), > + modeset_retry_work); > + connector = &intel_connector->base; > + > + /* Grab the locks before changing connector property*/ > + mutex_lock(&connector->dev->mode_config.mutex); > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, > + connector->name); > + list_for_each_entry(mode, &connector->modes, head) { > + mode->status = intel_dp_mode_valid(connector, > + mode); > + } > + drm_mode_prune_invalid(connector->dev, &connector->modes, > + verbose_prune); > + /* Set connector link status to BAD and send a Uevent to notify > + * userspace to do a modeset. > + */ > + drm_mode_connector_set_link_status_property(connector, > + DRM_MODE_LINK_STATUS_BAD); > + mutex_unlock(&connector->dev->mode_config.mutex); > + /* Send Hotplug uevent so userspace can reprobe */ > + drm_kms_helper_hotplug_event(connector->dev); > +} > + > bool > intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > struct intel_connector *intel_connector) > @@ -5689,6 +5779,10 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > enum port port = intel_dig_port->port; > int type; > > + /* Initialize the work for modeset in case of link train failure */ > + INIT_WORK(&intel_connector->modeset_retry_work, > + intel_dp_modeset_retry_work_fn); > + > if (WARN(intel_dig_port->max_lanes < 1, > "Not enough lanes (%d) for DP on port %c\n", > intel_dig_port->max_lanes, port_name(port))) > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > index 0048b52..10f81ab 100644 > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c > @@ -310,9 +310,15 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp) > DP_TRAINING_PATTERN_DISABLE); > } > > -void > +bool > intel_dp_start_link_train(struct intel_dp *intel_dp) > { > - intel_dp_link_training_clock_recovery(intel_dp); > - intel_dp_link_training_channel_equalization(intel_dp); > + bool ret; > + > + if (intel_dp_link_training_clock_recovery(intel_dp)) { > + ret = intel_dp_link_training_channel_equalization(intel_dp); > + if (ret) > + return true; > + } > + return false; Usually happy day scenario goes at the top indentation level, and errors are handled in the nested cases with early returns. Compare with: intel_dp_start_link_train(struct intel_dp *intel_dp) { if (!intel_dp_link_training_clock_recovery(intel_dp)) return false; if (!intel_dp_link_training_channel_equalization(intel_dp)) return false; return true; } > } > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 55ceb44..88918f4 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -312,6 +312,9 @@ struct intel_connector { > void *port; /* store this opaque as its illegal to dereference it */ > > struct intel_dp *mst_port; > + > + /* Work struct to schedule a uevent on link train failure */ > + struct work_struct modeset_retry_work; > }; > > struct dpll { > @@ -890,7 +893,6 @@ struct intel_dp { > int fallback_link_rate; > uint8_t fallback_lane_count; > int fallback_link_rate_index; > - bool link_train_failed; > uint8_t sink_count; > bool link_mst; > bool has_audio; > @@ -1389,7 +1391,7 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp, > bool link_mst); > int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp, > int link_rate, uint8_t lane_count); > -void intel_dp_start_link_train(struct intel_dp *intel_dp); > +bool intel_dp_start_link_train(struct intel_dp *intel_dp); > void intel_dp_stop_link_train(struct intel_dp *intel_dp); > void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode); > void intel_dp_encoder_reset(struct drm_encoder *encoder);
On Thu, Nov 17, 2016 at 02:49:02PM +0200, Jani Nikula wrote: > On Tue, 15 Nov 2016, Manasi Navare <manasi.d.navare@intel.com> wrote: > > If link training at a link rate optimal for a particular > > mode fails during modeset's atomic commit phase, then we > > let the modeset complete and then retry. We save the link rate > > value at which link training failed, update the link status property > > to "BAD" and use a lower link rate to prune the modes. It will redo > > the modeset on the current mode at lower link rate or if the current > > mode gets pruned due to lower link constraints then, it will send a > > hotplug uevent for userspace to handle it. > > > > This is also required to pass DP CTS tests 4.3.1.3, 4.3.1.4, > > 4.3.1.6. > > > > v5: > > * Move set link status to drm core (Daniel Vetter, Jani Nikula) > > v4: > > * Add fallback support for non DDI platforms too > > * Set connector->link status inside set_link_status function > > (Jani Nikula) > > v3: > > * Set link status property to BAd unconditionally (Jani Nikula) > > * Dont use two separate variables link_train_failed and link_status > > to indicate same thing (Jani Nikula) > > v2: > > * Squashed a few patches (Jani Nikula) > > > > Acked-by: Tony Cheng <tony.cheng@amd.com> > > Acked-by: Harry Wentland <Harry.wentland@amd.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Ville Syrjala <ville.syrjala@linux.intel.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 21 +++++- > > drivers/gpu/drm/i915/intel_dp.c | 102 +++++++++++++++++++++++++- > > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > > drivers/gpu/drm/i915/intel_drv.h | 6 +- > > 4 files changed, 131 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index 0ad4e16..57b7ee9 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1684,6 +1684,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > enum port port = intel_ddi_get_encoder_port(encoder); > > + struct intel_connector *intel_connector = intel_dp->attached_connector; > > + struct drm_connector *connector = &intel_connector->base; > > > > intel_dp_set_link_params(intel_dp, link_rate, lane_count, > > link_mst); > > @@ -1694,7 +1696,24 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > > intel_prepare_dp_ddi_buffers(encoder); > > intel_ddi_init_dp_buf_reg(encoder); > > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > - intel_dp_start_link_train(intel_dp); > > + if (!intel_dp_start_link_train(intel_dp)) { > > + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", > > + link_rate, lane_count); > > + if (!intel_dp_get_link_train_fallback_values(intel_dp, > > + link_rate, > > + lane_count)) > > + /* Schedule a Hotplug Uevent to userspace to start modeset */ > > + schedule_work(&intel_connector->modeset_retry_work); > > + } else { > > + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", > > + link_rate, lane_count); > > + intel_dp->fallback_link_rate_index = -1; > > + intel_dp->fallback_link_rate = 0; > > + intel_dp->fallback_lane_count = 0; > > + drm_mode_connector_set_link_status_property(connector, > > + DRM_MODE_LINK_STATUS_GOOD); > > + } > > Code block with intel_dp_start_link_train() failure handling. Okay. > > > + > > if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > > intel_dp_stop_link_train(intel_dp); > > } > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index e87b451..af2f8b2 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -353,8 +353,14 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp, > > target_clock = fixed_mode->clock; > > } > > > > - max_link_clock = intel_dp_max_link_rate(intel_dp); > > - max_lanes = intel_dp_max_lane_count(intel_dp); > > + /* Prune the modes using the fallback link rate/lane count */ > > + if (connector->link_status == DRM_MODE_LINK_STATUS_BAD) { > > + max_link_clock = intel_dp->fallback_link_rate; > > + max_lanes = intel_dp->fallback_lane_count; > > + } else { > > + max_link_clock = intel_dp_max_link_rate(intel_dp); > > + max_lanes = intel_dp_max_lane_count(intel_dp); > > + } > > > > max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes); > > mode_rate = intel_dp_link_required(target_clock, 18); > > @@ -1591,6 +1597,7 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp, > > 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; > > + struct drm_connector *connector = &intel_connector->base; > > int lane_count, clock; > > int min_lane_count = 1; > > int max_lane_count = intel_dp_max_lane_count(intel_dp); > > @@ -1638,6 +1645,12 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp, > > if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) > > return false; > > > > + /* Fall back to lower link rate in case of failure in previous modeset */ > > + if (connector->link_status == DRM_MODE_LINK_STATUS_BAD) { > > + min_lane_count = max_lane_count = intel_dp->fallback_lane_count; > > + min_clock = max_clock = intel_dp->fallback_link_rate_index; > > + } > > + > > DRM_DEBUG_KMS("DP link computation with max lane count %i " > > "max bw %d pixel clock %iKHz\n", > > max_lane_count, common_rates[max_clock], > > @@ -2781,6 +2794,8 @@ static void intel_enable_dp(struct intel_encoder *encoder, > > struct drm_device *dev = encoder->base.dev; > > struct drm_i915_private *dev_priv = to_i915(dev); > > struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc); > > + struct intel_connector *intel_connector = intel_dp->attached_connector; > > + struct drm_connector *connector = &intel_connector->base; > > uint32_t dp_reg = I915_READ(intel_dp->output_reg); > > enum pipe pipe = crtc->pipe; > > > > @@ -2811,7 +2826,23 @@ static void intel_enable_dp(struct intel_encoder *encoder, > > } > > > > intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > - intel_dp_start_link_train(intel_dp); > > + if (!intel_dp_start_link_train(intel_dp)) { > > + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", > > + intel_dp->link_rate, intel_dp->lane_count); > > + if (!intel_dp_get_link_train_fallback_values(intel_dp, > > + intel_dp->link_rate, > > + intel_dp->lane_count)) > > + /* Schedule a Hotplug Uevent to userspace to start modeset */ > > + schedule_work(&intel_connector->modeset_retry_work); > > + } else { > > + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", > > + intel_dp->link_rate, intel_dp->lane_count); > > + intel_dp->fallback_link_rate_index = -1; > > + intel_dp->fallback_link_rate = 0; > > + intel_dp->fallback_lane_count = 0; > > + drm_mode_connector_set_link_status_property(connector, > > + DRM_MODE_LINK_STATUS_GOOD); > > + } > > Second identical copy of intel_dp_start_link_train() failure handling. > > > intel_dp_stop_link_train(intel_dp); > > > > if (pipe_config->has_audio) { > > @@ -4018,6 +4049,8 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) > > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc); > > + struct intel_connector *intel_connector = intel_dp->attached_connector; > > + struct drm_connector *connector = &intel_connector->base; > > > > /* Suppress underruns caused by re-training */ > > intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); > > @@ -4025,7 +4058,23 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) > > intel_set_pch_fifo_underrun_reporting(dev_priv, > > intel_crtc_pch_transcoder(crtc), false); > > > > - intel_dp_start_link_train(intel_dp); > > + if (!intel_dp_start_link_train(intel_dp)) { > > + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", > > + intel_dp->link_rate, intel_dp->lane_count); > > + if (!intel_dp_get_link_train_fallback_values(intel_dp, > > + intel_dp->link_rate, > > + intel_dp->lane_count)) > > + /* Schedule a Hotplug Uevent to userspace to start modeset */ > > + schedule_work(&intel_connector->modeset_retry_work); > > + } else { > > + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", > > + intel_dp->link_rate, intel_dp->lane_count); > > + intel_dp->fallback_link_rate_index = -1; > > + intel_dp->fallback_link_rate = 0; > > + intel_dp->fallback_lane_count = 0; > > + drm_mode_connector_set_link_status_property(connector, > > + DRM_MODE_LINK_STATUS_GOOD); > > + } > > Third identical copy of intel_dp_start_link_train() failure > handling. Please abstract. > > Why don't all intel_dp_start_link_train() calls need this? Could the > above code be *within* intel_dp_start_link_train()? > Actually the same codeblock got repeated since I expanded this failure handling to all DDI and non DDI platforms and to all cases where link training happens. I think instead of adding to intel_dp_start_link_train, I can create another separate function for link_train_failure handling which can be called at all these places. Having it in a separate function would make it more intuitive that if start_linj_train failed then we are calling this failure handling code block. Let me know what you think. Regards Manasi > > intel_dp_stop_link_train(intel_dp); > > > > /* Keep underrun reporting disabled until things are stable */ > > @@ -4419,6 +4468,11 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > > intel_dp->compliance_test_active = 0; > > intel_dp->compliance_test_type = 0; > > intel_dp->compliance_test_data = 0; > > + intel_dp->fallback_link_rate_index = -1; > > + intel_dp->fallback_link_rate = 0; > > + intel_dp->fallback_lane_count = 0; > > + drm_mode_connector_set_link_status_property(connector, > > + DRM_MODE_LINK_STATUS_GOOD); > > > > if (intel_dp->is_mst) { > > DRM_DEBUG_KMS("MST device may have disappeared %d vs %d\n", > > @@ -4510,6 +4564,11 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, > > DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > > connector->base.id, connector->name); > > > > + /* If this is a retry due to link trianing failure */ > > + if (status == connector_status_connected && > > + connector->link_status == DRM_MODE_LINK_STATUS_BAD) > > + return status; > > + > > /* If full detect is not performed yet, do a full detect */ > > if (!intel_dp->detect_done) > > status = intel_dp_long_pulse(intel_dp->attached_connector); > > @@ -5677,6 +5736,37 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > > return false; > > } > > > > +static void intel_dp_modeset_retry_work_fn(struct work_struct *work) > > +{ > > + struct intel_connector *intel_connector; > > + struct drm_connector *connector; > > + struct drm_display_mode *mode; > > + bool verbose_prune = true; > > + > > + intel_connector = container_of(work, typeof(*intel_connector), > > + modeset_retry_work); > > + connector = &intel_connector->base; > > + > > + /* Grab the locks before changing connector property*/ > > + mutex_lock(&connector->dev->mode_config.mutex); > > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, > > + connector->name); > > + list_for_each_entry(mode, &connector->modes, head) { > > + mode->status = intel_dp_mode_valid(connector, > > + mode); > > + } > > + drm_mode_prune_invalid(connector->dev, &connector->modes, > > + verbose_prune); > > + /* Set connector link status to BAD and send a Uevent to notify > > + * userspace to do a modeset. > > + */ > > + drm_mode_connector_set_link_status_property(connector, > > + DRM_MODE_LINK_STATUS_BAD); > > + mutex_unlock(&connector->dev->mode_config.mutex); > > + /* Send Hotplug uevent so userspace can reprobe */ > > + drm_kms_helper_hotplug_event(connector->dev); > > +} > > + > > bool > > intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > > struct intel_connector *intel_connector) > > @@ -5689,6 +5779,10 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > > enum port port = intel_dig_port->port; > > int type; > > > > + /* Initialize the work for modeset in case of link train failure */ > > + INIT_WORK(&intel_connector->modeset_retry_work, > > + intel_dp_modeset_retry_work_fn); > > + > > if (WARN(intel_dig_port->max_lanes < 1, > > "Not enough lanes (%d) for DP on port %c\n", > > intel_dig_port->max_lanes, port_name(port))) > > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > > index 0048b52..10f81ab 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c > > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c > > @@ -310,9 +310,15 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp) > > DP_TRAINING_PATTERN_DISABLE); > > } > > > > -void > > +bool > > intel_dp_start_link_train(struct intel_dp *intel_dp) > > { > > - intel_dp_link_training_clock_recovery(intel_dp); > > - intel_dp_link_training_channel_equalization(intel_dp); > > + bool ret; > > + > > + if (intel_dp_link_training_clock_recovery(intel_dp)) { > > + ret = intel_dp_link_training_channel_equalization(intel_dp); > > + if (ret) > > + return true; > > + } > > + return false; > > Usually happy day scenario goes at the top indentation level, and errors > are handled in the nested cases with early returns. Compare with: > > intel_dp_start_link_train(struct intel_dp *intel_dp) > { > if (!intel_dp_link_training_clock_recovery(intel_dp)) > return false; > > if (!intel_dp_link_training_channel_equalization(intel_dp)) > return false; > > return true; > } > Agreed. > > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 55ceb44..88918f4 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -312,6 +312,9 @@ struct intel_connector { > > void *port; /* store this opaque as its illegal to dereference it */ > > > > struct intel_dp *mst_port; > > + > > + /* Work struct to schedule a uevent on link train failure */ > > + struct work_struct modeset_retry_work; > > }; > > > > struct dpll { > > @@ -890,7 +893,6 @@ struct intel_dp { > > int fallback_link_rate; > > uint8_t fallback_lane_count; > > int fallback_link_rate_index; > > - bool link_train_failed; > > uint8_t sink_count; > > bool link_mst; > > bool has_audio; > > @@ -1389,7 +1391,7 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp, > > bool link_mst); > > int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp, > > int link_rate, uint8_t lane_count); > > -void intel_dp_start_link_train(struct intel_dp *intel_dp); > > +bool intel_dp_start_link_train(struct intel_dp *intel_dp); > > void intel_dp_stop_link_train(struct intel_dp *intel_dp); > > void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode); > > void intel_dp_encoder_reset(struct drm_encoder *encoder); > > -- > Jani Nikula, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 0ad4e16..57b7ee9 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1684,6 +1684,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); enum port port = intel_ddi_get_encoder_port(encoder); + struct intel_connector *intel_connector = intel_dp->attached_connector; + struct drm_connector *connector = &intel_connector->base; intel_dp_set_link_params(intel_dp, link_rate, lane_count, link_mst); @@ -1694,7 +1696,24 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, intel_prepare_dp_ddi_buffers(encoder); intel_ddi_init_dp_buf_reg(encoder); intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); - intel_dp_start_link_train(intel_dp); + if (!intel_dp_start_link_train(intel_dp)) { + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", + link_rate, lane_count); + if (!intel_dp_get_link_train_fallback_values(intel_dp, + link_rate, + lane_count)) + /* Schedule a Hotplug Uevent to userspace to start modeset */ + schedule_work(&intel_connector->modeset_retry_work); + } else { + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", + link_rate, lane_count); + intel_dp->fallback_link_rate_index = -1; + intel_dp->fallback_link_rate = 0; + intel_dp->fallback_lane_count = 0; + drm_mode_connector_set_link_status_property(connector, + DRM_MODE_LINK_STATUS_GOOD); + } + if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) intel_dp_stop_link_train(intel_dp); } diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index e87b451..af2f8b2 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -353,8 +353,14 @@ int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp, target_clock = fixed_mode->clock; } - max_link_clock = intel_dp_max_link_rate(intel_dp); - max_lanes = intel_dp_max_lane_count(intel_dp); + /* Prune the modes using the fallback link rate/lane count */ + if (connector->link_status == DRM_MODE_LINK_STATUS_BAD) { + max_link_clock = intel_dp->fallback_link_rate; + max_lanes = intel_dp->fallback_lane_count; + } else { + max_link_clock = intel_dp_max_link_rate(intel_dp); + max_lanes = intel_dp_max_lane_count(intel_dp); + } max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes); mode_rate = intel_dp_link_required(target_clock, 18); @@ -1591,6 +1597,7 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp, 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; + struct drm_connector *connector = &intel_connector->base; int lane_count, clock; int min_lane_count = 1; int max_lane_count = intel_dp_max_lane_count(intel_dp); @@ -1638,6 +1645,12 @@ static int intel_dp_compute_bpp(struct intel_dp *intel_dp, if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) return false; + /* Fall back to lower link rate in case of failure in previous modeset */ + if (connector->link_status == DRM_MODE_LINK_STATUS_BAD) { + min_lane_count = max_lane_count = intel_dp->fallback_lane_count; + min_clock = max_clock = intel_dp->fallback_link_rate_index; + } + DRM_DEBUG_KMS("DP link computation with max lane count %i " "max bw %d pixel clock %iKHz\n", max_lane_count, common_rates[max_clock], @@ -2781,6 +2794,8 @@ static void intel_enable_dp(struct intel_encoder *encoder, struct drm_device *dev = encoder->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc); + struct intel_connector *intel_connector = intel_dp->attached_connector; + struct drm_connector *connector = &intel_connector->base; uint32_t dp_reg = I915_READ(intel_dp->output_reg); enum pipe pipe = crtc->pipe; @@ -2811,7 +2826,23 @@ static void intel_enable_dp(struct intel_encoder *encoder, } intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); - intel_dp_start_link_train(intel_dp); + if (!intel_dp_start_link_train(intel_dp)) { + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", + intel_dp->link_rate, intel_dp->lane_count); + if (!intel_dp_get_link_train_fallback_values(intel_dp, + intel_dp->link_rate, + intel_dp->lane_count)) + /* Schedule a Hotplug Uevent to userspace to start modeset */ + schedule_work(&intel_connector->modeset_retry_work); + } else { + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", + intel_dp->link_rate, intel_dp->lane_count); + intel_dp->fallback_link_rate_index = -1; + intel_dp->fallback_link_rate = 0; + intel_dp->fallback_lane_count = 0; + drm_mode_connector_set_link_status_property(connector, + DRM_MODE_LINK_STATUS_GOOD); + } intel_dp_stop_link_train(intel_dp); if (pipe_config->has_audio) { @@ -4018,6 +4049,8 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base; struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc); + struct intel_connector *intel_connector = intel_dp->attached_connector; + struct drm_connector *connector = &intel_connector->base; /* Suppress underruns caused by re-training */ intel_set_cpu_fifo_underrun_reporting(dev_priv, crtc->pipe, false); @@ -4025,7 +4058,23 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp) intel_set_pch_fifo_underrun_reporting(dev_priv, intel_crtc_pch_transcoder(crtc), false); - intel_dp_start_link_train(intel_dp); + if (!intel_dp_start_link_train(intel_dp)) { + DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane count = %d", + intel_dp->link_rate, intel_dp->lane_count); + if (!intel_dp_get_link_train_fallback_values(intel_dp, + intel_dp->link_rate, + intel_dp->lane_count)) + /* Schedule a Hotplug Uevent to userspace to start modeset */ + schedule_work(&intel_connector->modeset_retry_work); + } else { + DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane count = %d", + intel_dp->link_rate, intel_dp->lane_count); + intel_dp->fallback_link_rate_index = -1; + intel_dp->fallback_link_rate = 0; + intel_dp->fallback_lane_count = 0; + drm_mode_connector_set_link_status_property(connector, + DRM_MODE_LINK_STATUS_GOOD); + } intel_dp_stop_link_train(intel_dp); /* Keep underrun reporting disabled until things are stable */ @@ -4419,6 +4468,11 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, intel_dp->compliance_test_active = 0; intel_dp->compliance_test_type = 0; intel_dp->compliance_test_data = 0; + intel_dp->fallback_link_rate_index = -1; + intel_dp->fallback_link_rate = 0; + intel_dp->fallback_lane_count = 0; + drm_mode_connector_set_link_status_property(connector, + DRM_MODE_LINK_STATUS_GOOD); if (intel_dp->is_mst) { DRM_DEBUG_KMS("MST device may have disappeared %d vs %d\n", @@ -4510,6 +4564,11 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv, DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, connector->name); + /* If this is a retry due to link trianing failure */ + if (status == connector_status_connected && + connector->link_status == DRM_MODE_LINK_STATUS_BAD) + return status; + /* If full detect is not performed yet, do a full detect */ if (!intel_dp->detect_done) status = intel_dp_long_pulse(intel_dp->attached_connector); @@ -5677,6 +5736,37 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, return false; } +static void intel_dp_modeset_retry_work_fn(struct work_struct *work) +{ + struct intel_connector *intel_connector; + struct drm_connector *connector; + struct drm_display_mode *mode; + bool verbose_prune = true; + + intel_connector = container_of(work, typeof(*intel_connector), + modeset_retry_work); + connector = &intel_connector->base; + + /* Grab the locks before changing connector property*/ + mutex_lock(&connector->dev->mode_config.mutex); + DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id, + connector->name); + list_for_each_entry(mode, &connector->modes, head) { + mode->status = intel_dp_mode_valid(connector, + mode); + } + drm_mode_prune_invalid(connector->dev, &connector->modes, + verbose_prune); + /* Set connector link status to BAD and send a Uevent to notify + * userspace to do a modeset. + */ + drm_mode_connector_set_link_status_property(connector, + DRM_MODE_LINK_STATUS_BAD); + mutex_unlock(&connector->dev->mode_config.mutex); + /* Send Hotplug uevent so userspace can reprobe */ + drm_kms_helper_hotplug_event(connector->dev); +} + bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port, struct intel_connector *intel_connector) @@ -5689,6 +5779,10 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, enum port port = intel_dig_port->port; int type; + /* Initialize the work for modeset in case of link train failure */ + INIT_WORK(&intel_connector->modeset_retry_work, + intel_dp_modeset_retry_work_fn); + if (WARN(intel_dig_port->max_lanes < 1, "Not enough lanes (%d) for DP on port %c\n", intel_dig_port->max_lanes, port_name(port))) diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c index 0048b52..10f81ab 100644 --- a/drivers/gpu/drm/i915/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c @@ -310,9 +310,15 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp) DP_TRAINING_PATTERN_DISABLE); } -void +bool intel_dp_start_link_train(struct intel_dp *intel_dp) { - intel_dp_link_training_clock_recovery(intel_dp); - intel_dp_link_training_channel_equalization(intel_dp); + bool ret; + + if (intel_dp_link_training_clock_recovery(intel_dp)) { + ret = intel_dp_link_training_channel_equalization(intel_dp); + if (ret) + return true; + } + return false; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 55ceb44..88918f4 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -312,6 +312,9 @@ struct intel_connector { void *port; /* store this opaque as its illegal to dereference it */ struct intel_dp *mst_port; + + /* Work struct to schedule a uevent on link train failure */ + struct work_struct modeset_retry_work; }; struct dpll { @@ -890,7 +893,6 @@ struct intel_dp { int fallback_link_rate; uint8_t fallback_lane_count; int fallback_link_rate_index; - bool link_train_failed; uint8_t sink_count; bool link_mst; bool has_audio; @@ -1389,7 +1391,7 @@ void intel_dp_set_link_params(struct intel_dp *intel_dp, bool link_mst); int intel_dp_get_link_train_fallback_values(struct intel_dp *intel_dp, int link_rate, uint8_t lane_count); -void intel_dp_start_link_train(struct intel_dp *intel_dp); +bool intel_dp_start_link_train(struct intel_dp *intel_dp); void intel_dp_stop_link_train(struct intel_dp *intel_dp); void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode); void intel_dp_encoder_reset(struct drm_encoder *encoder);