On Mon, 2020-04-20 at 23:06 +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Sort out some of the mess between intel_ddi.c intel_dp.c by > introducing a .set_signal_levels() vfunc. Reviewed-by: José Roberto de Souza <jose.souza@xxxxxxxxx> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 81 +++++++--- > .../drm/i915/display/intel_display_types.h | 1 + > drivers/gpu/drm/i915/display/intel_dp.c | 146 +++++++++++----- > -- > 3 files changed, 150 insertions(+), 78 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index 7c223520ad6b..eb420069a469 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -2659,38 +2659,66 @@ static u32 intel_ddi_dp_level(struct intel_dp > *intel_dp) > return translate_signal_level(signal_levels); > } > > -u32 bxt_signal_levels(struct intel_dp *intel_dp) > +static void > +tgl_set_signal_levels(struct intel_dp *intel_dp) > { > - struct intel_digital_port *dport = dp_to_dig_port(intel_dp); > - struct drm_i915_private *dev_priv = to_i915(dport- > >base.base.dev); > - struct intel_encoder *encoder = &dport->base; > + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)- > >base; > int level = intel_ddi_dp_level(intel_dp); > > - if (INTEL_GEN(dev_priv) >= 12) > - tgl_ddi_vswing_sequence(encoder, intel_dp->link_rate, > - level, encoder->type); > - else if (INTEL_GEN(dev_priv) >= 11) > - icl_ddi_vswing_sequence(encoder, intel_dp->link_rate, > - level, encoder->type); > - else if (IS_CANNONLAKE(dev_priv)) > - cnl_ddi_vswing_sequence(encoder, level, encoder->type); > - else > - bxt_ddi_vswing_sequence(encoder, level, encoder->type); > - > - return 0; > + tgl_ddi_vswing_sequence(encoder, intel_dp->link_rate, > + level, encoder->type); > } > > -u32 ddi_signal_levels(struct intel_dp *intel_dp) > +static void > +icl_set_signal_levels(struct intel_dp *intel_dp) > { > - struct intel_digital_port *dport = dp_to_dig_port(intel_dp); > - struct drm_i915_private *dev_priv = to_i915(dport- > >base.base.dev); > - struct intel_encoder *encoder = &dport->base; > + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)- > >base; > int level = intel_ddi_dp_level(intel_dp); > > + icl_ddi_vswing_sequence(encoder, intel_dp->link_rate, > + level, encoder->type); > +} > + > +static void > +cnl_set_signal_levels(struct intel_dp *intel_dp) > +{ > + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)- > >base; > + int level = intel_ddi_dp_level(intel_dp); > + > + cnl_ddi_vswing_sequence(encoder, level, encoder->type); > +} > + > +static void > +bxt_set_signal_levels(struct intel_dp *intel_dp) > +{ > + struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)- > >base; > + int level = intel_ddi_dp_level(intel_dp); > + > + bxt_ddi_vswing_sequence(encoder, level, encoder->type); > +} > + > +static void > +hsw_set_signal_levels(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); > + int level = intel_ddi_dp_level(intel_dp); > + enum port port = encoder->port; > + u32 signal_levels; > + > + signal_levels = DDI_BUF_TRANS_SELECT(level); > + > + drm_dbg_kms(&dev_priv->drm, "Using signal levels %08x\n", > + signal_levels); > + > + intel_dp->DP &= ~DDI_BUF_EMP_MASK; > + intel_dp->DP |= signal_levels; > + > if (IS_GEN9_BC(dev_priv)) > skl_ddi_set_iboost(encoder, level, encoder->type); > > - return DDI_BUF_TRANS_SELECT(level); > + intel_de_write(dev_priv, DDI_BUF_CTL(port), intel_dp->DP); > + intel_de_posting_read(dev_priv, DDI_BUF_CTL(port)); > } > > static inline > @@ -4435,6 +4463,17 @@ intel_ddi_init_dp_connector(struct > intel_digital_port *intel_dig_port) > intel_ddi_prepare_link_retrain; > intel_dig_port->dp.set_link_train = intel_ddi_set_link_train; > > + if (INTEL_GEN(dev_priv) >= 12) > + intel_dig_port->dp.set_signal_levels = > tgl_set_signal_levels; > + else if (INTEL_GEN(dev_priv) >= 11) > + intel_dig_port->dp.set_signal_levels = > icl_set_signal_levels; > + else if (IS_CANNONLAKE(dev_priv)) > + intel_dig_port->dp.set_signal_levels = > cnl_set_signal_levels; > + else if (IS_GEN9_LP(dev_priv)) > + intel_dig_port->dp.set_signal_levels = > bxt_set_signal_levels; > + else > + intel_dig_port->dp.set_signal_levels = > hsw_set_signal_levels; > + > if (INTEL_GEN(dev_priv) < 12) { > intel_dig_port->dp.regs.dp_tp_ctl = DP_TP_CTL(port); > intel_dig_port->dp.regs.dp_tp_status = > DP_TP_STATUS(port); > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > b/drivers/gpu/drm/i915/display/intel_display_types.h > index 616850cc66bb..960ba14f01c8 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1369,6 +1369,7 @@ struct intel_dp { > /* This is called before a link training is starterd */ > void (*prepare_link_retrain)(struct intel_dp *intel_dp); > void (*set_link_train)(struct intel_dp *intel_dp, u8 > dp_train_pat); > + void (*set_signal_levels)(struct intel_dp *intel_dp); > > /* Displayport compliance testing */ > struct intel_dp_compliance compliance; > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index f120e2881f77..7d5e1e857ba7 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -4033,7 +4033,7 @@ intel_dp_pre_emphasis_max(struct intel_dp > *intel_dp, u8 voltage_swing) > } > } > > -static u32 vlv_signal_levels(struct intel_dp *intel_dp) > +static void vlv_set_signal_levels(struct intel_dp *intel_dp) > { > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)- > >base; > unsigned long demph_reg_value, preemph_reg_value, > @@ -4061,7 +4061,7 @@ static u32 vlv_signal_levels(struct intel_dp > *intel_dp) > uniqtranscale_reg_value = 0x5598DA3A; > break; > default: > - return 0; > + return; > } > break; > case DP_TRAIN_PRE_EMPH_LEVEL_1: > @@ -4080,7 +4080,7 @@ static u32 vlv_signal_levels(struct intel_dp > *intel_dp) > uniqtranscale_reg_value = 0x55ADDA3A; > break; > default: > - return 0; > + return; > } > break; > case DP_TRAIN_PRE_EMPH_LEVEL_2: > @@ -4095,7 +4095,7 @@ static u32 vlv_signal_levels(struct intel_dp > *intel_dp) > uniqtranscale_reg_value = 0x55ADDA3A; > break; > default: > - return 0; > + return; > } > break; > case DP_TRAIN_PRE_EMPH_LEVEL_3: > @@ -4106,20 +4106,18 @@ static u32 vlv_signal_levels(struct intel_dp > *intel_dp) > uniqtranscale_reg_value = 0x55ADDA3A; > break; > default: > - return 0; > + return; > } > break; > default: > - return 0; > + return; > } > > vlv_set_phy_signal_level(encoder, demph_reg_value, > preemph_reg_value, > uniqtranscale_reg_value, 0); > - > - return 0; > } > > -static u32 chv_signal_levels(struct intel_dp *intel_dp) > +static void chv_set_signal_levels(struct intel_dp *intel_dp) > { > struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)- > >base; > u32 deemph_reg_value, margin_reg_value; > @@ -4147,7 +4145,7 @@ static u32 chv_signal_levels(struct intel_dp > *intel_dp) > uniq_trans_scale = true; > break; > default: > - return 0; > + return; > } > break; > case DP_TRAIN_PRE_EMPH_LEVEL_1: > @@ -4165,7 +4163,7 @@ static u32 chv_signal_levels(struct intel_dp > *intel_dp) > margin_reg_value = 154; > break; > default: > - return 0; > + return; > } > break; > case DP_TRAIN_PRE_EMPH_LEVEL_2: > @@ -4179,7 +4177,7 @@ static u32 chv_signal_levels(struct intel_dp > *intel_dp) > margin_reg_value = 154; > break; > default: > - return 0; > + return; > } > break; > case DP_TRAIN_PRE_EMPH_LEVEL_3: > @@ -4189,21 +4187,18 @@ static u32 chv_signal_levels(struct intel_dp > *intel_dp) > margin_reg_value = 154; > break; > default: > - return 0; > + return; > } > break; > default: > - return 0; > + return; > } > > chv_set_phy_signal_level(encoder, deemph_reg_value, > margin_reg_value, uniq_trans_scale); > - > - return 0; > } > > -static u32 > -g4x_signal_levels(u8 train_set) > +static u32 g4x_signal_levels(u8 train_set) > { > u32 signal_levels = 0; > > @@ -4240,12 +4235,31 @@ g4x_signal_levels(u8 train_set) > return signal_levels; > } > > +static void > +g4x_set_signal_levels(struct intel_dp *intel_dp) > +{ > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > + u8 train_set = intel_dp->train_set[0]; > + u32 signal_levels; > + > + signal_levels = g4x_signal_levels(train_set); > + > + drm_dbg_kms(&dev_priv->drm, "Using signal levels %08x\n", > + signal_levels); > + > + intel_dp->DP &= ~(DP_VOLTAGE_MASK | DP_PRE_EMPHASIS_MASK); > + intel_dp->DP |= signal_levels; > + > + intel_de_write(dev_priv, intel_dp->output_reg, intel_dp->DP); > + intel_de_posting_read(dev_priv, intel_dp->output_reg); > +} > + > /* SNB CPU eDP voltage swing and pre-emphasis control */ > -static u32 > -snb_cpu_edp_signal_levels(u8 train_set) > +static u32 snb_cpu_edp_signal_levels(u8 train_set) > { > - int signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK | > - DP_TRAIN_PRE_EMPHASIS_MASK); > + u8 signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK | > + DP_TRAIN_PRE_EMPHASIS_MASK); > + > switch (signal_levels) { > case DP_TRAIN_VOLTAGE_SWING_LEVEL_0 | > DP_TRAIN_PRE_EMPH_LEVEL_0: > case DP_TRAIN_VOLTAGE_SWING_LEVEL_1 | > DP_TRAIN_PRE_EMPH_LEVEL_0: > @@ -4268,12 +4282,31 @@ snb_cpu_edp_signal_levels(u8 train_set) > } > } > > +static void > +snb_cpu_edp_set_signal_levels(struct intel_dp *intel_dp) > +{ > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > + u8 train_set = intel_dp->train_set[0]; > + u32 signal_levels; > + > + signal_levels = snb_cpu_edp_signal_levels(train_set); > + > + drm_dbg_kms(&dev_priv->drm, "Using signal levels %08x\n", > + signal_levels); > + > + intel_dp->DP &= ~EDP_LINK_TRAIN_VOL_EMP_MASK_SNB; > + intel_dp->DP |= signal_levels; > + > + intel_de_write(dev_priv, intel_dp->output_reg, intel_dp->DP); > + intel_de_posting_read(dev_priv, intel_dp->output_reg); > +} > + > /* IVB CPU eDP voltage swing and pre-emphasis control */ > -static u32 > -ivb_cpu_edp_signal_levels(u8 train_set) > +static u32 ivb_cpu_edp_signal_levels(u8 train_set) > { > - int signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK | > - DP_TRAIN_PRE_EMPHASIS_MASK); > + u8 signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK | > + DP_TRAIN_PRE_EMPHASIS_MASK); > + > switch (signal_levels) { > case DP_TRAIN_VOLTAGE_SWING_LEVEL_0 | > DP_TRAIN_PRE_EMPH_LEVEL_0: > return EDP_LINK_TRAIN_400MV_0DB_IVB; > @@ -4299,38 +4332,29 @@ ivb_cpu_edp_signal_levels(u8 train_set) > } > } > > -void > -intel_dp_set_signal_levels(struct intel_dp *intel_dp) > +static void > +ivb_cpu_edp_set_signal_levels(struct intel_dp *intel_dp) > { > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > - struct intel_digital_port *intel_dig_port = > dp_to_dig_port(intel_dp); > - enum port port = intel_dig_port->base.port; > - u32 signal_levels, mask = 0; > u8 train_set = intel_dp->train_set[0]; > + u32 signal_levels; > > - if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10) { > - signal_levels = bxt_signal_levels(intel_dp); > - } else if (HAS_DDI(dev_priv)) { > - signal_levels = ddi_signal_levels(intel_dp); > - mask = DDI_BUF_EMP_MASK; > - } else if (IS_CHERRYVIEW(dev_priv)) { > - signal_levels = chv_signal_levels(intel_dp); > - } else if (IS_VALLEYVIEW(dev_priv)) { > - signal_levels = vlv_signal_levels(intel_dp); > - } else if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) { > - signal_levels = ivb_cpu_edp_signal_levels(train_set); > - mask = EDP_LINK_TRAIN_VOL_EMP_MASK_IVB; > - } else if (IS_GEN(dev_priv, 6) && port == PORT_A) { > - signal_levels = snb_cpu_edp_signal_levels(train_set); > - mask = EDP_LINK_TRAIN_VOL_EMP_MASK_SNB; > - } else { > - signal_levels = g4x_signal_levels(train_set); > - mask = DP_VOLTAGE_MASK | DP_PRE_EMPHASIS_MASK; > - } > + signal_levels = ivb_cpu_edp_signal_levels(train_set); > > - if (mask) > - drm_dbg_kms(&dev_priv->drm, "Using signal levels > %08x\n", > - signal_levels); > + drm_dbg_kms(&dev_priv->drm, "Using signal levels %08x\n", > + signal_levels); > + > + intel_dp->DP &= ~EDP_LINK_TRAIN_VOL_EMP_MASK_IVB; > + intel_dp->DP |= signal_levels; > + > + intel_de_write(dev_priv, intel_dp->output_reg, intel_dp->DP); > + intel_de_posting_read(dev_priv, intel_dp->output_reg); > +} > + > +void intel_dp_set_signal_levels(struct intel_dp *intel_dp) > +{ > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > + u8 train_set = intel_dp->train_set[0]; > > drm_dbg_kms(&dev_priv->drm, "Using vswing level %d%s\n", > train_set & DP_TRAIN_VOLTAGE_SWING_MASK, > @@ -4341,10 +4365,7 @@ intel_dp_set_signal_levels(struct intel_dp > *intel_dp) > train_set & DP_TRAIN_MAX_PRE_EMPHASIS_REACHED ? > " (max)" : ""); > > - intel_dp->DP = (intel_dp->DP & ~mask) | signal_levels; > - > - intel_de_write(dev_priv, intel_dp->output_reg, intel_dp->DP); > - intel_de_posting_read(dev_priv, intel_dp->output_reg); > + intel_dp->set_signal_levels(intel_dp); > } > > void > @@ -8515,6 +8536,17 @@ bool intel_dp_init(struct drm_i915_private > *dev_priv, > else > intel_dig_port->dp.set_link_train = g4x_set_link_train; > > + if (IS_CHERRYVIEW(dev_priv)) > + intel_dig_port->dp.set_signal_levels = > chv_set_signal_levels; > + else if (IS_VALLEYVIEW(dev_priv)) > + intel_dig_port->dp.set_signal_levels = > vlv_set_signal_levels; > + else if (IS_IVYBRIDGE(dev_priv) && port == PORT_A) > + intel_dig_port->dp.set_signal_levels = > ivb_cpu_edp_set_signal_levels; > + else if (IS_GEN(dev_priv, 6) && port == PORT_A) > + intel_dig_port->dp.set_signal_levels = > snb_cpu_edp_set_signal_levels; > + else > + intel_dig_port->dp.set_signal_levels = > g4x_set_signal_levels; > + > intel_dig_port->dp.output_reg = output_reg; > intel_dig_port->max_lanes = 4; > intel_dig_port->dp.regs.dp_tp_ctl = DP_TP_CTL(port); _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx