[Intel-gfx] [PATCH 0/5] drm/i915/tgl: Fix Combo PHY DPLL fractional divider for 38.4MHz ref clock

[Intel-gfx] [PATCH 0/5] drm/i915/tgl: Fix Combo PHY DPLL fractional divider for 38.4MHz ref clock

[Imre Deak]

This patchset replaces [1], adding also a workaround for TGL BIOSes that
don't apply Display WA #22010492432. The first patch fixes an incorrect
BIOS PDIV programming I noticed while testing this patchset on an ASUS/SKL
system.

[1] https://patchwork.freedesktop.org/series/79486/

Imre Deak (5):
  drm/i915/skl: Work around incorrect BIOS WRPLL PDIV programming
  drm/i915: Factor out skl_wrpll_calc_freq()
  drm/i915/icl: Cross check the combo PLL WRPLL parameters wrt.
    hard-coded PLL freqs
  drm/i915/tgl: Fix Combo PHY DPLL fractional divider for 38.4MHz ref
    clock
  drm/i915/tgl: Add workaround for incorrect BIOS combo PHY DPLL
    programming

 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 232 ++++++++++++------
 drivers/gpu/drm/i915/i915_reg.h               |   4 +
 2 files changed, 161 insertions(+), 75 deletions(-)

-- 
2.25.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/5] drm/i915/skl: Work around incorrect BIOS WRPLL PDIV programming

[Imre Deak]

The BIOS of at least one ASUS-Z170M system with an SKL I have programs
the 101b WRPLL PDIV divider value, which is the encoding for PDIV=7 with
bit#0 incorrectly set.

This happens with the

"3840x2160": 30 262750 3840 3888 3920 4000 2160 2163 2168 2191 0x48 0x9

HDMI mode (scaled from a 1024x768 src fb) set by BIOS and the

ref_clock=24000, dco_integer=383, dco_fraction=5802, pdiv=7, qdiv=1, kdiv=1

WRPLL parameters (assuming PDIV=7 was the intended setting). This
corresponds to 262749 PLL frequency/port clock.

Later the driver sets the same mode for which it calculates the same
dco_int/dco_frac/div WRPLL parameters (with the correct PDIV=7 encoding).

Based on the above, let's assume that PDIV=7 was intended and the HW
just ignores bit#0 in the PDIV register field for this setting, treating
100b and 101b encodings the same way.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 8 ++++++++
 drivers/gpu/drm/i915/i915_reg.h               | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index e08684e34078..095b53fe3a21 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -1602,6 +1602,14 @@ static int skl_ddi_wrpll_get_freq(struct drm_i915_private *i915,
 	case DPLL_CFGCR2_PDIV_3:
 		p0 = 3;
 		break;
+	case DPLL_CFGCR2_PDIV_7 | (1 << DPLL_CFGCR2_PDIV_SHIFT):
+		/*
+		 * Incorrect ASUS-Z170M BIOS setting, the HW seems to ignore bit#0,
+		 * handling it the same way as PDIV_7.
+		 */
+		drm_err(&i915->drm, "Invalid WRPLL PDIV divider value, fixing it.\n");
+		p0 = 7;
+		break;
 	case DPLL_CFGCR2_PDIV_7:
 		p0 = 7;
 		break;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 47730a176698..f70e45bd3810 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10253,6 +10253,7 @@ enum skl_power_gate {
 #define  DPLL_CFGCR2_KDIV_3 (2 << 5)
 #define  DPLL_CFGCR2_KDIV_1 (3 << 5)
 #define  DPLL_CFGCR2_PDIV_MASK		(7 << 2)
+#define  DPLL_CFGCR2_PDIV_SHIFT		2
 #define  DPLL_CFGCR2_PDIV(x)		((x) << 2)
 #define  DPLL_CFGCR2_PDIV_1 (0 << 2)
 #define  DPLL_CFGCR2_PDIV_2 (1 << 2)
-- 
2.25.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/5] drm/i915/skl: Work around incorrect BIOS WRPLL PDIV programming

[Ville Syrjälä]

On Tue, Sep 29, 2020 at 03:29:25AM +0300, Imre Deak wrote:
> The BIOS of at least one ASUS-Z170M system with an SKL I have programs
> the 101b WRPLL PDIV divider value, which is the encoding for PDIV=7 with
> bit#0 incorrectly set.
> 
> This happens with the
> 
> "3840x2160": 30 262750 3840 3888 3920 4000 2160 2163 2168 2191 0x48 0x9
> 
> HDMI mode (scaled from a 1024x768 src fb) set by BIOS and the
> 
> ref_clock=24000, dco_integer=383, dco_fraction=5802, pdiv=7, qdiv=1, kdiv=1
> 
> WRPLL parameters (assuming PDIV=7 was the intended setting). This
> corresponds to 262749 PLL frequency/port clock.
> 
> Later the driver sets the same mode for which it calculates the same
> dco_int/dco_frac/div WRPLL parameters (with the correct PDIV=7 encoding).
> 
> Based on the above, let's assume that PDIV=7 was intended and the HW
> just ignores bit#0 in the PDIV register field for this setting, treating
> 100b and 101b encodings the same way.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 8 ++++++++
>  drivers/gpu/drm/i915/i915_reg.h               | 1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index e08684e34078..095b53fe3a21 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -1602,6 +1602,14 @@ static int skl_ddi_wrpll_get_freq(struct drm_i915_private *i915,
>  	case DPLL_CFGCR2_PDIV_3:
>  		p0 = 3;
>  		break;
> +	case DPLL_CFGCR2_PDIV_7 | (1 << DPLL_CFGCR2_PDIV_SHIFT):

Maybe we want a define for this?

> +		/*
> +		 * Incorrect ASUS-Z170M BIOS setting, the HW seems to ignore bit#0,
> +		 * handling it the same way as PDIV_7.
> +		 */
> +		drm_err(&i915->drm, "Invalid WRPLL PDIV divider value, fixing it.\n");

I wonder how many bug reports that will generate. Might want to make
it debug insteead.

> +		p0 = 7;
> +		break;

Or maybe fallthrough?

>  	case DPLL_CFGCR2_PDIV_7:
>  		p0 = 7;
>  		break;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 47730a176698..f70e45bd3810 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -10253,6 +10253,7 @@ enum skl_power_gate {
>  #define  DPLL_CFGCR2_KDIV_3 (2 << 5)
>  #define  DPLL_CFGCR2_KDIV_1 (3 << 5)
>  #define  DPLL_CFGCR2_PDIV_MASK		(7 << 2)
> +#define  DPLL_CFGCR2_PDIV_SHIFT		2
>  #define  DPLL_CFGCR2_PDIV(x)		((x) << 2)
>  #define  DPLL_CFGCR2_PDIV_1 (0 << 2)
>  #define  DPLL_CFGCR2_PDIV_2 (1 << 2)
> -- 
> 2.25.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/5] drm/i915/skl: Work around incorrect BIOS WRPLL PDIV programming

[Imre Deak]

On Thu, Oct 01, 2020 at 07:41:48PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 29, 2020 at 03:29:25AM +0300, Imre Deak wrote:
> > The BIOS of at least one ASUS-Z170M system with an SKL I have programs
> > the 101b WRPLL PDIV divider value, which is the encoding for PDIV=7 with
> > bit#0 incorrectly set.
> > 
> > This happens with the
> > 
> > "3840x2160": 30 262750 3840 3888 3920 4000 2160 2163 2168 2191 0x48 0x9
> > 
> > HDMI mode (scaled from a 1024x768 src fb) set by BIOS and the
> > 
> > ref_clock=24000, dco_integer=383, dco_fraction=5802, pdiv=7, qdiv=1, kdiv=1
> > 
> > WRPLL parameters (assuming PDIV=7 was the intended setting). This
> > corresponds to 262749 PLL frequency/port clock.
> > 
> > Later the driver sets the same mode for which it calculates the same
> > dco_int/dco_frac/div WRPLL parameters (with the correct PDIV=7 encoding).
> > 
> > Based on the above, let's assume that PDIV=7 was intended and the HW
> > just ignores bit#0 in the PDIV register field for this setting, treating
> > 100b and 101b encodings the same way.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 8 ++++++++
> >  drivers/gpu/drm/i915/i915_reg.h               | 1 +
> >  2 files changed, 9 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index e08684e34078..095b53fe3a21 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -1602,6 +1602,14 @@ static int skl_ddi_wrpll_get_freq(struct drm_i915_private *i915,
> >  	case DPLL_CFGCR2_PDIV_3:
> >  		p0 = 3;
> >  		break;
> > +	case DPLL_CFGCR2_PDIV_7 | (1 << DPLL_CFGCR2_PDIV_SHIFT):
> 
> Maybe we want a define for this?

Ok.

> 
> > +		/*
> > +		 * Incorrect ASUS-Z170M BIOS setting, the HW seems to ignore bit#0,
> > +		 * handling it the same way as PDIV_7.
> > +		 */
> > +		drm_err(&i915->drm, "Invalid WRPLL PDIV divider value, fixing it.\n");
> 
> I wonder how many bug reports that will generate. Might want to make
> it debug insteead.

I thought having reports for this is actually good, since BIOS vendors
should be notified then, but can change this to debug.

> 
> > +		p0 = 7;
> > +		break;
> 
> Or maybe fallthrough?

can do if you have an idea how to do that with the MISSING_CASE() added
later setting pdiv=7 by default.

> 
> >  	case DPLL_CFGCR2_PDIV_7:
> >  		p0 = 7;
> >  		break;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 47730a176698..f70e45bd3810 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -10253,6 +10253,7 @@ enum skl_power_gate {
> >  #define  DPLL_CFGCR2_KDIV_3 (2 << 5)
> >  #define  DPLL_CFGCR2_KDIV_1 (3 << 5)
> >  #define  DPLL_CFGCR2_PDIV_MASK		(7 << 2)
> > +#define  DPLL_CFGCR2_PDIV_SHIFT		2
> >  #define  DPLL_CFGCR2_PDIV(x)		((x) << 2)
> >  #define  DPLL_CFGCR2_PDIV_1 (0 << 2)
> >  #define  DPLL_CFGCR2_PDIV_2 (1 << 2)
> > -- 
> > 2.25.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/5] drm/i915: Factor out skl_wrpll_calc_freq()

[Imre Deak]

The WRPLL parameter -> frequency formula is the same for all platforms
starting with SKL. Factor out the helper for this for clarity and so
that we can use the same formula when selecting the ICL WRPLL dividers
for DP mode in an upcoming patch (to cross-check the frequency
calculated using the formula wrt. hard-coded freq values in the ICL DP
PLL tables).

While at it add the MISSING_CASE() for incorrect divider encodings.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 160 +++++++++++-------
 drivers/gpu/drm/i915/i915_reg.h               |   3 +
 2 files changed, 101 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 095b53fe3a21..e3370c8dccc8 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -1576,31 +1576,31 @@ static bool skl_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state)
 	return true;
 }
 
-static int skl_ddi_wrpll_get_freq(struct drm_i915_private *i915,
-				  const struct intel_shared_dpll *pll)
+static int skl_wrpll_calc_freq(int ref_clock,
+			       u32 dco_integer, u32 dco_fraction,
+			       u32 pdiv, u32 qdiv, u32 kdiv)
 {
-	const struct intel_dpll_hw_state *pll_state = &pll->state.hw_state;
-	int ref_clock = i915->dpll.ref_clks.nssc;
-	u32 p0, p1, p2, dco_freq;
+	u32 dco_freq;
 
-	p0 = pll_state->cfgcr2 & DPLL_CFGCR2_PDIV_MASK;
-	p2 = pll_state->cfgcr2 & DPLL_CFGCR2_KDIV_MASK;
+	dco_freq = ref_clock * dco_integer;
+	dco_freq += dco_fraction * ref_clock / 0x8000;
 
-	if (pll_state->cfgcr2 &  DPLL_CFGCR2_QDIV_MODE(1))
-		p1 = (pll_state->cfgcr2 & DPLL_CFGCR2_QDIV_RATIO_MASK) >> 8;
-	else
-		p1 = 1;
+	return dco_freq / (pdiv * qdiv * kdiv * 5);
+}
 
-
-	switch (p0) {
+static void skl_wrpll_decode_divs(struct drm_i915_private *i915,
+				  const struct skl_wrpll_params *wrpll_params,
+				  u32 *pdiv, u32 *qdiv, u32 *kdiv)
+{
+	switch (DPLL_CFGCR2_PDIV(wrpll_params->pdiv)) {
 	case DPLL_CFGCR2_PDIV_1:
-		p0 = 1;
+		*pdiv = 1;
 		break;
 	case DPLL_CFGCR2_PDIV_2:
-		p0 = 2;
+		*pdiv = 2;
 		break;
 	case DPLL_CFGCR2_PDIV_3:
-		p0 = 3;
+		*pdiv = 3;
 		break;
 	case DPLL_CFGCR2_PDIV_7 | (1 << DPLL_CFGCR2_PDIV_SHIFT):
 		/*
@@ -1608,38 +1608,63 @@ static int skl_ddi_wrpll_get_freq(struct drm_i915_private *i915,
 		 * handling it the same way as PDIV_7.
 		 */
 		drm_err(&i915->drm, "Invalid WRPLL PDIV divider value, fixing it.\n");
-		p0 = 7;
+		*pdiv = 7;
 		break;
+	default:
+		MISSING_CASE(wrpll_params->pdiv);
+		fallthrough;
 	case DPLL_CFGCR2_PDIV_7:
-		p0 = 7;
+		*pdiv = 7;
 		break;
 	}
 
-	switch (p2) {
+	*qdiv = wrpll_params->qdiv_mode ? wrpll_params->qdiv_ratio : 1;
+
+	switch (DPLL_CFGCR2_KDIV(wrpll_params->kdiv)) {
+	default:
+		MISSING_CASE(wrpll_params->kdiv);
+		fallthrough;
 	case DPLL_CFGCR2_KDIV_5:
-		p2 = 5;
+		*kdiv = 5;
 		break;
 	case DPLL_CFGCR2_KDIV_2:
-		p2 = 2;
+		*kdiv = 2;
 		break;
 	case DPLL_CFGCR2_KDIV_3:
-		p2 = 3;
+		*kdiv = 3;
 		break;
 	case DPLL_CFGCR2_KDIV_1:
-		p2 = 1;
+		*kdiv = 1;
 		break;
 	}
+}
 
-	dco_freq = (pll_state->cfgcr1 & DPLL_CFGCR1_DCO_INTEGER_MASK) *
-		   ref_clock;
+static int skl_ddi_wrpll_get_freq(struct drm_i915_private *i915,
+				  const struct intel_shared_dpll *pll)
+{
+	const struct intel_dpll_hw_state *pll_state = &pll->state.hw_state;
+	struct skl_wrpll_params wrpll_params = { };
+	int ref_clock = i915->dpll.ref_clks.nssc;
+	u32 pdiv;
+	u32 qdiv;
+	u32 kdiv;
+	u32 dco_integer;
+	u32 dco_fraction;
 
-	dco_freq += ((pll_state->cfgcr1 & DPLL_CFGCR1_DCO_FRACTION_MASK) >> 9) *
-		    ref_clock / 0x8000;
+	wrpll_params.pdiv = (pll_state->cfgcr2 & DPLL_CFGCR2_PDIV_MASK) >> DPLL_CFGCR2_PDIV_SHIFT;
+	wrpll_params.kdiv = (pll_state->cfgcr2 & DPLL_CFGCR2_KDIV_MASK) >> DPLL_CFGCR2_KDIV_SHIFT;
 
-	if (drm_WARN_ON(&i915->drm, p0 == 0 || p1 == 0 || p2 == 0))
-		return 0;
+	wrpll_params.qdiv_mode = !!(pll_state->cfgcr2 &  DPLL_CFGCR2_QDIV_MODE(1));
+	wrpll_params.qdiv_ratio = (pll_state->cfgcr2 & DPLL_CFGCR2_QDIV_RATIO_MASK) >>
+				  DPLL_CFGCR2_QDIV_RATIO_SHIFT;
 
-	return dco_freq / (p0 * p1 * p2 * 5);
+	skl_wrpll_decode_divs(i915, &wrpll_params, &pdiv, &qdiv, &kdiv);
+
+	dco_integer = pll_state->cfgcr1 & DPLL_CFGCR1_DCO_INTEGER_MASK;
+	dco_fraction = (pll_state->cfgcr1 & DPLL_CFGCR1_DCO_FRACTION_MASK) >>
+			DPLL_CFGCR1_DCO_FRACTION_SHIFT;
+
+	return skl_wrpll_calc_freq(ref_clock, dco_integer, dco_fraction, pdiv, qdiv, kdiv);
 }
 
 static bool
@@ -2630,60 +2655,71 @@ static bool cnl_ddi_hdmi_pll_dividers(struct intel_crtc_state *crtc_state)
 	return true;
 }
 
-static int __cnl_ddi_wrpll_get_freq(struct drm_i915_private *dev_priv,
-				    const struct intel_shared_dpll *pll,
-				    int ref_clock)
+static void cnl_wrpll_decode_divs(const struct skl_wrpll_params *wrpll_params,
+				  u32 *pdiv, u32 *qdiv, u32 *kdiv)
 {
-	const struct intel_dpll_hw_state *pll_state = &pll->state.hw_state;
-	u32 p0, p1, p2, dco_freq;
-
-	p0 = pll_state->cfgcr1 & DPLL_CFGCR1_PDIV_MASK;
-	p2 = pll_state->cfgcr1 & DPLL_CFGCR1_KDIV_MASK;
-
-	if (pll_state->cfgcr1 & DPLL_CFGCR1_QDIV_MODE(1))
-		p1 = (pll_state->cfgcr1 & DPLL_CFGCR1_QDIV_RATIO_MASK) >>
-			DPLL_CFGCR1_QDIV_RATIO_SHIFT;
-	else
-		p1 = 1;
-
-
-	switch (p0) {
+	switch (DPLL_CFGCR1_PDIV(wrpll_params->pdiv)) {
 	case DPLL_CFGCR1_PDIV_2:
-		p0 = 2;
+		*pdiv = 2;
 		break;
 	case DPLL_CFGCR1_PDIV_3:
-		p0 = 3;
+		*pdiv = 3;
 		break;
 	case DPLL_CFGCR1_PDIV_5:
-		p0 = 5;
+		*pdiv = 5;
 		break;
+	default:
+		MISSING_CASE(wrpll_params->pdiv);
+		fallthrough;
 	case DPLL_CFGCR1_PDIV_7:
-		p0 = 7;
+		*pdiv = 7;
 		break;
 	}
 
-	switch (p2) {
+	*qdiv = wrpll_params->qdiv_mode ? wrpll_params->qdiv_ratio : 1;
+
+	switch (DPLL_CFGCR1_KDIV(wrpll_params->kdiv)) {
 	case DPLL_CFGCR1_KDIV_1:
-		p2 = 1;
+		*kdiv = 1;
 		break;
 	case DPLL_CFGCR1_KDIV_2:
-		p2 = 2;
+		*kdiv = 2;
 		break;
+	default:
+		MISSING_CASE(wrpll_params->kdiv);
+		fallthrough;
 	case DPLL_CFGCR1_KDIV_3:
-		p2 = 3;
+		*kdiv = 3;
 		break;
 	}
+}
 
-	dco_freq = (pll_state->cfgcr0 & DPLL_CFGCR0_DCO_INTEGER_MASK) *
-		   ref_clock;
+static int __cnl_ddi_wrpll_get_freq(struct drm_i915_private *dev_priv,
+				    const struct intel_shared_dpll *pll,
+				    int ref_clock)
+{
+	const struct intel_dpll_hw_state *pll_state = &pll->state.hw_state;
+	struct skl_wrpll_params wrpll_params = { };
+	u32 pdiv;
+	u32 qdiv;
+	u32 kdiv;
+	u32 dco_integer;
+	u32 dco_fraction;
 
-	dco_freq += (((pll_state->cfgcr0 & DPLL_CFGCR0_DCO_FRACTION_MASK) >>
-		      DPLL_CFGCR0_DCO_FRACTION_SHIFT) * ref_clock) / 0x8000;
+	wrpll_params.pdiv = (pll_state->cfgcr1 & DPLL_CFGCR1_PDIV_MASK) >> DPLL_CFGCR1_PDIV_SHIFT;
+	wrpll_params.kdiv = (pll_state->cfgcr1 & DPLL_CFGCR1_KDIV_MASK) >> DPLL_CFGCR1_KDIV_SHIFT;
 
-	if (drm_WARN_ON(&dev_priv->drm, p0 == 0 || p1 == 0 || p2 == 0))
-		return 0;
+	wrpll_params.qdiv_mode = !!(pll_state->cfgcr1 & DPLL_CFGCR1_QDIV_MODE(1));
+	wrpll_params.qdiv_ratio = (pll_state->cfgcr1 & DPLL_CFGCR1_QDIV_RATIO_MASK) >>
+				  DPLL_CFGCR1_QDIV_RATIO_SHIFT;
 
-	return dco_freq / (p0 * p1 * p2 * 5);
+	cnl_wrpll_decode_divs(&wrpll_params, &pdiv, &qdiv, &kdiv);
+
+	dco_integer = pll_state->cfgcr0 & DPLL_CFGCR0_DCO_INTEGER_MASK;
+	dco_fraction = (pll_state->cfgcr0 & DPLL_CFGCR0_DCO_FRACTION_MASK) >>
+		       DPLL_CFGCR0_DCO_FRACTION_SHIFT;
+
+	return skl_wrpll_calc_freq(ref_clock, dco_integer, dco_fraction, pdiv, qdiv, kdiv);
 }
 
 static int cnl_ddi_wrpll_get_freq(struct drm_i915_private *i915,
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f70e45bd3810..4409c712030c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10237,6 +10237,7 @@ enum skl_power_gate {
 #define _DPLL3_CFGCR1	0x6C050
 #define  DPLL_CFGCR1_FREQ_ENABLE	(1 << 31)
 #define  DPLL_CFGCR1_DCO_FRACTION_MASK	(0x7fff << 9)
+#define  DPLL_CFGCR1_DCO_FRACTION_SHIFT	9
 #define  DPLL_CFGCR1_DCO_FRACTION(x)	((x) << 9)
 #define  DPLL_CFGCR1_DCO_INTEGER_MASK	(0x1ff)
 
@@ -10244,9 +10245,11 @@ enum skl_power_gate {
 #define _DPLL2_CFGCR2	0x6C04C
 #define _DPLL3_CFGCR2	0x6C054
 #define  DPLL_CFGCR2_QDIV_RATIO_MASK	(0xff << 8)
+#define  DPLL_CFGCR2_QDIV_RATIO_SHIFT	8
 #define  DPLL_CFGCR2_QDIV_RATIO(x)	((x) << 8)
 #define  DPLL_CFGCR2_QDIV_MODE(x)	((x) << 7)
 #define  DPLL_CFGCR2_KDIV_MASK		(3 << 5)
+#define  DPLL_CFGCR2_KDIV_SHIFT		5
 #define  DPLL_CFGCR2_KDIV(x)		((x) << 5)
 #define  DPLL_CFGCR2_KDIV_5 (0 << 5)
 #define  DPLL_CFGCR2_KDIV_2 (1 << 5)
-- 
2.25.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/5] drm/i915/icl: Cross check the combo PLL WRPLL parameters wrt. hard-coded PLL freqs

[Imre Deak]

When selecting the WRPLL dividers for a given port clock/PLL freq, the
hard-coded PLL freq in a table entry can be calculated using the rest of
parameters in the same entry. Cross-check if the hard coded values match
what we calculate with the formula.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 26 ++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index e3370c8dccc8..ded2b2dfe319 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3002,6 +3002,30 @@ static const struct skl_wrpll_params tgl_tbt_pll_38_4MHz_values = {
 	.pdiv = 0, .kdiv = 0, .qdiv_mode = 0, .qdiv_ratio = 0,
 };
 
+static int icl_wrpll_ref_clock(struct drm_i915_private *i915);
+
+static bool icl_dp_combo_pll_clock_match(struct drm_i915_private *i915, int clock,
+					 const struct icl_combo_pll_params *p)
+{
+	int ref_clock = icl_wrpll_ref_clock(i915);
+	int pll_freq;
+	u32 pdiv;
+	u32 qdiv;
+	u32 kdiv;
+
+	cnl_wrpll_decode_divs(&p->wrpll, &pdiv, &qdiv, &kdiv);
+
+	pll_freq = skl_wrpll_calc_freq(ref_clock,
+				       p->wrpll.dco_integer, p->wrpll.dco_fraction,
+				       pdiv, qdiv, kdiv);
+	drm_WARN_ON(&i915->drm, pll_freq != p->clock);
+
+	if (clock == pll_freq)
+		return true;
+
+	return false;
+}
+
 static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
 				  struct skl_wrpll_params *pll_params)
 {
@@ -3014,7 +3038,7 @@ static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(icl_dp_combo_pll_24MHz_values); i++) {
-		if (clock == params[i].clock) {
+		if (icl_dp_combo_pll_clock_match(dev_priv, clock, &params[i])) {
 			*pll_params = params[i].wrpll;
 			return true;
 		}
-- 
2.25.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/5] drm/i915/icl: Cross check the combo PLL WRPLL parameters wrt. hard-coded PLL freqs

[Ville Syrjälä]

On Tue, Sep 29, 2020 at 03:29:27AM +0300, Imre Deak wrote:
> When selecting the WRPLL dividers for a given port clock/PLL freq, the
> hard-coded PLL freq in a table entry can be calculated using the rest of
> parameters in the same entry. Cross-check if the hard coded values match
> what we calculate with the formula.

We've never done this on any other plaform I think. Why is this special?
Also, shouldn't the state checker catch this anyway?

> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 26 ++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index e3370c8dccc8..ded2b2dfe319 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -3002,6 +3002,30 @@ static const struct skl_wrpll_params tgl_tbt_pll_38_4MHz_values = {
>  	.pdiv = 0, .kdiv = 0, .qdiv_mode = 0, .qdiv_ratio = 0,
>  };
>  
> +static int icl_wrpll_ref_clock(struct drm_i915_private *i915);
> +
> +static bool icl_dp_combo_pll_clock_match(struct drm_i915_private *i915, int clock,
> +					 const struct icl_combo_pll_params *p)
> +{
> +	int ref_clock = icl_wrpll_ref_clock(i915);
> +	int pll_freq;
> +	u32 pdiv;
> +	u32 qdiv;
> +	u32 kdiv;
> +
> +	cnl_wrpll_decode_divs(&p->wrpll, &pdiv, &qdiv, &kdiv);
> +
> +	pll_freq = skl_wrpll_calc_freq(ref_clock,
> +				       p->wrpll.dco_integer, p->wrpll.dco_fraction,
> +				       pdiv, qdiv, kdiv);
> +	drm_WARN_ON(&i915->drm, pll_freq != p->clock);
> +
> +	if (clock == pll_freq)
> +		return true;
> +
> +	return false;
> +}
> +
>  static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
>  				  struct skl_wrpll_params *pll_params)
>  {
> @@ -3014,7 +3038,7 @@ static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(icl_dp_combo_pll_24MHz_values); i++) {
> -		if (clock == params[i].clock) {
> +		if (icl_dp_combo_pll_clock_match(dev_priv, clock, &params[i])) {
>  			*pll_params = params[i].wrpll;
>  			return true;
>  		}
> -- 
> 2.25.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/5] drm/i915/icl: Cross check the combo PLL WRPLL parameters wrt. hard-coded PLL freqs

[Ville Syrjälä]

On Thu, Oct 01, 2020 at 07:44:29PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 29, 2020 at 03:29:27AM +0300, Imre Deak wrote:
> > When selecting the WRPLL dividers for a given port clock/PLL freq, the
> > hard-coded PLL freq in a table entry can be calculated using the rest of
> > parameters in the same entry. Cross-check if the hard coded values match
> > what we calculate with the formula.
> 
> We've never done this on any other plaform I think. Why is this special?

I think if we do this we should do at init time for all such tables.
Ideally we'd do it at compile time but no constexpr in C :(

> Also, shouldn't the state checker catch this anyway?
> 
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 26 ++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index e3370c8dccc8..ded2b2dfe319 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -3002,6 +3002,30 @@ static const struct skl_wrpll_params tgl_tbt_pll_38_4MHz_values = {
> >  	.pdiv = 0, .kdiv = 0, .qdiv_mode = 0, .qdiv_ratio = 0,
> >  };
> >  
> > +static int icl_wrpll_ref_clock(struct drm_i915_private *i915);
> > +
> > +static bool icl_dp_combo_pll_clock_match(struct drm_i915_private *i915, int clock,
> > +					 const struct icl_combo_pll_params *p)
> > +{
> > +	int ref_clock = icl_wrpll_ref_clock(i915);
> > +	int pll_freq;
> > +	u32 pdiv;
> > +	u32 qdiv;
> > +	u32 kdiv;
> > +
> > +	cnl_wrpll_decode_divs(&p->wrpll, &pdiv, &qdiv, &kdiv);
> > +
> > +	pll_freq = skl_wrpll_calc_freq(ref_clock,
> > +				       p->wrpll.dco_integer, p->wrpll.dco_fraction,
> > +				       pdiv, qdiv, kdiv);
> > +	drm_WARN_ON(&i915->drm, pll_freq != p->clock);
> > +
> > +	if (clock == pll_freq)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> >  static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
> >  				  struct skl_wrpll_params *pll_params)
> >  {
> > @@ -3014,7 +3038,7 @@ static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
> >  	int i;
> >  
> >  	for (i = 0; i < ARRAY_SIZE(icl_dp_combo_pll_24MHz_values); i++) {
> > -		if (clock == params[i].clock) {
> > +		if (icl_dp_combo_pll_clock_match(dev_priv, clock, &params[i])) {
> >  			*pll_params = params[i].wrpll;
> >  			return true;
> >  		}
> > -- 
> > 2.25.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/5] drm/i915/icl: Cross check the combo PLL WRPLL parameters wrt. hard-coded PLL freqs

[Imre Deak]

On Thu, Oct 01, 2020 at 07:44:29PM +0300, Ville Syrjälä wrote:
> On Tue, Sep 29, 2020 at 03:29:27AM +0300, Imre Deak wrote:
> > When selecting the WRPLL dividers for a given port clock/PLL freq, the
> > hard-coded PLL freq in a table entry can be calculated using the rest of
> > parameters in the same entry. Cross-check if the hard coded values match
> > what we calculate with the formula.
> 
> We've never done this on any other plaform I think. Why is this special?

clock in icl_combo_pll_params is already defined by WRPLL params in the
same entry along with refclock. The driver needs to calculate already
this same clock when reading out the PLL HW state, so I thought it makes
sense to determine clock from WRPLL params when looking up an entry from
the PLL params table.

It's also used by the last patch in the patchset that needs to calculate
the clock both with the fractional divider WA applied and not applied.

> Also, shouldn't the state checker catch this anyway?

Afaics the PLL state verification only checks if the calculated /
programmed WRPLL parameters match what we read out.  But the point in
this patch was only to make the table lookup and the clock calculation
during HW readout uniform.

> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 26 ++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > index e3370c8dccc8..ded2b2dfe319 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> > @@ -3002,6 +3002,30 @@ static const struct skl_wrpll_params tgl_tbt_pll_38_4MHz_values = {
> >  	.pdiv = 0, .kdiv = 0, .qdiv_mode = 0, .qdiv_ratio = 0,
> >  };
> >  
> > +static int icl_wrpll_ref_clock(struct drm_i915_private *i915);
> > +
> > +static bool icl_dp_combo_pll_clock_match(struct drm_i915_private *i915, int clock,
> > +					 const struct icl_combo_pll_params *p)
> > +{
> > +	int ref_clock = icl_wrpll_ref_clock(i915);
> > +	int pll_freq;
> > +	u32 pdiv;
> > +	u32 qdiv;
> > +	u32 kdiv;
> > +
> > +	cnl_wrpll_decode_divs(&p->wrpll, &pdiv, &qdiv, &kdiv);
> > +
> > +	pll_freq = skl_wrpll_calc_freq(ref_clock,
> > +				       p->wrpll.dco_integer, p->wrpll.dco_fraction,
> > +				       pdiv, qdiv, kdiv);
> > +	drm_WARN_ON(&i915->drm, pll_freq != p->clock);
> > +
> > +	if (clock == pll_freq)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> >  static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
> >  				  struct skl_wrpll_params *pll_params)
> >  {
> > @@ -3014,7 +3038,7 @@ static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
> >  	int i;
> >  
> >  	for (i = 0; i < ARRAY_SIZE(icl_dp_combo_pll_24MHz_values); i++) {
> > -		if (clock == params[i].clock) {
> > +		if (icl_dp_combo_pll_clock_match(dev_priv, clock, &params[i])) {
> >  			*pll_params = params[i].wrpll;
> >  			return true;
> >  		}
> > -- 
> > 2.25.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 4/5] drm/i915/tgl: Fix Combo PHY DPLL fractional divider for 38.4MHz ref clock

[Imre Deak]

Apply Display WA #22010492432 for combo PHY PLLs too. This should fix a
problem where the PLL output frequency is slightly off with the current
PLL fractional divider value.

I haven't seen an actual case where this causes a problem, but let's
follow the spec. It's also needed on some EHL platforms, but for that we
also need a way to distinguish the affected EHL SKUs, so I leave that
for a follow-up.

v2:
- Apply the WA at one place when calculating the PLL dividers from the
  frequency and the frequency from the dividers for all the combo PLL
  use cases (DP, HDMI, TBT). (Ville)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 34 +++++++++++--------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index ded2b2dfe319..e7b058340a1a 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -2694,6 +2694,16 @@ static void cnl_wrpll_decode_divs(const struct skl_wrpll_params *wrpll_params,
 	}
 }
 
+/*
+ * Display WA #22010492432: tgl
+ * Program half of the nominal DCO divider fraction value.
+ */
+static bool
+tgl_combo_pll_div_frac_wa_needed(struct drm_i915_private *i915)
+{
+	return IS_TIGERLAKE(i915) && i915->dpll.ref_clks.nssc == 38400;
+}
+
 static int __cnl_ddi_wrpll_get_freq(struct drm_i915_private *dev_priv,
 				    const struct intel_shared_dpll *pll,
 				    int ref_clock)
@@ -2719,6 +2729,9 @@ static int __cnl_ddi_wrpll_get_freq(struct drm_i915_private *dev_priv,
 	dco_fraction = (pll_state->cfgcr0 & DPLL_CFGCR0_DCO_FRACTION_MASK) >>
 		       DPLL_CFGCR0_DCO_FRACTION_SHIFT;
 
+	if (tgl_combo_pll_div_frac_wa_needed(dev_priv))
+		dco_fraction *= 2;
+
 	return skl_wrpll_calc_freq(ref_clock, dco_integer, dco_fraction, pdiv, qdiv, kdiv);
 }
 
@@ -2992,16 +3005,6 @@ static const struct skl_wrpll_params tgl_tbt_pll_24MHz_values = {
 	/* the following params are unused */
 };
 
-/*
- * Display WA #22010492432: tgl
- * Divide the nominal .dco_fraction value by 2.
- */
-static const struct skl_wrpll_params tgl_tbt_pll_38_4MHz_values = {
-	.dco_integer = 0x54, .dco_fraction = 0x1800,
-	/* the following params are unused */
-	.pdiv = 0, .kdiv = 0, .qdiv_mode = 0, .qdiv_ratio = 0,
-};
-
 static int icl_wrpll_ref_clock(struct drm_i915_private *i915);
 
 static bool icl_dp_combo_pll_clock_match(struct drm_i915_private *i915, int clock,
@@ -3059,14 +3062,12 @@ static bool icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
 			MISSING_CASE(dev_priv->dpll.ref_clks.nssc);
 			fallthrough;
 		case 19200:
+		case 38400:
 			*pll_params = tgl_tbt_pll_19_2MHz_values;
 			break;
 		case 24000:
 			*pll_params = tgl_tbt_pll_24MHz_values;
 			break;
-		case 38400:
-			*pll_params = tgl_tbt_pll_38_4MHz_values;
-			break;
 		}
 	} else {
 		switch (dev_priv->dpll.ref_clks.nssc) {
@@ -3133,9 +3134,14 @@ static void icl_calc_dpll_state(struct drm_i915_private *i915,
 				const struct skl_wrpll_params *pll_params,
 				struct intel_dpll_hw_state *pll_state)
 {
+	u32 dco_fraction = pll_params->dco_fraction;
+
 	memset(pll_state, 0, sizeof(*pll_state));
 
-	pll_state->cfgcr0 = DPLL_CFGCR0_DCO_FRACTION(pll_params->dco_fraction) |
+	if (tgl_combo_pll_div_frac_wa_needed(i915))
+		dco_fraction = DIV_ROUND_CLOSEST(dco_fraction, 2);
+
+	pll_state->cfgcr0 = DPLL_CFGCR0_DCO_FRACTION(dco_fraction) |
 			    pll_params->dco_integer;
 
 	pll_state->cfgcr1 = DPLL_CFGCR1_QDIV_RATIO(pll_params->qdiv_ratio) |
-- 
2.25.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/5] drm/i915/tgl: Fix Combo PHY DPLL fractional divider for 38.4MHz ref clock

[Ville Syrjälä]

On Tue, Sep 29, 2020 at 03:29:28AM +0300, Imre Deak wrote:
> Apply Display WA #22010492432 for combo PHY PLLs too. This should fix a
> problem where the PLL output frequency is slightly off with the current
> PLL fractional divider value.
> 
> I haven't seen an actual case where this causes a problem, but let's
> follow the spec. It's also needed on some EHL platforms, but for that we
> also need a way to distinguish the affected EHL SKUs, so I leave that
> for a follow-up.
> 
> v2:
> - Apply the WA at one place when calculating the PLL dividers from the
>   frequency and the frequency from the dividers for all the combo PLL
>   use cases (DP, HDMI, TBT). (Ville)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 34 +++++++++++--------
>  1 file changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index ded2b2dfe319..e7b058340a1a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -2694,6 +2694,16 @@ static void cnl_wrpll_decode_divs(const struct skl_wrpll_params *wrpll_params,
>  	}
>  }
>  
> +/*
> + * Display WA #22010492432: tgl
> + * Program half of the nominal DCO divider fraction value.
> + */
> +static bool
> +tgl_combo_pll_div_frac_wa_needed(struct drm_i915_private *i915)
> +{
> +	return IS_TIGERLAKE(i915) && i915->dpll.ref_clks.nssc == 38400;
> +}
> +
>  static int __cnl_ddi_wrpll_get_freq(struct drm_i915_private *dev_priv,
>  				    const struct intel_shared_dpll *pll,
>  				    int ref_clock)
> @@ -2719,6 +2729,9 @@ static int __cnl_ddi_wrpll_get_freq(struct drm_i915_private *dev_priv,
>  	dco_fraction = (pll_state->cfgcr0 & DPLL_CFGCR0_DCO_FRACTION_MASK) >>
>  		       DPLL_CFGCR0_DCO_FRACTION_SHIFT;
>  
> +	if (tgl_combo_pll_div_frac_wa_needed(dev_priv))
> +		dco_fraction *= 2;
> +
>  	return skl_wrpll_calc_freq(ref_clock, dco_integer, dco_fraction, pdiv, qdiv, kdiv);
>  }
>  
> @@ -2992,16 +3005,6 @@ static const struct skl_wrpll_params tgl_tbt_pll_24MHz_values = {
>  	/* the following params are unused */
>  };
>  
> -/*
> - * Display WA #22010492432: tgl
> - * Divide the nominal .dco_fraction value by 2.
> - */
> -static const struct skl_wrpll_params tgl_tbt_pll_38_4MHz_values = {
> -	.dco_integer = 0x54, .dco_fraction = 0x1800,
> -	/* the following params are unused */
> -	.pdiv = 0, .kdiv = 0, .qdiv_mode = 0, .qdiv_ratio = 0,
> -};
> -
>  static int icl_wrpll_ref_clock(struct drm_i915_private *i915);
>  
>  static bool icl_dp_combo_pll_clock_match(struct drm_i915_private *i915, int clock,
> @@ -3059,14 +3062,12 @@ static bool icl_calc_tbt_pll(struct intel_crtc_state *crtc_state,
>  			MISSING_CASE(dev_priv->dpll.ref_clks.nssc);
>  			fallthrough;
>  		case 19200:
> +		case 38400:
>  			*pll_params = tgl_tbt_pll_19_2MHz_values;
>  			break;
>  		case 24000:
>  			*pll_params = tgl_tbt_pll_24MHz_values;
>  			break;
> -		case 38400:
> -			*pll_params = tgl_tbt_pll_38_4MHz_values;
> -			break;
>  		}
>  	} else {
>  		switch (dev_priv->dpll.ref_clks.nssc) {
> @@ -3133,9 +3134,14 @@ static void icl_calc_dpll_state(struct drm_i915_private *i915,
>  				const struct skl_wrpll_params *pll_params,
>  				struct intel_dpll_hw_state *pll_state)
>  {
> +	u32 dco_fraction = pll_params->dco_fraction;
> +
>  	memset(pll_state, 0, sizeof(*pll_state));
>  
> -	pll_state->cfgcr0 = DPLL_CFGCR0_DCO_FRACTION(pll_params->dco_fraction) |
> +	if (tgl_combo_pll_div_frac_wa_needed(i915))
> +		dco_fraction = DIV_ROUND_CLOSEST(dco_fraction, 2);
> +
> +	pll_state->cfgcr0 = DPLL_CFGCR0_DCO_FRACTION(dco_fraction) |
>  			    pll_params->dco_integer;
>  
>  	pll_state->cfgcr1 = DPLL_CFGCR1_QDIV_RATIO(pll_params->qdiv_ratio) |
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 5/5] drm/i915/tgl: Add workaround for incorrect BIOS combo PHY DPLL programming

[Imre Deak]

The TGL A stepping and some B stepping (display C stepping) BIOSes
program the combo PHY DPLL fractional divider value incorrectly, not
applying the Display #22010492432 workaround.

Add a workaround for such BIOS versions, so that the driver selects the
correct WRPLL parameter entry correctly (selecting the same entry from
the table whether or not the fractional divider was adjusted or not by
BIOS according to #22010492432).

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index e7b058340a1a..7968ceb23ab6 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3026,7 +3026,15 @@ static bool icl_dp_combo_pll_clock_match(struct drm_i915_private *i915, int cloc
 	if (clock == pll_freq)
 		return true;
 
-	return false;
+	if (!tgl_combo_pll_div_frac_wa_needed(i915) ||
+	    !IS_TGL_DISP_REVID(i915, TGL_REVID_A0, TGL_REVID_C0))
+		return false;
+
+	pll_freq = skl_wrpll_calc_freq(ref_clock,
+				       p->wrpll.dco_integer, p->wrpll.dco_fraction * 2,
+				       pdiv, qdiv, kdiv);
+
+	return clock == pll_freq;
 }
 
 static bool icl_calc_dp_combo_pll(struct intel_crtc_state *crtc_state,
-- 
2.25.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/tgl: Fix Combo PHY DPLL fractional divider for 38.4MHz ref clock

[Patchwork]

== Series Details ==

Series: drm/i915/tgl: Fix Combo PHY DPLL fractional divider for 38.4MHz ref clock
URL   : https://patchwork.freedesktop.org/series/82173/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/gt/intel_reset.c:1311:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/gvt/mmio.c:290:23: warning: memcpy with byte count of 279040
+drivers/gpu/drm/i915/i915_perf.c:1440:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1494:15: warning: memset with byte count of 16777216
+./include/linux/seqlock.h:752:24: warning: trying to copy expression type 31
+./include/linux/seqlock.h:778:16: warning: trying to copy expression type 31
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/tgl: Fix Combo PHY DPLL fractional divider for 38.4MHz ref clock

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 6719 bytes --]

== Series Details ==

Series: drm/i915/tgl: Fix Combo PHY DPLL fractional divider for 38.4MHz ref clock
URL   : https://patchwork.freedesktop.org/series/82173/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9068 -> Patchwork_18586
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/index.html

Known issues
------------

  Here are the changes found in Patchwork_18586 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_tiled_pread_basic:
    - fi-bsw-n3050:       [PASS][1] -> [DMESG-WARN][2] ([i915#1982])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9068/fi-bsw-n3050/igt@gem_tiled_pread_basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/fi-bsw-n3050/igt@gem_tiled_pread_basic.html

  * igt@kms_busy@basic@flip:
    - fi-kbl-x1275:       [PASS][3] -> [DMESG-WARN][4] ([i915#62] / [i915#92] / [i915#95])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9068/fi-kbl-x1275/igt@kms_busy@basic@flip.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/fi-kbl-x1275/igt@kms_busy@basic@flip.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-atomic:
    - fi-icl-u2:          [PASS][5] -> [DMESG-WARN][6] ([i915#1982])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9068/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2:
    - fi-skl-guc:         [PASS][7] -> [DMESG-WARN][8] ([i915#2203])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9068/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2.html

  
#### Possible fixes ####

  * {igt@core_hotunplug@unbind-rebind}:
    - fi-kbl-x1275:       [DMESG-WARN][9] ([i915#62] / [i915#92] / [i915#95]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9068/fi-kbl-x1275/igt@core_hotunplug@unbind-rebind.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/fi-kbl-x1275/igt@core_hotunplug@unbind-rebind.html

  * igt@kms_flip@basic-flip-vs-dpms@d-dsi1:
    - {fi-tgl-dsi}:       [DMESG-WARN][11] ([i915#1982]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9068/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-dpms@d-dsi1.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-dpms@d-dsi1.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1:
    - fi-icl-u2:          [DMESG-WARN][13] ([i915#1982]) -> [PASS][14] +1 similar issue
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9068/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html

  
#### Warnings ####

  * igt@gem_exec_suspend@basic-s0:
    - fi-kbl-x1275:       [DMESG-WARN][15] ([i915#62] / [i915#92]) -> [DMESG-WARN][16] ([i915#62] / [i915#92] / [i915#95]) +3 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9068/fi-kbl-x1275/igt@gem_exec_suspend@basic-s0.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/fi-kbl-x1275/igt@gem_exec_suspend@basic-s0.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-kbl-x1275:       [DMESG-WARN][17] ([i915#62] / [i915#92]) -> [DMESG-WARN][18] ([i915#1982] / [i915#62] / [i915#92] / [i915#95])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9068/fi-kbl-x1275/igt@gem_exec_suspend@basic-s3.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/fi-kbl-x1275/igt@gem_exec_suspend@basic-s3.html

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-x1275:       [DMESG-FAIL][19] ([i915#62] / [i915#95]) -> [DMESG-FAIL][20] ([i915#62])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9068/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - fi-kbl-x1275:       [DMESG-WARN][21] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][22] ([i915#62] / [i915#92]) +3 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9068/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  * igt@vgem_basic@unload:
    - fi-kbl-x1275:       [DMESG-WARN][23] ([i915#95]) -> [DMESG-WARN][24] ([i915#62] / [i915#92])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9068/fi-kbl-x1275/igt@vgem_basic@unload.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/fi-kbl-x1275/igt@vgem_basic@unload.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (46 -> 39)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_9068 -> Patchwork_18586

  CI-20190529: 20190529
  CI_DRM_9068: a41d308b2f54b33a4a90011f4fe16dc5ef7e49a1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5790: 722a3eb9734f04030508d244df9dff55c5ab686c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18586: 6520c445f2d425ceebd3b7e6e2eb3b3aa0d4c892 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6520c445f2d4 drm/i915/tgl: Add workaround for incorrect BIOS combo PHY DPLL programming
a6fb67e5b7e2 drm/i915/tgl: Fix Combo PHY DPLL fractional divider for 38.4MHz ref clock
4be8359615d9 drm/i915/icl: Cross check the combo PLL WRPLL parameters wrt. hard-coded PLL freqs
a9de3d654f6f drm/i915: Factor out skl_wrpll_calc_freq()
885b445fa0a6 drm/i915/skl: Work around incorrect BIOS WRPLL PDIV programming

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18586/index.html

[-- Attachment #1.2: Type: text/html, Size: 9454 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx