On 12/01/2016 14:04, Daniel Vetter wrote:
On Tue, Jan 12, 2016 at 11:50:34AM +0000, John Harrison wrote:
On 12/01/2016 11:28, Chris Wilson wrote:
On Tue, Jan 12, 2016 at 11:11:20AM +0000, John Harrison wrote:
On 12/01/2016 00:20, Chris Wilson wrote:
On Mon, Jan 11, 2016 at 06:42:31PM +0000, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>
A later patch in this series re-organises the batch buffer submission
code. Part of that is to reduce the scope of a pm_get/put pair.
Specifically, they previously wrapped the entire submission path from
the very start to the very end, now they only wrap the actual hardware
submission part in the back half.
However, as you haven't fixed the ordering issue that requires rpm_get
before struct_mutex, this is broken.
Why does 'intel_runtime_pm_get' require the struct mutex to be held?
It has certainly not complained at me about trying to do stuff
without it.
Because it depends upon the struct_mutex and rpm doesn't have sufficient
lockdep integration to be able to warn about using rpm from the
incorrect contexts.
Where? What does the 'pm_runtime_get_sync' call turn into? There are already
other places in the driver which call intel_runtime_pm_get() immediately
after grabbing the mutex lock. Also, the description comment for _pm_get()
does not mention anything about mutexes at all.
If you nest rpm_get within dev->struct_mutex that's a bug and could
deadlock. Where does this happen? And for any such place we need a new
subtest in pm_rpm.
The first two hits when grepping the driver are in
'i915_gem_seqno_info()' and 'i915_interrupt_info()' in i915_debugfs.c.
Both say:
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
intel_runtime_pm_get(dev_priv);
-Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
