diff mbox series

[v3,1/2] drm/i915: Move SAGV block time to dev_priv

Message ID 20191004221449.1317-1-james.ausmus@intel.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/2] drm/i915: Move SAGV block time to dev_priv | expand

Commit Message

James Ausmus Oct. 4, 2019, 10:14 p.m. UTC
In prep for newer platforms having more complicated ways to determine
the SAGV block time, move the variable to dev_priv, and extract the
setting to an initial setup 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.

v2: Shorten the function name (Ville), return directly (Ville), move
sagv_block_time_us value to dev_priv (Ville)

v3: Change sagv_block_time_us to u32 (Lucas), Change fallback value to
-1 (Lucas), use intel_has_sagv for setup check rather than hand-rolling
(Lucas)

Cc: Ville Syrjälä <ville.syrjala@linux.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/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/intel_pm.c | 33 ++++++++++++++++++++++++---------
 2 files changed, 26 insertions(+), 9 deletions(-)

Comments

Ville Syrjälä Oct. 8, 2019, 11:29 a.m. UTC | #1
On Fri, Oct 04, 2019 at 03:14:48PM -0700, James Ausmus wrote:
> In prep for newer platforms having more complicated ways to determine
> the SAGV block time, move the variable to dev_priv, and extract the
> setting to an initial setup 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.
> 
> v2: Shorten the function name (Ville), return directly (Ville), move
> sagv_block_time_us value to dev_priv (Ville)
> 
> v3: Change sagv_block_time_us to u32 (Lucas), Change fallback value to
> -1 (Lucas), use intel_has_sagv for setup check rather than hand-rolling
> (Lucas)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.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/i915_drv.h |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c | 33 ++++++++++++++++++++++++---------
>  2 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cde4c7fb5570..1d9a9e827261 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1560,6 +1560,8 @@ struct drm_i915_private {
>  		I915_SAGV_NOT_CONTROLLED
>  	} sagv_status;
>  
> +	u32 sagv_block_time_us;
> +

u32 seems a bit excessive. Although the pcode command doesn't document
any upper bound so maybe u32 is really the correct choice here. And we'd
need to put it somewhere else to avoid the hole anyway.

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

>  	struct {
>  		/*
>  		 * Raw watermark latency values:
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index bfcf03ab5245..0ffcafe97216 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3642,6 +3642,26 @@ intel_has_sagv(struct drm_i915_private *dev_priv)
>  		dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED;
>  }
>  
> +static void
> +skl_setup_sagv_block_time(struct drm_i915_private *dev_priv)
> +{
> +	if (IS_GEN(dev_priv, 11)) {
> +		dev_priv->sagv_block_time_us = 10;
> +		return;
> +	} else if (IS_GEN(dev_priv, 10)) {
> +		dev_priv->sagv_block_time_us = 20;
> +		return;
> +	} else if (IS_GEN(dev_priv, 9)) {
> +		dev_priv->sagv_block_time_us = 30;
> +		return;
> +	} else {
> +		MISSING_CASE(INTEL_GEN(dev_priv));
> +	}
> +
> +	/* Default to an unusable block time */
> +	dev_priv->sagv_block_time_us = -1;
> +}
> +
>  /*
>   * SAGV dynamically adjusts the system agent voltage and clock frequencies
>   * depending on power and performance requirements. The display engine access
> @@ -3730,18 +3750,10 @@ bool intel_can_enable_sagv(struct intel_atomic_state *state)
>  	struct intel_crtc_state *crtc_state;
>  	enum pipe pipe;
>  	int level, latency;
> -	int sagv_block_time_us;
>  
>  	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;
> -
>  	/*
>  	 * If there are no active CRTCs, no additional checks need be performed
>  	 */
> @@ -3788,7 +3800,7 @@ bool intel_can_enable_sagv(struct intel_atomic_state *state)
>  		 * incur memory latencies higher than sagv_block_time_us we
>  		 * can't enable SAGV.
>  		 */
> -		if (latency < sagv_block_time_us)
> +		if (latency < dev_priv->sagv_block_time_us)
>  			return false;
>  	}
>  
> @@ -9013,6 +9025,9 @@ void intel_init_pm(struct drm_i915_private *dev_priv)
>  	else if (IS_GEN(dev_priv, 5))
>  		i915_ironlake_get_mem_freq(dev_priv);
>  
> +	if (intel_has_sagv(dev_priv))
> +		skl_setup_sagv_block_time(dev_priv);
> +
>  	/* For FIFO watermark updates */
>  	if (INTEL_GEN(dev_priv) >= 9) {
>  		skl_setup_wm_latency(dev_priv);
> -- 
> 2.22.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cde4c7fb5570..1d9a9e827261 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1560,6 +1560,8 @@  struct drm_i915_private {
 		I915_SAGV_NOT_CONTROLLED
 	} sagv_status;
 
+	u32 sagv_block_time_us;
+
 	struct {
 		/*
 		 * Raw watermark latency values:
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index bfcf03ab5245..0ffcafe97216 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3642,6 +3642,26 @@  intel_has_sagv(struct drm_i915_private *dev_priv)
 		dev_priv->sagv_status != I915_SAGV_NOT_CONTROLLED;
 }
 
+static void
+skl_setup_sagv_block_time(struct drm_i915_private *dev_priv)
+{
+	if (IS_GEN(dev_priv, 11)) {
+		dev_priv->sagv_block_time_us = 10;
+		return;
+	} else if (IS_GEN(dev_priv, 10)) {
+		dev_priv->sagv_block_time_us = 20;
+		return;
+	} else if (IS_GEN(dev_priv, 9)) {
+		dev_priv->sagv_block_time_us = 30;
+		return;
+	} else {
+		MISSING_CASE(INTEL_GEN(dev_priv));
+	}
+
+	/* Default to an unusable block time */
+	dev_priv->sagv_block_time_us = -1;
+}
+
 /*
  * SAGV dynamically adjusts the system agent voltage and clock frequencies
  * depending on power and performance requirements. The display engine access
@@ -3730,18 +3750,10 @@  bool intel_can_enable_sagv(struct intel_atomic_state *state)
 	struct intel_crtc_state *crtc_state;
 	enum pipe pipe;
 	int level, latency;
-	int sagv_block_time_us;
 
 	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;
-
 	/*
 	 * If there are no active CRTCs, no additional checks need be performed
 	 */
@@ -3788,7 +3800,7 @@  bool intel_can_enable_sagv(struct intel_atomic_state *state)
 		 * incur memory latencies higher than sagv_block_time_us we
 		 * can't enable SAGV.
 		 */
-		if (latency < sagv_block_time_us)
+		if (latency < dev_priv->sagv_block_time_us)
 			return false;
 	}
 
@@ -9013,6 +9025,9 @@  void intel_init_pm(struct drm_i915_private *dev_priv)
 	else if (IS_GEN(dev_priv, 5))
 		i915_ironlake_get_mem_freq(dev_priv);
 
+	if (intel_has_sagv(dev_priv))
+		skl_setup_sagv_block_time(dev_priv);
+
 	/* For FIFO watermark updates */
 	if (INTEL_GEN(dev_priv) >= 9) {
 		skl_setup_wm_latency(dev_priv);