diff mbox

[v2,4/5] drm/i915: Preserve the state of power wells not explicitly enabled

Message ID 1487345986-26511-5-git-send-email-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Imre Deak Feb. 17, 2017, 3:39 p.m. UTC
Atm, power wells that BIOS has enabled, but which we don't explicitly
enable during power domain initialization would get disabled as we clear
the BIOS request bit in the given power well sync_hw hook. To prevent
this copy over any set request bits in the BIOS request register to the
driver request register and clear the BIOS request bit only afterwards.

This doesn't make a difference now, since we enable all power wells
during power domain initialization. A follow-up patchset will add power
wells for which this isn't true, so fix up the inconsistency.

Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Ander Conselvan de Oliveira Feb. 20, 2017, 9:28 a.m. UTC | #1
On Fri, 2017-02-17 at 17:39 +0200, Imre Deak wrote:
> Atm, power wells that BIOS has enabled, but which we don't explicitly
> enable during power domain initialization would get disabled as we clear
> the BIOS request bit in the given power well sync_hw hook. To prevent
> this copy over any set request bits in the BIOS request register to the
> driver request register and clear the BIOS request bit only afterwards.
> 
> This doesn't make a difference now, since we enable all power wells
> during power domain initialization. A follow-up patchset will add power
> wells for which this isn't true, so fix up the inconsistency.
> 
> Cc: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 62c99a9..44d4da3 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -830,12 +830,14 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  static void hsw_power_well_sync_hw(struct drm_i915_private *dev_priv,
>  				   struct i915_power_well *power_well)
>  {
> -	/*
> -	 * We're taking over the BIOS, so clear any requests made by it since
> -	 * the driver is in charge now.
> -	 */
> -	if (I915_READ(HSW_PWR_WELL_BIOS) & HSW_PWR_WELL_ENABLE_REQUEST)
> +	/* Take over the request bit if set by BIOS. */
> +	if (I915_READ(HSW_PWR_WELL_BIOS) & HSW_PWR_WELL_ENABLE_REQUEST) {
> +		if (!(I915_READ(HSW_PWR_WELL_DRIVER) &
> +		      HSW_PWR_WELL_ENABLE_REQUEST))
> +			I915_WRITE(HSW_PWR_WELL_DRIVER,
> +				   HSW_PWR_WELL_ENABLE_REQUEST);
>  		I915_WRITE(HSW_PWR_WELL_BIOS, 0);
> +	}
>  }
>  
>  static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
> @@ -865,8 +867,12 @@ static void skl_power_well_sync_hw(struct drm_i915_private *dev_priv,
>  	uint32_t mask = SKL_POWER_WELL_REQ(power_well->id);
>  	uint32_t bios_req = I915_READ(HSW_PWR_WELL_BIOS);
>  
> -	/* Clear any request made by BIOS as driver is taking over */
> +	/* Take over the request bit if set by BIOS. */
>  	if (bios_req & mask) {
> +		uint32_t drv_req = I915_READ(HSW_PWR_WELL_DRIVER);
> +
> +		if (!(drv_req & mask))
> +			I915_WRITE(HSW_PWR_WELL_DRIVER, drv_req | mask);
>  		I915_WRITE(HSW_PWR_WELL_BIOS, bios_req & ~mask);
>  	}
>  }
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 62c99a9..44d4da3 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -830,12 +830,14 @@  static void skl_set_power_well(struct drm_i915_private *dev_priv,
 static void hsw_power_well_sync_hw(struct drm_i915_private *dev_priv,
 				   struct i915_power_well *power_well)
 {
-	/*
-	 * We're taking over the BIOS, so clear any requests made by it since
-	 * the driver is in charge now.
-	 */
-	if (I915_READ(HSW_PWR_WELL_BIOS) & HSW_PWR_WELL_ENABLE_REQUEST)
+	/* Take over the request bit if set by BIOS. */
+	if (I915_READ(HSW_PWR_WELL_BIOS) & HSW_PWR_WELL_ENABLE_REQUEST) {
+		if (!(I915_READ(HSW_PWR_WELL_DRIVER) &
+		      HSW_PWR_WELL_ENABLE_REQUEST))
+			I915_WRITE(HSW_PWR_WELL_DRIVER,
+				   HSW_PWR_WELL_ENABLE_REQUEST);
 		I915_WRITE(HSW_PWR_WELL_BIOS, 0);
+	}
 }
 
 static void hsw_power_well_enable(struct drm_i915_private *dev_priv,
@@ -865,8 +867,12 @@  static void skl_power_well_sync_hw(struct drm_i915_private *dev_priv,
 	uint32_t mask = SKL_POWER_WELL_REQ(power_well->id);
 	uint32_t bios_req = I915_READ(HSW_PWR_WELL_BIOS);
 
-	/* Clear any request made by BIOS as driver is taking over */
+	/* Take over the request bit if set by BIOS. */
 	if (bios_req & mask) {
+		uint32_t drv_req = I915_READ(HSW_PWR_WELL_DRIVER);
+
+		if (!(drv_req & mask))
+			I915_WRITE(HSW_PWR_WELL_DRIVER, drv_req | mask);
 		I915_WRITE(HSW_PWR_WELL_BIOS, bios_req & ~mask);
 	}
 }