[Intel-gfx] [PATCH 27/36] drm/i915: Split control of rps and rc6

[Sagar Arun Kamble]

On 3/14/2018 3:07 PM, Chris Wilson wrote:
> Allow ourselves to individually toggle rps or rc6. This will be used
> later when we want to enable rps/rc6 at different phases during the
> device bring up.
>
> Whilst here, convert the intel_$verb_gt_powersave over to
> intel_gt_pm_$verb scheme.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
<snip>
> +void intel_gt_pm_init(struct drm_i915_private *dev_priv)
>   {
>   	struct intel_rps *rps = &dev_priv->gt_pm.rps;
>   
> @@ -2475,22 +2477,13 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
>   	/* Finally allow us to boost to max by default */
>   	rps->boost_freq = rps->max_freq;
>   
> -	mutex_unlock(&rps->lock);
> -}
> -
> -static inline void intel_enable_llc_pstate(struct drm_i915_private *i915)
> -{
> -	lockdep_assert_held(&i915->gt_pm.rps.lock);
> -
> -	if (i915->gt_pm.llc_pstate.enabled)
> -		return;
> -
> -	gen6_update_ring_freq(i915);
> +	if (HAS_LLC(dev_priv))
> +		gen6_update_ring_freq(dev_priv);
Ring frequency table update has to be done on resuming from sleep or 
reset as well hence we will
need to possibly move it either __enable_rps or gt_pm_sanitize(provided 
we guard against "rps initialized")
Verified on my SKL system. Otherwise, patch looks good to me.

Thanks,
Sagar
>   
> -	i915->gt_pm.llc_pstate.enabled = true;
> +	mutex_unlock(&rps->lock);
>   }
>   
> -static void intel_enable_rc6(struct drm_i915_private *dev_priv)
> +static void __enable_rc6(struct drm_i915_private *dev_priv)
>   {
>   	lockdep_assert_held(&dev_priv->gt_pm.rps.lock);
>   
> @@ -2511,7 +2504,7 @@ static void intel_enable_rc6(struct drm_i915_private *dev_priv)
>   	dev_priv->gt_pm.rc6.enabled = true;
>   }
>   
> -static void intel_enable_rps(struct drm_i915_private *dev_priv)
> +static void __enable_rps(struct drm_i915_private *dev_priv)
>   {
>   	struct intel_rps *rps = &dev_priv->gt_pm.rps;
>   
> @@ -2546,37 +2539,27 @@ static void intel_enable_rps(struct drm_i915_private *dev_priv)
>   	rps->enabled = true;
>   }
>   
> -void intel_enable_gt_powersave(struct drm_i915_private *dev_priv)
> +void intel_gt_pm_enable_rc6(struct drm_i915_private *dev_priv)
>   {
> -	/* Powersaving is controlled by the host when inside a VM */
> -	if (intel_vgpu_active(dev_priv))
> +	if (!HAS_RC6(dev_priv))
>   		return;
>   
>   	mutex_lock(&dev_priv->gt_pm.rps.lock);
> -
> -	if (HAS_RC6(dev_priv))
> -		intel_enable_rc6(dev_priv);
> -	if (HAS_RPS(dev_priv))
> -		intel_enable_rps(dev_priv);
> -	if (HAS_LLC(dev_priv))
> -		intel_enable_llc_pstate(dev_priv);
> -
> +	__enable_rc6(dev_priv);
>   	mutex_unlock(&dev_priv->gt_pm.rps.lock);
>   }
>   
> -static inline void intel_disable_llc_pstate(struct drm_i915_private *i915)
> +void intel_gt_pm_enable_rps(struct drm_i915_private *dev_priv)
>   {
> -	lockdep_assert_held(&i915->gt_pm.rps.lock);
> -
> -	if (!i915->gt_pm.llc_pstate.enabled)
> +	if (!HAS_RPS(dev_priv))
>   		return;
>   
> -	/* Currently there is no HW configuration to be done to disable. */
> -
> -	i915->gt_pm.llc_pstate.enabled = false;
> +	mutex_lock(&dev_priv->gt_pm.rps.lock);
> +	__enable_rps(dev_priv);
> +	mutex_unlock(&dev_priv->gt_pm.rps.lock);
>   }
>   
> -static void intel_disable_rc6(struct drm_i915_private *dev_priv)
> +static void __disable_rc6(struct drm_i915_private *dev_priv)
>   {
>   	lockdep_assert_held(&dev_priv->gt_pm.rps.lock);
>   
> @@ -2595,7 +2578,14 @@ static void intel_disable_rc6(struct drm_i915_private *dev_priv)
>   	dev_priv->gt_pm.rc6.enabled = false;
>   }
>   
> -static void intel_disable_rps(struct drm_i915_private *dev_priv)
> +void intel_gt_pm_disable_rc6(struct drm_i915_private *dev_priv)
> +{
> +	mutex_lock(&dev_priv->gt_pm.rps.lock);
> +	__disable_rc6(dev_priv);
> +	mutex_unlock(&dev_priv->gt_pm.rps.lock);
> +}
> +
> +static void __disable_rps(struct drm_i915_private *dev_priv)
>   {
>   	lockdep_assert_held(&dev_priv->gt_pm.rps.lock);
>   
> @@ -2616,19 +2606,14 @@ static void intel_disable_rps(struct drm_i915_private *dev_priv)
>   	dev_priv->gt_pm.rps.enabled = false;
>   }
>   
> -void intel_disable_gt_powersave(struct drm_i915_private *dev_priv)
> +void intel_gt_pm_disable_rps(struct drm_i915_private *dev_priv)
>   {
>   	mutex_lock(&dev_priv->gt_pm.rps.lock);
> -
> -	intel_disable_rc6(dev_priv);
> -	intel_disable_rps(dev_priv);
> -	if (HAS_LLC(dev_priv))
> -		intel_disable_llc_pstate(dev_priv);
> -
> +	__disable_rps(dev_priv);
>   	mutex_unlock(&dev_priv->gt_pm.rps.lock);
>   }
>   
> -void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
> +void intel_gt_pm_fini(struct drm_i915_private *dev_priv)
>   {
>   	if (IS_VALLEYVIEW(dev_priv))
>   		valleyview_cleanup_gt_powersave(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_gt_pm.h b/drivers/gpu/drm/i915/intel_gt_pm.h
> index 722325bbb6cc..5975c63f46bf 100644
> --- a/drivers/gpu/drm/i915/intel_gt_pm.h
> +++ b/drivers/gpu/drm/i915/intel_gt_pm.h
> @@ -31,12 +31,16 @@ struct intel_rps_client;
>   void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
>   void intel_gpu_ips_teardown(void);
>   
> -void intel_sanitize_gt_powersave(struct drm_i915_private *dev_priv);
> +void intel_gt_pm_sanitize(struct drm_i915_private *dev_priv);
>   
> -void intel_init_gt_powersave(struct drm_i915_private *dev_priv);
> -void intel_enable_gt_powersave(struct drm_i915_private *dev_priv);
> -void intel_disable_gt_powersave(struct drm_i915_private *dev_priv);
> -void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv);
> +void intel_gt_pm_init(struct drm_i915_private *dev_priv);
> +void intel_gt_pm_fini(struct drm_i915_private *dev_priv);
> +
> +void intel_gt_pm_enable_rps(struct drm_i915_private *dev_priv);
> +void intel_gt_pm_disable_rps(struct drm_i915_private *dev_priv);
> +
> +void intel_gt_pm_enable_rc6(struct drm_i915_private *dev_priv);
> +void intel_gt_pm_disable_rc6(struct drm_i915_private *dev_priv);
>   
>   void intel_gt_pm_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir);
>   

-- 
Thanks,
Sagar