[Intel-gfx] [PATCH] drm/i915/display/dg1: Correctly map DPLLs during state readout

[Lucas De Marchi]

On Wed, Jun 30, 2021 at 02:05:22PM -0700, Jose Souza wrote:
>_DG1_DPCLKA0_CFGCR0 maps between DPLL 0 and 1 with one bit for phy A
>and B while _DG1_DPCLKA1_CFGCR0 maps between DPLL 2 and 3 with one
>bit for phy C and D.
>
>Reusing _cnl_ddi_get_pll() don't take that into cosideration returing
>DPLL 0 and 1 for phy C and D.
>
>That is a regression introduced in the refactor done in commit
>c9fdceaa64dc ("drm/i915: Move DDI clock readout to encoder->get_config()").
>While at it also dropping the macros previously used, not reusing it
>to improve readability.
>
>BSpec: 50286
>Fixes: c9fdceaa64dc ("drm/i915: Move DDI clock readout to encoder->get_config()")
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>Signed-off-by: José Roberto de Souza <jose.souza at intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

thanks
Lucas De Marchi

>---
> drivers/gpu/drm/i915/display/intel_ddi.c | 19 ++++++++++++++++---
> drivers/gpu/drm/i915/i915_reg.h          |  3 ---
> 2 files changed, 16 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>index 91fd85bee1d27..26a3aa73fcc43 100644
>--- a/drivers/gpu/drm/i915/display/intel_ddi.c
>+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>@@ -1738,10 +1738,23 @@ static struct intel_shared_dpll *dg1_ddi_get_pll(struct intel_encoder *encoder)
> {
> 	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> 	enum phy phy = intel_port_to_phy(i915, encoder->port);
>+	enum intel_dpll_id id;
>+	u32 val;
>
>-	return _cnl_ddi_get_pll(i915, DG1_DPCLKA_CFGCR0(phy),
>-				DG1_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy),
>-				DG1_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy));
>+	val = intel_de_read(i915, DG1_DPCLKA_CFGCR0(phy));
>+	val &= DG1_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
>+	val >>= DG1_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy);
>+	id = val;
>+
>+	/*
>+	 * _DG1_DPCLKA0_CFGCR0 maps between DPLL 0 and 1 with one bit for phy A
>+	 * and B while _DG1_DPCLKA1_CFGCR0 maps between DPLL 2 and 3 with one
>+	 * bit for phy C and D.
>+	 */
>+	if (phy >= PHY_C)
>+		id += DPLL_ID_DG1_DPLL2;
>+
>+	return intel_get_shared_dpll_by_id(i915, id);
> }
>
> static void icl_ddi_combo_enable_clock(struct intel_encoder *encoder,
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>index 65c155b141899..16a19239d86dd 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -10525,7 +10525,6 @@ enum skl_power_gate {
> #define _DG1_DPCLKA1_CFGCR0				0x16C280
> #define _DG1_DPCLKA_PHY_IDX(phy)			((phy) % 2)
> #define _DG1_DPCLKA_PLL_IDX(pll)			((pll) % 2)
>-#define _DG1_PHY_DPLL_MAP(phy)				((phy) >= PHY_C ? DPLL_ID_DG1_DPLL2 : DPLL_ID_DG1_DPLL0)
> #define DG1_DPCLKA_CFGCR0(phy)				_MMIO_PHY((phy) / 2, \
> 								  _DG1_DPCLKA_CFGCR0, \
> 								  _DG1_DPCLKA1_CFGCR0)
>@@ -10533,8 +10532,6 @@ enum skl_power_gate {
> #define   DG1_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy)	(_DG1_DPCLKA_PHY_IDX(phy) * 2)
> #define   DG1_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy)	(_DG1_DPCLKA_PLL_IDX(pll) << DG1_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
> #define   DG1_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy)	(0x3 << DG1_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
>-#define   DG1_DPCLKA_CFGCR0_DDI_CLK_SEL_DPLL_MAP(clk_sel, phy) \
>-	(((clk_sel) >> DG1_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy)) + _DG1_PHY_DPLL_MAP(phy))
>
> /* ADLS Clocks */
> #define _ADLS_DPCLKA_CFGCR0			0x164280
>-- 
>2.32.0
>