[Intel-gfx] [PATCH] drm/i915: add missing forcewake put on i915_wa_registers()

[Paulo Zanoni]

2014-10-22 6:56 GMT-02:00 Siluvery, Arun <arun.siluvery at linux.intel.com>:
> On 22/10/2014 08:35, Ville Syrjälä wrote:
>>
>> On Tue, Oct 21, 2014 at 07:40:35PM +0200, Daniel Vetter wrote:
>>>
>>> On Tue, Oct 21, 2014 at 02:58:08PM -0200, Paulo Zanoni wrote:
>>>>
>>>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>>>
>>>> Otherwise, a simple "cat" to the debugfs file can make the machine use
>>>> much more power than needed, and prevent it from runtime suspending.
>>>>
>>>> Related commit:
>>>>
>>>>      commit 8452e1d173a16d9812422a2272c4ab0f0ba81057
>>>>      Author: Mika Kuoppala <mika.kuoppala at linux.intel.com>
>>>>      Date:   Tue Oct 7 17:21:26 2014 +0300
>>>>          drm/i915: Build workaround list in ring initialization
>>>>
>>>> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
>>>> Cc: Arun Siluvery <arun.siluvery at linux.intel.com>
>>>> Testcase: igt/pm_rpm/debugfs-read
>>>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>>
>>>
>>> tbh I'm not even sure we want to do the manual forcewake get here -
>>> I915_READ will do it for us, and this is a debug interface. So no one
>>> should care about perf. Mika, is that right? If so I'd like to merge the
>>> inverse patch which drops the fw_get.
>>
>>
>> Don't we still need the idle msg disable+poll CSPWRFSM trick here on
>> gen8? That also needs forcewake around it.
>>
>
> I had a chat with Mika on this yesterday and he seem to agree that forcewake
> is probably not required here. I couldn't send the patch yesterday but as
> per Ville's comments looks like we need forcewake here?

Looks like Daniel removed the forcewake get from the original patch.

>
> regards
> Arun
>
>
>>> -Daniel
>>>
>>>> ---
>>>>   drivers/gpu/drm/i915/i915_debugfs.c | 1 +
>>>>   1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>>>> b/drivers/gpu/drm/i915/i915_debugfs.c
>>>> index 9600285..36a4baa 100644
>>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>>> @@ -2671,6 +2671,7 @@ static int i915_wa_registers(struct seq_file *m,
>>>> void *unused)
>>>>                            addr, value, mask, read, ok ? "OK" : "FAIL");
>>>>         }
>>>>
>>>> +       gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
>>>>         intel_runtime_pm_put(dev_priv);
>>>>         mutex_unlock(&dev->struct_mutex);
>>>>
>>>> --
>>>> 2.1.1
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>>
>>>
>>> --
>>> Daniel Vetter
>>> Software Engineer, Intel Corporation
>>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni