Re: [PATCH 1/6] drm/i915: Bring UP Power Wells before disabling RC6.

["S, Deepak" <deepak.s@intel.com>]

On 4/8/2014 6:13 PM, Ville Syrjälä wrote:
> On Mon, Apr 07, 2014 at 02:36:20PM -0700, Ben Widawsky wrote:
>> On Mon, Apr 07, 2014 at 05:01:46PM -0300, Rodrigo Vivi wrote:
>>> From: Deepak S <deepak.s@intel.com>
>>>
>>> We need do forcewake before Disabling RC6, This is what the BIOS
>>> expects while going into suspend.
>>>
>>> v2: updated commit message. (Daniel)
>>>
>>> Signed-off-by: Deepak S <deepak.s@intel.com>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_pm.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index 04af065..ad2ff99 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -3198,8 +3198,14 @@ static void valleyview_disable_rps(struct drm_device *dev)
>>>   {
>>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>>
>>> +	/* we're doing forcewake before Disabling RC6,
>>> +	 * This what the BIOS expects when going into suspend */
>>> +	gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
>>> +
>>>   	I915_WRITE(GEN6_RC_CONTROL, 0);
>>>
>>> +	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
>>> +
>>>   	gen6_disable_rps_interrupts(dev);
>>>   }
>>>
>>
>> Isn't the forcewake done as part of I915_WRITE sufficient?
>
> Writes don't do forcewake, nor is the register even part of the
> VLV forcewake ranges.
>
> I guess the rationale for this patche is still a bit vague. But if it's
> really needed, I wonder whether we should do this same dance for !VLV
> too? Do we have any "GPU stuck in wrong power state after suspend" type of
> bugs still around?

One of suggestion form the HW team was to Bring the wells up before we 
disable RC6 at run-time. We did see some issue when we enabled D0ix.

I think the is a good practice to make sure we bring-up the wells before 
we disable RC6. At least this avoids the cases where wells are not up 
before we can access the Next register after disable.