Re: [PATCH 01/11] drm/i915: Accurately track when we mark the hardware as idle/busy

Paulo Zanoni
2014-02-21T17:04:34Z

2014-02-21 13:55 GMT-03:00 Chris Wilson <chris@chris-wilson.co.uk>:
> On Fri, Feb 21, 2014 at 01:52:18PM -0300, Paulo Zanoni wrote:
>> From: Chris Wilson <chris@chris-wilson.co.uk>
>>
>> We currently call intel_mark_idle() too often, as we do so as a
>> side-effect of processing the request queue. However, we the calls to
>> intel_mark_idle() are expected to be paired with a call to
>> intel_mark_busy() (or else we try to idle the hardware by accessing
>> registers that are already disabled). Make the idle/busy tracking
>> explicit to prevent the multiple calls.
>>
>> v2: From Paulo
>> - Make it compile
>> - Drop the __i915_add_request chunk
>>
>> Reported-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 8 ++++++++
>> drivers/gpu/drm/i915/intel_display.c | 9 +++++++++
>> 2 files changed, 17 insertions(+)
>>
>>
>> Chris did not reply to my review comments yet, so I just went and implemented
>> them. We need at least an ACK form him here before merging.
>
> Didn't see them... Why have you altered the logic?

See the comment at the __i915_add_request chunk:

http://lists.freedesktop.org/archives/intel-gfx/2014-February/040334.html

Maybe I just broke your patch :)
If my review doesn't make sense, we can stick to your version, it
should do the job, and I can retest everything easily.

> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre

--
Paulo Zanoni