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

Kamble, Sagar A
2016-07-01T06:49:10Z

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;
> }