Re: [PATCH 04/14] drm/i915/dpio: Fix VLV DPIO PLL register dword numbering

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 22 Apr 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The spreadsheet defines the PLL register block as having
> the dwords in the following order:
>
> block   dwords    offsets
> PLL1    0x0-0x7   0x00-0x1f
> PLL2    0x0-0x7   0x20-0x2f
> PLL1ext 0x10-0x1f 0x40-0x5f
> PLL2ext 0x10-0x1f 0x60-0x7f
>
> So dword indexes 0x8-0xf don't even exist. Renumber
> our register defines to match.
>
> Note that the spreadsheet used hex numbering whereas our
> defiens are in decimal. Perhaps we should change that?
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I am, again, taking your word for it, instead of going on a wild goose
chase trying to find all the specs. The patch matches the commit
message,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  drivers/gpu/drm/i915/display/intel_dpll.c | 18 ++++++++---------
>  drivers/gpu/drm/i915/i915_reg.h           | 24 +++++++++++------------
>  2 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll.c b/drivers/gpu/drm/i915/display/intel_dpll.c
> index 7e8aca3c87ec..b95032651da0 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll.c
> @@ -1875,19 +1875,19 @@ static void vlv_pllb_recal_opamp(struct drm_i915_private *dev_priv,
>  	 * PLLB opamp always calibrates to max value of 0x3f, force enable it
>  	 * and set it to a reasonable value instead.
>  	 */
> -	reg_val = vlv_dpio_read(dev_priv, phy, VLV_PLL_DW9(1));
> +	reg_val = vlv_dpio_read(dev_priv, phy, VLV_PLL_DW17(1));
>  	reg_val &= 0xffffff00;
>  	reg_val |= 0x00000030;
> -	vlv_dpio_write(dev_priv, phy, VLV_PLL_DW9(1), reg_val);
> +	vlv_dpio_write(dev_priv, phy, VLV_PLL_DW17(1), reg_val);
>  
>  	reg_val = vlv_dpio_read(dev_priv, phy, VLV_REF_DW11);
>  	reg_val &= 0x00ffffff;
>  	reg_val |= 0x8c000000;
>  	vlv_dpio_write(dev_priv, phy, VLV_REF_DW11, reg_val);
>  
> -	reg_val = vlv_dpio_read(dev_priv, phy, VLV_PLL_DW9(1));
> +	reg_val = vlv_dpio_read(dev_priv, phy, VLV_PLL_DW17(1));
>  	reg_val &= 0xffffff00;
> -	vlv_dpio_write(dev_priv, phy, VLV_PLL_DW9(1), reg_val);
> +	vlv_dpio_write(dev_priv, phy, VLV_PLL_DW17(1), reg_val);
>  
>  	reg_val = vlv_dpio_read(dev_priv, phy, VLV_REF_DW11);
>  	reg_val &= 0x00ffffff;
> @@ -1923,9 +1923,9 @@ static void vlv_prepare_pll(const struct intel_crtc_state *crtc_state)
>  	vlv_dpio_write(dev_priv, phy, VLV_PCS_DW17_BCAST, 0x0100000f);
>  
>  	/* Disable target IRef on PLL */
> -	reg_val = vlv_dpio_read(dev_priv, phy, VLV_PLL_DW8(pipe));
> +	reg_val = vlv_dpio_read(dev_priv, phy, VLV_PLL_DW16(pipe));
>  	reg_val &= 0x00ffffff;
> -	vlv_dpio_write(dev_priv, phy, VLV_PLL_DW8(pipe), reg_val);
> +	vlv_dpio_write(dev_priv, phy, VLV_PLL_DW16(pipe), reg_val);
>  
>  	/* Disable fast lock */
>  	vlv_dpio_write(dev_priv, phy, VLV_CMN_DW0, 0x610);
> @@ -1951,10 +1951,10 @@ static void vlv_prepare_pll(const struct intel_crtc_state *crtc_state)
>  	if (crtc_state->port_clock == 162000 ||
>  	    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG) ||
>  	    intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> -		vlv_dpio_write(dev_priv, phy, VLV_PLL_DW10(pipe),
> +		vlv_dpio_write(dev_priv, phy, VLV_PLL_DW18(pipe),
>  				 0x009f0003);
>  	else
> -		vlv_dpio_write(dev_priv, phy, VLV_PLL_DW10(pipe),
> +		vlv_dpio_write(dev_priv, phy, VLV_PLL_DW18(pipe),
>  				 0x00d0000f);
>  
>  	if (intel_crtc_has_dp_encoder(crtc_state)) {
> @@ -1981,7 +1981,7 @@ static void vlv_prepare_pll(const struct intel_crtc_state *crtc_state)
>  		coreclk |= 0x01000000;
>  	vlv_dpio_write(dev_priv, phy, VLV_PLL_DW7(pipe), coreclk);
>  
> -	vlv_dpio_write(dev_priv, phy, VLV_PLL_DW11(pipe), 0x87871000);
> +	vlv_dpio_write(dev_priv, phy, VLV_PLL_DW19(pipe), 0x87871000);
>  
>  	vlv_dpio_put(dev_priv);
>  }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8f3c83d2ab8d..747221f8ac72 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -229,21 +229,21 @@
>  #define _VLV_PLL_DW7_CH1		0x803c
>  #define VLV_PLL_DW7(ch) _PIPE(ch, _VLV_PLL_DW7_CH0, _VLV_PLL_DW7_CH1)
>  
> -#define _VLV_PLL_DW8_CH0		0x8040
> -#define _VLV_PLL_DW8_CH1		0x8060
> -#define VLV_PLL_DW8(ch) _PIPE(ch, _VLV_PLL_DW8_CH0, _VLV_PLL_DW8_CH1)
> +#define _VLV_PLL_DW16_CH0		0x8040
> +#define _VLV_PLL_DW16_CH1		0x8060
> +#define VLV_PLL_DW16(ch) _PIPE(ch, _VLV_PLL_DW16_CH0, _VLV_PLL_DW16_CH1)
>  
> -#define _VLV_PLL_DW9_CH0		0x8044
> -#define _VLV_PLL_DW9_CH1		0x8064
> -#define VLV_PLL_DW9(ch) _PIPE(ch, _VLV_PLL_DW9_CH0, _VLV_PLL_DW9_CH1)
> +#define _VLV_PLL_DW17_CH0		0x8044
> +#define _VLV_PLL_DW17_CH1		0x8064
> +#define VLV_PLL_DW17(ch) _PIPE(ch, _VLV_PLL_DW17_CH0, _VLV_PLL_DW17_CH1)
>  
> -#define _VLV_PLL_DW10_CH0		0x8048
> -#define _VLV_PLL_DW10_CH1		0x8068
> -#define VLV_PLL_DW10(ch) _PIPE(ch, _VLV_PLL_DW10_CH0, _VLV_PLL_DW10_CH1)
> +#define _VLV_PLL_DW18_CH0		0x8048
> +#define _VLV_PLL_DW18_CH1		0x8068
> +#define VLV_PLL_DW18(ch) _PIPE(ch, _VLV_PLL_DW18_CH0, _VLV_PLL_DW18_CH1)
>  
> -#define _VLV_PLL_DW11_CH0		0x804c
> -#define _VLV_PLL_DW11_CH1		0x806c
> -#define VLV_PLL_DW11(ch) _PIPE(ch, _VLV_PLL_DW11_CH0, _VLV_PLL_DW11_CH1)
> +#define _VLV_PLL_DW19_CH0		0x804c
> +#define _VLV_PLL_DW19_CH1		0x806c
> +#define VLV_PLL_DW19(ch) _PIPE(ch, _VLV_PLL_DW19_CH0, _VLV_PLL_DW19_CH1)
>  
>  /* Spec for ref block start counts at DW8 */
>  #define VLV_REF_DW11			0x80ac

-- 
Jani Nikula, Intel