On Thu, Sep 19, 2019 at 2:36 PM Souza, Jose <jose.souza@xxxxxxxxx> wrote:
>
> On Thu, 2019-09-19 at 14:27 -0700, Lucas De Marchi wrote:
> > On Wed, Sep 18, 2019 at 5:07 PM José Roberto de Souza
> > <jose.souza@xxxxxxxxx> wrote:
> > > Extending ICL mg calculations to also support dkl calculations.
> > >
> > > BSpec: 49204
> > >
> > > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@xxxxxxxxx>
> > > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_ddi.c | 29 +++-
> > > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 148 +++++++++++---
> > > ----
> > > 2 files changed, 114 insertions(+), 63 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index 78c6974a52d4..2717e7522a38 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -1436,11 +1436,30 @@ static int icl_calc_mg_pll_link(struct
> > > drm_i915_private *dev_priv,
> > >
> > > ref_clock = dev_priv->cdclk.hw.ref;
> > >
> > > - m1 = pll_state->mg_pll_div1 & MG_PLL_DIV1_FBPREDIV_MASK;
> > > - m2_int = pll_state->mg_pll_div0 &
> > > MG_PLL_DIV0_FBDIV_INT_MASK;
> > > - m2_frac = (pll_state->mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H)
> > > ?
> > > - (pll_state->mg_pll_div0 &
> > > MG_PLL_DIV0_FBDIV_FRAC_MASK) >>
> > > - MG_PLL_DIV0_FBDIV_FRAC_SHIFT : 0;
> > > + if (INTEL_GEN(dev_priv) >= 12) {
> > > + m1 = pll_state->mg_pll_div0 &
> > > DKL_PLL_DIV0_FBPREDIV_MASK;
> > > + m1 = m1 >> DKL_PLL_DIV0_FBPREDIV_SHIFT;
> > > + m2_int = pll_state->mg_pll_div0 &
> > > DKL_PLL_DIV0_FBDIV_INT_MASK;
> > > +
> > > + if (pll_state->mg_pll_bias &
> > > DKL_PLL_BIAS_FRAC_EN_H) {
> > > + m2_frac = pll_state->mg_pll_bias &
> > > + DKL_PLL_BIAS_FBDIV_FRAC_MASK;
> > > + m2_frac = m2_frac >>
> > > DKL_PLL_BIAS_FBDIV_SHIFT;
> > > + } else {
> > > + m2_frac = 0;
> > > + }
> > > + } else {
> > > + m1 = pll_state->mg_pll_div1 &
> > > MG_PLL_DIV1_FBPREDIV_MASK;
> > > + m2_int = pll_state->mg_pll_div0 &
> > > MG_PLL_DIV0_FBDIV_INT_MASK;
> > > +
> > > + if (pll_state->mg_pll_div0 & MG_PLL_DIV0_FRACNEN_H)
> > > {
> > > + m2_frac = pll_state->mg_pll_div0 &
> > > + MG_PLL_DIV0_FBDIV_FRAC_MASK;
> > > + m2_frac = m2_frac >>
> > > MG_PLL_DIV0_FBDIV_FRAC_SHIFT;
> > > + } else {
> > > + m2_frac = 0;
> > > + }
> > > + }
> > >
> > > switch (pll_state->mg_clktop2_hsclkctl &
> > > MG_CLKTOP2_HSCLKCTL_HSDIV_RATIO_MASK) {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > index 21249997940d..e6a7280da408 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > > @@ -2607,7 +2607,8 @@ enum intel_dpll_id icl_tc_port_to_pll_id(enum
> > > tc_port tc_port)
> > >
> > > static bool icl_mg_pll_find_divisors(int clock_khz, bool is_dp,
> > > bool use_ssc,
> > > u32 *target_dco_khz,
> > > - struct intel_dpll_hw_state
> > > *state)
> > > + struct intel_dpll_hw_state
> > > *state,
> > > + bool is_dkl)
> > > {
> > > u32 dco_min_freq, dco_max_freq;
> > > int div1_vals[] = {7, 5, 3, 2};
> > > @@ -2627,10 +2628,14 @@ static bool icl_mg_pll_find_divisors(int
> > > clock_khz, bool is_dp, bool use_ssc,
> > >
> > > if (dco < dco_min_freq || dco >
> > > dco_max_freq)
> > > continue;
> > > -
> > > if (div2 >= 2) {
> > > - a_divratio = is_dp ? 10 : 5;
> > > - tlinedrv = 2;
> > > + if (is_dkl) {
> > > + a_divratio = 5;
> > > + tlinedrv = 1;
> > > + } else {
> > > + a_divratio = is_dp ? 10 :
> > > 5;
> > > + tlinedrv = 2;
> > > + }
> > > } else {
> > > a_divratio = 5;
> > > tlinedrv = 0;
> > > @@ -2693,11 +2698,12 @@ static bool icl_calc_mg_pll_state(struct
> > > intel_crtc_state *crtc_state,
> > > u64 tmp;
> > > bool use_ssc = false;
> > > bool is_dp = !intel_crtc_has_type(crtc_state,
> > > INTEL_OUTPUT_HDMI);
> > > + bool is_dkl = INTEL_GEN(dev_priv) >= 12;
> > >
> > > memset(pll_state, 0, sizeof(*pll_state));
> > >
> > > if (!icl_mg_pll_find_divisors(clock, is_dp, use_ssc,
> > > &dco_khz,
> > > - pll_state)) {
> > > + pll_state, is_dkl)) {
> > > DRM_DEBUG_KMS("Failed to find divisors for clock
> > > %d\n", clock);
> > > return false;
> > > }
> > > @@ -2705,8 +2711,12 @@ static bool icl_calc_mg_pll_state(struct
> > > intel_crtc_state *crtc_state,
> > > m1div = 2;
> > > m2div_int = dco_khz / (refclk_khz * m1div);
> > > if (m2div_int > 255) {
> > > + if (is_dkl)
> > > + goto no_mdiv;
> >
> > just to jump 2 lines? It's not an error path, so `if (!is_dkl)` would
> > do it better IMO
>
> Okay
>
> >
> > > +
> > > m1div = 4;
> > > m2div_int = dco_khz / (refclk_khz * m1div);
> > > +no_mdiv:
> > > if (m2div_int > 255) {
> > > DRM_DEBUG_KMS("Failed to find mdiv for
> > > clock %d\n",
> > > clock);
> > > @@ -2732,7 +2742,7 @@ static bool icl_calc_mg_pll_state(struct
> > > intel_crtc_state *crtc_state,
> > > break;
> > > case 38400:
> > > iref_ndiv = 2;
> > > - iref_trim = 28;
> > > + iref_trim = is_dkl ? 24 : 28;
> >
> > refclk_khz / iref_ndiv == 19200. So iref_trim should still be 28 for
> > dkl, no?
>
> From BSpec:
>
> $iref_trim = (($refclk_mhz/$iref_ndiv) <= 19.2) ? 28 :
> (($refclk_mhz/$iref_ndiv) > 19.2 &&
> ($refclk_mhz/$iref_ndiv) <= 25) ? 25 : 24;
>
> So
> if ((refclk_khz / iref_ndiv) <= 19.2)
> iref_trim = 28
> else if ((refclk_khz / iref_ndiv) <= 25)
> iref_trim = 25
> else
> iref_trim = 24
>
we already have a switch (refclk_khz). And in that case refclk_khz is
38400. There's no case that leads to iref_trim == 24, unless we are
missing a refclk_khz addition. In that case it should be added
regardless of is_dkl.
Lucas De Marchi
>
> >
> > > iref_pulse_w = 1;
> > > break;
> > > default:
> > > @@ -2786,60 +2796,82 @@ static bool icl_calc_mg_pll_state(struct
> > > intel_crtc_state *crtc_state,
> > > }
> > > ssc_steplog = 4;
> > >
> > > - pll_state->mg_pll_div0 = (m2div_rem > 0 ?
> > > MG_PLL_DIV0_FRACNEN_H : 0) |
> > > - MG_PLL_DIV0_FBDIV_FRAC(m2div_frac
> > > ) |
> > > - MG_PLL_DIV0_FBDIV_INT(m2div_int);
> > > -
> > > - pll_state->mg_pll_div1 =
> > > MG_PLL_DIV1_IREF_NDIVRATIO(iref_ndiv) |
> > > - MG_PLL_DIV1_DITHER_DIV_2 |
> > > - MG_PLL_DIV1_NDIVRATIO(1) |
> > > - MG_PLL_DIV1_FBPREDIV(m1div);
> > > -
> > > - pll_state->mg_pll_lf =
> > > MG_PLL_LF_TDCTARGETCNT(tdc_targetcnt) |
> > > - MG_PLL_LF_AFCCNTSEL_512 |
> > > - MG_PLL_LF_GAINCTRL(1) |
> > > - MG_PLL_LF_INT_COEFF(int_coeff) |
> > > - MG_PLL_LF_PROP_COEFF(prop_coeff);
> > > -
> > > - pll_state->mg_pll_frac_lock =
> > > MG_PLL_FRAC_LOCK_TRUELOCK_CRIT_32 |
> > > - MG_PLL_FRAC_LOCK_EARLYLOCK_CR
> > > IT_32 |
> > > - MG_PLL_FRAC_LOCK_LOCKTHRESH(1
> > > 0) |
> > > - MG_PLL_FRAC_LOCK_DCODITHEREN
> > > |
> > > - MG_PLL_FRAC_LOCK_FEEDFWRDGAIN
> > > (feedfwgain);
> > > - if (use_ssc || m2div_rem > 0)
> > > - pll_state->mg_pll_frac_lock |=
> > > MG_PLL_FRAC_LOCK_FEEDFWRDCAL_EN;
> > > -
> > > - pll_state->mg_pll_ssc = (use_ssc ? MG_PLL_SSC_EN : 0) |
> > > - MG_PLL_SSC_TYPE(2) |
> > > - MG_PLL_SSC_STEPLENGTH(ssc_steplen)
> > > |
> > > - MG_PLL_SSC_STEPNUM(ssc_steplog) |
> > > - MG_PLL_SSC_FLLEN |
> > > - MG_PLL_SSC_STEPSIZE(ssc_stepsize);
> > > -
> > > - pll_state->mg_pll_tdc_coldst_bias =
> > > MG_PLL_TDC_COLDST_COLDSTART |
> > > - MG_PLL_TDC_COLDST_IREFI
> > > NT_EN |
> > > - MG_PLL_TDC_COLDST_REFBI
> > > AS_START_PULSE_W(iref_pulse_w) |
> > > - MG_PLL_TDC_TDCOVCCORR_E
> > > N |
> > > - MG_PLL_TDC_TDCSEL(3);
> > > -
> > > - pll_state->mg_pll_bias = MG_PLL_BIAS_BIAS_GB_SEL(3) |
> > > - MG_PLL_BIAS_INIT_DCOAMP(0x3F) |
> > > - MG_PLL_BIAS_BIAS_BONUS(10) |
> > > - MG_PLL_BIAS_BIASCAL_EN |
> > > - MG_PLL_BIAS_CTRIM(12) |
> > > - MG_PLL_BIAS_VREF_RDAC(4) |
> > > - MG_PLL_BIAS_IREFTRIM(iref_trim);
> > > -
> > > - if (refclk_khz == 38400) {
> > > - pll_state->mg_pll_tdc_coldst_bias_mask =
> > > MG_PLL_TDC_COLDST_COLDSTART;
> > > - pll_state->mg_pll_bias_mask = 0;
> > > + if (is_dkl) {
> > > + pll_state->mg_pll_div0 =
> > > DKL_PLL_DIV0_INTEG_COEFF(int_coeff) |
> > > + DKL_PLL_DIV0_PROP_COEFF(pr
> > > op_coeff) |
> > > + DKL_PLL_DIV0_FBPREDIV(m1di
> > > v) |
> > > + DKL_PLL_DIV0_FBDIV_INT(m2d
> > > iv_int);
> > > +
> > > + pll_state->mg_pll_div1 =
> > > DKL_PLL_DIV1_IREF_TRIM(iref_trim) |
> > > + DKL_PLL_DIV1_TDC_TARGET_CN
> > > T(tdc_targetcnt);
> > > +
> > > + pll_state->mg_pll_ssc =
> > > DKL_PLL_SSC_IREF_NDIV_RATIO(iref_ndiv) |
> > > + DKL_PLL_SSC_STEP_LEN(ssc_st
> > > eplen) |
> > > + DKL_PLL_SSC_STEP_NUM(ssc_st
> > > eplog) |
> > > + (use_ssc ? DKL_PLL_SSC_EN :
> > > 0);
> > > +
> > > + pll_state->mg_pll_bias = (m2div_frac ?
> > > DKL_PLL_BIAS_FRAC_EN_H : 0) |
> > > + DKL_PLL_BIAS_FBDIV_FRAC(m
> > > 2div_frac);
> > > +
> > > + pll_state->mg_pll_tdc_coldst_bias =
> > > + DKL_PLL_TDC_SSC_STEP_SIZE(ssc_steps
> > > ize) |
> > > + DKL_PLL_TDC_FEED_FWD_GAIN(feedfwgai
> > > n);
> > > } else {
> > > - pll_state->mg_pll_tdc_coldst_bias_mask = -1U;
> > > - pll_state->mg_pll_bias_mask = -1U;
> > > - }
> > > + pll_state->mg_pll_div0 = (m2div_rem > 0 ?
> > > MG_PLL_DIV0_FRACNEN_H : 0) |
> > > + MG_PLL_DIV0_FBDIV_FRAC(m2
> > > div_frac) |
> > > + MG_PLL_DIV0_FBDIV_INT(m2d
> > > iv_int);
> > > +
> > > + pll_state->mg_pll_div1 =
> > > MG_PLL_DIV1_IREF_NDIVRATIO(iref_ndiv) |
> > > + MG_PLL_DIV1_DITHER_DIV_2 |
> > > + MG_PLL_DIV1_NDIVRATIO(1) |
> > > + MG_PLL_DIV1_FBPREDIV(m1div
> > > );
> > > +
> > > + pll_state->mg_pll_lf =
> > > MG_PLL_LF_TDCTARGETCNT(tdc_targetcnt) |
> > > + MG_PLL_LF_AFCCNTSEL_512 |
> > > + MG_PLL_LF_GAINCTRL(1) |
> > > + MG_PLL_LF_INT_COEFF(int_coef
> > > f) |
> > > + MG_PLL_LF_PROP_COEFF(prop_co
> > > eff);
> > > +
> > > + pll_state->mg_pll_frac_lock =
> > > MG_PLL_FRAC_LOCK_TRUELOCK_CRIT_32 |
> > > + MG_PLL_FRAC_LOCK_EARL
> > > YLOCK_CRIT_32 |
> > > + MG_PLL_FRAC_LOCK_LOCK
> > > THRESH(10) |
> > > + MG_PLL_FRAC_LOCK_DCOD
> > > ITHEREN |
> > > + MG_PLL_FRAC_LOCK_FEED
> > > FWRDGAIN(feedfwgain);
> > > + if (use_ssc || m2div_rem > 0)
> > > + pll_state->mg_pll_frac_lock |=
> > > MG_PLL_FRAC_LOCK_FEEDFWRDCAL_EN;
> > > +
> > > + pll_state->mg_pll_ssc = (use_ssc ? MG_PLL_SSC_EN :
> > > 0) |
> > > + MG_PLL_SSC_TYPE(2) |
> > > + MG_PLL_SSC_STEPLENGTH(ssc_s
> > > teplen) |
> > > + MG_PLL_SSC_STEPNUM(ssc_step
> > > log) |
> > > + MG_PLL_SSC_FLLEN |
> > > + MG_PLL_SSC_STEPSIZE(ssc_ste
> > > psize);
> > > +
> > > + pll_state->mg_pll_tdc_coldst_bias =
> > > MG_PLL_TDC_COLDST_COLDSTART |
> > > + MG_PLL_TDC_COLD
> > > ST_IREFINT_EN |
> > > + MG_PLL_TDC_COLD
> > > ST_REFBIAS_START_PULSE_W(iref_pulse_w) |
> > > + MG_PLL_TDC_TDCO
> > > VCCORR_EN |
> > > + MG_PLL_TDC_TDCS
> > > EL(3);
> > > +
> > > + pll_state->mg_pll_bias = MG_PLL_BIAS_BIAS_GB_SEL(3)
> > > |
> > > + MG_PLL_BIAS_INIT_DCOAMP(0x
> > > 3F) |
> > > + MG_PLL_BIAS_BIAS_BONUS(10)
> > > |
> > > + MG_PLL_BIAS_BIASCAL_EN |
> > > + MG_PLL_BIAS_CTRIM(12) |
> > > + MG_PLL_BIAS_VREF_RDAC(4) |
> > > + MG_PLL_BIAS_IREFTRIM(iref_
> > > trim);
> > > +
> > > + if (refclk_khz == 38400) {
> > > + pll_state->mg_pll_tdc_coldst_bias_mask =
> > > MG_PLL_TDC_COLDST_COLDSTART;
> > > + pll_state->mg_pll_bias_mask = 0;
> > > + } else {
> > > + pll_state->mg_pll_tdc_coldst_bias_mask =
> > > -1U;
> > > + pll_state->mg_pll_bias_mask = -1U;
> > > + }
> > >
> > > - pll_state->mg_pll_tdc_coldst_bias &= pll_state-
> > > >mg_pll_tdc_coldst_bias_mask;
> > > - pll_state->mg_pll_bias &= pll_state->mg_pll_bias_mask;
> > > + pll_state->mg_pll_tdc_coldst_bias &= pll_state-
> > > >mg_pll_tdc_coldst_bias_mask;
> > > + pll_state->mg_pll_bias &= pll_state-
> > > >mg_pll_bias_mask;
> > > + }
> >
> > This whole hunk would be simpler with a preparatory patch. git
> > doesn't
> > detect the code move, so not
> > easy to double check the MG_ code didn't change. But I confirmed with
> > using git show --ignore-space-change,
> > so lgtm.
> >
> > Lucas De Marchi
> >
> > > return true;
> > > }
> > > --
> > > 2.23.0
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> >
--
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
