diff mbox

[v2,1/1] drm/i915/gen9: Check BIOS RC6 setup before enabling RC6

Message ID 1446204649-18987-1-git-send-email-sagar.a.kamble@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sagar.a.kamble@intel.com Oct. 30, 2015, 11:30 a.m. UTC
RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
configuration registers. If those are not setup Driver should not enable RC6.
For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
to know if BIOS has enabled HW/SW RC6.
This will also enable user to control RC6 using BIOS settings alone.

v2: Had placed logic in gen8 function by mistake. Fixed it. Ensuring RPM is
not enabled in case BIOS disabled RC6.

Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Daniel Vetter Oct. 30, 2015, 4:08 p.m. UTC | #1
On Fri, Oct 30, 2015 at 05:00:49PM +0530, Sagar Arun Kamble wrote:
> RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
> configuration registers. If those are not setup Driver should not enable RC6.
> For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
> to know if BIOS has enabled HW/SW RC6.
> This will also enable user to control RC6 using BIOS settings alone.
> 
> v2: Had placed logic in gen8 function by mistake. Fixed it. Ensuring RPM is
> not enabled in case BIOS disabled RC6.
> 
> Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2c7c9fc..1ec52f2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -30,6 +30,7 @@
>  #include "intel_drv.h"
>  #include "../../../platform/x86/intel_ips.h"
>  #include <linux/module.h>
> +#include <linux/pm_runtime.h>
>  
>  /**
>   * RC6 is a special power stage which allows the GPU to enter an very
> @@ -4763,6 +4764,20 @@ static void gen9_enable_rc6(struct drm_device *dev)
>  	struct intel_engine_cs *ring;
>  	uint32_t rc6_mask = 0;
>  	int unused;
> +	bool hw_rc6_enabled, sw_rc6_enabled;
> +	struct device *device = &dev->pdev->dev;
> +
> +	/* Check if BIOS has enabled HW/SW RC6. Only then enable RC6 */
> +	hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
> +				(GEN6_RC_CTL_RC6_ENABLE | GEN6_RC_CTL_HW_ENABLE);
> +	sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) & GEN6_RC_CTL_HW_ENABLE)
> +				&& (I915_READ(GEN6_RC_STATE) & 0x40000);

Please add a define for the magic value 0x40000. Also, should we check
this on earlier platforms too? In that case a small helper and calling it
everywhere would be great.
-Daniel

> +	if (!(hw_rc6_enabled || sw_rc6_enabled)) {
> +		i915.enable_rc6 = 0;
> +		pm_runtime_forbid(device);
> +		DRM_INFO("RC6 disabled by BIOS, disabled runtime PM support\n");
> +	}
> +
>  
>  	/* 1a: Software RC state - RC0 */
>  	I915_WRITE(GEN6_RC_STATE, 0);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter Oct. 30, 2015, 4:09 p.m. UTC | #2
On Fri, Oct 30, 2015 at 05:00:49PM +0530, Sagar Arun Kamble wrote:
> RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
> configuration registers. If those are not setup Driver should not enable RC6.
> For implementing this, driver can check RC_CTRL0 and RC_CTRL1 values
> to know if BIOS has enabled HW/SW RC6.
> This will also enable user to control RC6 using BIOS settings alone.
> 
> v2: Had placed logic in gen8 function by mistake. Fixed it. Ensuring RPM is
> not enabled in case BIOS disabled RC6.
> 
> Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 2c7c9fc..1ec52f2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -30,6 +30,7 @@
>  #include "intel_drv.h"
>  #include "../../../platform/x86/intel_ips.h"
>  #include <linux/module.h>
> +#include <linux/pm_runtime.h>
>  
>  /**
>   * RC6 is a special power stage which allows the GPU to enter an very
> @@ -4763,6 +4764,20 @@ static void gen9_enable_rc6(struct drm_device *dev)
>  	struct intel_engine_cs *ring;
>  	uint32_t rc6_mask = 0;
>  	int unused;
> +	bool hw_rc6_enabled, sw_rc6_enabled;
> +	struct device *device = &dev->pdev->dev;
> +
> +	/* Check if BIOS has enabled HW/SW RC6. Only then enable RC6 */
> +	hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
> +				(GEN6_RC_CTL_RC6_ENABLE | GEN6_RC_CTL_HW_ENABLE);
> +	sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) & GEN6_RC_CTL_HW_ENABLE)
> +				&& (I915_READ(GEN6_RC_STATE) & 0x40000);
> +	if (!(hw_rc6_enabled || sw_rc6_enabled)) {
> +		i915.enable_rc6 = 0;
> +		pm_runtime_forbid(device);
> +		DRM_INFO("RC6 disabled by BIOS, disabled runtime PM support\n");

One more: You don't disable runtime PM here, only RC6. Please fix this.
-Daniel

> +	}
> +
>  
>  	/* 1a: Software RC state - RC0 */
>  	I915_WRITE(GEN6_RC_STATE, 0);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2c7c9fc..1ec52f2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -30,6 +30,7 @@ 
 #include "intel_drv.h"
 #include "../../../platform/x86/intel_ips.h"
 #include <linux/module.h>
+#include <linux/pm_runtime.h>
 
 /**
  * RC6 is a special power stage which allows the GPU to enter an very
@@ -4763,6 +4764,20 @@  static void gen9_enable_rc6(struct drm_device *dev)
 	struct intel_engine_cs *ring;
 	uint32_t rc6_mask = 0;
 	int unused;
+	bool hw_rc6_enabled, sw_rc6_enabled;
+	struct device *device = &dev->pdev->dev;
+
+	/* Check if BIOS has enabled HW/SW RC6. Only then enable RC6 */
+	hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
+				(GEN6_RC_CTL_RC6_ENABLE | GEN6_RC_CTL_HW_ENABLE);
+	sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) & GEN6_RC_CTL_HW_ENABLE)
+				&& (I915_READ(GEN6_RC_STATE) & 0x40000);
+	if (!(hw_rc6_enabled || sw_rc6_enabled)) {
+		i915.enable_rc6 = 0;
+		pm_runtime_forbid(device);
+		DRM_INFO("RC6 disabled by BIOS, disabled runtime PM support\n");
+	}
+
 
 	/* 1a: Software RC state - RC0 */
 	I915_WRITE(GEN6_RC_STATE, 0);