Re: [PATCH 1/3] drm/i915: Extract SAGV block time function

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Sep 25, 2019 at 01:33:50PM -0700, James Ausmus wrote:
> In prep for newer platforms having more complicated ways to determine
> the SAGV block time, extract the setting to a separate function. While
> we're at it, update the if ladder to follow the new gen -> old gen order
> preference, and warn on any non-specified gen.
> 
> Cc: Ville Syrjälä <ville.syrjala@intel.com>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: James Ausmus <james.ausmus@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6bed2ed14574..5ad72dcb0faa 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3662,6 +3662,23 @@ intel_has_sagv(struct drm_i915_private *dev_priv)
>  		dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED;
>  }
>  
> +static int
> +intel_get_sagv_block_time_us(struct drm_i915_private *dev_priv)

The "_get_" in the name seems a bit superfluous.

> +{
> +	int sagv_block_time_us = 1000; /* Default to unusable block time */
> +
> +	if (IS_GEN(dev_priv, 11))
> +		sagv_block_time_us = 10;
> +	else if (IS_GEN(dev_priv, 10))
> +		sagv_block_time_us = 20;
> +	else if (IS_GEN(dev_priv, 9))
> +		sagv_block_time_us = 30;
> +	else
> +		MISSING_CASE(INTEL_GEN(dev_priv));
> +
> +	return sagv_block_time_us;

Could just return directly w/o the temp variable.

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

> +}
> +
>  /*
>   * SAGV dynamically adjusts the system agent voltage and clock frequencies
>   * depending on power and performance requirements. The display engine access
> @@ -3755,12 +3772,7 @@ bool intel_can_enable_sagv(struct intel_atomic_state *state)
>  	if (!intel_has_sagv(dev_priv))
>  		return false;
>  
> -	if (IS_GEN(dev_priv, 9))
> -		sagv_block_time_us = 30;
> -	else if (IS_GEN(dev_priv, 10))
> -		sagv_block_time_us = 20;
> -	else
> -		sagv_block_time_us = 10;
> +	sagv_block_time_us = intel_get_sagv_block_time_us(dev_priv);
>  
>  	/*
>  	 * If there are no active CRTCs, no additional checks need be performed
> -- 
> 2.22.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx