Re: [Intel-gfx] [PATCH v2 08/13] drm/i915/tgl: Add dkl phy programming sequences
On Fri, 2019-09-20 at 14:44 -0700, Lucas De Marchi wrote: > On Wed, Sep 18, 2019 at 5:07 PM José Roberto de Souza > wrote: > > From: Clinton A Taylor > > > > Added DKL Phy sequences and helpers functions to program voltage > > swing, clock gating and dp mode. > > > > It is not written in DP enabling sequence but "PHY Clockgating > > programming" states that clock gating should be enabled after the > > link training but doing so causes all the following trainings to > > fail > > so not enabling it for. > > > > v2: > > Setting the right HIP_INDEX_REG bits (José) > > > > BSpec: 49292 > > BSpec: 49190 > > > > Signed-off-by: José Roberto de Souza > > Signed-off-by: Clinton A Taylor > > --- > > drivers/gpu/drm/i915/display/intel_ddi.c | 246 > > +-- > > 1 file changed, 234 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > > b/drivers/gpu/drm/i915/display/intel_ddi.c > > index 81792a04e0aa..fd271118d1f5 100644 > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > @@ -586,6 +586,25 @@ static const struct icl_mg_phy_ddi_buf_trans > > icl_mg_phy_ddi_translations[] = { > > { 0x0, 0x00, 0x00 },/* 3 0 */ > > }; > > > > +struct tgl_dkl_phy_ddi_buf_trans { > > + u32 dkl_vswing_control; > > + u32 dkl_preshoot_control; > > + u32 dkl_de_emphasis_control; > > +}; > > + > > +static const struct tgl_dkl_phy_ddi_buf_trans > > tgl_dkl_phy_ddi_translations[] = { > > comment here like in the icl version with the meaning of the comments > would be good Okay > > > + { 0x7, 0x0, 0x00 }, /* 0 0 400mV 0 dB */ > > + { 0x5, 0x0, 0x03 }, /* 0 1 400mV 3.5 dB */ > > + { 0x2, 0x0, 0x0b }, /* 0 2 400mV 6 dB */ > > + { 0x0, 0x0, 0x19 }, /* 0 3 400mV 9.5 dB */ > > + { 0x5, 0x0, 0x00 }, /* 1 0 600mV 0 dB */ > > pre-emphasis here is 1. And the others below are wrong, too. I thought that too but based on the index_to_dp_signal_levels table and the values on "Pre-emphasis dB" column is mostly likely that BSpec has a typo, issue was already open for that on BSpec. > > > + { 0x2, 0x0, 0x03 }, /* 1 1 600mV 3.5 dB */ > > + { 0x0, 0x0, 0x14 }, /* 1 2 600mV 6 dB */ > > + { 0x2, 0x0, 0x00 }, /* 2 0 800mV 0 dB */ > > + { 0x0, 0x0, 0x0B }, /* 2 1 800mV 3.5 dB */ > > + { 0x0, 0x0, 0x00 }, /* 3 0 1200mV 0 dBHDMI > > Default */ > > +}; > > + > > static const struct ddi_buf_trans * > > bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int > > *n_entries) > > { > > @@ -873,11 +892,15 @@ static int intel_ddi_hdmi_level(struct > > drm_i915_private *dev_priv, enum port por > > level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift; > > > > if (INTEL_GEN(dev_priv) >= 11) { > > - if (intel_phy_is_combo(dev_priv, phy)) > > + if (intel_phy_is_combo(dev_priv, phy)) { > > icl_get_combo_buf_trans(dev_priv, > > INTEL_OUTPUT_HDMI, > > 0, _entries); > > - else > > - n_entries = > > ARRAY_SIZE(icl_mg_phy_ddi_translations); > > + } else { > > + if (INTEL_GEN(dev_priv) >= 12) > > + n_entries = > > ARRAY_SIZE(tgl_dkl_phy_ddi_translations); > > + else > > + n_entries = > > ARRAY_SIZE(icl_mg_phy_ddi_translations); > > + } > > default_entry = n_entries - 1; > > I think plain ladder would be better. Just add one for > INTEL_GEN(dev_priv) >= 12) Okay > > > } else if (IS_CANNONLAKE(dev_priv)) { > > cnl_get_buf_trans_hdmi(dev_priv, _entries); > > @@ -2308,11 +2331,15 @@ u8 intel_ddi_dp_voltage_max(struct > > intel_encoder *encoder) > > int n_entries; > > > > if (INTEL_GEN(dev_priv) >= 11) { > > - if (intel_phy_is_combo(dev_priv, phy)) > > + if (intel_phy_is_combo(dev_priv, phy)) { > > icl_get_combo_buf_trans(dev_priv, encoder- > > >type, > > intel_dp- > > >link_rate, _entries); > > - else > > - n_entries = > > ARRAY_SIZE(icl_mg_phy_ddi_translations); > > + } else { > > + if (INTEL_GEN(dev_priv) >= 12) > > + n_entries = > > ARRAY_SIZE(tgl_dkl_phy_ddi_translations); > > + else > > + n_entries = > > ARRAY_SIZE(icl_mg_phy_ddi_translations); > > + } > > ditto Okay > > > } else if (IS_CANNONLAKE(dev_priv)) { > > if (encoder->type == INTEL_OUTPUT_EDP) > >
Re: [Intel-gfx] [PATCH v2 08/13] drm/i915/tgl: Add dkl phy programming sequences
On Wed, Sep 18, 2019 at 5:07 PM José Roberto de Souza wrote: > > From: Clinton A Taylor > > Added DKL Phy sequences and helpers functions to program voltage > swing, clock gating and dp mode. > > It is not written in DP enabling sequence but "PHY Clockgating > programming" states that clock gating should be enabled after the > link training but doing so causes all the following trainings to fail > so not enabling it for. > > v2: > Setting the right HIP_INDEX_REG bits (José) > > BSpec: 49292 > BSpec: 49190 > > Signed-off-by: José Roberto de Souza > Signed-off-by: Clinton A Taylor > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 246 +-- > 1 file changed, 234 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index 81792a04e0aa..fd271118d1f5 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -586,6 +586,25 @@ static const struct icl_mg_phy_ddi_buf_trans > icl_mg_phy_ddi_translations[] = { > { 0x0, 0x00, 0x00 },/* 3 0 */ > }; > > +struct tgl_dkl_phy_ddi_buf_trans { > + u32 dkl_vswing_control; > + u32 dkl_preshoot_control; > + u32 dkl_de_emphasis_control; > +}; > + > +static const struct tgl_dkl_phy_ddi_buf_trans tgl_dkl_phy_ddi_translations[] > = { comment here like in the icl version with the meaning of the comments would be good > + { 0x7, 0x0, 0x00 }, /* 0 0 400mV 0 dB */ > + { 0x5, 0x0, 0x03 }, /* 0 1 400mV 3.5 dB */ > + { 0x2, 0x0, 0x0b }, /* 0 2 400mV 6 dB */ > + { 0x0, 0x0, 0x19 }, /* 0 3 400mV 9.5 dB */ > + { 0x5, 0x0, 0x00 }, /* 1 0 600mV 0 dB */ pre-emphasis here is 1. And the others below are wrong, too. > + { 0x2, 0x0, 0x03 }, /* 1 1 600mV 3.5 dB */ > + { 0x0, 0x0, 0x14 }, /* 1 2 600mV 6 dB */ > + { 0x2, 0x0, 0x00 }, /* 2 0 800mV 0 dB */ > + { 0x0, 0x0, 0x0B }, /* 2 1 800mV 3.5 dB */ > + { 0x0, 0x0, 0x00 }, /* 3 0 1200mV 0 dBHDMI Default */ > +}; > + > static const struct ddi_buf_trans * > bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries) > { > @@ -873,11 +892,15 @@ static int intel_ddi_hdmi_level(struct drm_i915_private > *dev_priv, enum port por > level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift; > > if (INTEL_GEN(dev_priv) >= 11) { > - if (intel_phy_is_combo(dev_priv, phy)) > + if (intel_phy_is_combo(dev_priv, phy)) { > icl_get_combo_buf_trans(dev_priv, INTEL_OUTPUT_HDMI, > 0, _entries); > - else > - n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations); > + } else { > + if (INTEL_GEN(dev_priv) >= 12) > + n_entries = > ARRAY_SIZE(tgl_dkl_phy_ddi_translations); > + else > + n_entries = > ARRAY_SIZE(icl_mg_phy_ddi_translations); > + } > default_entry = n_entries - 1; I think plain ladder would be better. Just add one for INTEL_GEN(dev_priv) >= 12) > } else if (IS_CANNONLAKE(dev_priv)) { > cnl_get_buf_trans_hdmi(dev_priv, _entries); > @@ -2308,11 +2331,15 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder > *encoder) > int n_entries; > > if (INTEL_GEN(dev_priv) >= 11) { > - if (intel_phy_is_combo(dev_priv, phy)) > + if (intel_phy_is_combo(dev_priv, phy)) { > icl_get_combo_buf_trans(dev_priv, encoder->type, > intel_dp->link_rate, > _entries); > - else > - n_entries = ARRAY_SIZE(icl_mg_phy_ddi_translations); > + } else { > + if (INTEL_GEN(dev_priv) >= 12) > + n_entries = > ARRAY_SIZE(tgl_dkl_phy_ddi_translations); > + else > + n_entries = > ARRAY_SIZE(icl_mg_phy_ddi_translations); > + } ditto > } else if (IS_CANNONLAKE(dev_priv)) { > if (encoder->type == INTEL_OUTPUT_EDP) > cnl_get_buf_trans_edp(dev_priv, _entries); > @@ -2749,6 +2776,66 @@ static void icl_ddi_vswing_sequence(struct > intel_encoder *encoder, > icl_mg_phy_ddi_vswing_sequence(encoder, link_clock, level); > } > > +static void > +tgl_dkl_phy_ddi_vswing_sequence(struct intel_encoder *encoder, int > link_clock, > + u32 level) > +{ > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > + enum tc_port tc_port = intel_port_to_tc(dev_priv, encoder->port); > + const struct