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

2018-09-13 Thread Paulo Zanoni
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 
> Cc: Jose Souza 
> Signed-off-by: Anusha Srivatsa 
> ---
>  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_CFGCR00x164000
>  #define _ICL_DPLL1_CFGCR00x164080
> +#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_CFGCR10x164004
>  #define _ICL_DPLL1_CFGCR10x164084
> +#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


[Intel-gfx] [PATCH] drm/i915/tbt: Add CFGCR0/1 registers for TBT

2018-09-13 Thread Anusha Srivatsa
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 
Cc: Jose Souza 
Signed-off-by: Anusha Srivatsa 
---
 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, _ICL_DPLL0_CFGCR0, \
  _ICL_DPLL1_CFGCR0)
 
 #define _ICL_DPLL0_CFGCR1  0x164004
 #define _ICL_DPLL1_CFGCR1  0x164084
+#define ICL_TBTPLL_CFGCR1  _MMIO(0x164080)
 #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:
-- 
2.17.1

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