[Intel-gfx] [PATCH 5/8] drm/i915: Consolidate {bxt, cnl, icl}_uninit_cdclk

Ville Syrjälä ville.syrjala at linux.intel.com
Tue Sep 10 12:39:45 UTC 2019


On Fri, Sep 06, 2019 at 05:21:40PM -0700, Matt Roper wrote:
> The uninitialize flow is the same on all of these platforms, aside from
> calculating a different frequency level.
> 
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 48 +++++++---------------
>  1 file changed, 14 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index f8c2a706990b..a70fec82d2bc 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1692,7 +1692,18 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
>  
>  	cdclk_state.cdclk = cdclk_state.bypass;
>  	cdclk_state.vco = 0;
> -	cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
> +	if (IS_ELKHARTLAKE(dev_priv))
> +		cdclk_state.voltage_level =
> +			ehl_calc_voltage_level(cdclk_state.cdclk);
> +	else if (INTEL_GEN(dev_priv) >= 11)
> +		cdclk_state.voltage_level =
> +			icl_calc_voltage_level(cdclk_state.cdclk);
> +	else if (INTEL_GEN(dev_priv) >= 10)
> +		cdclk_state.voltage_level =
> +			cnl_calc_voltage_level(cdclk_state.cdclk);
> +	else
> +		cdclk_state.voltage_level =
> +			bxt_calc_voltage_level(cdclk_state.cdclk);
>  
>  	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>  }
> @@ -1738,22 +1749,6 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
>  	bxt_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
>  }
>  
> -static void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
> -{
> -	struct intel_cdclk_state cdclk_state = dev_priv->cdclk.hw;
> -
> -	cdclk_state.cdclk = cdclk_state.bypass;
> -	cdclk_state.vco = 0;
> -	if (IS_ELKHARTLAKE(dev_priv))
> -		cdclk_state.voltage_level =
> -			ehl_calc_voltage_level(cdclk_state.cdclk);
> -	else
> -		cdclk_state.voltage_level =
> -			icl_calc_voltage_level(cdclk_state.cdclk);
> -
> -	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> -}
> -
>  static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_cdclk_state cdclk_state;
> @@ -1773,17 +1768,6 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
>  	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>  }
>  
> -static void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
> -{
> -	struct intel_cdclk_state cdclk_state = dev_priv->cdclk.hw;
> -
> -	cdclk_state.cdclk = cdclk_state.bypass;
> -	cdclk_state.vco = 0;
> -	cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
> -
> -	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> -}
> -
>  /**
>   * intel_cdclk_init - Initialize CDCLK
>   * @i915: i915 device
> @@ -1814,14 +1798,10 @@ void intel_cdclk_init(struct drm_i915_private *i915)
>   */
>  void intel_cdclk_uninit(struct drm_i915_private *i915)
>  {
> -	if (INTEL_GEN(i915) >= 11)
> -		icl_uninit_cdclk(i915);
> -	else if (IS_CANNONLAKE(i915))
> -		cnl_uninit_cdclk(i915);
> +	if (IS_GEN9_LP(i915) || INTEL_GEN(i915) >= 10)

I think the general concensus has been to list platforms
from new to old. Might be good to stick to that even
within conditions like this. There may have been other cases like this
in these patches that I didn't spot.

Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

> +		bxt_uninit_cdclk(i915);
>  	else if (IS_GEN9_BC(i915))
>  		skl_uninit_cdclk(i915);
> -	else if (IS_GEN9_LP(i915))
> -		bxt_uninit_cdclk(i915);
>  }
>  
>  /**
> -- 
> 2.20.1

-- 
Ville Syrjälä
Intel


More information about the Intel-gfx mailing list