Re: [PATCH 03/31] drm/i915: Separate RPS and RC6 handling for BDW

From: "Szwichtenberg, Radoslaw" <radoslaw.szwichtenberg@intel.com>
To: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Kamble, Sagar A" <sagar.a.kamble@intel.com>
Subject: Re: [PATCH 03/31] drm/i915: Separate RPS and RC6 handling for BDW
Date: Wed, 20 Sep 2017 11:14:06 +0000	[thread overview]
Message-ID: <1505906043.21576.30.camel@intel.com> (raw)
In-Reply-To: <1505842927-13327-4-git-send-email-sagar.a.kamble@intel.com>

On Tue, 2017-09-19 at 23:11 +0530, Sagar Arun Kamble wrote:
> This patch separates RC6 and RPS enabling for BDW.
> RC6/RPS Disabling are handled through gen6 functions.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f78a1e8..6de69ae 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6613,7 +6613,7 @@ static void gen9_enable_rc6(struct drm_i915_private
> *dev_priv)
>  	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>  }
>  
> -static void gen8_enable_rps(struct drm_i915_private *dev_priv)
> +static void gen8_enable_rc6(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_engine_cs *engine;
>  	enum intel_engine_id id;
> @@ -6645,16 +6645,18 @@ static void gen8_enable_rps(struct drm_i915_private
> *dev_priv)
>  	if (intel_enable_rc6() & INTEL_RC6_ENABLE)
>  		rc6_mask = GEN6_RC_CTL_RC6_ENABLE;
>  	intel_print_rc6_info(dev_priv, rc6_mask);
> -	if (IS_BROADWELL(dev_priv))
> -		I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> -				GEN7_RC_CTL_TO_MODE |
> -				rc6_mask);
> -	else
> -		I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> -				GEN6_RC_CTL_EI_MODE(1) |
> -				rc6_mask);
> +	I915_WRITE(GEN6_RC_CONTROL, GEN6_RC_CTL_HW_ENABLE |
> +			GEN7_RC_CTL_TO_MODE |
> +			rc6_mask);
>  
> -	/* 4 Program defaults and thresholds for RPS*/
> +	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> +}
> +
> +static void gen8_enable_rps(struct drm_i915_private *dev_priv)
> +{
> +	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> +
> +	/* 1 Program defaults and thresholds for RPS*/
>  	I915_WRITE(GEN6_RPNSWREQ,
>  		   HSW_FREQUENCY(dev_priv->rps.rp1_freq));
>  	I915_WRITE(GEN6_RC_VIDEO_FREQ,
> @@ -6674,7 +6676,7 @@ static void gen8_enable_rps(struct drm_i915_private
> *dev_priv)
>  
>  	I915_WRITE(GEN6_RP_IDLE_HYSTERSIS, 10);
>  
> -	/* 5: Enable RPS */
> +	/* 2: Enable RPS */
>  	I915_WRITE(GEN6_RP_CONTROL,
>  		   GEN6_RP_MEDIA_TURBO |
>  		   GEN6_RP_MEDIA_HW_NORMAL_MODE |
> @@ -6683,7 +6685,7 @@ static void gen8_enable_rps(struct drm_i915_private
> *dev_priv)
>  		   GEN6_RP_UP_BUSY_AVG |
>  		   GEN6_RP_DOWN_IDLE_AVG);
>  
> -	/* 6: Ring frequency + overclocking (our driver does this later */
> +	/* 3: Ring frequency + overclocking (our driver does this later */
This comment looks invalid (no overclocking done here). Also closing bracket
missing and maybe one white line to be removed :)

-Radek
>  
>  	reset_rps(dev_priv, gen6_set_rps);
>  
> @@ -7976,6 +7978,7 @@ void intel_enable_gt_powersave(struct drm_i915_private
> *dev_priv)
>  		if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv))
>  			gen6_update_ring_freq(dev_priv);
>  	} else if (IS_BROADWELL(dev_priv)) {
> +		gen8_enable_rc6(dev_priv);
>  		gen8_enable_rps(dev_priv);
>  		gen6_update_ring_freq(dev_priv);
>  	} else if (INTEL_GEN(dev_priv) >= 6) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx