| Message ID | 20190923195513.207536-5-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | TGL TC enabling v3 | expand |
On Mon, Sep 23, 2019 at 12:55 PM José Roberto de Souza <jose.souza@intel.com> wrote: > > From: Clinton A Taylor <clinton.a.taylor@intel.com> > > 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é) > > v3: > Adding the meaning of each column of tgl_dkl_phy_ddi_translations > Adding if gen >= 12 on intel_ddi_hdmi_level() and > intel_ddi_pre_enable_hdmi() instead of reuse part of gen >= 11 if we should treat these patch versioning the same way of the commit messages, avoiding the gerund. > > BSpec: 49292 > BSpec: 49190 > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > Signed-off-by: Clinton A Taylor <clinton.a.taylor@intel.com> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 249 ++++++++++++++++++++++- > 1 file changed, 240 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > index 33cd766f9eea..1ab3e0c4c0a1 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -586,6 +586,26 @@ 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[] = { > + /* VS pre-emp Non-trans mV Pre-emph dB */ > + { 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 */ > + { 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 dB HDMI default */ > +}; > + > static const struct ddi_buf_trans * > bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries) > { > @@ -872,7 +892,14 @@ 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_GEN(dev_priv) >= 12) { > + if (intel_phy_is_combo(dev_priv, phy)) > + icl_get_combo_buf_trans(dev_priv, INTEL_OUTPUT_HDMI, > + 0, &n_entries); > + else > + n_entries = ARRAY_SIZE(tgl_dkl_phy_ddi_translations); > + default_entry = n_entries - 1; > + } else if (INTEL_GEN(dev_priv) == 11) { > if (intel_phy_is_combo(dev_priv, phy)) > icl_get_combo_buf_trans(dev_priv, INTEL_OUTPUT_HDMI, > 0, &n_entries); > @@ -2313,7 +2340,13 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder) > enum phy phy = intel_port_to_phy(dev_priv, port); > int n_entries; > > - if (INTEL_GEN(dev_priv) >= 11) { > + if (INTEL_GEN(dev_priv) >= 12) { > + if (intel_phy_is_combo(dev_priv, phy)) > + icl_get_combo_buf_trans(dev_priv, encoder->type, > + intel_dp->link_rate, &n_entries); > + else > + n_entries = ARRAY_SIZE(tgl_dkl_phy_ddi_translations); > + } else if (INTEL_GEN(dev_priv) == 11) { > if (intel_phy_is_combo(dev_priv, phy)) > icl_get_combo_buf_trans(dev_priv, encoder->type, > intel_dp->link_rate, &n_entries); > @@ -2755,6 +2788,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 tgl_dkl_phy_ddi_buf_trans *ddi_translations; > + u32 n_entries, val, ln; > + > + n_entries = ARRAY_SIZE(tgl_dkl_phy_ddi_translations); > + ddi_translations = tgl_dkl_phy_ddi_translations; > + > + if (level >= n_entries) > + level = n_entries - 1; > + > + /* > + * All registers programmed here use HIP_INDEX_REG 0 or 1 > + */ this comment is bogus as it's actually HIP_INDEX_VAL we are talking about, and that's pretty clear by the loop. > + for (ln = 0; ln < 2; ln++) { > + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, ln)); > + > + /* All the registers are RMW */ > + val = I915_READ(DKL_TX_DPCNTL0(tc_port)); > + val &= ~(DKL_TX_PRESHOOT_COEFF_MASK | > + DKL_TX_DE_EMPAHSIS_COEFF_MASK | > + DKL_TX_VSWING_CONTROL_MASK); > + val |= DKL_TX_VSWING_CONTROL(ddi_translations[level].dkl_vswing_control); > + val |= DKL_TX_DE_EMPHASIS_COEFF(ddi_translations[level].dkl_de_emphasis_control); > + val |= DKL_TX_PRESHOOT_COEFF(ddi_translations[level].dkl_preshoot_control); > + I915_WRITE(DKL_TX_DPCNTL0(tc_port), val); > + > + val = I915_READ(DKL_TX_DPCNTL1(tc_port)); > + val &= ~(DKL_TX_PRESHOOT_COEFF_MASK | > + DKL_TX_DE_EMPAHSIS_COEFF_MASK | > + DKL_TX_VSWING_CONTROL_MASK); > + val |= DKL_TX_VSWING_CONTROL(ddi_translations[level].dkl_vswing_control); > + val |= DKL_TX_DE_EMPHASIS_COEFF(ddi_translations[level].dkl_de_emphasis_control); > + val |= DKL_TX_PRESHOOT_COEFF(ddi_translations[level].dkl_preshoot_control); > + I915_WRITE(DKL_TX_DPCNTL1(tc_port), val); > + > + val = I915_READ(DKL_TX_DPCNTL2(tc_port)); > + val &= ~DKL_TX_DP20BITMODE; > + I915_WRITE(DKL_TX_DPCNTL2(tc_port), val); > + } > +} > + > +static void tgl_ddi_vswing_sequence(struct intel_encoder *encoder, > + int link_clock, > + u32 level, > + enum intel_output_type type) > +{ > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > + enum phy phy = intel_port_to_phy(dev_priv, encoder->port); > + > + if (intel_phy_is_combo(dev_priv, phy)) > + icl_combo_phy_ddi_vswing_sequence(encoder, level, type); > + else > + tgl_dkl_phy_ddi_vswing_sequence(encoder, link_clock, level); > +} > + > static u32 translate_signal_level(int signal_levels) > { > int i; > @@ -2786,7 +2879,10 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp) > struct intel_encoder *encoder = &dport->base; > int level = intel_ddi_dp_level(intel_dp); > > - if (INTEL_GEN(dev_priv) >= 11) > + 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)) > @@ -3033,6 +3129,34 @@ static void intel_ddi_clk_disable(struct intel_encoder *encoder) > } > } > > +static void > +tgl_phy_set_clock_gating(struct intel_digital_port *dig_port, bool enable) > +{ > + struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev); > + enum port port = dig_port->base.port; > + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); > + u32 val, bits; > + int ln; > + > + if (tc_port == PORT_TC_NONE) > + return; > + > + bits = DKL_DP_MODE_CFG_TR2PWR_GATING | DKL_DP_MODE_CFG_TRPWR_GATING | > + DKL_DP_MODE_CFG_CLNPWR_GATING | DKL_DP_MODE_CFG_DIGPWR_GATING | > + DKL_DP_MODE_CFG_GAONPWR_GATING; > + > + for (ln = 0; ln < 2; ln++) { > + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, ln)); > + > + val = I915_READ(DKL_DP_MODE(tc_port)); > + if (enable) > + val |= bits; > + else > + val &= ~bits; > + I915_WRITE(DKL_DP_MODE(tc_port), val); > + } > +} > + > static void > icl_phy_set_clock_gating(struct intel_digital_port *dig_port, bool enable) > { > @@ -3132,6 +3256,95 @@ static void icl_program_mg_dp_mode(struct intel_digital_port *intel_dig_port) > I915_WRITE(MG_DP_MODE(1, port), ln1); > } > > +static void tgl_program_dkl_dp_mode(struct intel_digital_port *intel_dig_port) > +{ > + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev); > + enum port port = intel_dig_port->base.port; > + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); > + u32 ln0, ln1, lane_mask, pin_mask; > + int num_lanes; > + > + if (tc_port == PORT_TC_NONE || > + intel_dig_port->tc_mode == TC_PORT_TBT_ALT) > + return; In icl_program_mg_dp_mode() this is if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT) if intel_dig_port is not backed by a TC phy, tc_mode should be TC_PORT_TBT_ALT since that's 0. > + > + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, 0x0)); > + ln0 = I915_READ(DKL_DP_MODE(tc_port)); > + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, 0x1)); > + ln1 = I915_READ(DKL_DP_MODE(tc_port)); > + > + num_lanes = intel_dig_port->dp.lane_count; in this file this is often called width > + > + switch (intel_dig_port->tc_mode) { > + case TC_PORT_DP_ALT: > + ln0 &= ~(DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X1_MODE); typo: the second one should be DKL_DP_MODE_CFG_DP_X2_MODE but.... for DKL we have the exact same table as for MG. Why do we need these changes here while we don't for ICL? > + ln1 &= ~(DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X2_MODE); > + > + lane_mask = intel_tc_port_get_lane_mask(intel_dig_port); /* DPX4TXLATC */ > + pin_mask = intel_tc_port_get_pin_assignment_mask(intel_dig_port); /* DPPATC */ > + > + switch (pin_mask) { From a quick look to the table, we don't change based on pin_mask, but rather based on lane_mask, just like for MG. Btw, lane_mask is unused in this function now and we don't get a warning just because it's misused in a MISSING_CASE() below. The values do change based on width, but I'm not sure why MG doesn't need to take that into account. +Imre Deak Lucas De Marchi > + case 0x0: > + if (num_lanes == 1) { > + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE; > + } else { > + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; > + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; > + } > + break; > + case 0x1: > + if (num_lanes == 4) { > + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; > + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; > + } > + break; > + case 0x2: > + if (num_lanes == 2) { > + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; > + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; > + } > + break; > + case 0x3: > + case 0x5: > + if (num_lanes == 1) { > + ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE; > + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE; > + } else { > + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; > + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; > + } > + break; > + case 0x4: > + case 0x6: > + if (num_lanes == 1) { > + ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE; > + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE; > + } else { > + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; > + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; > + } > + break; > + default: > + MISSING_CASE(lane_mask); > + } > + break; > + > + case TC_PORT_LEGACY: > + ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X2_MODE; > + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X2_MODE; > + break; > + > + default: > + MISSING_CASE(intel_dig_port->tc_mode); > + return; > + } > + > + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, 0x0)); > + I915_WRITE(DKL_DP_MODE(tc_port), ln0); > + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, 0x1)); > + I915_WRITE(DKL_DP_MODE(tc_port), ln1); > +} > + > static void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp, > const struct intel_crtc_state *crtc_state) > { > @@ -3218,7 +3431,7 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder, > dig_port->ddi_io_power_domain); > > /* 6. */ > - icl_program_mg_dp_mode(dig_port); > + tgl_program_dkl_dp_mode(dig_port); > > /* > * 7.a - Steps in this function should only be executed ov´er MST > @@ -3231,10 +3444,10 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder, > intel_ddi_config_transcoder_func(crtc_state); > > /* 7.d */ > - icl_phy_set_clock_gating(dig_port, false); > + tgl_phy_set_clock_gating(dig_port, false); > > /* 7.e */ > - icl_ddi_vswing_sequence(encoder, crtc_state->port_clock, level, > + tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock, level, > encoder->type); > > /* 7.f */ > @@ -3266,6 +3479,15 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder, > /* 7.k */ > intel_dp_stop_link_train(intel_dp); > > + /* > + * TODO: enable clock gating > + * > + * 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 now. > + */ > + > /* 7.l */ > intel_ddi_enable_fec(encoder, crtc_state); > intel_dsc_enable(encoder, crtc_state); > @@ -3371,9 +3593,15 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, > intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain); > > icl_program_mg_dp_mode(dig_port); > - icl_phy_set_clock_gating(dig_port, false); > + if (INTEL_GEN(dev_priv) >= 12) > + tgl_phy_set_clock_gating(dig_port, false); > + else > + icl_phy_set_clock_gating(dig_port, false); > > - if (INTEL_GEN(dev_priv) >= 11) > + if (INTEL_GEN(dev_priv) >= 12) > + tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock, > + level, INTEL_OUTPUT_HDMI); > + else if (INTEL_GEN(dev_priv) == 11) > icl_ddi_vswing_sequence(encoder, crtc_state->port_clock, > level, INTEL_OUTPUT_HDMI); > else if (IS_CANNONLAKE(dev_priv)) > @@ -3383,7 +3611,10 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, > else > intel_prepare_hdmi_ddi_buffers(encoder, level); > > - icl_phy_set_clock_gating(dig_port, true); > + if (INTEL_GEN(dev_priv) >= 12) > + tgl_phy_set_clock_gating(dig_port, true); > + else > + icl_phy_set_clock_gating(dig_port, true); > > if (IS_GEN9_BC(dev_priv)) > skl_ddi_set_iboost(encoder, level, INTEL_OUTPUT_HDMI); > -- > 2.23.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Sep 23, 2019 at 03:02:54PM -0700, Lucas De Marchi wrote: > On Mon, Sep 23, 2019 at 12:55 PM José Roberto de Souza > <jose.souza@intel.com> wrote: > > [...] > > > + ln1 &= ~(DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X2_MODE); > > + > > + lane_mask = intel_tc_port_get_lane_mask(intel_dig_port); /* DPX4TXLATC */ > > + pin_mask = intel_tc_port_get_pin_assignment_mask(intel_dig_port); /* DPPATC */ > > + > > + switch (pin_mask) { > > From a quick look to the table, we don't change based on pin_mask, but > rather based on lane_mask, just like for MG. There are differences, for instance pin_mask=0x4,0x6/lane_mask=0x3 vs. pin_mask=0x2/lane_mask=0x3. > Btw, lane_mask is unused in this function now and we don't get a > warning just because it's misused in a > MISSING_CASE() below. > > The values do change based on width, but I'm not sure why MG doesn't > need to take that into account. Yes, looks like crtc_state->lane_count should be considered. Not sure why the MG version doesn't do that either, I assume it was an addition to BSpec only after the function was added. The same applies to pin_mask. > +Imre Deak > > Lucas De Marchi > > > + case 0x0: > > + if (num_lanes == 1) { > > + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE; > > + } else { > > + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; > > + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; > > + } > > + break; > > + case 0x1: > > + if (num_lanes == 4) { > > + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; > > + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; > > + } > > + break; > > + case 0x2: > > + if (num_lanes == 2) { > > + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; > > + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; > > + } > > + break; > > + case 0x3: > > + case 0x5: > > + if (num_lanes == 1) { > > + ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE; > > + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE; > > + } else { > > + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; > > + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; > > + } > > + break; > > + case 0x4: > > + case 0x6: > > + if (num_lanes == 1) { > > + ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE; > > + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE; > > + } else { > > + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; > > + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; > > + } > > + break; > > + default: > > + MISSING_CASE(lane_mask); > > + } > > + break; > > + > > + case TC_PORT_LEGACY: > > + ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X2_MODE; > > + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X2_MODE; > > + break; > > + > > + default: > > + MISSING_CASE(intel_dig_port->tc_mode); > > + return; > > + } > > + > > + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, 0x0)); > > + I915_WRITE(DKL_DP_MODE(tc_port), ln0); > > + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, 0x1)); > > + I915_WRITE(DKL_DP_MODE(tc_port), ln1); > > +} > > + > > static void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp, > > const struct intel_crtc_state *crtc_state) > > { > > @@ -3218,7 +3431,7 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder, > > dig_port->ddi_io_power_domain); > > > > /* 6. */ > > - icl_program_mg_dp_mode(dig_port); > > + tgl_program_dkl_dp_mode(dig_port); > > > > /* > > * 7.a - Steps in this function should only be executed ov´er MST > > @@ -3231,10 +3444,10 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder, > > intel_ddi_config_transcoder_func(crtc_state); > > > > /* 7.d */ > > - icl_phy_set_clock_gating(dig_port, false); > > + tgl_phy_set_clock_gating(dig_port, false); > > > > /* 7.e */ > > - icl_ddi_vswing_sequence(encoder, crtc_state->port_clock, level, > > + tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock, level, > > encoder->type); > > > > /* 7.f */ > > @@ -3266,6 +3479,15 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder, > > /* 7.k */ > > intel_dp_stop_link_train(intel_dp); > > > > + /* > > + * TODO: enable clock gating > > + * > > + * 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 now. > > + */ > > + > > /* 7.l */ > > intel_ddi_enable_fec(encoder, crtc_state); > > intel_dsc_enable(encoder, crtc_state); > > @@ -3371,9 +3593,15 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, > > intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain); > > > > icl_program_mg_dp_mode(dig_port); > > - icl_phy_set_clock_gating(dig_port, false); > > + if (INTEL_GEN(dev_priv) >= 12) > > + tgl_phy_set_clock_gating(dig_port, false); > > + else > > + icl_phy_set_clock_gating(dig_port, false); > > > > - if (INTEL_GEN(dev_priv) >= 11) > > + if (INTEL_GEN(dev_priv) >= 12) > > + tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock, > > + level, INTEL_OUTPUT_HDMI); > > + else if (INTEL_GEN(dev_priv) == 11) > > icl_ddi_vswing_sequence(encoder, crtc_state->port_clock, > > level, INTEL_OUTPUT_HDMI); > > else if (IS_CANNONLAKE(dev_priv)) > > @@ -3383,7 +3611,10 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, > > else > > intel_prepare_hdmi_ddi_buffers(encoder, level); > > > > - icl_phy_set_clock_gating(dig_port, true); > > + if (INTEL_GEN(dev_priv) >= 12) > > + tgl_phy_set_clock_gating(dig_port, true); > > + else > > + icl_phy_set_clock_gating(dig_port, true); > > > > if (IS_GEN9_BC(dev_priv)) > > skl_ddi_set_iboost(encoder, level, INTEL_OUTPUT_HDMI); > > -- > > 2.23.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Lucas De Marchi
On Tue, 2019-09-24 at 16:00 +0300, Imre Deak wrote: > On Mon, Sep 23, 2019 at 03:02:54PM -0700, Lucas De Marchi wrote: > > On Mon, Sep 23, 2019 at 12:55 PM José Roberto de Souza > > <jose.souza@intel.com> wrote: > > > [...] > > > + ln1 &= ~(DKL_DP_MODE_CFG_DP_X1_MODE | > > > DKL_DP_MODE_CFG_DP_X2_MODE); > > > + > > > + lane_mask = > > > intel_tc_port_get_lane_mask(intel_dig_port); /* DPX4TXLATC */ > > > + pin_mask = > > > intel_tc_port_get_pin_assignment_mask(intel_dig_port); /* DPPATC > > > */ > > > + > > > + switch (pin_mask) { > > > > From a quick look to the table, we don't change based on pin_mask, > > but > > rather based on lane_mask, just like for MG. > > There are differences, for instance pin_mask=0x4,0x6/lane_mask=0x3 > vs. > pin_mask=0x2/lane_mask=0x3. > > > Btw, lane_mask is unused in this function now and we don't get a > > warning just because it's misused in a > > MISSING_CASE() below. > > > > The values do change based on width, but I'm not sure why MG > > doesn't > > need to take that into account. > > Yes, looks like crtc_state->lane_count should be considered. Not sure > why the MG version doesn't do that either, I assume it was an > addition > to BSpec only after the function was added. The same applies to > pin_mask. Looking more carefully, FIA takes care of flip/reversal lanes, so for us just matter what lanes can be used. So dropping the patch adding intel_tc_port_get_pin_assignment_mask(), dropping tgl_program_dkl_dp_mode and tgl_phy_set_clock_gating() and reusing ICL versions with a couple of "if gen > 12" to access dkl register. One odd thing that I notice is that we use port instead of tc_port in most MG registers, those MG registers uses a macro that subtract port and PORT_C to get the right register, thinking in send a patch changing all of those to receive as parameter tc_port to make it consistent, what you guys think? > > > +Imre Deak > > > > Lucas De Marchi > > > > > + case 0x0: > > > + if (num_lanes == 1) { > > > + ln1 |= > > > DKL_DP_MODE_CFG_DP_X1_MODE; > > > + } else { > > > + ln0 |= > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > + ln1 |= > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > + } > > > + break; > > > + case 0x1: > > > + if (num_lanes == 4) { > > > + ln0 |= > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > + ln1 |= > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > + } > > > + break; > > > + case 0x2: > > > + if (num_lanes == 2) { > > > + ln0 |= > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > + ln1 |= > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > + } > > > + break; > > > + case 0x3: > > > + case 0x5: > > > + if (num_lanes == 1) { > > > + ln0 |= > > > DKL_DP_MODE_CFG_DP_X1_MODE; > > > + ln1 |= > > > DKL_DP_MODE_CFG_DP_X1_MODE; > > > + } else { > > > + ln0 |= > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > + ln1 |= > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > + } > > > + break; > > > + case 0x4: > > > + case 0x6: > > > + if (num_lanes == 1) { > > > + ln0 |= > > > DKL_DP_MODE_CFG_DP_X1_MODE; > > > + ln1 |= > > > DKL_DP_MODE_CFG_DP_X1_MODE; > > > + } else { > > > + ln0 |= > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > + ln1 |= > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > + } > > > + break; > > > + default: > > > + MISSING_CASE(lane_mask); > > > + } > > > + break; > > > + > > > + case TC_PORT_LEGACY: > > > + ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE | > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE | > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > + break; > > > + > > > + default: > > > + MISSING_CASE(intel_dig_port->tc_mode); > > > + return; > > > + } > > > + > > > + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, > > > 0x0)); > > > + I915_WRITE(DKL_DP_MODE(tc_port), ln0); > > > + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, > > > 0x1)); > > > + I915_WRITE(DKL_DP_MODE(tc_port), ln1); > > > +} > > > + > > > static void intel_dp_sink_set_fec_ready(struct intel_dp > > > *intel_dp, > > > const struct > > > intel_crtc_state *crtc_state) > > > { > > > @@ -3218,7 +3431,7 @@ static void tgl_ddi_pre_enable_dp(struct > > > intel_encoder *encoder, > > > dig_port- > > > >ddi_io_power_domain); > > > > > > /* 6. */ > > > - icl_program_mg_dp_mode(dig_port); > > > + tgl_program_dkl_dp_mode(dig_port); > > > > > > /* > > > * 7.a - Steps in this function should only be executed > > > ov´er MST > > > @@ -3231,10 +3444,10 @@ static void tgl_ddi_pre_enable_dp(struct > > > intel_encoder *encoder, > > > intel_ddi_config_transcoder_func(crtc_state); > > > > > > /* 7.d */ > > > - icl_phy_set_clock_gating(dig_port, false); > > > + tgl_phy_set_clock_gating(dig_port, false); > > > > > > /* 7.e */ > > > - icl_ddi_vswing_sequence(encoder, crtc_state->port_clock, > > > level, > > > + tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock, > > > level, > > > encoder->type); > > > > > > /* 7.f */ > > > @@ -3266,6 +3479,15 @@ static void tgl_ddi_pre_enable_dp(struct > > > intel_encoder *encoder, > > > /* 7.k */ > > > intel_dp_stop_link_train(intel_dp); > > > > > > + /* > > > + * TODO: enable clock gating > > > + * > > > + * 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 now. > > > + */ > > > + > > > /* 7.l */ > > > intel_ddi_enable_fec(encoder, crtc_state); > > > intel_dsc_enable(encoder, crtc_state); > > > @@ -3371,9 +3593,15 @@ static void > > > intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, > > > intel_display_power_get(dev_priv, dig_port- > > > >ddi_io_power_domain); > > > > > > icl_program_mg_dp_mode(dig_port); > > > - icl_phy_set_clock_gating(dig_port, false); > > > + if (INTEL_GEN(dev_priv) >= 12) > > > + tgl_phy_set_clock_gating(dig_port, false); > > > + else > > > + icl_phy_set_clock_gating(dig_port, false); > > > > > > - if (INTEL_GEN(dev_priv) >= 11) > > > + if (INTEL_GEN(dev_priv) >= 12) > > > + tgl_ddi_vswing_sequence(encoder, crtc_state- > > > >port_clock, > > > + level, > > > INTEL_OUTPUT_HDMI); > > > + else if (INTEL_GEN(dev_priv) == 11) > > > icl_ddi_vswing_sequence(encoder, crtc_state- > > > >port_clock, > > > level, > > > INTEL_OUTPUT_HDMI); > > > else if (IS_CANNONLAKE(dev_priv)) > > > @@ -3383,7 +3611,10 @@ static void > > > intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, > > > else > > > intel_prepare_hdmi_ddi_buffers(encoder, level); > > > > > > - icl_phy_set_clock_gating(dig_port, true); > > > + if (INTEL_GEN(dev_priv) >= 12) > > > + tgl_phy_set_clock_gating(dig_port, true); > > > + else > > > + icl_phy_set_clock_gating(dig_port, true); > > > > > > if (IS_GEN9_BC(dev_priv)) > > > skl_ddi_set_iboost(encoder, level, > > > INTEL_OUTPUT_HDMI); > > > -- > > > 2.23.0 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > -- > > Lucas De Marchi
On Wed, Sep 25, 2019 at 02:21:52AM +0300, Souza, Jose wrote: > On Tue, 2019-09-24 at 16:00 +0300, Imre Deak wrote: > > On Mon, Sep 23, 2019 at 03:02:54PM -0700, Lucas De Marchi wrote: > > > On Mon, Sep 23, 2019 at 12:55 PM José Roberto de Souza > > > <jose.souza@intel.com> wrote: > > > > [...] > > > > + ln1 &= ~(DKL_DP_MODE_CFG_DP_X1_MODE | > > > > DKL_DP_MODE_CFG_DP_X2_MODE); > > > > + > > > > + lane_mask = > > > > intel_tc_port_get_lane_mask(intel_dig_port); /* DPX4TXLATC */ > > > > + pin_mask = > > > > intel_tc_port_get_pin_assignment_mask(intel_dig_port); /* DPPATC > > > > */ > > > > + > > > > + switch (pin_mask) { > > > > > > From a quick look to the table, we don't change based on pin_mask, > > > but > > > rather based on lane_mask, just like for MG. > > > > There are differences, for instance pin_mask=0x4,0x6/lane_mask=0x3 > > vs. > > pin_mask=0x2/lane_mask=0x3. > > > > > Btw, lane_mask is unused in this function now and we don't get a > > > warning just because it's misused in a > > > MISSING_CASE() below. > > > > > > The values do change based on width, but I'm not sure why MG > > > doesn't > > > need to take that into account. > > > > Yes, looks like crtc_state->lane_count should be considered. Not sure > > why the MG version doesn't do that either, I assume it was an > > addition > > to BSpec only after the function was added. The same applies to > > pin_mask. > > Looking more carefully, FIA takes care of flip/reversal lanes, so for > us just matter what lanes can be used. I think both the pin assignment and crtc_state->lane_count must be taken into account, see the above example with different pin assignment but same lane mask, where you have to program the DP_MODE registers differently. ICL would need to be fixed and in the end TGL and ICL would program DP_MODE the same way. > > So dropping the patch adding intel_tc_port_get_pin_assignment_mask(), > dropping tgl_program_dkl_dp_mode and tgl_phy_set_clock_gating() and > reusing ICL versions with a couple of "if gen > 12" to access dkl > register. > > One odd thing that I notice is that we use port instead of tc_port in > most MG registers, those MG registers uses a macro that subtract port > and PORT_C to get the right register, thinking in send a patch changing > all of those to receive as parameter tc_port to make it consistent, > what you guys think? > > > > > > > +Imre Deak > > > > > > Lucas De Marchi > > > > > > > + case 0x0: > > > > + if (num_lanes == 1) { > > > > + ln1 |= > > > > DKL_DP_MODE_CFG_DP_X1_MODE; > > > > + } else { > > > > + ln0 |= > > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > > + ln1 |= > > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > > + } > > > > + break; > > > > + case 0x1: > > > > + if (num_lanes == 4) { > > > > + ln0 |= > > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > > + ln1 |= > > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > > + } > > > > + break; > > > > + case 0x2: > > > > + if (num_lanes == 2) { > > > > + ln0 |= > > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > > + ln1 |= > > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > > + } > > > > + break; > > > > + case 0x3: > > > > + case 0x5: > > > > + if (num_lanes == 1) { > > > > + ln0 |= > > > > DKL_DP_MODE_CFG_DP_X1_MODE; > > > > + ln1 |= > > > > DKL_DP_MODE_CFG_DP_X1_MODE; > > > > + } else { > > > > + ln0 |= > > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > > + ln1 |= > > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > > + } > > > > + break; > > > > + case 0x4: > > > > + case 0x6: > > > > + if (num_lanes == 1) { > > > > + ln0 |= > > > > DKL_DP_MODE_CFG_DP_X1_MODE; > > > > + ln1 |= > > > > DKL_DP_MODE_CFG_DP_X1_MODE; > > > > + } else { > > > > + ln0 |= > > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > > + ln1 |= > > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > > + } > > > > + break; > > > > + default: > > > > + MISSING_CASE(lane_mask); > > > > + } > > > > + break; > > > > + > > > > + case TC_PORT_LEGACY: > > > > + ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE | > > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > > + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE | > > > > DKL_DP_MODE_CFG_DP_X2_MODE; > > > > + break; > > > > + > > > > + default: > > > > + MISSING_CASE(intel_dig_port->tc_mode); > > > > + return; > > > > + } > > > > + > > > > + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, > > > > 0x0)); > > > > + I915_WRITE(DKL_DP_MODE(tc_port), ln0); > > > > + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, > > > > 0x1)); > > > > + I915_WRITE(DKL_DP_MODE(tc_port), ln1); > > > > +} > > > > + > > > > static void intel_dp_sink_set_fec_ready(struct intel_dp > > > > *intel_dp, > > > > const struct > > > > intel_crtc_state *crtc_state) > > > > { > > > > @@ -3218,7 +3431,7 @@ static void tgl_ddi_pre_enable_dp(struct > > > > intel_encoder *encoder, > > > > dig_port- > > > > >ddi_io_power_domain); > > > > > > > > /* 6. */ > > > > - icl_program_mg_dp_mode(dig_port); > > > > + tgl_program_dkl_dp_mode(dig_port); > > > > > > > > /* > > > > * 7.a - Steps in this function should only be executed > > > > ov´er MST > > > > @@ -3231,10 +3444,10 @@ static void tgl_ddi_pre_enable_dp(struct > > > > intel_encoder *encoder, > > > > intel_ddi_config_transcoder_func(crtc_state); > > > > > > > > /* 7.d */ > > > > - icl_phy_set_clock_gating(dig_port, false); > > > > + tgl_phy_set_clock_gating(dig_port, false); > > > > > > > > /* 7.e */ > > > > - icl_ddi_vswing_sequence(encoder, crtc_state->port_clock, > > > > level, > > > > + tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock, > > > > level, > > > > encoder->type); > > > > > > > > /* 7.f */ > > > > @@ -3266,6 +3479,15 @@ static void tgl_ddi_pre_enable_dp(struct > > > > intel_encoder *encoder, > > > > /* 7.k */ > > > > intel_dp_stop_link_train(intel_dp); > > > > > > > > + /* > > > > + * TODO: enable clock gating > > > > + * > > > > + * 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 now. > > > > + */ > > > > + > > > > /* 7.l */ > > > > intel_ddi_enable_fec(encoder, crtc_state); > > > > intel_dsc_enable(encoder, crtc_state); > > > > @@ -3371,9 +3593,15 @@ static void > > > > intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, > > > > intel_display_power_get(dev_priv, dig_port- > > > > >ddi_io_power_domain); > > > > > > > > icl_program_mg_dp_mode(dig_port); > > > > - icl_phy_set_clock_gating(dig_port, false); > > > > + if (INTEL_GEN(dev_priv) >= 12) > > > > + tgl_phy_set_clock_gating(dig_port, false); > > > > + else > > > > + icl_phy_set_clock_gating(dig_port, false); > > > > > > > > - if (INTEL_GEN(dev_priv) >= 11) > > > > + if (INTEL_GEN(dev_priv) >= 12) > > > > + tgl_ddi_vswing_sequence(encoder, crtc_state- > > > > >port_clock, > > > > + level, > > > > INTEL_OUTPUT_HDMI); > > > > + else if (INTEL_GEN(dev_priv) == 11) > > > > icl_ddi_vswing_sequence(encoder, crtc_state- > > > > >port_clock, > > > > level, > > > > INTEL_OUTPUT_HDMI); > > > > else if (IS_CANNONLAKE(dev_priv)) > > > > @@ -3383,7 +3611,10 @@ static void > > > > intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, > > > > else > > > > intel_prepare_hdmi_ddi_buffers(encoder, level); > > > > > > > > - icl_phy_set_clock_gating(dig_port, true); > > > > + if (INTEL_GEN(dev_priv) >= 12) > > > > + tgl_phy_set_clock_gating(dig_port, true); > > > > + else > > > > + icl_phy_set_clock_gating(dig_port, true); > > > > > > > > if (IS_GEN9_BC(dev_priv)) > > > > skl_ddi_set_iboost(encoder, level, > > > > INTEL_OUTPUT_HDMI); > > > > -- > > > > 2.23.0 > > > > > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > > > > -- > > > Lucas De Marchi
On Tue, Sep 24, 2019 at 4:21 PM Souza, Jose <jose.souza@intel.com> wrote: > > On Tue, 2019-09-24 at 16:00 +0300, Imre Deak wrote: > > On Mon, Sep 23, 2019 at 03:02:54PM -0700, Lucas De Marchi wrote: > One odd thing that I notice is that we use port instead of tc_port in > most MG registers, those MG registers uses a macro that subtract port > and PORT_C to get the right register, thinking in send a patch changing > all of those to receive as parameter tc_port to make it consistent, > what you guys think? I think it's a leftover from previous implementation. For use them in TGL we do have to change to tc_port since TC ports in TGL start with PORT_D. Lucas De Marchi
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 33cd766f9eea..1ab3e0c4c0a1 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -586,6 +586,26 @@ 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[] = { + /* VS pre-emp Non-trans mV Pre-emph dB */ + { 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 */ + { 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 dB HDMI default */ +}; + static const struct ddi_buf_trans * bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries) { @@ -872,7 +892,14 @@ 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_GEN(dev_priv) >= 12) { + if (intel_phy_is_combo(dev_priv, phy)) + icl_get_combo_buf_trans(dev_priv, INTEL_OUTPUT_HDMI, + 0, &n_entries); + else + n_entries = ARRAY_SIZE(tgl_dkl_phy_ddi_translations); + default_entry = n_entries - 1; + } else if (INTEL_GEN(dev_priv) == 11) { if (intel_phy_is_combo(dev_priv, phy)) icl_get_combo_buf_trans(dev_priv, INTEL_OUTPUT_HDMI, 0, &n_entries); @@ -2313,7 +2340,13 @@ u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder) enum phy phy = intel_port_to_phy(dev_priv, port); int n_entries; - if (INTEL_GEN(dev_priv) >= 11) { + if (INTEL_GEN(dev_priv) >= 12) { + if (intel_phy_is_combo(dev_priv, phy)) + icl_get_combo_buf_trans(dev_priv, encoder->type, + intel_dp->link_rate, &n_entries); + else + n_entries = ARRAY_SIZE(tgl_dkl_phy_ddi_translations); + } else if (INTEL_GEN(dev_priv) == 11) { if (intel_phy_is_combo(dev_priv, phy)) icl_get_combo_buf_trans(dev_priv, encoder->type, intel_dp->link_rate, &n_entries); @@ -2755,6 +2788,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 tgl_dkl_phy_ddi_buf_trans *ddi_translations; + u32 n_entries, val, ln; + + n_entries = ARRAY_SIZE(tgl_dkl_phy_ddi_translations); + ddi_translations = tgl_dkl_phy_ddi_translations; + + if (level >= n_entries) + level = n_entries - 1; + + /* + * All registers programmed here use HIP_INDEX_REG 0 or 1 + */ + for (ln = 0; ln < 2; ln++) { + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, ln)); + + /* All the registers are RMW */ + val = I915_READ(DKL_TX_DPCNTL0(tc_port)); + val &= ~(DKL_TX_PRESHOOT_COEFF_MASK | + DKL_TX_DE_EMPAHSIS_COEFF_MASK | + DKL_TX_VSWING_CONTROL_MASK); + val |= DKL_TX_VSWING_CONTROL(ddi_translations[level].dkl_vswing_control); + val |= DKL_TX_DE_EMPHASIS_COEFF(ddi_translations[level].dkl_de_emphasis_control); + val |= DKL_TX_PRESHOOT_COEFF(ddi_translations[level].dkl_preshoot_control); + I915_WRITE(DKL_TX_DPCNTL0(tc_port), val); + + val = I915_READ(DKL_TX_DPCNTL1(tc_port)); + val &= ~(DKL_TX_PRESHOOT_COEFF_MASK | + DKL_TX_DE_EMPAHSIS_COEFF_MASK | + DKL_TX_VSWING_CONTROL_MASK); + val |= DKL_TX_VSWING_CONTROL(ddi_translations[level].dkl_vswing_control); + val |= DKL_TX_DE_EMPHASIS_COEFF(ddi_translations[level].dkl_de_emphasis_control); + val |= DKL_TX_PRESHOOT_COEFF(ddi_translations[level].dkl_preshoot_control); + I915_WRITE(DKL_TX_DPCNTL1(tc_port), val); + + val = I915_READ(DKL_TX_DPCNTL2(tc_port)); + val &= ~DKL_TX_DP20BITMODE; + I915_WRITE(DKL_TX_DPCNTL2(tc_port), val); + } +} + +static void tgl_ddi_vswing_sequence(struct intel_encoder *encoder, + int link_clock, + u32 level, + enum intel_output_type type) +{ + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); + enum phy phy = intel_port_to_phy(dev_priv, encoder->port); + + if (intel_phy_is_combo(dev_priv, phy)) + icl_combo_phy_ddi_vswing_sequence(encoder, level, type); + else + tgl_dkl_phy_ddi_vswing_sequence(encoder, link_clock, level); +} + static u32 translate_signal_level(int signal_levels) { int i; @@ -2786,7 +2879,10 @@ u32 bxt_signal_levels(struct intel_dp *intel_dp) struct intel_encoder *encoder = &dport->base; int level = intel_ddi_dp_level(intel_dp); - if (INTEL_GEN(dev_priv) >= 11) + 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)) @@ -3033,6 +3129,34 @@ static void intel_ddi_clk_disable(struct intel_encoder *encoder) } } +static void +tgl_phy_set_clock_gating(struct intel_digital_port *dig_port, bool enable) +{ + struct drm_i915_private *dev_priv = to_i915(dig_port->base.base.dev); + enum port port = dig_port->base.port; + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); + u32 val, bits; + int ln; + + if (tc_port == PORT_TC_NONE) + return; + + bits = DKL_DP_MODE_CFG_TR2PWR_GATING | DKL_DP_MODE_CFG_TRPWR_GATING | + DKL_DP_MODE_CFG_CLNPWR_GATING | DKL_DP_MODE_CFG_DIGPWR_GATING | + DKL_DP_MODE_CFG_GAONPWR_GATING; + + for (ln = 0; ln < 2; ln++) { + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, ln)); + + val = I915_READ(DKL_DP_MODE(tc_port)); + if (enable) + val |= bits; + else + val &= ~bits; + I915_WRITE(DKL_DP_MODE(tc_port), val); + } +} + static void icl_phy_set_clock_gating(struct intel_digital_port *dig_port, bool enable) { @@ -3132,6 +3256,95 @@ static void icl_program_mg_dp_mode(struct intel_digital_port *intel_dig_port) I915_WRITE(MG_DP_MODE(1, port), ln1); } +static void tgl_program_dkl_dp_mode(struct intel_digital_port *intel_dig_port) +{ + struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev); + enum port port = intel_dig_port->base.port; + enum tc_port tc_port = intel_port_to_tc(dev_priv, port); + u32 ln0, ln1, lane_mask, pin_mask; + int num_lanes; + + if (tc_port == PORT_TC_NONE || + intel_dig_port->tc_mode == TC_PORT_TBT_ALT) + return; + + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, 0x0)); + ln0 = I915_READ(DKL_DP_MODE(tc_port)); + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, 0x1)); + ln1 = I915_READ(DKL_DP_MODE(tc_port)); + + num_lanes = intel_dig_port->dp.lane_count; + + switch (intel_dig_port->tc_mode) { + case TC_PORT_DP_ALT: + ln0 &= ~(DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X1_MODE); + ln1 &= ~(DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X2_MODE); + + lane_mask = intel_tc_port_get_lane_mask(intel_dig_port); /* DPX4TXLATC */ + pin_mask = intel_tc_port_get_pin_assignment_mask(intel_dig_port); /* DPPATC */ + + switch (pin_mask) { + case 0x0: + if (num_lanes == 1) { + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE; + } else { + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; + } + break; + case 0x1: + if (num_lanes == 4) { + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; + } + break; + case 0x2: + if (num_lanes == 2) { + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; + } + break; + case 0x3: + case 0x5: + if (num_lanes == 1) { + ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE; + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE; + } else { + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; + } + break; + case 0x4: + case 0x6: + if (num_lanes == 1) { + ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE; + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE; + } else { + ln0 |= DKL_DP_MODE_CFG_DP_X2_MODE; + ln1 |= DKL_DP_MODE_CFG_DP_X2_MODE; + } + break; + default: + MISSING_CASE(lane_mask); + } + break; + + case TC_PORT_LEGACY: + ln0 |= DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X2_MODE; + ln1 |= DKL_DP_MODE_CFG_DP_X1_MODE | DKL_DP_MODE_CFG_DP_X2_MODE; + break; + + default: + MISSING_CASE(intel_dig_port->tc_mode); + return; + } + + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, 0x0)); + I915_WRITE(DKL_DP_MODE(tc_port), ln0); + I915_WRITE(HIP_INDEX_REG(tc_port), HIP_INDEX_VAL(tc_port, 0x1)); + I915_WRITE(DKL_DP_MODE(tc_port), ln1); +} + static void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state) { @@ -3218,7 +3431,7 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder, dig_port->ddi_io_power_domain); /* 6. */ - icl_program_mg_dp_mode(dig_port); + tgl_program_dkl_dp_mode(dig_port); /* * 7.a - Steps in this function should only be executed over MST @@ -3231,10 +3444,10 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder, intel_ddi_config_transcoder_func(crtc_state); /* 7.d */ - icl_phy_set_clock_gating(dig_port, false); + tgl_phy_set_clock_gating(dig_port, false); /* 7.e */ - icl_ddi_vswing_sequence(encoder, crtc_state->port_clock, level, + tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock, level, encoder->type); /* 7.f */ @@ -3266,6 +3479,15 @@ static void tgl_ddi_pre_enable_dp(struct intel_encoder *encoder, /* 7.k */ intel_dp_stop_link_train(intel_dp); + /* + * TODO: enable clock gating + * + * 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 now. + */ + /* 7.l */ intel_ddi_enable_fec(encoder, crtc_state); intel_dsc_enable(encoder, crtc_state); @@ -3371,9 +3593,15 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain); icl_program_mg_dp_mode(dig_port); - icl_phy_set_clock_gating(dig_port, false); + if (INTEL_GEN(dev_priv) >= 12) + tgl_phy_set_clock_gating(dig_port, false); + else + icl_phy_set_clock_gating(dig_port, false); - if (INTEL_GEN(dev_priv) >= 11) + if (INTEL_GEN(dev_priv) >= 12) + tgl_ddi_vswing_sequence(encoder, crtc_state->port_clock, + level, INTEL_OUTPUT_HDMI); + else if (INTEL_GEN(dev_priv) == 11) icl_ddi_vswing_sequence(encoder, crtc_state->port_clock, level, INTEL_OUTPUT_HDMI); else if (IS_CANNONLAKE(dev_priv)) @@ -3383,7 +3611,10 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder, else intel_prepare_hdmi_ddi_buffers(encoder, level); - icl_phy_set_clock_gating(dig_port, true); + if (INTEL_GEN(dev_priv) >= 12) + tgl_phy_set_clock_gating(dig_port, true); + else + icl_phy_set_clock_gating(dig_port, true); if (IS_GEN9_BC(dev_priv)) skl_ddi_set_iboost(encoder, level, INTEL_OUTPUT_HDMI);