public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 04/10] drm/i915: Perform static RPS frequency setup before userspace
Date: Mon, 11 Jul 2016 16:31:33 +0300	[thread overview]
Message-ID: <87eg70ti6i.fsf@intel.com> (raw)
In-Reply-To: <1468055535-19740-5-git-send-email-chris@chris-wilson.co.uk>

Chris Wilson <chris@chris-wilson.co.uk> writes:

> As these RPS frequency values are part of our userspace interface, they
> must be established before that userspace interface is registered.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 98 +++++++++++++----------------------------
>  1 file changed, 31 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index df72f8e7b4b3..54f739fbd133 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5102,35 +5102,31 @@ int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6)
>  
>  static void gen6_init_rps_frequencies(struct drm_i915_private *dev_priv)
>  {
> -	uint32_t rp_state_cap;
> -	u32 ddcc_status = 0;
> -	int ret;
> -
>  	/* All of these values are in units of 50MHz */
> -	dev_priv->rps.cur_freq		= 0;
> +
>  	/* static values from HW: RP0 > RP1 > RPn (min_freq) */
>  	if (IS_BROXTON(dev_priv)) {
> -		rp_state_cap = I915_READ(BXT_RP_STATE_CAP);
> +		u32 rp_state_cap = I915_READ(BXT_RP_STATE_CAP);
>  		dev_priv->rps.rp0_freq = (rp_state_cap >> 16) & 0xff;
>  		dev_priv->rps.rp1_freq = (rp_state_cap >>  8) & 0xff;
>  		dev_priv->rps.min_freq = (rp_state_cap >>  0) & 0xff;
>  	} else {
> -		rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
> +		u32 rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
>  		dev_priv->rps.rp0_freq = (rp_state_cap >>  0) & 0xff;
>  		dev_priv->rps.rp1_freq = (rp_state_cap >>  8) & 0xff;
>  		dev_priv->rps.min_freq = (rp_state_cap >> 16) & 0xff;
>  	}
> -
>  	/* hw_max = RP0 until we check for overclocking */
> -	dev_priv->rps.max_freq		= dev_priv->rps.rp0_freq;
> +	dev_priv->rps.max_freq = dev_priv->rps.rp0_freq;
>  
>  	dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv) ||
>  	    IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
> -		ret = sandybridge_pcode_read(dev_priv,
> -					HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
> -					&ddcc_status);
> -		if (0 == ret)
> +		u32 ddcc_status = 0;
> +
> +		if (sandybridge_pcode_read(dev_priv,
> +					   HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
> +					   &ddcc_status) == 0)


We do the read now without forcewake, but this should not matter.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

>  			dev_priv->rps.efficient_freq =
>  				clamp_t(u8,
>  					((ddcc_status >> 8) & 0xff),
> @@ -5140,30 +5136,14 @@ static void gen6_init_rps_frequencies(struct drm_i915_private *dev_priv)
>  
>  	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv)) {
>  		/* Store the frequency values in 16.66 MHZ units, which is
> -		   the natural hardware unit for SKL */
> +		 * the natural hardware unit for SKL
> +		 */
>  		dev_priv->rps.rp0_freq *= GEN9_FREQ_SCALER;
>  		dev_priv->rps.rp1_freq *= GEN9_FREQ_SCALER;
>  		dev_priv->rps.min_freq *= GEN9_FREQ_SCALER;
>  		dev_priv->rps.max_freq *= GEN9_FREQ_SCALER;
>  		dev_priv->rps.efficient_freq *= GEN9_FREQ_SCALER;
>  	}
> -
> -	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
> -	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
> -
> -	/* Preserve min/max settings in case of re-init */
> -	if (dev_priv->rps.max_freq_softlimit == 0)
> -		dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;
> -
> -	if (dev_priv->rps.min_freq_softlimit == 0) {
> -		if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> -			dev_priv->rps.min_freq_softlimit =
> -				max_t(int, dev_priv->rps.efficient_freq,
> -				      intel_freq_opcode(dev_priv, 450));
> -		else
> -			dev_priv->rps.min_freq_softlimit =
> -				dev_priv->rps.min_freq;
> -	}
>  }
>  
>  static void reset_rps(struct drm_i915_private *dev_priv,
> @@ -5183,8 +5163,6 @@ static void gen9_enable_rps(struct drm_i915_private *dev_priv)
>  {
>  	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>  
> -	gen6_init_rps_frequencies(dev_priv);
> -
>  	/* WaGsvDisableTurbo: Workaround to disable turbo on BXT A* */
>  	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_A1)) {
>  		/*
> @@ -5301,9 +5279,6 @@ static void gen8_enable_rps(struct drm_i915_private *dev_priv)
>  	/* 2a: Disable RC states. */
>  	I915_WRITE(GEN6_RC_CONTROL, 0);
>  
> -	/* Initialize rps frequencies */
> -	gen6_init_rps_frequencies(dev_priv);
> -
>  	/* 2b: Program RC6 thresholds.*/
>  	I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16);
>  	I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); /* 12500 * 1280ns */
> @@ -5392,9 +5367,6 @@ static void gen6_enable_rps(struct drm_i915_private *dev_priv)
>  
>  	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>  
> -	/* Initialize rps frequencies */
> -	gen6_init_rps_frequencies(dev_priv);
> -
>  	/* disable the counters and set deterministic thresholds */
>  	I915_WRITE(GEN6_RC_CONTROL, 0);
>  
> @@ -5778,8 +5750,6 @@ static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
>  
>  	vlv_init_gpll_ref_freq(dev_priv);
>  
> -	mutex_lock(&dev_priv->rps.hw_lock);
> -
>  	val = vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS);
>  	switch ((val >> 6) & 3) {
>  	case 0:
> @@ -5815,18 +5785,6 @@ static void valleyview_init_gt_powersave(struct drm_i915_private *dev_priv)
>  	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
>  			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
>  			 dev_priv->rps.min_freq);
> -
> -	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
> -	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
> -
> -	/* Preserve min/max settings in case of re-init */
> -	if (dev_priv->rps.max_freq_softlimit == 0)
> -		dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;
> -
> -	if (dev_priv->rps.min_freq_softlimit == 0)
> -		dev_priv->rps.min_freq_softlimit = dev_priv->rps.min_freq;
> -
> -	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
>  static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
> @@ -5837,8 +5795,6 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
>  
>  	vlv_init_gpll_ref_freq(dev_priv);
>  
> -	mutex_lock(&dev_priv->rps.hw_lock);
> -
>  	mutex_lock(&dev_priv->sb_lock);
>  	val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
>  	mutex_unlock(&dev_priv->sb_lock);
> @@ -5880,18 +5836,6 @@ static void cherryview_init_gt_powersave(struct drm_i915_private *dev_priv)
>  		   dev_priv->rps.rp1_freq |
>  		   dev_priv->rps.min_freq) & 1,
>  		  "Odd GPU freq values\n");
> -
> -	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
> -	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
> -
> -	/* Preserve min/max settings in case of re-init */
> -	if (dev_priv->rps.max_freq_softlimit == 0)
> -		dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;
> -
> -	if (dev_priv->rps.min_freq_softlimit == 0)
> -		dev_priv->rps.min_freq_softlimit = dev_priv->rps.min_freq;
> -
> -	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
>  static void valleyview_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
> @@ -6559,10 +6503,30 @@ void intel_init_gt_powersave(struct drm_i915_private *dev_priv)
>  		intel_runtime_pm_get(dev_priv);
>  	}
>  
> +	mutex_lock(&dev_priv->rps.hw_lock);
> +
> +	/* Initialize RPS limits (for userspace) */
>  	if (IS_CHERRYVIEW(dev_priv))
>  		cherryview_init_gt_powersave(dev_priv);
>  	else if (IS_VALLEYVIEW(dev_priv))
>  		valleyview_init_gt_powersave(dev_priv);
> +	else
> +		gen6_init_rps_frequencies(dev_priv);
> +
> +	/* Derive initial user preferences/limits from the hardware limits */
> +	dev_priv->rps.idle_freq = dev_priv->rps.min_freq;
> +	dev_priv->rps.cur_freq = dev_priv->rps.idle_freq;
> +
> +	dev_priv->rps.max_freq_softlimit = dev_priv->rps.max_freq;
> +	dev_priv->rps.min_freq_softlimit = dev_priv->rps.min_freq;
> +
> +	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> +		dev_priv->rps.min_freq_softlimit =
> +			max_t(int,
> +			      dev_priv->rps.efficient_freq,
> +			      intel_freq_opcode(dev_priv, 450));
> +
> +	mutex_unlock(&dev_priv->rps.hw_lock);
>  }
>  
>  void intel_cleanup_gt_powersave(struct drm_i915_private *dev_priv)
> -- 
> 2.8.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2016-07-11 13:32 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-09  9:12 Mika's rps/rc6 fixes Chris Wilson
2016-07-09  9:12 ` [PATCH 01/10] drm/i915/breadcrumbs: Queue hangcheck before sleeping Chris Wilson
2016-07-11 11:42   ` Mika Kuoppala
2016-07-09  9:12 ` [PATCH 02/10] drm/i915: Kick hangcheck from retire worker Chris Wilson
2016-07-11 12:21   ` Mika Kuoppala
2016-07-11 12:46     ` Chris Wilson
2016-07-09  9:12 ` [PATCH 03/10] drm/i915: Preserve current RPS frequency across init Chris Wilson
2016-07-09  9:12 ` [PATCH 04/10] drm/i915: Perform static RPS frequency setup before userspace Chris Wilson
2016-07-11 13:31   ` Mika Kuoppala [this message]
2016-07-09  9:12 ` [PATCH 05/10] drm/i915: Move overclocking detection to alongside RPS frequency detection Chris Wilson
2016-07-11 13:37   ` Mika Kuoppala
2016-07-09  9:12 ` [PATCH 06/10] drm/i915: Define a separate variable and control for RPS waitboost frequency Chris Wilson
2016-07-11 11:39   ` Mika Kuoppala
2016-07-11 11:49     ` Chris Wilson
2016-07-11 12:55       ` Mika Kuoppala
2016-07-11 13:09         ` Chris Wilson
2016-07-09  9:12 ` [PATCH 07/10] drm/i915: Remove superfluous powersave work flushing Chris Wilson
2016-07-11 14:11   ` Mika Kuoppala
2016-07-09  9:12 ` [PATCH 08/10] drm/i915: Defer enabling rc6 til after we submit the first batch/context Chris Wilson
2016-07-11 14:14   ` Mika Kuoppala
2016-07-11 14:24     ` Chris Wilson
2016-07-09  9:12 ` [PATCH 09/10] drm/i915: Hide gen6_update_ring_freq() Chris Wilson
2016-07-09  9:12 ` [PATCH 10/10] drm/i915: Remove temporary RPM wakeref assert disables Chris Wilson
2016-07-11 15:30   ` Mika Kuoppala
2016-07-09  9:58 ` ✗ Ro.CI.BAT: failure for series starting with [01/10] drm/i915/breadcrumbs: Queue hangcheck before sleeping Patchwork
2016-07-09 10:08   ` Chris Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87eg70ti6i.fsf@intel.com \
    --to=mika.kuoppala@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox