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

From: Daniel Vetter <daniel@ffwll.ch>
To: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2 1/1] drm/i915/gen9: Check BIOS RC6 setup before enabling RC6
Date: Fri, 30 Oct 2015 17:09:01 +0100	[thread overview]
Message-ID: <20151030160901.GL16848@phenom.ffwll.local> (raw)
In-Reply-To: <1446204649-18987-1-git-send-email-sagar.a.kamble@intel.com>

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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx