Re: [PATCH] drm/i915/tbt: Add CFGCR0/1 registers for TBT

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Qui, 2018-09-13 às 15:15 -0700, Anusha Srivatsa escreveu:
> We were using the default CFGCR0/1 instead of using
> TBT specific CFGCR0 and CFGCR1 registers during
> PLL sequence.
> 
> Add missing TBTPLL_CFGCR0/1 registers and plumb
> them in the existing PLL sequence.
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Jose Souza <jose.souza@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h       |  2 ++
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 18 ++++++++++++++++--
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 4948b352bf4c..e299ce7210fb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9552,11 +9552,13 @@ enum skl_power_gate {
>  
>  #define _ICL_DPLL0_CFGCR0		0x164000
>  #define _ICL_DPLL1_CFGCR0		0x164080
> +#define ICL_TBTPLL_CFGCR0		_MMIO(0x164100)
>  #define ICL_DPLL_CFGCR0(pll)		_MMIO_PLL(pll, 

As discussed this in person before, ICL_DPLL_CFGCR0() uses _PICK_EVEN,
which will evaluate to the correct address for TBT.

If you want, you may do this only for informational purposes:
    #define _ICL_TBTPLL_CRFCR0 0x164100
but it is not required.


> _ICL_DPLL0_CFGCR0, \
>  						  _ICL_DPLL1_CFGCR0)
>  
>  #define _ICL_DPLL0_CFGCR1		0x164004
>  #define _ICL_DPLL1_CFGCR1		0x164084
> +#define ICL_TBTPLL_CFGCR1		_MMIO(0x164080)

The address added by this patch is just wrong.

For a correct address, the same point as ICL_DPLL_CFGCR0() applies:
_PICK_EVEN does the magic and our code uses the correct value.

There's no need for all that code below. The current code is even valid
in case we add more DPLLs of the same type, as long as their IDs match
the indexing.


>  #define ICL_DPLL_CFGCR1(pll)		_MMIO_PLL(pll,
> _ICL_DPLL0_CFGCR1, \
>  						  _ICL_DPLL1_CFGCR1)
>  
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index e6cac9225536..5b297445fbf7 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2968,10 +2968,12 @@ static bool icl_pll_get_hw_state(struct
> drm_i915_private *dev_priv,
>  	switch (id) {
>  	case DPLL_ID_ICL_DPLL0:
>  	case DPLL_ID_ICL_DPLL1:
> -	case DPLL_ID_ICL_TBTPLL:
>  		hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(id));
>  		hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(id));
>  		break;
> +	case DPLL_ID_ICL_TBTPLL:
> +		hw_state->cfgcr0 = I915_READ(ICL_TBTPLL_CFGCR0);
> +		hw_state->cfgcr1 = I915_READ(ICL_TBTPLL_CFGCR1);
>  	case DPLL_ID_ICL_MGPLL1:
>  	case DPLL_ID_ICL_MGPLL2:
>  	case DPLL_ID_ICL_MGPLL3:
> @@ -3035,6 +3037,16 @@ static void icl_dpll_write(struct
> drm_i915_private *dev_priv,
>  	POSTING_READ(ICL_DPLL_CFGCR1(id));
>  }
>  
> +static void icl_tbtpll_write(struct drm_i915_private *dev_priv,
> +			     struct intel_shared_dpll *pll)
> +{
> +	struct intel_dpll_hw_state *hw_state = &pll->state.hw_state;
> +
> +	I915_WRITE(ICL_TBTPLL_CFGCR0, hw_state->cfgcr0);
> +	I915_WRITE(ICL_TBTPLL_CFGCR1, hw_state->cfgcr1);
> +	POSTING_READ(ICL_TBTPLL_CFGCR1);
> +}
> +
>  static void icl_mg_pll_write(struct drm_i915_private *dev_priv,
>  			     struct intel_shared_dpll *pll)
>  {
> @@ -3107,9 +3119,11 @@ static void icl_pll_enable(struct
> drm_i915_private *dev_priv,
>  	switch (id) {
>  	case DPLL_ID_ICL_DPLL0:
>  	case DPLL_ID_ICL_DPLL1:
> -	case DPLL_ID_ICL_TBTPLL:
>  		icl_dpll_write(dev_priv, pll);
>  		break;
> +	case DPLL_ID_ICL_TBTPLL:
> +		icl_tbtpll_write(dev_priv, pll);
> +		break;
>  	case DPLL_ID_ICL_MGPLL1:
>  	case DPLL_ID_ICL_MGPLL2:
>  	case DPLL_ID_ICL_MGPLL3:
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx