* [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* 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
* ✓ 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: ✓ 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
* 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
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