diff mbox series

drm/i915/ehl: Use an id of 4 while accessing DPLL4's CR0 and CR1

Message ID 20190717021316.18610-1-vivek.kasireddy@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/ehl: Use an id of 4 while accessing DPLL4's CR0 and CR1 | expand

Commit Message

Vivek Kasireddy July 17, 2019, 2:13 a.m. UTC
Although, DPLL4 enable and disable is associated with MGPLL1_ENABLE
register, we can use ICL_DPLL_CFGCR0/CR1 macros to access this dpll's
CR0 and CR1 registers by passing an id of 4 to these macros.

Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Comments

Ville Syrjälä July 18, 2019, 5:29 p.m. UTC | #1
On Tue, Jul 16, 2019 at 07:13:16PM -0700, Vivek Kasireddy wrote:
> Although, DPLL4 enable and disable is associated with MGPLL1_ENABLE
> register, we can use ICL_DPLL_CFGCR0/CR1 macros to access this dpll's
> CR0 and CR1 registers by passing an id of 4 to these macros.
> 
> Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 319a26a1ec10..f9bdf8514a53 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -3127,8 +3127,13 @@ static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  		hw_state->cfgcr0 = I915_READ(TGL_DPLL_CFGCR0(id));
>  		hw_state->cfgcr1 = I915_READ(TGL_DPLL_CFGCR1(id));
>  	} else {
> -		hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(id));
> -		hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(id));
> +		if (IS_ELKHARTLAKE(dev_priv) && id == DPLL_ID_EHL_DPLL4) {
> +			hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(4));
> +			hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(4));
> +		} else {
> +			hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(id));
> +			hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(id));
> +		}
>  	}
>  
>  	ret = true;
> @@ -3169,8 +3174,13 @@ static void icl_dpll_write(struct drm_i915_private *dev_priv,
>  		cfgcr0_reg = TGL_DPLL_CFGCR0(id);
>  		cfgcr1_reg = TGL_DPLL_CFGCR1(id);
>  	} else {
> -		cfgcr0_reg = ICL_DPLL_CFGCR0(id);
> -		cfgcr1_reg = ICL_DPLL_CFGCR1(id);
> +		if (IS_ELKHARTLAKE(dev_priv) && id == DPLL_ID_EHL_DPLL4) {
> +			cfgcr0_reg = ICL_DPLL_CFGCR0(4);
> +			cfgcr1_reg = ICL_DPLL_CFGCR1(4);
> +		} else {
> +			cfgcr0_reg = ICL_DPLL_CFGCR0(id);
> +			cfgcr1_reg = ICL_DPLL_CFGCR1(id);
> +		}

I was a bit worried this would also affect other parts of the code, but 
at least ICL_DPCLKA_CFGCR0_DDI_CLK_SEL() seems to do the right thing
with the id==2, and I couldn't immediately spot other issues.

Also surprising that ci didn't get confused by the fact that this was posted
as a reply to another series. Generally you should avoid replying with
anything except direct replacements for the original patches in the
series.

Pushed to dinq. Thanks for the patch.

>  	}
>  
>  	I915_WRITE(cfgcr0_reg, hw_state->cfgcr0);
> -- 
> 2.21.0
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 319a26a1ec10..f9bdf8514a53 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -3127,8 +3127,13 @@  static bool icl_pll_get_hw_state(struct drm_i915_private *dev_priv,
 		hw_state->cfgcr0 = I915_READ(TGL_DPLL_CFGCR0(id));
 		hw_state->cfgcr1 = I915_READ(TGL_DPLL_CFGCR1(id));
 	} else {
-		hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(id));
-		hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(id));
+		if (IS_ELKHARTLAKE(dev_priv) && id == DPLL_ID_EHL_DPLL4) {
+			hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(4));
+			hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(4));
+		} else {
+			hw_state->cfgcr0 = I915_READ(ICL_DPLL_CFGCR0(id));
+			hw_state->cfgcr1 = I915_READ(ICL_DPLL_CFGCR1(id));
+		}
 	}
 
 	ret = true;
@@ -3169,8 +3174,13 @@  static void icl_dpll_write(struct drm_i915_private *dev_priv,
 		cfgcr0_reg = TGL_DPLL_CFGCR0(id);
 		cfgcr1_reg = TGL_DPLL_CFGCR1(id);
 	} else {
-		cfgcr0_reg = ICL_DPLL_CFGCR0(id);
-		cfgcr1_reg = ICL_DPLL_CFGCR1(id);
+		if (IS_ELKHARTLAKE(dev_priv) && id == DPLL_ID_EHL_DPLL4) {
+			cfgcr0_reg = ICL_DPLL_CFGCR0(4);
+			cfgcr1_reg = ICL_DPLL_CFGCR1(4);
+		} else {
+			cfgcr0_reg = ICL_DPLL_CFGCR0(id);
+			cfgcr1_reg = ICL_DPLL_CFGCR1(id);
+		}
 	}
 
 	I915_WRITE(cfgcr0_reg, hw_state->cfgcr0);