diff mbox

[v6,1/1] drm/i915/bxt: Check BIOS RC6 setup before enabling RC6

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

Commit Message

sagar.a.kamble@intel.com Jan. 29, 2016, 5:52 p.m. UTC
RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
setup 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.
RC6 related instability can be avoided by disabling via BIOS settings
till driver fixes it.

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

v3: Need to disable RPM if RC6 is disabled due to BIOS settings. (Daniel)
Runtime PM enabling happens before gen9_enable_rc6.
Moved the updation of enable_rc6 parameter in intel_uncore_sanitize.

v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for bxt. (Imre)

v5: Caching reserved stolen base and size in the driver private data.
Reorganized RC6 setup check. Moved from gen9_enable_rc6 to intel_uncore_sanitize. (Imre)

v6: Rebasing on the patch submitted by Imre that moves gem_init_stolen earlier in the load.

Cc: Imre Deak <imre.deak@intel.com>
Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.h    |  2 ++
 drivers/gpu/drm/i915/i915_gem_stolen.c |  3 ++
 drivers/gpu/drm/i915/i915_reg.h        | 11 +++++++
 drivers/gpu/drm/i915/intel_drv.h       |  1 +
 drivers/gpu/drm/i915/intel_pm.c        | 60 ++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_uncore.c    |  2 ++
 6 files changed, 77 insertions(+), 2 deletions(-)

Comments

Imre Deak Feb. 2, 2016, 4:18 p.m. UTC | #1
On pe, 2016-01-29 at 23:22 +0530, Sagar Arun Kamble wrote:
> RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
> setup 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.
> RC6 related instability can be avoided by disabling via BIOS settings
> till driver fixes it.
> 
> v2: Had placed logic in gen8 function by mistake. Fixed it.
> Ensuring RPM is not enabled in case BIOS disabled RC6.
> 
> v3: Need to disable RPM if RC6 is disabled due to BIOS settings. (Daniel)
> Runtime PM enabling happens before gen9_enable_rc6.
> Moved the updation of enable_rc6 parameter in intel_uncore_sanitize.
> 
> v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for bxt. (Imre)
> 
> v5: Caching reserved stolen base and size in the driver private data.
> Reorganized RC6 setup check. Moved from gen9_enable_rc6 to intel_uncore_sanitize. (Imre)
> 
> v6: Rebasing on the patch submitted by Imre that moves gem_init_stolen earlier in the load.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.h    |  2 ++
>  drivers/gpu/drm/i915/i915_gem_stolen.c |  3 ++
>  drivers/gpu/drm/i915/i915_reg.h        | 11 +++++++
>  drivers/gpu/drm/i915/intel_drv.h       |  1 +
>  drivers/gpu/drm/i915/intel_pm.c        | 60 ++++++++++++++++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_uncore.c    |  2 ++
>  6 files changed, 77 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index f520c90..66a6da2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -342,6 +342,8 @@ struct i915_gtt {
>  
>  	size_t stolen_size;		/* Total size of stolen memory */
>  	size_t stolen_usable_size;	/* Total size minus BIOS reserved */
> +	size_t stolen_reserved_base;
> +	size_t stolen_reserved_size;
>  	u64 mappable_end;		/* End offset that we can CPU map */
>  	struct io_mapping *mappable;	/* Mapping to our CPU mappable region */
>  	phys_addr_t mappable_base;	/* PA of our GMADR */
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index c384dc9..ba1a00d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -458,6 +458,9 @@ int i915_gem_init_stolen(struct drm_device *dev)
>  		return 0;
>  	}
>  
> +	dev_priv->gtt.stolen_reserved_base = reserved_base;
> +	dev_priv->gtt.stolen_reserved_size = reserved_size;
> +
>  	/* It is possible for the reserved area to end before the end of stolen
>  	 * memory, so just consider the start. */
>  	reserved_total = stolen_top - reserved_base;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 65e32a3..00aac28 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6781,6 +6781,16 @@ enum skl_disp_power_wells {
>  
>  #define  VLV_PMWGICZ				_MMIO(0x1300a4)
>  
> +#define  RC6_LOCATION				_MMIO(0xD40)
> +#define  RC6_CTX_IN_DRAM			1
> +#define  RC6_CTX_BASE				_MMIO(0xD48)
> +#define  RC6_CTX_BASE_MASK			0xFFFFFFF0
> +#define  PWRCTX_MAXCNT_RCSUNIT			_MMIO(0x2054)
> +#define  PWRCTX_MAXCNT_VCSUNIT0			_MMIO(0x12054)
> +#define  PWRCTX_MAXCNT_BCSUNIT			_MMIO(0x22054)
> +#define  PWRCTX_MAXCNT_VECSUNIT			_MMIO(0x1A054)
> +#define  PWRCTX_MAXCNT_VCSUNIT1			_MMIO(0x1C054)
> +#define  IDLE_TIME_MASK				0xFFFFF
>  #define  FORCEWAKE				_MMIO(0xA18C)
>  #define  FORCEWAKE_VLV				_MMIO(0x1300b0)
>  #define  FORCEWAKE_ACK_VLV			_MMIO(0x1300b4)
> @@ -6919,6 +6929,7 @@ enum skl_disp_power_wells {
>  #define GEN6_RPDEUC				_MMIO(0xA084)
>  #define GEN6_RPDEUCSW				_MMIO(0xA088)
>  #define GEN6_RC_STATE				_MMIO(0xA094)
> +#define RC6_STATE				(1<<18)
>  #define GEN6_RC1_WAKE_RATE_LIMIT		_MMIO(0xA098)
>  #define GEN6_RC6_WAKE_RATE_LIMIT		_MMIO(0xA09C)
>  #define GEN6_RC6pp_WAKE_RATE_LIMIT		_MMIO(0xA0A0)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f620023..aaa2051a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1560,6 +1560,7 @@ void skl_wm_get_hw_state(struct drm_device *dev);
>  void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
>  			  struct skl_ddb_allocation *ddb /* out */);
>  uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
> +int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6);
>  
>  /* intel_sdvo.c */
>  bool intel_sdvo_init(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 31bc4ea..8929f69 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4558,12 +4558,69 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
>  			      onoff(mode & GEN6_RC_CTL_RC6_ENABLE));
>  }
>  
> -static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
> +static bool bxt_check_bios_rc6_setup(const struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	bool enable_rc6 = true;
> +	unsigned long rc6_ctx_base;
> +	bool bios_hw_rc6_enabled, bios_sw_rc6_enabled;
> +
> +	bios_hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
> +				(GEN6_RC_CTL_RC6_ENABLE |
> +					GEN6_RC_CTL_HW_ENABLE);
> +	bios_sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) &
> +				GEN6_RC_CTL_HW_ENABLE) &&
> +				(I915_READ(GEN6_RC_STATE) & RC6_STATE);
> +
> +	if (!(I915_READ(RC6_LOCATION) & RC6_CTX_IN_DRAM)) {
> +		DRM_DEBUG_KMS("RC6 Base location not set properly.\n");
> +		enable_rc6 = false;
> +	}
> +
> +	rc6_ctx_base = I915_READ(RC6_CTX_BASE) & RC6_CTX_BASE_MASK;
> +	if (!((rc6_ctx_base >= dev_priv->gtt.stolen_reserved_base) &&
> +		(rc6_ctx_base <= (dev_priv->gtt.stolen_reserved_base +
> +					dev_priv->gtt.stolen_reserved_size)))) {
> +		DRM_DEBUG_KMS("RC6 Base address not as expected.\n");
> +		enable_rc6 = false;
> +	}
> +
> +	if (!(((I915_READ(PWRCTX_MAXCNT_RCSUNIT) & IDLE_TIME_MASK) > 1) &&
> +	      ((I915_READ(PWRCTX_MAXCNT_VCSUNIT0) & IDLE_TIME_MASK) > 1) &&
> +	      ((I915_READ(PWRCTX_MAXCNT_BCSUNIT) & IDLE_TIME_MASK) > 1) &&
> +	      ((I915_READ(PWRCTX_MAXCNT_VECSUNIT) & IDLE_TIME_MASK) > 1))) {
> +		DRM_DEBUG_KMS("Engine Idle wait time not set properly.\n");
> +		enable_rc6 = false;
> +	}
> +
> +	if (HAS_BSD2(dev) &&
> +		((I915_READ(PWRCTX_MAXCNT_VCSUNIT1) & IDLE_TIME_MASK) > 1)) {
> +		DRM_DEBUG_KMS("VCSUNIT1 Idle wait time not set properly.\n");
> +		enable_rc6 = false;
> +	}

I missed this from the previous review: this check would make sense on 
SKL, but since the function is BXT specific it shouldn't be here, so
please remove checking PWRCTX_MAXCNT_VCSUNIT1 for now. With that this
is:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> +
> +	if (!bios_hw_rc6_enabled && !bios_sw_rc6_enabled) {
> +		DRM_DEBUG_KMS("HW/SW RC6 is not enabled by BIOS.\n");
> +		enable_rc6 = false;
> +	}
> +
> +	return enable_rc6;
> +}
> +
> +int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
>  {
>  	/* No RC6 before Ironlake and code is gone for ilk. */
>  	if (INTEL_INFO(dev)->gen < 6)
>  		return 0;
>  
> +	if (!enable_rc6)
> +		return 0;
> +
> +	if (IS_BROXTON(dev) && !bxt_check_bios_rc6_setup(dev)) {
> +		DRM_INFO("RC6 disabled by BIOS\n");
> +		return 0;
> +	}
> +
>  	/* Respect the kernel parameter if it is set */
>  	if (enable_rc6 >= 0) {
>  		int mask;
> @@ -6053,7 +6110,6 @@ void intel_init_gt_powersave(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
>  	/*
>  	 * RPM depends on RC6 to save restore the GT HW context, so make RC6 a
>  	 * requirement.
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index bfa79e5..436d8f2 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -400,6 +400,8 @@ void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake)
>  
>  void intel_uncore_sanitize(struct drm_device *dev)
>  {
> +	i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
> +
>  	/* BIOS often leaves RC6 enabled, but disable it for hw init */
>  	intel_disable_gt_powersave(dev);
>  }
sagar.a.kamble@intel.com Feb. 2, 2016, 6:06 p.m. UTC | #2
Thanks for the review Imre.
I Kept that BSD2 check thinking some BXT revision might support it :)
Will remove the check and send the patch.

Thanks
Sagar

On 2/2/2016 9:48 PM, Imre Deak wrote:
> On pe, 2016-01-29 at 23:22 +0530, Sagar Arun Kamble wrote:
>> RC6 setup is shared between BIOS and Driver. BIOS sets up subset of RC6
>> setup 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.
>> RC6 related instability can be avoided by disabling via BIOS settings
>> till driver fixes it.
>>
>> v2: Had placed logic in gen8 function by mistake. Fixed it.
>> Ensuring RPM is not enabled in case BIOS disabled RC6.
>>
>> v3: Need to disable RPM if RC6 is disabled due to BIOS settings. (Daniel)
>> Runtime PM enabling happens before gen9_enable_rc6.
>> Moved the updation of enable_rc6 parameter in intel_uncore_sanitize.
>>
>> v4: Added elaborate check for BIOS RC6 setup. Prepared check_pctx for bxt. (Imre)
>>
>> v5: Caching reserved stolen base and size in the driver private data.
>> Reorganized RC6 setup check. Moved from gen9_enable_rc6 to intel_uncore_sanitize. (Imre)
>>
>> v6: Rebasing on the patch submitted by Imre that moves gem_init_stolen earlier in the load.
>>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Change-Id: If89518708e133be6b3c7c6f90869fb66224b7b87
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_gem_gtt.h    |  2 ++
>>   drivers/gpu/drm/i915/i915_gem_stolen.c |  3 ++
>>   drivers/gpu/drm/i915/i915_reg.h        | 11 +++++++
>>   drivers/gpu/drm/i915/intel_drv.h       |  1 +
>>   drivers/gpu/drm/i915/intel_pm.c        | 60 ++++++++++++++++++++++++++++++++--
>>   drivers/gpu/drm/i915/intel_uncore.c    |  2 ++
>>   6 files changed, 77 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
>> index f520c90..66a6da2 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
>> @@ -342,6 +342,8 @@ struct i915_gtt {
>>   
>>   	size_t stolen_size;		/* Total size of stolen memory */
>>   	size_t stolen_usable_size;	/* Total size minus BIOS reserved */
>> +	size_t stolen_reserved_base;
>> +	size_t stolen_reserved_size;
>>   	u64 mappable_end;		/* End offset that we can CPU map */
>>   	struct io_mapping *mappable;	/* Mapping to our CPU mappable region */
>>   	phys_addr_t mappable_base;	/* PA of our GMADR */
>> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> index c384dc9..ba1a00d 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
>> @@ -458,6 +458,9 @@ int i915_gem_init_stolen(struct drm_device *dev)
>>   		return 0;
>>   	}
>>   
>> +	dev_priv->gtt.stolen_reserved_base = reserved_base;
>> +	dev_priv->gtt.stolen_reserved_size = reserved_size;
>> +
>>   	/* It is possible for the reserved area to end before the end of stolen
>>   	 * memory, so just consider the start. */
>>   	reserved_total = stolen_top - reserved_base;
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 65e32a3..00aac28 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6781,6 +6781,16 @@ enum skl_disp_power_wells {
>>   
>>   #define  VLV_PMWGICZ				_MMIO(0x1300a4)
>>   
>> +#define  RC6_LOCATION				_MMIO(0xD40)
>> +#define  RC6_CTX_IN_DRAM			1
>> +#define  RC6_CTX_BASE				_MMIO(0xD48)
>> +#define  RC6_CTX_BASE_MASK			0xFFFFFFF0
>> +#define  PWRCTX_MAXCNT_RCSUNIT			_MMIO(0x2054)
>> +#define  PWRCTX_MAXCNT_VCSUNIT0			_MMIO(0x12054)
>> +#define  PWRCTX_MAXCNT_BCSUNIT			_MMIO(0x22054)
>> +#define  PWRCTX_MAXCNT_VECSUNIT			_MMIO(0x1A054)
>> +#define  PWRCTX_MAXCNT_VCSUNIT1			_MMIO(0x1C054)
>> +#define  IDLE_TIME_MASK				0xFFFFF
>>   #define  FORCEWAKE				_MMIO(0xA18C)
>>   #define  FORCEWAKE_VLV				_MMIO(0x1300b0)
>>   #define  FORCEWAKE_ACK_VLV			_MMIO(0x1300b4)
>> @@ -6919,6 +6929,7 @@ enum skl_disp_power_wells {
>>   #define GEN6_RPDEUC				_MMIO(0xA084)
>>   #define GEN6_RPDEUCSW				_MMIO(0xA088)
>>   #define GEN6_RC_STATE				_MMIO(0xA094)
>> +#define RC6_STATE				(1<<18)
>>   #define GEN6_RC1_WAKE_RATE_LIMIT		_MMIO(0xA098)
>>   #define GEN6_RC6_WAKE_RATE_LIMIT		_MMIO(0xA09C)
>>   #define GEN6_RC6pp_WAKE_RATE_LIMIT		_MMIO(0xA0A0)
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index f620023..aaa2051a 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1560,6 +1560,7 @@ void skl_wm_get_hw_state(struct drm_device *dev);
>>   void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
>>   			  struct skl_ddb_allocation *ddb /* out */);
>>   uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
>> +int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6);
>>   
>>   /* intel_sdvo.c */
>>   bool intel_sdvo_init(struct drm_device *dev,
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 31bc4ea..8929f69 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -4558,12 +4558,69 @@ static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
>>   			      onoff(mode & GEN6_RC_CTL_RC6_ENABLE));
>>   }
>>   
>> -static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
>> +static bool bxt_check_bios_rc6_setup(const struct drm_device *dev)
>> +{
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	bool enable_rc6 = true;
>> +	unsigned long rc6_ctx_base;
>> +	bool bios_hw_rc6_enabled, bios_sw_rc6_enabled;
>> +
>> +	bios_hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
>> +				(GEN6_RC_CTL_RC6_ENABLE |
>> +					GEN6_RC_CTL_HW_ENABLE);
>> +	bios_sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) &
>> +				GEN6_RC_CTL_HW_ENABLE) &&
>> +				(I915_READ(GEN6_RC_STATE) & RC6_STATE);
>> +
>> +	if (!(I915_READ(RC6_LOCATION) & RC6_CTX_IN_DRAM)) {
>> +		DRM_DEBUG_KMS("RC6 Base location not set properly.\n");
>> +		enable_rc6 = false;
>> +	}
>> +
>> +	rc6_ctx_base = I915_READ(RC6_CTX_BASE) & RC6_CTX_BASE_MASK;
>> +	if (!((rc6_ctx_base >= dev_priv->gtt.stolen_reserved_base) &&
>> +		(rc6_ctx_base <= (dev_priv->gtt.stolen_reserved_base +
>> +					dev_priv->gtt.stolen_reserved_size)))) {
>> +		DRM_DEBUG_KMS("RC6 Base address not as expected.\n");
>> +		enable_rc6 = false;
>> +	}
>> +
>> +	if (!(((I915_READ(PWRCTX_MAXCNT_RCSUNIT) & IDLE_TIME_MASK) > 1) &&
>> +	      ((I915_READ(PWRCTX_MAXCNT_VCSUNIT0) & IDLE_TIME_MASK) > 1) &&
>> +	      ((I915_READ(PWRCTX_MAXCNT_BCSUNIT) & IDLE_TIME_MASK) > 1) &&
>> +	      ((I915_READ(PWRCTX_MAXCNT_VECSUNIT) & IDLE_TIME_MASK) > 1))) {
>> +		DRM_DEBUG_KMS("Engine Idle wait time not set properly.\n");
>> +		enable_rc6 = false;
>> +	}
>> +
>> +	if (HAS_BSD2(dev) &&
>> +		((I915_READ(PWRCTX_MAXCNT_VCSUNIT1) & IDLE_TIME_MASK) > 1)) {
>> +		DRM_DEBUG_KMS("VCSUNIT1 Idle wait time not set properly.\n");
>> +		enable_rc6 = false;
>> +	}
> I missed this from the previous review: this check would make sense on
> SKL, but since the function is BXT specific it shouldn't be here, so
> please remove checking PWRCTX_MAXCNT_VCSUNIT1 for now. With that this
> is:
> Reviewed-by: Imre Deak <imre.deak@intel.com>
>
>> +
>> +	if (!bios_hw_rc6_enabled && !bios_sw_rc6_enabled) {
>> +		DRM_DEBUG_KMS("HW/SW RC6 is not enabled by BIOS.\n");
>> +		enable_rc6 = false;
>> +	}
>> +
>> +	return enable_rc6;
>> +}
>> +
>> +int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
>>   {
>>   	/* No RC6 before Ironlake and code is gone for ilk. */
>>   	if (INTEL_INFO(dev)->gen < 6)
>>   		return 0;
>>   
>> +	if (!enable_rc6)
>> +		return 0;
>> +
>> +	if (IS_BROXTON(dev) && !bxt_check_bios_rc6_setup(dev)) {
>> +		DRM_INFO("RC6 disabled by BIOS\n");
>> +		return 0;
>> +	}
>> +
>>   	/* Respect the kernel parameter if it is set */
>>   	if (enable_rc6 >= 0) {
>>   		int mask;
>> @@ -6053,7 +6110,6 @@ void intel_init_gt_powersave(struct drm_device *dev)
>>   {
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>   
>> -	i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
>>   	/*
>>   	 * RPM depends on RC6 to save restore the GT HW context, so make RC6 a
>>   	 * requirement.
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index bfa79e5..436d8f2 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -400,6 +400,8 @@ void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake)
>>   
>>   void intel_uncore_sanitize(struct drm_device *dev)
>>   {
>> +	i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
>> +
>>   	/* BIOS often leaves RC6 enabled, but disable it for hw init */
>>   	intel_disable_gt_powersave(dev);
>>   }
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index f520c90..66a6da2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -342,6 +342,8 @@  struct i915_gtt {
 
 	size_t stolen_size;		/* Total size of stolen memory */
 	size_t stolen_usable_size;	/* Total size minus BIOS reserved */
+	size_t stolen_reserved_base;
+	size_t stolen_reserved_size;
 	u64 mappable_end;		/* End offset that we can CPU map */
 	struct io_mapping *mappable;	/* Mapping to our CPU mappable region */
 	phys_addr_t mappable_base;	/* PA of our GMADR */
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index c384dc9..ba1a00d 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -458,6 +458,9 @@  int i915_gem_init_stolen(struct drm_device *dev)
 		return 0;
 	}
 
+	dev_priv->gtt.stolen_reserved_base = reserved_base;
+	dev_priv->gtt.stolen_reserved_size = reserved_size;
+
 	/* It is possible for the reserved area to end before the end of stolen
 	 * memory, so just consider the start. */
 	reserved_total = stolen_top - reserved_base;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 65e32a3..00aac28 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6781,6 +6781,16 @@  enum skl_disp_power_wells {
 
 #define  VLV_PMWGICZ				_MMIO(0x1300a4)
 
+#define  RC6_LOCATION				_MMIO(0xD40)
+#define  RC6_CTX_IN_DRAM			1
+#define  RC6_CTX_BASE				_MMIO(0xD48)
+#define  RC6_CTX_BASE_MASK			0xFFFFFFF0
+#define  PWRCTX_MAXCNT_RCSUNIT			_MMIO(0x2054)
+#define  PWRCTX_MAXCNT_VCSUNIT0			_MMIO(0x12054)
+#define  PWRCTX_MAXCNT_BCSUNIT			_MMIO(0x22054)
+#define  PWRCTX_MAXCNT_VECSUNIT			_MMIO(0x1A054)
+#define  PWRCTX_MAXCNT_VCSUNIT1			_MMIO(0x1C054)
+#define  IDLE_TIME_MASK				0xFFFFF
 #define  FORCEWAKE				_MMIO(0xA18C)
 #define  FORCEWAKE_VLV				_MMIO(0x1300b0)
 #define  FORCEWAKE_ACK_VLV			_MMIO(0x1300b4)
@@ -6919,6 +6929,7 @@  enum skl_disp_power_wells {
 #define GEN6_RPDEUC				_MMIO(0xA084)
 #define GEN6_RPDEUCSW				_MMIO(0xA088)
 #define GEN6_RC_STATE				_MMIO(0xA094)
+#define RC6_STATE				(1<<18)
 #define GEN6_RC1_WAKE_RATE_LIMIT		_MMIO(0xA098)
 #define GEN6_RC6_WAKE_RATE_LIMIT		_MMIO(0xA09C)
 #define GEN6_RC6pp_WAKE_RATE_LIMIT		_MMIO(0xA0A0)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f620023..aaa2051a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1560,6 +1560,7 @@  void skl_wm_get_hw_state(struct drm_device *dev);
 void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
 			  struct skl_ddb_allocation *ddb /* out */);
 uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
+int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6);
 
 /* intel_sdvo.c */
 bool intel_sdvo_init(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 31bc4ea..8929f69 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4558,12 +4558,69 @@  static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
 			      onoff(mode & GEN6_RC_CTL_RC6_ENABLE));
 }
 
-static int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
+static bool bxt_check_bios_rc6_setup(const struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	bool enable_rc6 = true;
+	unsigned long rc6_ctx_base;
+	bool bios_hw_rc6_enabled, bios_sw_rc6_enabled;
+
+	bios_hw_rc6_enabled = I915_READ(GEN6_RC_CONTROL) &
+				(GEN6_RC_CTL_RC6_ENABLE |
+					GEN6_RC_CTL_HW_ENABLE);
+	bios_sw_rc6_enabled = !(I915_READ(GEN6_RC_CONTROL) &
+				GEN6_RC_CTL_HW_ENABLE) &&
+				(I915_READ(GEN6_RC_STATE) & RC6_STATE);
+
+	if (!(I915_READ(RC6_LOCATION) & RC6_CTX_IN_DRAM)) {
+		DRM_DEBUG_KMS("RC6 Base location not set properly.\n");
+		enable_rc6 = false;
+	}
+
+	rc6_ctx_base = I915_READ(RC6_CTX_BASE) & RC6_CTX_BASE_MASK;
+	if (!((rc6_ctx_base >= dev_priv->gtt.stolen_reserved_base) &&
+		(rc6_ctx_base <= (dev_priv->gtt.stolen_reserved_base +
+					dev_priv->gtt.stolen_reserved_size)))) {
+		DRM_DEBUG_KMS("RC6 Base address not as expected.\n");
+		enable_rc6 = false;
+	}
+
+	if (!(((I915_READ(PWRCTX_MAXCNT_RCSUNIT) & IDLE_TIME_MASK) > 1) &&
+	      ((I915_READ(PWRCTX_MAXCNT_VCSUNIT0) & IDLE_TIME_MASK) > 1) &&
+	      ((I915_READ(PWRCTX_MAXCNT_BCSUNIT) & IDLE_TIME_MASK) > 1) &&
+	      ((I915_READ(PWRCTX_MAXCNT_VECSUNIT) & IDLE_TIME_MASK) > 1))) {
+		DRM_DEBUG_KMS("Engine Idle wait time not set properly.\n");
+		enable_rc6 = false;
+	}
+
+	if (HAS_BSD2(dev) &&
+		((I915_READ(PWRCTX_MAXCNT_VCSUNIT1) & IDLE_TIME_MASK) > 1)) {
+		DRM_DEBUG_KMS("VCSUNIT1 Idle wait time not set properly.\n");
+		enable_rc6 = false;
+	}
+
+	if (!bios_hw_rc6_enabled && !bios_sw_rc6_enabled) {
+		DRM_DEBUG_KMS("HW/SW RC6 is not enabled by BIOS.\n");
+		enable_rc6 = false;
+	}
+
+	return enable_rc6;
+}
+
+int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6)
 {
 	/* No RC6 before Ironlake and code is gone for ilk. */
 	if (INTEL_INFO(dev)->gen < 6)
 		return 0;
 
+	if (!enable_rc6)
+		return 0;
+
+	if (IS_BROXTON(dev) && !bxt_check_bios_rc6_setup(dev)) {
+		DRM_INFO("RC6 disabled by BIOS\n");
+		return 0;
+	}
+
 	/* Respect the kernel parameter if it is set */
 	if (enable_rc6 >= 0) {
 		int mask;
@@ -6053,7 +6110,6 @@  void intel_init_gt_powersave(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
 	/*
 	 * RPM depends on RC6 to save restore the GT HW context, so make RC6 a
 	 * requirement.
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index bfa79e5..436d8f2 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -400,6 +400,8 @@  void intel_uncore_early_sanitize(struct drm_device *dev, bool restore_forcewake)
 
 void intel_uncore_sanitize(struct drm_device *dev)
 {
+	i915.enable_rc6 = sanitize_rc6_option(dev, i915.enable_rc6);
+
 	/* BIOS often leaves RC6 enabled, but disable it for hw init */
 	intel_disable_gt_powersave(dev);
 }