Message ID | 1473294615-27061-1-git-send-email-manasi.d.navare@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Reviewed-by: Mika Kahola <mika.kahola@intel.com> On Wed, 2016-09-07 at 17:30 -0700, Manasi Navare wrote: > According to the DisplayPort Spec, in case of Clock Recovery failure > the link training sequence should fall back to the lower link rate > followed by lower lane count until CR succeeds. > On CR success, the sequence proceeds with Channel EQ. > In case of Channel EQ failures, it should fallback to > lower link rate and lane count and start the CR phase again. > > v4: > * Fixed the link rate fallback loop (Manasi Navare) > v3: > * Fixed some rebase issues (Mika Kahola) > v2: > * Add a helper function to return index of requested link rate > into common_rates array > * Changed the link rate fallback loop to make use > of common_rates array (Mika Kahola) > * Changed INTEL_INFO to INTEL_GEN (David Weinehall) > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 109 > +++++++++++++++++++++++--- > drivers/gpu/drm/i915/intel_dp.c | 15 ++++ > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > drivers/gpu/drm/i915/intel_drv.h | 6 +- > 4 files changed, 128 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 25e7973..1278daa 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1634,19 +1634,18 @@ void intel_ddi_clk_select(struct > intel_encoder *encoder, > } > } > > -static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > +static void intel_ddi_pre_enable_edp(struct intel_encoder *encoder, > int link_rate, uint32_t > lane_count, > - struct intel_shared_dpll *pll, > - bool link_mst) > + struct intel_shared_dpll *pll) > { > 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); > > intel_dp_set_link_params(intel_dp, link_rate, lane_count, > - link_mst); > - if (encoder->type == INTEL_OUTPUT_EDP) > - intel_edp_panel_on(intel_dp); > + false); > + > + intel_edp_panel_on(intel_dp); > > intel_ddi_clk_select(encoder, pll); > intel_prepare_dp_ddi_buffers(encoder); > @@ -1657,6 +1656,28 @@ static void intel_ddi_pre_enable_dp(struct > intel_encoder *encoder, > intel_dp_stop_link_train(intel_dp); > } > > +static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > + int link_rate, uint32_t > lane_count, > + struct intel_shared_dpll *pll, > + bool link_mst) > +{ > + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > + struct drm_i915_private *dev_priv = to_i915(encoder- > >base.dev); > + struct intel_shared_dpll_config tmp_pll_config; > + > + /* Disable the PLL and obtain the PLL for Link Training > + * that starts with highest link rate and lane count. > + */ > + tmp_pll_config = pll->config; > + pll->funcs.disable(dev_priv, pll); > + 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))) > + drm_kms_helper_hotplug_event(encoder->base.dev); > + pll->config = tmp_pll_config; > +} > + > static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, > bool has_hdmi_sink, > struct drm_display_mode > *adjusted_mode, > @@ -1690,20 +1711,26 @@ static void intel_ddi_pre_enable(struct > intel_encoder *intel_encoder, > struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); > int type = intel_encoder->type; > > - if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) { > + if (type == INTEL_OUTPUT_EDP) > + intel_ddi_pre_enable_edp(intel_encoder, > + crtc->config->port_clock, > + crtc->config->lane_count, > + crtc->config->shared_dpll); > + > + if (type == INTEL_OUTPUT_DP) > intel_ddi_pre_enable_dp(intel_encoder, > crtc->config->port_clock, > crtc->config->lane_count, > crtc->config->shared_dpll, > intel_crtc_has_type(crtc- > >config, > INTEL_OU > TPUT_DP_MST)); > - } > - if (type == INTEL_OUTPUT_HDMI) { > + > + if (type == INTEL_OUTPUT_HDMI) > intel_ddi_pre_enable_hdmi(intel_encoder, > crtc->config- > >has_hdmi_sink, > &crtc->config- > >base.adjusted_mode, > crtc->config- > >shared_dpll); > - } > + > } > > static void intel_ddi_post_disable(struct intel_encoder > *intel_encoder, > @@ -2432,6 +2459,68 @@ intel_ddi_get_link_dpll(struct intel_dp > *intel_dp, int clock) > return pll; > } > > +bool > +intel_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate, > + uint8_t max_lane_count, bool link_mst) > +{ > + 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; > + int link_rate, link_rate_index; > + uint8_t lane_count; > + int common_rates[DP_MAX_SUPPORTED_RATES] = {}; > + bool ret = false; > + > + link_rate_index = intel_dp_link_rate_index(intel_dp, > common_rates, > + max_link_rate); > + if (link_rate_index < 0) { > + DRM_ERROR("Invalid Link Rate\n"); > + return false; > + } > + for (lane_count = max_lane_count; lane_count > 0; lane_count > >>= 1) { > + for (; link_rate_index >= 0; link_rate_index --) { > + link_rate = common_rates[link_rate_index]; > + pll = intel_ddi_get_link_dpll(intel_dp, > link_rate); > + if (pll == NULL) { > + DRM_ERROR("Could not find DPLL for > link " > + "training.\n"); > + return false; > + } > + tmp_pll_config = pll->config; > + pll->funcs.enable(dev_priv, pll); > + > + intel_dp_set_link_params(intel_dp, > link_rate, > + lane_count, > link_mst); > + > + intel_ddi_clk_select(encoder, pll); > + intel_prepare_dp_ddi_buffers(encoder); > + intel_ddi_init_dp_buf_reg(encoder); > + intel_dp_sink_dpms(intel_dp, > DRM_MODE_DPMS_ON); > + ret = intel_dp_start_link_train(intel_dp); > + if (ret) > + break; > + > + /* 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) { > + DRM_DEBUG_KMS("Link Training successful at > link rate: " > + "%d lane:%d\n", link_rate, > lane_count); > + break; > + } > + } > + intel_dp_stop_link_train(intel_dp); > + > + if (!lane_count) > + DRM_ERROR("Link Training Failed\n"); > + > + return ret; > +} > + > 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 75ac62f..1378116 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1443,6 +1443,21 @@ intel_dp_max_link_rate(struct intel_dp > *intel_dp) > return rates[len - 1]; > } > > +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int > *common_rates, > + int link_rate) > +{ > + int common_len; > + int index; > + > + common_len = intel_dp_common_rates(intel_dp, common_rates); > + for (index = common_len - 1; index >= 0; index--) { > + if (link_rate == common_rates[index]) > + return index; > + } > + > + return -1; > +} > + > int intel_dp_rate_select(struct intel_dp *intel_dp, int rate) > { > return rate_to_index(rate, intel_dp->sink_rates); > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c > b/drivers/gpu/drm/i915/intel_dp_link_training.c > index c438b02..f1e08f0 100644 > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c > @@ -313,9 +313,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 ca51e1a..5b97a7d4 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1160,6 +1160,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_ddi_link_train(struct intel_dp *intel_dp, int > max_link_rate, > + uint8_t max_lane_count, bool link_mst); > 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, > @@ -1381,7 +1383,7 @@ bool intel_dp_init_connector(struct > intel_digital_port *intel_dig_port, > void intel_dp_set_link_params(struct intel_dp *intel_dp, > int link_rate, uint8_t lane_count, > bool link_mst); > -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); > @@ -1403,6 +1405,8 @@ void intel_dp_add_properties(struct intel_dp > *intel_dp, struct drm_connector *co > void intel_dp_mst_suspend(struct drm_device *dev); > void intel_dp_mst_resume(struct drm_device *dev); > int intel_dp_max_link_rate(struct intel_dp *intel_dp); > +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int > *common_rates, > + int link_rate); > int intel_dp_rate_select(struct intel_dp *intel_dp, int rate); > void intel_dp_hot_plug(struct intel_encoder *intel_encoder); > void intel_power_sequencer_reset(struct drm_i915_private *dev_priv);
On Wed, Sep 7, 2016 at 5:30 PM, Manasi Navare <manasi.d.navare@intel.com> wrote: > According to the DisplayPort Spec, in case of Clock Recovery failure > the link training sequence should fall back to the lower link rate > followed by lower lane count until CR succeeds. > On CR success, the sequence proceeds with Channel EQ. > In case of Channel EQ failures, it should fallback to > lower link rate and lane count and start the CR phase again. > > v4: > * Fixed the link rate fallback loop (Manasi Navare) > v3: > * Fixed some rebase issues (Mika Kahola) > v2: > * Add a helper function to return index of requested link rate > into common_rates array > * Changed the link rate fallback loop to make use > of common_rates array (Mika Kahola) > * Changed INTEL_INFO to INTEL_GEN (David Weinehall) > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 109 +++++++++++++++++++++++--- > drivers/gpu/drm/i915/intel_dp.c | 15 ++++ > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > drivers/gpu/drm/i915/intel_drv.h | 6 +- > 4 files changed, 128 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 25e7973..1278daa 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1634,19 +1634,18 @@ void intel_ddi_clk_select(struct intel_encoder *encoder, > } > } > > -static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > +static void intel_ddi_pre_enable_edp(struct intel_encoder *encoder, > int link_rate, uint32_t lane_count, > - struct intel_shared_dpll *pll, > - bool link_mst) > + struct intel_shared_dpll *pll) > { > 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); > > intel_dp_set_link_params(intel_dp, link_rate, lane_count, > - link_mst); > - if (encoder->type == INTEL_OUTPUT_EDP) > - intel_edp_panel_on(intel_dp); > + false); > + > + intel_edp_panel_on(intel_dp); > > intel_ddi_clk_select(encoder, pll); > intel_prepare_dp_ddi_buffers(encoder); > @@ -1657,6 +1656,28 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > intel_dp_stop_link_train(intel_dp); > } > > +static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > + int link_rate, uint32_t lane_count, > + struct intel_shared_dpll *pll, > + bool link_mst) > +{ > + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > + struct intel_shared_dpll_config tmp_pll_config; > + > + /* Disable the PLL and obtain the PLL for Link Training > + * that starts with highest link rate and lane count. > + */ > + tmp_pll_config = pll->config; > + pll->funcs.disable(dev_priv, pll); > + 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))) > + drm_kms_helper_hotplug_event(encoder->base.dev); > + pll->config = tmp_pll_config; > +} > + > static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, > bool has_hdmi_sink, > struct drm_display_mode *adjusted_mode, > @@ -1690,20 +1711,26 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder, > struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); > int type = intel_encoder->type; > > - if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) { > + if (type == INTEL_OUTPUT_EDP) > + intel_ddi_pre_enable_edp(intel_encoder, > + crtc->config->port_clock, > + crtc->config->lane_count, > + crtc->config->shared_dpll); > + > + if (type == INTEL_OUTPUT_DP) > intel_ddi_pre_enable_dp(intel_encoder, > crtc->config->port_clock, > crtc->config->lane_count, > crtc->config->shared_dpll, > intel_crtc_has_type(crtc->config, > INTEL_OUTPUT_DP_MST)); > - } > - if (type == INTEL_OUTPUT_HDMI) { > + > + if (type == INTEL_OUTPUT_HDMI) > intel_ddi_pre_enable_hdmi(intel_encoder, > crtc->config->has_hdmi_sink, > &crtc->config->base.adjusted_mode, > crtc->config->shared_dpll); > - } > + > } > > static void intel_ddi_post_disable(struct intel_encoder *intel_encoder, > @@ -2432,6 +2459,68 @@ intel_ddi_get_link_dpll(struct intel_dp *intel_dp, int clock) > return pll; > } > > +bool > +intel_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate, > + uint8_t max_lane_count, bool link_mst) > +{ > + 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; > + int link_rate, link_rate_index; > + uint8_t lane_count; > + int common_rates[DP_MAX_SUPPORTED_RATES] = {}; > + bool ret = false; > + > + link_rate_index = intel_dp_link_rate_index(intel_dp, common_rates, > + max_link_rate); > + if (link_rate_index < 0) { > + DRM_ERROR("Invalid Link Rate\n"); > + return false; > + } > + for (lane_count = max_lane_count; lane_count > 0; lane_count >>= 1) { > + for (; link_rate_index >= 0; link_rate_index --) { ERROR: space prohibited before that '--' (ctx:WxB) #148: FILE: drivers/gpu/drm/i915/intel_ddi.c:2483: + for (; link_rate_index >= 0; link_rate_index --) { ^ > + link_rate = common_rates[link_rate_index]; > + pll = intel_ddi_get_link_dpll(intel_dp, link_rate); > + if (pll == NULL) { > + DRM_ERROR("Could not find DPLL for link " > + "training.\n"); The most common way in our code is to not split the comment and go over 80 cols. checkpatch complains either ways anyway: WARNING: quoted string split across lines #153: FILE: drivers/gpu/drm/i915/intel_ddi.c:2488: + DRM_ERROR("Could not find DPLL for link " + "training.\n"); Not sure what our maintainer prefer. Jani? > + return false; > + } > + tmp_pll_config = pll->config; > + pll->funcs.enable(dev_priv, pll); > + > + intel_dp_set_link_params(intel_dp, link_rate, > + lane_count, link_mst); > + > + intel_ddi_clk_select(encoder, pll); > + intel_prepare_dp_ddi_buffers(encoder); > + intel_ddi_init_dp_buf_reg(encoder); > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > + ret = intel_dp_start_link_train(intel_dp); > + if (ret) > + break; > + > + /* Disable port followed by PLL for next retry/clean up */ And here it goes over 80 cols the most common is to /* * Disable... * ...clean up */ > + intel_ddi_post_disable(encoder, NULL, NULL); > + 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); also here and in 2 other places in drm-i915-Make-DP-link-training-channel-equalization-.patch Thanks, Rodrigo. > + break; > + } > + } > + intel_dp_stop_link_train(intel_dp); > + > + if (!lane_count) > + DRM_ERROR("Link Training Failed\n"); > + > + return ret; > +} > + > 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 75ac62f..1378116 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1443,6 +1443,21 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp) > return rates[len - 1]; > } > > +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int *common_rates, > + int link_rate) > +{ > + int common_len; > + int index; > + > + common_len = intel_dp_common_rates(intel_dp, common_rates); > + for (index = common_len - 1; index >= 0; index--) { > + if (link_rate == common_rates[index]) > + return index; > + } > + > + return -1; > +} > + > int intel_dp_rate_select(struct intel_dp *intel_dp, int rate) > { > return rate_to_index(rate, intel_dp->sink_rates); > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > index c438b02..f1e08f0 100644 > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c > @@ -313,9 +313,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 ca51e1a..5b97a7d4 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1160,6 +1160,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_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate, > + uint8_t max_lane_count, bool link_mst); > 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, > @@ -1381,7 +1383,7 @@ bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > void intel_dp_set_link_params(struct intel_dp *intel_dp, > int link_rate, uint8_t lane_count, > bool link_mst); > -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); > @@ -1403,6 +1405,8 @@ void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *co > void intel_dp_mst_suspend(struct drm_device *dev); > void intel_dp_mst_resume(struct drm_device *dev); > int intel_dp_max_link_rate(struct intel_dp *intel_dp); > +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int *common_rates, > + int link_rate); > int intel_dp_rate_select(struct intel_dp *intel_dp, int rate); > void intel_dp_hot_plug(struct intel_encoder *intel_encoder); > void intel_power_sequencer_reset(struct drm_i915_private *dev_priv); > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 08 Sep 2016, Manasi Navare <manasi.d.navare@intel.com> wrote: > According to the DisplayPort Spec, in case of Clock Recovery failure > the link training sequence should fall back to the lower link rate > followed by lower lane count until CR succeeds. > On CR success, the sequence proceeds with Channel EQ. > In case of Channel EQ failures, it should fallback to > lower link rate and lane count and start the CR phase again. > > v4: > * Fixed the link rate fallback loop (Manasi Navare) > v3: > * Fixed some rebase issues (Mika Kahola) > v2: > * Add a helper function to return index of requested link rate > into common_rates array > * Changed the link rate fallback loop to make use > of common_rates array (Mika Kahola) > * Changed INTEL_INFO to INTEL_GEN (David Weinehall) > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 109 +++++++++++++++++++++++--- > drivers/gpu/drm/i915/intel_dp.c | 15 ++++ > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > drivers/gpu/drm/i915/intel_drv.h | 6 +- > 4 files changed, 128 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 25e7973..1278daa 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1634,19 +1634,18 @@ void intel_ddi_clk_select(struct intel_encoder *encoder, > } > } > > -static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > +static void intel_ddi_pre_enable_edp(struct intel_encoder *encoder, > int link_rate, uint32_t lane_count, > - struct intel_shared_dpll *pll, > - bool link_mst) > + struct intel_shared_dpll *pll) > { > 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); > > intel_dp_set_link_params(intel_dp, link_rate, lane_count, > - link_mst); > - if (encoder->type == INTEL_OUTPUT_EDP) > - intel_edp_panel_on(intel_dp); > + false); > + > + intel_edp_panel_on(intel_dp); > > intel_ddi_clk_select(encoder, pll); > intel_prepare_dp_ddi_buffers(encoder); > @@ -1657,6 +1656,28 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > intel_dp_stop_link_train(intel_dp); > } > > +static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > + int link_rate, uint32_t lane_count, > + struct intel_shared_dpll *pll, > + bool link_mst) > +{ > + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > + struct intel_shared_dpll_config tmp_pll_config; > + > + /* Disable the PLL and obtain the PLL for Link Training > + * that starts with highest link rate and lane count. > + */ > + tmp_pll_config = pll->config; > + pll->funcs.disable(dev_priv, pll); > + 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))) Redundant parens. > + drm_kms_helper_hotplug_event(encoder->base.dev); > + pll->config = tmp_pll_config; > +} > + > static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, > bool has_hdmi_sink, > struct drm_display_mode *adjusted_mode, > @@ -1690,20 +1711,26 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder, > struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); > int type = intel_encoder->type; > > - if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) { > + if (type == INTEL_OUTPUT_EDP) > + intel_ddi_pre_enable_edp(intel_encoder, > + crtc->config->port_clock, > + crtc->config->lane_count, > + crtc->config->shared_dpll); > + > + if (type == INTEL_OUTPUT_DP) > intel_ddi_pre_enable_dp(intel_encoder, > crtc->config->port_clock, > crtc->config->lane_count, > crtc->config->shared_dpll, > intel_crtc_has_type(crtc->config, > INTEL_OUTPUT_DP_MST)); > - } > - if (type == INTEL_OUTPUT_HDMI) { > + > + if (type == INTEL_OUTPUT_HDMI) > intel_ddi_pre_enable_hdmi(intel_encoder, > crtc->config->has_hdmi_sink, > &crtc->config->base.adjusted_mode, > crtc->config->shared_dpll); > - } > + > } > > static void intel_ddi_post_disable(struct intel_encoder *intel_encoder, > @@ -2432,6 +2459,68 @@ intel_ddi_get_link_dpll(struct intel_dp *intel_dp, int clock) > return pll; > } > > +bool > +intel_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate, > + uint8_t max_lane_count, bool link_mst) > +{ > + 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; > + int link_rate, link_rate_index; > + uint8_t lane_count; > + int common_rates[DP_MAX_SUPPORTED_RATES] = {}; > + bool ret = false; > + > + link_rate_index = intel_dp_link_rate_index(intel_dp, common_rates, > + max_link_rate); > + if (link_rate_index < 0) { > + DRM_ERROR("Invalid Link Rate\n"); > + return false; > + } > + for (lane_count = max_lane_count; lane_count > 0; lane_count >>= 1) { > + for (; link_rate_index >= 0; link_rate_index --) { Once you've reached link_rate_index < 0 here, you won't try lower lane counts anymore and the nested loop is useless, i.e. you never reset link_rate_index back. > + link_rate = common_rates[link_rate_index]; > + pll = intel_ddi_get_link_dpll(intel_dp, link_rate); > + if (pll == NULL) { > + DRM_ERROR("Could not find DPLL for link " > + "training.\n"); > + return false; > + } > + tmp_pll_config = pll->config; > + pll->funcs.enable(dev_priv, pll); > + > + intel_dp_set_link_params(intel_dp, link_rate, > + lane_count, link_mst); > + > + intel_ddi_clk_select(encoder, pll); > + intel_prepare_dp_ddi_buffers(encoder); > + intel_ddi_init_dp_buf_reg(encoder); > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > + ret = intel_dp_start_link_train(intel_dp); > + if (ret) > + break; > + > + /* 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) { > + DRM_DEBUG_KMS("Link Training successful at link rate: " > + "%d lane:%d\n", link_rate, lane_count); > + break; > + } > + } > + intel_dp_stop_link_train(intel_dp); > + > + if (!lane_count) > + DRM_ERROR("Link Training Failed\n"); > + > + return ret; > +} > + > 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 75ac62f..1378116 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1443,6 +1443,21 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp) > return rates[len - 1]; > } > > +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int *common_rates, > + int link_rate) > +{ > + int common_len; > + int index; > + > + common_len = intel_dp_common_rates(intel_dp, common_rates); > + for (index = common_len - 1; index >= 0; index--) { > + if (link_rate == common_rates[index]) > + return index; > + } I think it's always easier to have the paradigm for loop, and do the math using the length and index, like this: common_len = intel_dp_common_rates(intel_dp, common_rates); for (i = 0; i < common_len; i++) if (link_rate == common_rates[common_len - i - 1]) return i; > + > + return -1; > +} > + > int intel_dp_rate_select(struct intel_dp *intel_dp, int rate) > { > return rate_to_index(rate, intel_dp->sink_rates); > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > index c438b02..f1e08f0 100644 > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c > @@ -313,9 +313,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; > + } Prefer early returns instead of adding more indentation. > + return false; > } > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index ca51e1a..5b97a7d4 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1160,6 +1160,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_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate, > + uint8_t max_lane_count, bool link_mst); > 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, > @@ -1381,7 +1383,7 @@ bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > void intel_dp_set_link_params(struct intel_dp *intel_dp, > int link_rate, uint8_t lane_count, > bool link_mst); > -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); > @@ -1403,6 +1405,8 @@ void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *co > void intel_dp_mst_suspend(struct drm_device *dev); > void intel_dp_mst_resume(struct drm_device *dev); > int intel_dp_max_link_rate(struct intel_dp *intel_dp); > +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int *common_rates, > + int link_rate); > int intel_dp_rate_select(struct intel_dp *intel_dp, int rate); > void intel_dp_hot_plug(struct intel_encoder *intel_encoder); > void intel_power_sequencer_reset(struct drm_i915_private *dev_priv);
On Fri, 09 Sep 2016, Rodrigo Vivi <rodrigo.vivi@gmail.com> wrote: > On Wed, Sep 7, 2016 at 5:30 PM, Manasi Navare <manasi.d.navare@intel.com> wrote: >> According to the DisplayPort Spec, in case of Clock Recovery failure >> the link training sequence should fall back to the lower link rate >> followed by lower lane count until CR succeeds. >> On CR success, the sequence proceeds with Channel EQ. >> In case of Channel EQ failures, it should fallback to >> lower link rate and lane count and start the CR phase again. >> >> v4: >> * Fixed the link rate fallback loop (Manasi Navare) >> v3: >> * Fixed some rebase issues (Mika Kahola) >> v2: >> * Add a helper function to return index of requested link rate >> into common_rates array >> * Changed the link rate fallback loop to make use >> of common_rates array (Mika Kahola) >> * Changed INTEL_INFO to INTEL_GEN (David Weinehall) >> >> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> >> --- >> drivers/gpu/drm/i915/intel_ddi.c | 109 +++++++++++++++++++++++--- >> drivers/gpu/drm/i915/intel_dp.c | 15 ++++ >> drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- >> drivers/gpu/drm/i915/intel_drv.h | 6 +- >> 4 files changed, 128 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c >> index 25e7973..1278daa 100644 >> --- a/drivers/gpu/drm/i915/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/intel_ddi.c >> @@ -1634,19 +1634,18 @@ void intel_ddi_clk_select(struct intel_encoder *encoder, >> } >> } >> >> -static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, >> +static void intel_ddi_pre_enable_edp(struct intel_encoder *encoder, >> int link_rate, uint32_t lane_count, >> - struct intel_shared_dpll *pll, >> - bool link_mst) >> + struct intel_shared_dpll *pll) >> { >> 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); >> >> intel_dp_set_link_params(intel_dp, link_rate, lane_count, >> - link_mst); >> - if (encoder->type == INTEL_OUTPUT_EDP) >> - intel_edp_panel_on(intel_dp); >> + false); >> + >> + intel_edp_panel_on(intel_dp); >> >> intel_ddi_clk_select(encoder, pll); >> intel_prepare_dp_ddi_buffers(encoder); >> @@ -1657,6 +1656,28 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, >> intel_dp_stop_link_train(intel_dp); >> } >> >> +static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, >> + int link_rate, uint32_t lane_count, >> + struct intel_shared_dpll *pll, >> + bool link_mst) >> +{ >> + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); >> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> + struct intel_shared_dpll_config tmp_pll_config; >> + >> + /* Disable the PLL and obtain the PLL for Link Training >> + * that starts with highest link rate and lane count. >> + */ >> + tmp_pll_config = pll->config; >> + pll->funcs.disable(dev_priv, pll); >> + 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))) >> + drm_kms_helper_hotplug_event(encoder->base.dev); >> + pll->config = tmp_pll_config; >> +} >> + >> static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, >> bool has_hdmi_sink, >> struct drm_display_mode *adjusted_mode, >> @@ -1690,20 +1711,26 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder, >> struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); >> int type = intel_encoder->type; >> >> - if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) { >> + if (type == INTEL_OUTPUT_EDP) >> + intel_ddi_pre_enable_edp(intel_encoder, >> + crtc->config->port_clock, >> + crtc->config->lane_count, >> + crtc->config->shared_dpll); >> + >> + if (type == INTEL_OUTPUT_DP) >> intel_ddi_pre_enable_dp(intel_encoder, >> crtc->config->port_clock, >> crtc->config->lane_count, >> crtc->config->shared_dpll, >> intel_crtc_has_type(crtc->config, >> INTEL_OUTPUT_DP_MST)); >> - } >> - if (type == INTEL_OUTPUT_HDMI) { >> + >> + if (type == INTEL_OUTPUT_HDMI) >> intel_ddi_pre_enable_hdmi(intel_encoder, >> crtc->config->has_hdmi_sink, >> &crtc->config->base.adjusted_mode, >> crtc->config->shared_dpll); >> - } >> + >> } >> >> static void intel_ddi_post_disable(struct intel_encoder *intel_encoder, >> @@ -2432,6 +2459,68 @@ intel_ddi_get_link_dpll(struct intel_dp *intel_dp, int clock) >> return pll; >> } >> >> +bool >> +intel_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate, >> + uint8_t max_lane_count, bool link_mst) >> +{ >> + 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; >> + int link_rate, link_rate_index; >> + uint8_t lane_count; >> + int common_rates[DP_MAX_SUPPORTED_RATES] = {}; >> + bool ret = false; >> + >> + link_rate_index = intel_dp_link_rate_index(intel_dp, common_rates, >> + max_link_rate); >> + if (link_rate_index < 0) { >> + DRM_ERROR("Invalid Link Rate\n"); >> + return false; >> + } >> + for (lane_count = max_lane_count; lane_count > 0; lane_count >>= 1) { >> + for (; link_rate_index >= 0; link_rate_index --) { > > ERROR: space prohibited before that '--' (ctx:WxB) > #148: FILE: drivers/gpu/drm/i915/intel_ddi.c:2483: > + for (; link_rate_index >= 0; link_rate_index --) { > ^ >> + link_rate = common_rates[link_rate_index]; >> + pll = intel_ddi_get_link_dpll(intel_dp, link_rate); >> + if (pll == NULL) { >> + DRM_ERROR("Could not find DPLL for link " >> + "training.\n"); > > The most common way in our code is to not split the comment and go over 80 cols. > > checkpatch complains either ways anyway: > > WARNING: quoted string split across lines > #153: FILE: drivers/gpu/drm/i915/intel_ddi.c:2488: > + DRM_ERROR("Could not find DPLL for link " > + "training.\n"); > > Not sure what our maintainer prefer. Jani? In this case, I'd prefer rigorous review of the functional aspects of the changes. > > >> + return false; >> + } >> + tmp_pll_config = pll->config; >> + pll->funcs.enable(dev_priv, pll); >> + >> + intel_dp_set_link_params(intel_dp, link_rate, >> + lane_count, link_mst); >> + >> + intel_ddi_clk_select(encoder, pll); >> + intel_prepare_dp_ddi_buffers(encoder); >> + intel_ddi_init_dp_buf_reg(encoder); >> + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); >> + ret = intel_dp_start_link_train(intel_dp); >> + if (ret) >> + break; >> + >> + /* Disable port followed by PLL for next retry/clean up */ > > And here it goes over 80 cols the most common is to > > /* > * Disable... > * ...clean up > */ > >> + intel_ddi_post_disable(encoder, NULL, NULL); >> + 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); > > also here and in 2 other places in > drm-i915-Make-DP-link-training-channel-equalization-.patch > > Thanks, > Rodrigo. > >> + break; >> + } >> + } >> + intel_dp_stop_link_train(intel_dp); >> + >> + if (!lane_count) >> + DRM_ERROR("Link Training Failed\n"); >> + >> + return ret; >> +} >> + >> 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 75ac62f..1378116 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -1443,6 +1443,21 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp) >> return rates[len - 1]; >> } >> >> +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int *common_rates, >> + int link_rate) >> +{ >> + int common_len; >> + int index; >> + >> + common_len = intel_dp_common_rates(intel_dp, common_rates); >> + for (index = common_len - 1; index >= 0; index--) { >> + if (link_rate == common_rates[index]) >> + return index; >> + } >> + >> + return -1; >> +} >> + >> int intel_dp_rate_select(struct intel_dp *intel_dp, int rate) >> { >> return rate_to_index(rate, intel_dp->sink_rates); >> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c >> index c438b02..f1e08f0 100644 >> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c >> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c >> @@ -313,9 +313,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 ca51e1a..5b97a7d4 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -1160,6 +1160,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_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate, >> + uint8_t max_lane_count, bool link_mst); >> 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, >> @@ -1381,7 +1383,7 @@ bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port, >> void intel_dp_set_link_params(struct intel_dp *intel_dp, >> int link_rate, uint8_t lane_count, >> bool link_mst); >> -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); >> @@ -1403,6 +1405,8 @@ void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *co >> void intel_dp_mst_suspend(struct drm_device *dev); >> void intel_dp_mst_resume(struct drm_device *dev); >> int intel_dp_max_link_rate(struct intel_dp *intel_dp); >> +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int *common_rates, >> + int link_rate); >> int intel_dp_rate_select(struct intel_dp *intel_dp, int rate); >> void intel_dp_hot_plug(struct intel_encoder *intel_encoder); >> void intel_power_sequencer_reset(struct drm_i915_private *dev_priv); >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Sep 09, 2016 at 10:11:06AM +0300, Jani Nikula wrote: > On Thu, 08 Sep 2016, Manasi Navare <manasi.d.navare@intel.com> wrote: > > According to the DisplayPort Spec, in case of Clock Recovery failure > > the link training sequence should fall back to the lower link rate > > followed by lower lane count until CR succeeds. > > On CR success, the sequence proceeds with Channel EQ. > > In case of Channel EQ failures, it should fallback to > > lower link rate and lane count and start the CR phase again. > > > > v4: > > * Fixed the link rate fallback loop (Manasi Navare) > > v3: > > * Fixed some rebase issues (Mika Kahola) > > v2: > > * Add a helper function to return index of requested link rate > > into common_rates array > > * Changed the link rate fallback loop to make use > > of common_rates array (Mika Kahola) > > * Changed INTEL_INFO to INTEL_GEN (David Weinehall) > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 109 +++++++++++++++++++++++--- > > drivers/gpu/drm/i915/intel_dp.c | 15 ++++ > > drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- > > drivers/gpu/drm/i915/intel_drv.h | 6 +- > > 4 files changed, 128 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index 25e7973..1278daa 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1634,19 +1634,18 @@ void intel_ddi_clk_select(struct intel_encoder *encoder, > > } > > } > > > > -static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > > +static void intel_ddi_pre_enable_edp(struct intel_encoder *encoder, > > int link_rate, uint32_t lane_count, > > - struct intel_shared_dpll *pll, > > - bool link_mst) > > + struct intel_shared_dpll *pll) > > { > > 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); > > > > intel_dp_set_link_params(intel_dp, link_rate, lane_count, > > - link_mst); > > - if (encoder->type == INTEL_OUTPUT_EDP) > > - intel_edp_panel_on(intel_dp); > > + false); > > + > > + intel_edp_panel_on(intel_dp); > > > > intel_ddi_clk_select(encoder, pll); > > intel_prepare_dp_ddi_buffers(encoder); > > @@ -1657,6 +1656,28 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > > intel_dp_stop_link_train(intel_dp); > > } > > > > +static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, > > + int link_rate, uint32_t lane_count, > > + struct intel_shared_dpll *pll, > > + bool link_mst) > > +{ > > + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > + struct intel_shared_dpll_config tmp_pll_config; > > + > > + /* Disable the PLL and obtain the PLL for Link Training > > + * that starts with highest link rate and lane count. > > + */ > > + tmp_pll_config = pll->config; > > + pll->funcs.disable(dev_priv, pll); > > + 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))) > > Redundant parens. > > > + drm_kms_helper_hotplug_event(encoder->base.dev); > > + pll->config = tmp_pll_config; > > +} > > + > > static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, > > bool has_hdmi_sink, > > struct drm_display_mode *adjusted_mode, > > @@ -1690,20 +1711,26 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder, > > struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); > > int type = intel_encoder->type; > > > > - if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) { > > + if (type == INTEL_OUTPUT_EDP) > > + intel_ddi_pre_enable_edp(intel_encoder, > > + crtc->config->port_clock, > > + crtc->config->lane_count, > > + crtc->config->shared_dpll); > > + > > + if (type == INTEL_OUTPUT_DP) > > intel_ddi_pre_enable_dp(intel_encoder, > > crtc->config->port_clock, > > crtc->config->lane_count, > > crtc->config->shared_dpll, > > intel_crtc_has_type(crtc->config, > > INTEL_OUTPUT_DP_MST)); > > - } > > - if (type == INTEL_OUTPUT_HDMI) { > > + > > + if (type == INTEL_OUTPUT_HDMI) > > intel_ddi_pre_enable_hdmi(intel_encoder, > > crtc->config->has_hdmi_sink, > > &crtc->config->base.adjusted_mode, > > crtc->config->shared_dpll); > > - } > > + > > } > > > > static void intel_ddi_post_disable(struct intel_encoder *intel_encoder, > > @@ -2432,6 +2459,68 @@ intel_ddi_get_link_dpll(struct intel_dp *intel_dp, int clock) > > return pll; > > } > > > > +bool > > +intel_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate, > > + uint8_t max_lane_count, bool link_mst) > > +{ > > + 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; > > + int link_rate, link_rate_index; > > + uint8_t lane_count; > > + int common_rates[DP_MAX_SUPPORTED_RATES] = {}; > > + bool ret = false; > > + > > + link_rate_index = intel_dp_link_rate_index(intel_dp, common_rates, > > + max_link_rate); > > + if (link_rate_index < 0) { > > + DRM_ERROR("Invalid Link Rate\n"); > > + return false; > > + } > > + for (lane_count = max_lane_count; lane_count > 0; lane_count >>= 1) { > > + for (; link_rate_index >= 0; link_rate_index --) { > > Once you've reached link_rate_index < 0 here, you won't try lower lane > counts anymore and the nested loop is useless, i.e. you never reset > link_rate_index back. > Thanks for the review comments. Yes this reseting of link rate index got lost when the logic was changed to use common rates. I will make the necessary changes. > > + link_rate = common_rates[link_rate_index]; > > + pll = intel_ddi_get_link_dpll(intel_dp, link_rate); > > + if (pll == NULL) { > > + DRM_ERROR("Could not find DPLL for link " > > + "training.\n"); > > + return false; > > + } > > + tmp_pll_config = pll->config; > > + pll->funcs.enable(dev_priv, pll); > > + > > + intel_dp_set_link_params(intel_dp, link_rate, > > + lane_count, link_mst); > > + > > + intel_ddi_clk_select(encoder, pll); > > + intel_prepare_dp_ddi_buffers(encoder); > > + intel_ddi_init_dp_buf_reg(encoder); > > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > + ret = intel_dp_start_link_train(intel_dp); > > + if (ret) > > + break; > > + > > + /* 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) { > > + DRM_DEBUG_KMS("Link Training successful at link rate: " > > + "%d lane:%d\n", link_rate, lane_count); > > + break; > > + } > > + } > > + intel_dp_stop_link_train(intel_dp); > > + > > + if (!lane_count) > > + DRM_ERROR("Link Training Failed\n"); > > + > > + return ret; > > +} > > + > > 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 75ac62f..1378116 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -1443,6 +1443,21 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp) > > return rates[len - 1]; > > } > > > > +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int *common_rates, > > + int link_rate) > > +{ > > + int common_len; > > + int index; > > + > > + common_len = intel_dp_common_rates(intel_dp, common_rates); > > + for (index = common_len - 1; index >= 0; index--) { > > + if (link_rate == common_rates[index]) > > + return index; > > + } > > I think it's always easier to have the paradigm for loop, and do the > math using the length and index, like this: > > common_len = intel_dp_common_rates(intel_dp, common_rates); > for (i = 0; i < common_len; i++) > if (link_rate == common_rates[common_len - i - 1]) > return i; > Ok agreed I will change the loop to just common_len and do math using index and length. But we need to start comparing from the highest index since we will be starting the link training at highest link rate so lesser iterations if we start comparing from the highest index. Manasi > > + > > + return -1; > > +} > > + > > int intel_dp_rate_select(struct intel_dp *intel_dp, int rate) > > { > > return rate_to_index(rate, intel_dp->sink_rates); > > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c > > index c438b02..f1e08f0 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c > > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c > > @@ -313,9 +313,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; > > + } > > Prefer early returns instead of adding more indentation. > > > + return false; > > } > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index ca51e1a..5b97a7d4 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1160,6 +1160,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_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate, > > + uint8_t max_lane_count, bool link_mst); > > 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, > > @@ -1381,7 +1383,7 @@ bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port, > > void intel_dp_set_link_params(struct intel_dp *intel_dp, > > int link_rate, uint8_t lane_count, > > bool link_mst); > > -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); > > @@ -1403,6 +1405,8 @@ void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *co > > void intel_dp_mst_suspend(struct drm_device *dev); > > void intel_dp_mst_resume(struct drm_device *dev); > > int intel_dp_max_link_rate(struct intel_dp *intel_dp); > > +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int *common_rates, > > + int link_rate); > > int intel_dp_rate_select(struct intel_dp *intel_dp, int rate); > > void intel_dp_hot_plug(struct intel_encoder *intel_encoder); > > void intel_power_sequencer_reset(struct drm_i915_private *dev_priv); > > -- > 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 25e7973..1278daa 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1634,19 +1634,18 @@ void intel_ddi_clk_select(struct intel_encoder *encoder, } } -static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, +static void intel_ddi_pre_enable_edp(struct intel_encoder *encoder, int link_rate, uint32_t lane_count, - struct intel_shared_dpll *pll, - bool link_mst) + struct intel_shared_dpll *pll) { 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); intel_dp_set_link_params(intel_dp, link_rate, lane_count, - link_mst); - if (encoder->type == INTEL_OUTPUT_EDP) - intel_edp_panel_on(intel_dp); + false); + + intel_edp_panel_on(intel_dp); intel_ddi_clk_select(encoder, pll); intel_prepare_dp_ddi_buffers(encoder); @@ -1657,6 +1656,28 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, intel_dp_stop_link_train(intel_dp); } +static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, + int link_rate, uint32_t lane_count, + struct intel_shared_dpll *pll, + bool link_mst) +{ + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base); + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); + struct intel_shared_dpll_config tmp_pll_config; + + /* Disable the PLL and obtain the PLL for Link Training + * that starts with highest link rate and lane count. + */ + tmp_pll_config = pll->config; + pll->funcs.disable(dev_priv, pll); + 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))) + drm_kms_helper_hotplug_event(encoder->base.dev); + pll->config = tmp_pll_config; +} + static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, bool has_hdmi_sink, struct drm_display_mode *adjusted_mode, @@ -1690,20 +1711,26 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder, struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); int type = intel_encoder->type; - if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) { + if (type == INTEL_OUTPUT_EDP) + intel_ddi_pre_enable_edp(intel_encoder, + crtc->config->port_clock, + crtc->config->lane_count, + crtc->config->shared_dpll); + + if (type == INTEL_OUTPUT_DP) intel_ddi_pre_enable_dp(intel_encoder, crtc->config->port_clock, crtc->config->lane_count, crtc->config->shared_dpll, intel_crtc_has_type(crtc->config, INTEL_OUTPUT_DP_MST)); - } - if (type == INTEL_OUTPUT_HDMI) { + + if (type == INTEL_OUTPUT_HDMI) intel_ddi_pre_enable_hdmi(intel_encoder, crtc->config->has_hdmi_sink, &crtc->config->base.adjusted_mode, crtc->config->shared_dpll); - } + } static void intel_ddi_post_disable(struct intel_encoder *intel_encoder, @@ -2432,6 +2459,68 @@ intel_ddi_get_link_dpll(struct intel_dp *intel_dp, int clock) return pll; } +bool +intel_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate, + uint8_t max_lane_count, bool link_mst) +{ + 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; + int link_rate, link_rate_index; + uint8_t lane_count; + int common_rates[DP_MAX_SUPPORTED_RATES] = {}; + bool ret = false; + + link_rate_index = intel_dp_link_rate_index(intel_dp, common_rates, + max_link_rate); + if (link_rate_index < 0) { + DRM_ERROR("Invalid Link Rate\n"); + return false; + } + for (lane_count = max_lane_count; lane_count > 0; lane_count >>= 1) { + for (; link_rate_index >= 0; link_rate_index --) { + link_rate = common_rates[link_rate_index]; + pll = intel_ddi_get_link_dpll(intel_dp, link_rate); + if (pll == NULL) { + DRM_ERROR("Could not find DPLL for link " + "training.\n"); + return false; + } + tmp_pll_config = pll->config; + pll->funcs.enable(dev_priv, pll); + + intel_dp_set_link_params(intel_dp, link_rate, + lane_count, link_mst); + + intel_ddi_clk_select(encoder, pll); + intel_prepare_dp_ddi_buffers(encoder); + intel_ddi_init_dp_buf_reg(encoder); + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); + ret = intel_dp_start_link_train(intel_dp); + if (ret) + break; + + /* 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) { + DRM_DEBUG_KMS("Link Training successful at link rate: " + "%d lane:%d\n", link_rate, lane_count); + break; + } + } + intel_dp_stop_link_train(intel_dp); + + if (!lane_count) + DRM_ERROR("Link Training Failed\n"); + + return ret; +} + 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 75ac62f..1378116 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1443,6 +1443,21 @@ intel_dp_max_link_rate(struct intel_dp *intel_dp) return rates[len - 1]; } +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int *common_rates, + int link_rate) +{ + int common_len; + int index; + + common_len = intel_dp_common_rates(intel_dp, common_rates); + for (index = common_len - 1; index >= 0; index--) { + if (link_rate == common_rates[index]) + return index; + } + + return -1; +} + int intel_dp_rate_select(struct intel_dp *intel_dp, int rate) { return rate_to_index(rate, intel_dp->sink_rates); diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c index c438b02..f1e08f0 100644 --- a/drivers/gpu/drm/i915/intel_dp_link_training.c +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c @@ -313,9 +313,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 ca51e1a..5b97a7d4 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1160,6 +1160,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_ddi_link_train(struct intel_dp *intel_dp, int max_link_rate, + uint8_t max_lane_count, bool link_mst); 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, @@ -1381,7 +1383,7 @@ bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port, void intel_dp_set_link_params(struct intel_dp *intel_dp, int link_rate, uint8_t lane_count, bool link_mst); -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); @@ -1403,6 +1405,8 @@ void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *co void intel_dp_mst_suspend(struct drm_device *dev); void intel_dp_mst_resume(struct drm_device *dev); int intel_dp_max_link_rate(struct intel_dp *intel_dp); +int intel_dp_link_rate_index(struct intel_dp *intel_dp, int *common_rates, + int link_rate); int intel_dp_rate_select(struct intel_dp *intel_dp, int rate); void intel_dp_hot_plug(struct intel_encoder *intel_encoder); void intel_power_sequencer_reset(struct drm_i915_private *dev_priv);
According to the DisplayPort Spec, in case of Clock Recovery failure the link training sequence should fall back to the lower link rate followed by lower lane count until CR succeeds. On CR success, the sequence proceeds with Channel EQ. In case of Channel EQ failures, it should fallback to lower link rate and lane count and start the CR phase again. v4: * Fixed the link rate fallback loop (Manasi Navare) v3: * Fixed some rebase issues (Mika Kahola) v2: * Add a helper function to return index of requested link rate into common_rates array * Changed the link rate fallback loop to make use of common_rates array (Mika Kahola) * Changed INTEL_INFO to INTEL_GEN (David Weinehall) Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> --- drivers/gpu/drm/i915/intel_ddi.c | 109 +++++++++++++++++++++++--- drivers/gpu/drm/i915/intel_dp.c | 15 ++++ drivers/gpu/drm/i915/intel_dp_link_training.c | 12 ++- drivers/gpu/drm/i915/intel_drv.h | 6 +- 4 files changed, 128 insertions(+), 14 deletions(-)