Re: [PATCH 2/2] drm/i915/bxt: Fix sanity check for BIOS RC6 setup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxx>

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@xxxxxxxxx>
Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
---
   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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux