[Intel-gfx] [PATCH v4 02/38] drm/i915: Explicit power enable during deferred context initialisation

[Daniel Vetter]

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 at Intel.com wrote:
> >>>>From: John Harrison <John.C.Harrison at Intel.com>
> >>>>
> >>>>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.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch