From: "Souza, Jose" <jose.souza@intel.com>
To: "Deak, Imre" <imre.deak@intel.com>
Cc: "intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>,
"De Marchi, Lucas" <lucas.demarchi@intel.com>
Subject: Re: [PATCH v4 2/4] drm/i915/tgl: Add dkl phy programming sequences
Date: Thu, 26 Sep 2019 21:02:11 +0000 [thread overview]
Message-ID: <2fe551d821db7fdcf02870c4e34c8c281f4f3b41.camel@intel.com> (raw)
In-Reply-To: <20190926153400.GC20817@ideak-desk.fi.intel.com>
On Thu, 2019-09-26 at 18:34 +0300, Imre Deak wrote:
> On Wed, Sep 25, 2019 at 04:45:07PM -0700, José Roberto de Souza
> 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
> >
> > v4:
> > Moved the DP_MODE lane programing to another patch as ICL also
> > needed it
> > Sharing icl_phy_set_clock_gating() and icl_program_mg_dp_mode()
> > with
> > TGL as bits and programing as now it almost identical to ICL
> >
> > BSpec: 49292
> > BSpec: 49190
> >
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > 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 | 175
> > ++++++++++++++++++++---
> > drivers/gpu/drm/i915/i915_reg.h | 8 --
> > 2 files changed, 155 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 316cedb16935..4da7940f1fcf 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);
> > @@ -2334,7 +2361,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);
> > @@ -2776,6 +2809,63 @@ 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;
> > +
> > + for (ln = 0; ln < 2; ln++) {
>
> nit:
> dpcntl_val = DKL_TX_VSWING_CONTROL(...) | ...;
> dpcntl_mask = ~(DKL_TX_PRESHOOT_COEFF_MASK | ...);
>
> > + 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 &= ~dpcnt_mask;
> val |= dpcnt_val;
>
Nice idea, moved dpcntl_val/mask even out of the loop.
> > + 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_co
> > ntrol);
> > + 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 &= ~dpcnt_mask;
> val |= dpcnt_val;
>
> > + 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_co
> > ntrol);
> > + 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;
> > @@ -2807,7 +2897,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))
> > @@ -3071,38 +3164,61 @@ icl_phy_set_clock_gating(struct
> > intel_digital_port *dig_port, bool enable)
> > MG_DP_MODE_CFG_GAONPWR_GATING;
> >
> > for (ln = 0; ln < 2; ln++) {
> > - val = I915_READ(MG_DP_MODE(ln, port));
> > + if (INTEL_GEN(dev_priv) >= 12) {
> > + I915_WRITE(HIP_INDEX_REG(tc_port),
> > HIP_INDEX_VAL(tc_port, ln));
> > + val = I915_READ(DKL_DP_MODE(tc_port));
> > + } else {
> > + val = I915_READ(MG_DP_MODE(ln, port));
> > + }
> > +
> > if (enable)
> > val |= bits;
> > else
> > val &= ~bits;
> > - I915_WRITE(MG_DP_MODE(ln, port), val);
> > +
> > + if (INTEL_GEN(dev_priv) >= 12)
> > + I915_WRITE(DKL_DP_MODE(tc_port), val);
> > + else
> > + I915_WRITE(MG_DP_MODE(ln, port), val);
> > }
> >
> > - bits = MG_MISC_SUS0_CFG_TR2PWR_GATING |
> > MG_MISC_SUS0_CFG_CL2PWR_GATING |
> > - MG_MISC_SUS0_CFG_GAONPWR_GATING |
> > MG_MISC_SUS0_CFG_TRPWR_GATING |
> > - MG_MISC_SUS0_CFG_CL1PWR_GATING |
> > MG_MISC_SUS0_CFG_DGPWR_GATING;
> > + if (INTEL_GEN(dev_priv) == 11) {
>
> nit:
> if (GEN >= 12)
> return;
>
> Reviewed-by: Imre Deak <imre.deak@intel.com>
Thanks
>
> > + bits = MG_MISC_SUS0_CFG_TR2PWR_GATING |
> > + MG_MISC_SUS0_CFG_CL2PWR_GATING |
> > + MG_MISC_SUS0_CFG_GAONPWR_GATING |
> > + MG_MISC_SUS0_CFG_TRPWR_GATING |
> > + MG_MISC_SUS0_CFG_CL1PWR_GATING |
> > + MG_MISC_SUS0_CFG_DGPWR_GATING;
> >
> > - val = I915_READ(MG_MISC_SUS0(tc_port));
> > - if (enable)
> > - val |= (bits | MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE(3));
> > - else
> > - val &= ~(bits |
> > MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE_MASK);
> > - I915_WRITE(MG_MISC_SUS0(tc_port), val);
> > + val = I915_READ(MG_MISC_SUS0(tc_port));
> > + if (enable)
> > + val |= (bits |
> > MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE(3));
> > + else
> > + val &= ~(bits |
> > MG_MISC_SUS0_SUSCLK_DYNCLKGATE_MODE_MASK);
> > + I915_WRITE(MG_MISC_SUS0(tc_port), val);
> > + }
> > }
> >
> > static void icl_program_mg_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, pin_assignment;
> > u8 width;
> >
> > if (intel_dig_port->tc_mode == TC_PORT_TBT_ALT)
> > return;
> >
> > - ln0 = I915_READ(MG_DP_MODE(0, port));
> > - ln1 = I915_READ(MG_DP_MODE(1, port));
> > + if (INTEL_GEN(dev_priv) >= 12) {
> > + 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));
> > + } else {
> > + ln0 = I915_READ(MG_DP_MODE(0, port));
> > + ln1 = I915_READ(MG_DP_MODE(1, port));
> > + }
> >
> > ln0 &= ~(MG_DP_MODE_CFG_DP_X1_MODE |
> > MG_DP_MODE_CFG_DP_X1_MODE);
> > ln1 &= ~(MG_DP_MODE_CFG_DP_X1_MODE |
> > MG_DP_MODE_CFG_DP_X2_MODE);
> > @@ -3157,8 +3273,15 @@ static void icl_program_mg_dp_mode(struct
> > intel_digital_port *intel_dig_port)
> > MISSING_CASE(pin_assignment);
> > }
> >
> > - I915_WRITE(MG_DP_MODE(0, port), ln0);
> > - I915_WRITE(MG_DP_MODE(1, port), ln1);
> > + if (INTEL_GEN(dev_priv) >= 12) {
> > + 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);
> > + } else {
> > + I915_WRITE(MG_DP_MODE(0, port), ln0);
> > + I915_WRITE(MG_DP_MODE(1, port), ln1);
> > + }
> > }
> >
> > static void intel_dp_sink_set_fec_ready(struct intel_dp *intel_dp,
> > @@ -3263,7 +3386,7 @@ static void tgl_ddi_pre_enable_dp(struct
> > intel_encoder *encoder,
> > icl_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 */
> > @@ -3295,6 +3418,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);
> > @@ -3402,7 +3534,10 @@ static void intel_ddi_pre_enable_hdmi(struct
> > intel_encoder *encoder,
> > icl_program_mg_dp_mode(dig_port);
> > 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))
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index bcf449c1d152..7f6b59aa6348 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -10245,14 +10245,6 @@ enum skl_power_gate {
> > _DKL_TX_DW18)
> >
> > #define _DKL_DP_MODE 0xA0
> > -#define DKL_DP_MODE_CFG_GAONPWR_GATING (1 << 1)
> > -#define DKL_DP_MODE_CFG_DIGPWR_GATING (1 << 2)
> > -#define DKL_DP_MODE_CFG_CLNPWR_GATING (1 << 3)
> > -#define DKL_DP_MODE_CFG_TRPWR_GATING (1 << 4)
> > -#define DKL_DP_MODE_CFG_TR2PWR_GATING (1 << 5)
> > -#define DKL_DP_MODE_CFG_GATING_CTRL_MASK (0x1f << 1)
> > -#define DKL_DP_MODE_CFG_DP_X1_MODE (1 <<
> > 6)
> > -#define DKL_DP_MODE_CFG_DP_X2_MODE (1 <<
> > 7)
> > #define DKL_DP_MODE(tc_port) _MMIO(_PORT(tc_port, \
> > _DKL_PHY1_BASE, \
> > _DKL_PHY2_BASE) +
> > \
> > --
> > 2.23.0
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2019-09-26 21:02 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-25 23:45 [PATCH v4 0/4] TGL TC enabling v4 José Roberto de Souza
2019-09-25 23:45 ` [PATCH v4 1/4] drm/i915/tc: Update DP_MODE programming José Roberto de Souza
2019-09-26 12:02 ` Imre Deak
2019-09-26 19:35 ` Souza, Jose
2019-09-26 19:41 ` Imre Deak
2019-09-25 23:45 ` [PATCH v4 2/4] drm/i915/tgl: Add dkl phy programming sequences José Roberto de Souza
2019-09-26 15:34 ` Imre Deak
2019-09-26 21:02 ` Souza, Jose [this message]
2019-09-25 23:45 ` [PATCH v4 3/4] drm/i915/tgl: Fix dkl link training José Roberto de Souza
2019-09-26 13:06 ` Imre Deak
2019-09-25 23:45 ` [PATCH v4 4/4] drm/i915/tgl: initialize TC and TBT ports José Roberto de Souza
2019-09-26 0:58 ` ✓ Fi.CI.BAT: success for TGL TC enabling (rev4) Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2fe551d821db7fdcf02870c4e34c8c281f4f3b41.camel@intel.com \
--to=jose.souza@intel.com \
--cc=imre.deak@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=lucas.demarchi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.