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

[Kamble, Sagar A]

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 at 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 at intel.com>
>>> Signed-off-by: Imre Deak <imre.deak at 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;
>>>    	}
>>>