public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Fix log type for RC6 debug messages
@ 2016-06-29 16:13 Imre Deak
  2016-06-29 16:13 ` [PATCH 2/2] drm/i915/bxt: Fix sanity check for BIOS RC6 setup Imre Deak
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Imre Deak @ 2016-06-29 16:13 UTC (permalink / raw)
  To: intel-gfx

RC6 isn't really a KMS feature, so use the more proper DRIVER log type
for RC6 related debug messages.

CC: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d7f8ba8..5dce264 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4973,14 +4973,15 @@ static void intel_print_rc6_info(struct drm_i915_private *dev_priv, u32 mode)
 			mode = 0;
 	}
 	if (HAS_RC6p(dev_priv))
-		DRM_DEBUG_KMS("Enabling RC6 states: RC6 %s RC6p %s RC6pp %s\n",
-			      onoff(mode & GEN6_RC_CTL_RC6_ENABLE),
-			      onoff(mode & GEN6_RC_CTL_RC6p_ENABLE),
-			      onoff(mode & GEN6_RC_CTL_RC6pp_ENABLE));
+		DRM_DEBUG_DRIVER("Enabling RC6 states: "
+				 "RC6 %s RC6p %s RC6pp %s\n",
+				 onoff(mode & GEN6_RC_CTL_RC6_ENABLE),
+				 onoff(mode & GEN6_RC_CTL_RC6p_ENABLE),
+				 onoff(mode & GEN6_RC_CTL_RC6pp_ENABLE));
 
 	else
-		DRM_DEBUG_KMS("Enabling RC6 states: RC6 %s\n",
-			      onoff(mode & GEN6_RC_CTL_RC6_ENABLE));
+		DRM_DEBUG_DRIVER("Enabling RC6 states: RC6 %s\n",
+				 onoff(mode & GEN6_RC_CTL_RC6_ENABLE));
 }
 
 static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
@@ -4990,7 +4991,7 @@ static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
 	unsigned long rc6_ctx_base;
 
 	if (!(I915_READ(RC6_LOCATION) & RC6_CTX_IN_DRAM)) {
-		DRM_DEBUG_KMS("RC6 Base location not set properly.\n");
+		DRM_DEBUG_DRIVER("RC6 Base location not set properly.\n");
 		enable_rc6 = false;
 	}
 
@@ -5002,7 +5003,7 @@ static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
 	if (!((rc6_ctx_base >= ggtt->stolen_reserved_base) &&
 	      (rc6_ctx_base + PAGE_SIZE <= ggtt->stolen_reserved_base +
 					ggtt->stolen_reserved_size))) {
-		DRM_DEBUG_KMS("RC6 Base address not as expected.\n");
+		DRM_DEBUG_DRIVER("RC6 Base address not as expected.\n");
 		enable_rc6 = false;
 	}
 
@@ -5010,7 +5011,7 @@ static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
 	      ((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");
+		DRM_DEBUG_DRIVER("Engine Idle wait time not set properly.\n");
 		enable_rc6 = false;
 	}
 
@@ -5018,7 +5019,7 @@ static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
 					    GEN6_RC_CTL_HW_ENABLE)) &&
 	    ((I915_READ(GEN6_RC_CONTROL) & GEN6_RC_CTL_HW_ENABLE) ||
 	     !(I915_READ(GEN6_RC_STATE) & RC6_STATE))) {
-		DRM_DEBUG_KMS("HW/SW RC6 is not enabled by BIOS.\n");
+		DRM_DEBUG_DRIVER("HW/SW RC6 is not enabled by BIOS.\n");
 		enable_rc6 = false;
 	}
 
@@ -5050,8 +5051,9 @@ int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6)
 			mask = INTEL_RC6_ENABLE;
 
 		if ((enable_rc6 & mask) != enable_rc6)
-			DRM_DEBUG_KMS("Adjusting RC6 mask to %d (requested %d, valid %d)\n",
-				      enable_rc6 & mask, enable_rc6, mask);
+			DRM_DEBUG_DRIVER("Adjusting RC6 mask to %d "
+					 "(requested %d, valid %d)\n",
+					 enable_rc6 & mask, enable_rc6, mask);
 
 		return enable_rc6 & mask;
 	}
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 2/2] drm/i915/bxt: Fix sanity check for BIOS RC6 setup
  2016-06-29 16:13 [PATCH 1/2] drm/i915: Fix log type for RC6 debug messages Imre Deak
@ 2016-06-29 16:13 ` Imre Deak
  2016-07-01  6:49   ` Kamble, Sagar A
  2016-06-29 17:06 ` ✓ Ro.CI.BAT: success for series starting with [1/2] drm/i915: Fix log type for RC6 debug messages Patchwork
  2016-07-01  6:37 ` [PATCH 1/2] " Kamble, Sagar A
  2 siblings, 1 reply; 8+ messages in thread
From: Imre Deak @ 2016-06-29 16:13 UTC (permalink / raw)
  To: intel-gfx

BXT BIOS has two options related to GPU power management: "RC6(Render
Standby)" and "GT PM Support". The assumption so far was that disabling
either of these options would leave RC6 uninitialized. According to my
tests this isn't so: for a proper RC6 setup we only need the "GT PM
Support" option to be enabled while the "RC6" option only controls
whether RC6 is left enabled or not by BIOS. OTOH we were missing a few
checks to ensure a proper RC6 setup. Add these now and don't fail the
sanity check if RC6 is disabled. This fixes a problem where RC6 remains
disabled after reloading the driver, since we explicitly disable RC6
during unloading.

CC: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  5 +++++
 drivers/gpu/drm/i915/intel_pm.c | 19 ++++++++++++++-----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c6bfbf8..92b4046 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7085,12 +7085,17 @@ enum {
 #define GEN6_RC6pp_THRESHOLD			_MMIO(0xA0C0)
 #define GEN6_PMINTRMSK				_MMIO(0xA168)
 #define   GEN8_PMINTR_REDIRECT_TO_NON_DISP	(1<<31)
+#define GEN8_MISC_CTRL0				_MMIO(0xA180)
 #define VLV_PWRDWNUPCTL				_MMIO(0xA294)
 #define GEN9_MEDIA_PG_IDLE_HYSTERESIS		_MMIO(0xA0C4)
 #define GEN9_RENDER_PG_IDLE_HYSTERESIS		_MMIO(0xA0C8)
 #define GEN9_PG_ENABLE				_MMIO(0xA210)
 #define GEN9_RENDER_PG_ENABLE			(1<<0)
 #define GEN9_MEDIA_PG_ENABLE			(1<<1)
+#define GEN8_PUSHBUS_CONTROL			_MMIO(0xA248)
+#define GEN8_PUSHBUS_ENABLE			_MMIO(0xA250)
+#define GEN8_PUSHBUS_SHIFT			_MMIO(0xA25C)
+
 
 #define VLV_CHICKEN_3				_MMIO(VLV_DISPLAY_BASE + 0x7040C)
 #define  PIXEL_OVERLAP_CNT_MASK			(3 << 30)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5dce264..fe76991 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5015,11 +5015,20 @@ static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
 		enable_rc6 = false;
 	}
 
-	if (!(I915_READ(GEN6_RC_CONTROL) & (GEN6_RC_CTL_RC6_ENABLE |
-					    GEN6_RC_CTL_HW_ENABLE)) &&
-	    ((I915_READ(GEN6_RC_CONTROL) & GEN6_RC_CTL_HW_ENABLE) ||
-	     !(I915_READ(GEN6_RC_STATE) & RC6_STATE))) {
-		DRM_DEBUG_DRIVER("HW/SW RC6 is not enabled by BIOS.\n");
+	if (!I915_READ(GEN8_PUSHBUS_CONTROL) ||
+	    !I915_READ(GEN8_PUSHBUS_ENABLE) ||
+	    !I915_READ(GEN8_PUSHBUS_SHIFT)) {
+		DRM_DEBUG_DRIVER("Pushbus not setup properly.\n");
+		enable_rc6 = false;
+	}
+
+	if (!I915_READ(GEN6_GFXPAUSE)) {
+		DRM_DEBUG_DRIVER("GFX pause not setup properly.\n");
+		enable_rc6 = false;
+	}
+
+	if (!I915_READ(GEN8_MISC_CTRL0)) {
+		DRM_DEBUG_DRIVER("GPM control not setup properly.\n");
 		enable_rc6 = false;
 	}
 
-- 
2.5.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* ✓ Ro.CI.BAT: success for series starting with [1/2] drm/i915: Fix log type for RC6 debug messages
  2016-06-29 16:13 [PATCH 1/2] drm/i915: Fix log type for RC6 debug messages Imre Deak
  2016-06-29 16:13 ` [PATCH 2/2] drm/i915/bxt: Fix sanity check for BIOS RC6 setup Imre Deak
@ 2016-06-29 17:06 ` Patchwork
  2016-07-01 12:03   ` Imre Deak
  2016-07-01  6:37 ` [PATCH 1/2] " Kamble, Sagar A
  2 siblings, 1 reply; 8+ messages in thread
From: Patchwork @ 2016-06-29 17:06 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Fix log type for RC6 debug messages
URL   : https://patchwork.freedesktop.org/series/9285/
State : success

== Summary ==

Series 9285v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/9285/revisions/1/mbox

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)

fi-skl-i5-6260u  total:229  pass:202  dwarn:0   dfail:0   fail:2   skip:25 
fi-snb-i7-2600   total:229  pass:174  dwarn:0   dfail:0   fail:2   skip:53 
ro-bdw-i5-5250u  total:229  pass:202  dwarn:2   dfail:1   fail:2   skip:22 
ro-bdw-i7-5557U  total:229  pass:202  dwarn:1   dfail:1   fail:2   skip:23 
ro-bdw-i7-5600u  total:229  pass:190  dwarn:0   dfail:1   fail:0   skip:38 
ro-byt-n2820     total:229  pass:178  dwarn:0   dfail:1   fail:5   skip:45 
ro-hsw-i3-4010u  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31 
ro-hsw-i7-4770r  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31 
ro-ilk-i7-620lm  total:229  pass:155  dwarn:0   dfail:1   fail:3   skip:70 
ro-ilk1-i5-650   total:224  pass:155  dwarn:0   dfail:1   fail:3   skip:65 
ro-ivb-i7-3770   total:229  pass:186  dwarn:0   dfail:1   fail:2   skip:40 
ro-ivb2-i7-3770  total:229  pass:190  dwarn:0   dfail:1   fail:2   skip:36 
ro-skl3-i5-6260u total:229  pass:206  dwarn:1   dfail:1   fail:2   skip:19 
ro-snb-i7-2620M  total:229  pass:179  dwarn:0   dfail:1   fail:1   skip:48 
fi-kbl-qkkr failed to connect after reboot
fi-skl-i7-6700k failed to connect after reboot
ro-bsw-n3050 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1334/

8a6521c drm-intel-nightly: 2016y-06m-29d-16h-08m-16s UTC integration manifest
4595c784 drm/i915/bxt: Fix sanity check for BIOS RC6 setup
52064d0 drm/i915: Fix log type for RC6 debug messages

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/i915: Fix log type for RC6 debug messages
  2016-06-29 16:13 [PATCH 1/2] drm/i915: Fix log type for RC6 debug messages Imre Deak
  2016-06-29 16:13 ` [PATCH 2/2] drm/i915/bxt: Fix sanity check for BIOS RC6 setup Imre Deak
  2016-06-29 17:06 ` ✓ Ro.CI.BAT: success for series starting with [1/2] drm/i915: Fix log type for RC6 debug messages Patchwork
@ 2016-07-01  6:37 ` Kamble, Sagar A
  2 siblings, 0 replies; 8+ messages in thread
From: Kamble, Sagar A @ 2016-07-01  6:37 UTC (permalink / raw)
  To: Imre Deak, intel-gfx

Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>

On 6/29/2016 9:43 PM, Imre Deak wrote:
> RC6 isn't really a KMS feature, so use the more proper DRIVER log type
> for RC6 related debug messages.
>
> CC: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_pm.c | 26 ++++++++++++++------------
>   1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d7f8ba8..5dce264 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4973,14 +4973,15 @@ static void intel_print_rc6_info(struct drm_i915_private *dev_priv, u32 mode)
>   			mode = 0;
>   	}
>   	if (HAS_RC6p(dev_priv))
> -		DRM_DEBUG_KMS("Enabling RC6 states: RC6 %s RC6p %s RC6pp %s\n",
> -			      onoff(mode & GEN6_RC_CTL_RC6_ENABLE),
> -			      onoff(mode & GEN6_RC_CTL_RC6p_ENABLE),
> -			      onoff(mode & GEN6_RC_CTL_RC6pp_ENABLE));
> +		DRM_DEBUG_DRIVER("Enabling RC6 states: "
> +				 "RC6 %s RC6p %s RC6pp %s\n",
> +				 onoff(mode & GEN6_RC_CTL_RC6_ENABLE),
> +				 onoff(mode & GEN6_RC_CTL_RC6p_ENABLE),
> +				 onoff(mode & GEN6_RC_CTL_RC6pp_ENABLE));
>   
>   	else
> -		DRM_DEBUG_KMS("Enabling RC6 states: RC6 %s\n",
> -			      onoff(mode & GEN6_RC_CTL_RC6_ENABLE));
> +		DRM_DEBUG_DRIVER("Enabling RC6 states: RC6 %s\n",
> +				 onoff(mode & GEN6_RC_CTL_RC6_ENABLE));
>   }
>   
>   static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
> @@ -4990,7 +4991,7 @@ static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
>   	unsigned long rc6_ctx_base;
>   
>   	if (!(I915_READ(RC6_LOCATION) & RC6_CTX_IN_DRAM)) {
> -		DRM_DEBUG_KMS("RC6 Base location not set properly.\n");
> +		DRM_DEBUG_DRIVER("RC6 Base location not set properly.\n");
>   		enable_rc6 = false;
>   	}
>   
> @@ -5002,7 +5003,7 @@ static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
>   	if (!((rc6_ctx_base >= ggtt->stolen_reserved_base) &&
>   	      (rc6_ctx_base + PAGE_SIZE <= ggtt->stolen_reserved_base +
>   					ggtt->stolen_reserved_size))) {
> -		DRM_DEBUG_KMS("RC6 Base address not as expected.\n");
> +		DRM_DEBUG_DRIVER("RC6 Base address not as expected.\n");
>   		enable_rc6 = false;
>   	}
>   
> @@ -5010,7 +5011,7 @@ static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
>   	      ((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");
> +		DRM_DEBUG_DRIVER("Engine Idle wait time not set properly.\n");
>   		enable_rc6 = false;
>   	}
>   
> @@ -5018,7 +5019,7 @@ static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
>   					    GEN6_RC_CTL_HW_ENABLE)) &&
>   	    ((I915_READ(GEN6_RC_CONTROL) & GEN6_RC_CTL_HW_ENABLE) ||
>   	     !(I915_READ(GEN6_RC_STATE) & RC6_STATE))) {
> -		DRM_DEBUG_KMS("HW/SW RC6 is not enabled by BIOS.\n");
> +		DRM_DEBUG_DRIVER("HW/SW RC6 is not enabled by BIOS.\n");
>   		enable_rc6 = false;
>   	}
>   
> @@ -5050,8 +5051,9 @@ int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6)
>   			mask = INTEL_RC6_ENABLE;
>   
>   		if ((enable_rc6 & mask) != enable_rc6)
> -			DRM_DEBUG_KMS("Adjusting RC6 mask to %d (requested %d, valid %d)\n",
> -				      enable_rc6 & mask, enable_rc6, mask);
> +			DRM_DEBUG_DRIVER("Adjusting RC6 mask to %d "
> +					 "(requested %d, valid %d)\n",
> +					 enable_rc6 & mask, enable_rc6, mask);
>   
>   		return enable_rc6 & mask;
>   	}

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/i915/bxt: Fix sanity check for BIOS RC6 setup
  2016-06-29 16:13 ` [PATCH 2/2] drm/i915/bxt: Fix sanity check for BIOS RC6 setup Imre Deak
@ 2016-07-01  6:49   ` Kamble, Sagar A
  2016-07-01  9:15     ` Imre Deak
  0 siblings, 1 reply; 8+ messages in thread
From: Kamble, Sagar A @ 2016-07-01  6:49 UTC (permalink / raw)
  To: Imre Deak, intel-gfx

Have seen BIOS having option "RC6" disabled and "GTPM" enabled for cases 
where there are RC6 specific issues.
GTPM option entails setup for other features as well I guess.
In such cases - Can we output some DRM_INFO log saying BIOS has disabled 
RC6 although setup is available.

Do we need to also check for other unit level clock gating register 
setup done by BIOS like: GEN7_MISCCPCTL, GEN6_UCGCTL1 to GEN6_UCGCTL4, 
GEN8_UCGCTL6 etc.

Thanks
Sagar


On 6/29/2016 9:43 PM, Imre Deak wrote:
> BXT BIOS has two options related to GPU power management: "RC6(Render
> Standby)" and "GT PM Support". The assumption so far was that disabling
> either of these options would leave RC6 uninitialized. According to my
> tests this isn't so: for a proper RC6 setup we only need the "GT PM
> Support" option to be enabled while the "RC6" option only controls
> whether RC6 is left enabled or not by BIOS. OTOH we were missing a few
> checks to ensure a proper RC6 setup. Add these now and don't fail the
> sanity check if RC6 is disabled. This fixes a problem where RC6 remains
> disabled after reloading the driver, since we explicitly disable RC6
> during unloading.
>
> CC: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h |  5 +++++
>   drivers/gpu/drm/i915/intel_pm.c | 19 ++++++++++++++-----
>   2 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c6bfbf8..92b4046 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7085,12 +7085,17 @@ enum {
>   #define GEN6_RC6pp_THRESHOLD			_MMIO(0xA0C0)
>   #define GEN6_PMINTRMSK				_MMIO(0xA168)
>   #define   GEN8_PMINTR_REDIRECT_TO_NON_DISP	(1<<31)
> +#define GEN8_MISC_CTRL0				_MMIO(0xA180)
>   #define VLV_PWRDWNUPCTL				_MMIO(0xA294)
>   #define GEN9_MEDIA_PG_IDLE_HYSTERESIS		_MMIO(0xA0C4)
>   #define GEN9_RENDER_PG_IDLE_HYSTERESIS		_MMIO(0xA0C8)
>   #define GEN9_PG_ENABLE				_MMIO(0xA210)
>   #define GEN9_RENDER_PG_ENABLE			(1<<0)
>   #define GEN9_MEDIA_PG_ENABLE			(1<<1)
> +#define GEN8_PUSHBUS_CONTROL			_MMIO(0xA248)
> +#define GEN8_PUSHBUS_ENABLE			_MMIO(0xA250)
> +#define GEN8_PUSHBUS_SHIFT			_MMIO(0xA25C)
> +
>   
>   #define VLV_CHICKEN_3				_MMIO(VLV_DISPLAY_BASE + 0x7040C)
>   #define  PIXEL_OVERLAP_CNT_MASK			(3 << 30)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5dce264..fe76991 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5015,11 +5015,20 @@ static bool bxt_check_bios_rc6_setup(struct drm_i915_private *dev_priv)
>   		enable_rc6 = false;
>   	}
>   
> -	if (!(I915_READ(GEN6_RC_CONTROL) & (GEN6_RC_CTL_RC6_ENABLE |
> -					    GEN6_RC_CTL_HW_ENABLE)) &&
> -	    ((I915_READ(GEN6_RC_CONTROL) & GEN6_RC_CTL_HW_ENABLE) ||
> -	     !(I915_READ(GEN6_RC_STATE) & RC6_STATE))) {
> -		DRM_DEBUG_DRIVER("HW/SW RC6 is not enabled by BIOS.\n");
> +	if (!I915_READ(GEN8_PUSHBUS_CONTROL) ||
> +	    !I915_READ(GEN8_PUSHBUS_ENABLE) ||
> +	    !I915_READ(GEN8_PUSHBUS_SHIFT)) {
> +		DRM_DEBUG_DRIVER("Pushbus not setup properly.\n");
> +		enable_rc6 = false;
> +	}
> +
> +	if (!I915_READ(GEN6_GFXPAUSE)) {
> +		DRM_DEBUG_DRIVER("GFX pause not setup properly.\n");
> +		enable_rc6 = false;
> +	}
> +
> +	if (!I915_READ(GEN8_MISC_CTRL0)) {
> +		DRM_DEBUG_DRIVER("GPM control not setup properly.\n");
>   		enable_rc6 = false;
>   	}
>   

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/i915/bxt: Fix sanity check for BIOS RC6 setup
  2016-07-01  6:49   ` Kamble, Sagar A
@ 2016-07-01  9:15     ` Imre Deak
  2016-07-01 10:56       ` Kamble, Sagar A
  0 siblings, 1 reply; 8+ messages in thread
From: Imre Deak @ 2016-07-01  9:15 UTC (permalink / raw)
  To: Kamble, Sagar A, intel-gfx

On pe, 2016-07-01 at 12:19 +0530, Kamble, Sagar A wrote:

> Have seen BIOS having option "RC6" disabled and "GTPM" enabled for cases 
> where there are RC6 specific issues.

It's possible although I haven't seen any based on the specs I have and
the tests I ran. In any case the checks I added should catch any such
missing setup and if there is something on top of that we need to add
those (along with an update to the specification). 

> GTPM option entails setup for other features as well I guess.

Yes, it affects RPS setup too, but my point was that disabling it is
what leaves RC6 unconfigured. I guess this doesn't really matter in the
end, the main thing is that we check all the RC6 specific registers.

> In such cases - Can we output some DRM_INFO log saying BIOS has disabled 
> RC6 although setup is available.

Yes, can add that, but since it's something we'd need for debugging I'd
use DRM_DEBUG.

> Do we need to also check for other unit level clock gating register 
> setup done by BIOS like: GEN7_MISCCPCTL, GEN6_UCGCTL1 to
> GEN6_UCGCTL4, 
> GEN8_UCGCTL6 etc.

These are subject to change with later HW steppings. In any case their
default value is the more conservative scenario with clock gating
disabled, which should still allow RC6 functionality. They can be also
enabled/disabled separately from the GTPM option in BIOS setup (via
sub-options to GTPM), something we haven't checked so far anyway.

Are you ok if I add a debug print for these too?

--Imre

> 
> Thanks
> Sagar
> 
> 
> On 6/29/2016 9:43 PM, Imre Deak wrote:
> > BXT BIOS has two options related to GPU power management:
> > "RC6(Render
> > Standby)" and "GT PM Support". The assumption so far was that
> > disabling
> > either of these options would leave RC6 uninitialized. According to
> > my
> > tests this isn't so: for a proper RC6 setup we only need the "GT PM
> > Support" option to be enabled while the "RC6" option only controls
> > whether RC6 is left enabled or not by BIOS. OTOH we were missing a
> > few
> > checks to ensure a proper RC6 setup. Add these now and don't fail
> > the
> > sanity check if RC6 is disabled. This fixes a problem where RC6
> > remains
> > disabled after reloading the driver, since we explicitly disable
> > RC6
> > during unloading.
> > 
> > CC: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_reg.h |  5 +++++
> >   drivers/gpu/drm/i915/intel_pm.c | 19 ++++++++++++++-----
> >   2 files changed, 19 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index c6bfbf8..92b4046 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7085,12 +7085,17 @@ enum {
> >   #define GEN6_RC6pp_THRESHOLD			_MMIO(0xA0C0)
> >   #define GEN6_PMINTRMSK				_MMIO(0xA16
> > 8)
> >   #define   GEN8_PMINTR_REDIRECT_TO_NON_DISP	(1<<31)
> > +#define GEN8_MISC_CTRL0				_MMIO(0xA18
> > 0)
> >   #define VLV_PWRDWNUPCTL				_MMIO(0xA2
> > 94)
> >   #define GEN9_MEDIA_PG_IDLE_HYSTERESIS		_MMIO(0xA0C4
> > )
> >   #define GEN9_RENDER_PG_IDLE_HYSTERESIS		_MMIO(0xA0C
> > 8)
> >   #define GEN9_PG_ENABLE				_MMIO(0xA21
> > 0)
> >   #define GEN9_RENDER_PG_ENABLE			(1<<0)
> >   #define GEN9_MEDIA_PG_ENABLE			(1<<1)
> > +#define GEN8_PUSHBUS_CONTROL			_MMIO(0xA248)
> > +#define GEN8_PUSHBUS_ENABLE			_MMIO(0xA250)
> > +#define GEN8_PUSHBUS_SHIFT			_MMIO(0xA25C)
> > +
> >   
> >   #define VLV_CHICKEN_3				_MMIO(VLV_DI
> > SPLAY_BASE + 0x7040C)
> >   #define  PIXEL_OVERLAP_CNT_MASK			(3 << 30)
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 5dce264..fe76991 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -5015,11 +5015,20 @@ static bool bxt_check_bios_rc6_setup(struct
> > drm_i915_private *dev_priv)
> >   		enable_rc6 = false;
> >   	}
> >   
> > -	if (!(I915_READ(GEN6_RC_CONTROL) & (GEN6_RC_CTL_RC6_ENABLE
> > |
> > -					    GEN6_RC_CTL_HW_ENABLE)
> > ) &&
> > -	    ((I915_READ(GEN6_RC_CONTROL) & GEN6_RC_CTL_HW_ENABLE)
> > ||
> > -	     !(I915_READ(GEN6_RC_STATE) & RC6_STATE))) {
> > -		DRM_DEBUG_DRIVER("HW/SW RC6 is not enabled by
> > BIOS.\n");
> > +	if (!I915_READ(GEN8_PUSHBUS_CONTROL) ||
> > +	    !I915_READ(GEN8_PUSHBUS_ENABLE) ||
> > +	    !I915_READ(GEN8_PUSHBUS_SHIFT)) {
> > +		DRM_DEBUG_DRIVER("Pushbus not setup properly.\n");
> > +		enable_rc6 = false;
> > +	}
> > +
> > +	if (!I915_READ(GEN6_GFXPAUSE)) {
> > +		DRM_DEBUG_DRIVER("GFX pause not setup
> > properly.\n");
> > +		enable_rc6 = false;
> > +	}
> > +
> > +	if (!I915_READ(GEN8_MISC_CTRL0)) {
> > +		DRM_DEBUG_DRIVER("GPM control not setup
> > properly.\n");
> >   		enable_rc6 = false;
> >   	}
> >   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/i915/bxt: Fix sanity check for BIOS RC6 setup
  2016-07-01  9:15     ` Imre Deak
@ 2016-07-01 10:56       ` Kamble, Sagar A
  0 siblings, 0 replies; 8+ messages in thread
From: Kamble, Sagar A @ 2016-07-01 10:56 UTC (permalink / raw)
  To: imre.deak, intel-gfx



On 7/1/2016 2:45 PM, Imre Deak wrote:
> On pe, 2016-07-01 at 12:19 +0530, Kamble, Sagar A wrote:
>
>> Have seen BIOS having option "RC6" disabled and "GTPM" enabled for cases
>> where there are RC6 specific issues.
> It's possible although I haven't seen any based on the specs I have and
> the tests I ran. In any case the checks I added should catch any such
> missing setup and if there is something on top of that we need to add
> those (along with an update to the specification).
>
>> GTPM option entails setup for other features as well I guess.
> Yes, it affects RPS setup too, but my point was that disabling it is
> what leaves RC6 unconfigured. I guess this doesn't really matter in the
> end, the main thing is that we check all the RC6 specific registers.
>
>> In such cases - Can we output some DRM_INFO log saying BIOS has disabled
>> RC6 although setup is available.
> Yes, can add that, but since it's something we'd need for debugging I'd
> use DRM_DEBUG.
Fine. With this:
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>
>> Do we need to also check for other unit level clock gating register
>> setup done by BIOS like: GEN7_MISCCPCTL, GEN6_UCGCTL1 to
>> GEN6_UCGCTL4,
>> GEN8_UCGCTL6 etc.
> These are subject to change with later HW steppings. In any case their
> default value is the more conservative scenario with clock gating
> disabled, which should still allow RC6 functionality. They can be also
> enabled/disabled separately from the GTPM option in BIOS setup (via
> sub-options to GTPM), something we haven't checked so far anyway.
>
> Are you ok if I add a debug print for these too?
We can skip given that these are not so much impacting RC6 as you said.


>
> --Imre
>
>> Thanks
>> Sagar
>>
>>
>> On 6/29/2016 9:43 PM, Imre Deak wrote:
>>> BXT BIOS has two options related to GPU power management:
>>> "RC6(Render
>>> Standby)" and "GT PM Support". The assumption so far was that
>>> disabling
>>> either of these options would leave RC6 uninitialized. According to
>>> my
>>> tests this isn't so: for a proper RC6 setup we only need the "GT PM
>>> Support" option to be enabled while the "RC6" option only controls
>>> whether RC6 is left enabled or not by BIOS. OTOH we were missing a
>>> few
>>> checks to ensure a proper RC6 setup. Add these now and don't fail
>>> the
>>> sanity check if RC6 is disabled. This fixes a problem where RC6
>>> remains
>>> disabled after reloading the driver, since we explicitly disable
>>> RC6
>>> during unloading.
>>>
>>> CC: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/i915_reg.h |  5 +++++
>>>    drivers/gpu/drm/i915/intel_pm.c | 19 ++++++++++++++-----
>>>    2 files changed, 19 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>>> b/drivers/gpu/drm/i915/i915_reg.h
>>> index c6bfbf8..92b4046 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -7085,12 +7085,17 @@ enum {
>>>    #define GEN6_RC6pp_THRESHOLD			_MMIO(0xA0C0)
>>>    #define GEN6_PMINTRMSK				_MMIO(0xA16
>>> 8)
>>>    #define   GEN8_PMINTR_REDIRECT_TO_NON_DISP	(1<<31)
>>> +#define GEN8_MISC_CTRL0				_MMIO(0xA18
>>> 0)
>>>    #define VLV_PWRDWNUPCTL				_MMIO(0xA2
>>> 94)
>>>    #define GEN9_MEDIA_PG_IDLE_HYSTERESIS		_MMIO(0xA0C4
>>> )
>>>    #define GEN9_RENDER_PG_IDLE_HYSTERESIS		_MMIO(0xA0C
>>> 8)
>>>    #define GEN9_PG_ENABLE				_MMIO(0xA21
>>> 0)
>>>    #define GEN9_RENDER_PG_ENABLE			(1<<0)
>>>    #define GEN9_MEDIA_PG_ENABLE			(1<<1)
>>> +#define GEN8_PUSHBUS_CONTROL			_MMIO(0xA248)
>>> +#define GEN8_PUSHBUS_ENABLE			_MMIO(0xA250)
>>> +#define GEN8_PUSHBUS_SHIFT			_MMIO(0xA25C)
>>> +
>>>    
>>>    #define VLV_CHICKEN_3				_MMIO(VLV_DI
>>> SPLAY_BASE + 0x7040C)
>>>    #define  PIXEL_OVERLAP_CNT_MASK			(3 << 30)
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c
>>> b/drivers/gpu/drm/i915/intel_pm.c
>>> index 5dce264..fe76991 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -5015,11 +5015,20 @@ static bool bxt_check_bios_rc6_setup(struct
>>> drm_i915_private *dev_priv)
>>>    		enable_rc6 = false;
>>>    	}
>>>    
>>> -	if (!(I915_READ(GEN6_RC_CONTROL) & (GEN6_RC_CTL_RC6_ENABLE
>>> |
>>> -					    GEN6_RC_CTL_HW_ENABLE)
>>> ) &&
>>> -	    ((I915_READ(GEN6_RC_CONTROL) & GEN6_RC_CTL_HW_ENABLE)
>>> ||
>>> -	     !(I915_READ(GEN6_RC_STATE) & RC6_STATE))) {
>>> -		DRM_DEBUG_DRIVER("HW/SW RC6 is not enabled by
>>> BIOS.\n");
>>> +	if (!I915_READ(GEN8_PUSHBUS_CONTROL) ||
>>> +	    !I915_READ(GEN8_PUSHBUS_ENABLE) ||
>>> +	    !I915_READ(GEN8_PUSHBUS_SHIFT)) {
>>> +		DRM_DEBUG_DRIVER("Pushbus not setup properly.\n");
>>> +		enable_rc6 = false;
>>> +	}
>>> +
>>> +	if (!I915_READ(GEN6_GFXPAUSE)) {
>>> +		DRM_DEBUG_DRIVER("GFX pause not setup
>>> properly.\n");
>>> +		enable_rc6 = false;
>>> +	}
>>> +
>>> +	if (!I915_READ(GEN8_MISC_CTRL0)) {
>>> +		DRM_DEBUG_DRIVER("GPM control not setup
>>> properly.\n");
>>>    		enable_rc6 = false;
>>>    	}
>>>    

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: ✓ Ro.CI.BAT: success for series starting with [1/2] drm/i915: Fix log type for RC6 debug messages
  2016-06-29 17:06 ` ✓ Ro.CI.BAT: success for series starting with [1/2] drm/i915: Fix log type for RC6 debug messages Patchwork
@ 2016-07-01 12:03   ` Imre Deak
  0 siblings, 0 replies; 8+ messages in thread
From: Imre Deak @ 2016-07-01 12:03 UTC (permalink / raw)
  To: intel-gfx, sagar

On ke, 2016-06-29 at 17:06 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/2] drm/i915: Fix log type for RC6
> debug messages
> URL   : https://patchwork.freedesktop.org/series/9285/
> State : success

Thanks for the review, I pushed the patches to -dinq.

> == Summary ==
> 
> Series 9285v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/9285/revisions/1/mbox
> 
> Test kms_pipe_crc_basic:
>         Subgroup suspend-read-crc-pipe-c:
>                 dmesg-warn -> SKIP       (ro-bdw-i5-5250u)
> 
> fi-skl-i5-
> 6260u  total:229  pass:202  dwarn:0   dfail:0   fail:2   skip:25 
> fi-snb-i7-
> 2600   total:229  pass:174  dwarn:0   dfail:0   fail:2   skip:53 
> ro-bdw-i5-
> 5250u  total:229  pass:202  dwarn:2   dfail:1   fail:2   skip:22 
> ro-bdw-i7-
> 5557U  total:229  pass:202  dwarn:1   dfail:1   fail:2   skip:23 
> ro-bdw-i7-
> 5600u  total:229  pass:190  dwarn:0   dfail:1   fail:0   skip:38 
> ro-byt-
> n2820     total:229  pass:178  dwarn:0   dfail:1   fail:5   skip:45 
> ro-hsw-i3-
> 4010u  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31 
> ro-hsw-i7-
> 4770r  total:229  pass:195  dwarn:0   dfail:1   fail:2   skip:31 
> ro-ilk-i7-
> 620lm  total:229  pass:155  dwarn:0   dfail:1   fail:3   skip:70 
> ro-ilk1-i5-
> 650   total:224  pass:155  dwarn:0   dfail:1   fail:3   skip:65 
> ro-ivb-i7-
> 3770   total:229  pass:186  dwarn:0   dfail:1   fail:2   skip:40 
> ro-ivb2-i7-
> 3770  total:229  pass:190  dwarn:0   dfail:1   fail:2   skip:36 
> ro-skl3-i5-6260u
> total:229  pass:206  dwarn:1   dfail:1   fail:2   skip:19 
> ro-snb-i7-
> 2620M  total:229  pass:179  dwarn:0   dfail:1   fail:1   skip:48 
> fi-kbl-qkkr failed to connect after reboot
> fi-skl-i7-6700k failed to connect after reboot
> ro-bsw-n3050 failed to connect after reboot
> 
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1334/
> 
> 8a6521c drm-intel-nightly: 2016y-06m-29d-16h-08m-16s UTC integration
> manifest
> 4595c784 drm/i915/bxt: Fix sanity check for BIOS RC6 setup
> 52064d0 drm/i915: Fix log type for RC6 debug messages
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-07-01 12:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-29 16:13 [PATCH 1/2] drm/i915: Fix log type for RC6 debug messages Imre Deak
2016-06-29 16:13 ` [PATCH 2/2] drm/i915/bxt: Fix sanity check for BIOS RC6 setup Imre Deak
2016-07-01  6:49   ` Kamble, Sagar A
2016-07-01  9:15     ` Imre Deak
2016-07-01 10:56       ` Kamble, Sagar A
2016-06-29 17:06 ` ✓ Ro.CI.BAT: success for series starting with [1/2] drm/i915: Fix log type for RC6 debug messages Patchwork
2016-07-01 12:03   ` Imre Deak
2016-07-01  6:37 ` [PATCH 1/2] " Kamble, Sagar A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox