[Intel-gfx] [PATCH v6 02/23] drm/i915/gen9: Separate RPS and RC6 handling

[Chris Wilson]

On Thu, Mar 16, 2017 at 11:58:06PM +0530, Sagar Arun Kamble wrote:
> With GuC based SLPC, frequency control will be moved to GuC and Host will
> continue to control RC6 and Ring frequency setup. SLPC can be enabled in
> the GuC setup path and can happen in parallel in GuC with other i915 setup.
> Hence we can do away with deferred RPS enabling. This needs separate
> handling of RPS, RC6 and ring frequencies in driver flows. We can still use
> the *gt_powersave routines with separate status variables of RPS, RC6 and
> SLPC. With this patch, RC6 and ring frequencies setup(if applicable) are
> tracked through rps.rc6_enabled and RPS through rps.enabled.
> Also, Active RPS check in suspend flow is needed for platforms with RC6
> and RPS enabling/disabling coupled together. RPM suspend depends only on
> RC6 though. Hence Active RPS check is done only for non-Gen9 platforms.
> Moved setting of rps.enabled to platform level functions as there is case
> of disabling of RPS in gen9_enable_rps.
> 
> v2: Changing parameter to dev_priv for IS_GEN9 and HAS_RUNTIME_PM and line
>     spacing changes. (David)
>     and commit message update for checkpatch issues.
> 
> v3: Rebase.
> 
> v4: Commit message update.
> 
> v5: Updated intel_enable_gt_powersave and intel_disable_gt_powersave
>     routines with separated RPS and RC6 handling and rebase. Commit message
>     update.(Sagar)
> 
> v6: Added comments at the definition of rc6_enabled.
> 
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |  6 ++++-
>  drivers/gpu/drm/i915/i915_drv.h |  5 ++++
>  drivers/gpu/drm/i915/intel_pm.c | 54 +++++++++++++++++++++++++++++++++--------
>  3 files changed, 54 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9164167..0302d24 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2358,7 +2358,11 @@ static int intel_runtime_suspend(struct device *kdev)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	int ret;
>  
> -	if (WARN_ON_ONCE(!(dev_priv->rps.enabled && intel_enable_rc6())))
> +	if (WARN_ON_ONCE(!intel_enable_rc6()))
> +		return -ENODEV;
> +
> +	if (WARN_ON_ONCE((IS_GEN9(dev_priv) && !dev_priv->rps.rc6_enabled) ||
> +			 (!IS_GEN9(dev_priv) && !dev_priv->rps.enabled)))
>  		return -ENODEV;
>  
>  	if (WARN_ON_ONCE(!HAS_RUNTIME_PM(dev_priv)))
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 355bc545..7bafcd3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1372,7 +1372,12 @@ struct intel_gen6_power_mgmt {
>  	struct list_head clients;
>  	bool client_boost;
>  
> +	/*
> +	 * For platforms prior to Gen9, RPS and RC6 status is tracked
> +	 * through "enabled". For Gen9, RC6 is tracked through "rc6_enabled".
> +	 */
>  	bool enabled;
> +	bool rc6_enabled;

Does that sound sane?

Let's have rc6_enabled mean rc6_enabled whatever the gen, and
rps_enabled mean rps_enabled whatever the gen.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre