[Intel-gfx] [PATCH 7/7] drm/i915: GEM operations need to be done under the big lock

[Chris Wilson]

On Mon, Jan 11, 2016 at 03:00:20PM +0000, Chris Wilson wrote:
> On Mon, Jan 11, 2016 at 02:47:17PM +0000, Tvrtko Ursulin wrote:
> > 
> > On 11/01/16 14:38, Chris Wilson wrote:
> > >On Mon, Jan 11, 2016 at 02:08:41PM +0000, Tvrtko Ursulin wrote:
> > >>From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > >>
> > >>VMA creation and GEM list management need the big lock.
> > >>
> > >>v2:
> > >>
> > >>Mutex unlock ended on the wrong path somehow. (0-day, Julia Lawall)
> > >>
> > >>Not to mention drm_gem_object_unreference was there in existing
> > >>code with no mutex held.
> > >>
> > >>v3:
> > >>
> > >>Some callers of i915_gem_object_create_stolen_for_preallocated
> > >>already hold the lock so move the mutex into the other caller
> > >>as well.
> > >
> > >intel_pm.c valleyview_setup_pctx?
> > 
> > Already holds it traced by the WARN_ON at its top.
> 
> Which is a nice little mutex inversion of its own. :|
> i.e. rpm vs struct_mutex bug

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b1fb43fcfeea..c8f684f8799c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16018,9 +16018,7 @@ void intel_modeset_gem_init(struct drm_device *dev)
        struct drm_crtc *c;
        struct drm_i915_gem_object *obj;
 
-       mutex_lock(&dev->struct_mutex);
        intel_init_gt_powersave(dev);
-       mutex_unlock(&dev->struct_mutex);
 
        intel_modeset_init_hw(dev);
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a082b4577599..90e5bf7a2402 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5224,8 +5224,6 @@ static void cherryview_setup_pctx(struct drm_device *dev)
        u32 pcbr;
        int pctx_size = 32*1024;
 
-       WARN_ON(!mutex_is_locked(&dev->struct_mutex));
-
        pcbr = I915_READ(VLV_PCBR);
        if ((pcbr >> VLV_PCBR_ADDR_SHIFT) == 0) {
                DRM_DEBUG_DRIVER("BIOS didn't set up PCBR, fixing up\n");
@@ -5247,8 +5245,7 @@ static void valleyview_setup_pctx(struct drm_device *dev)
        u32 pcbr;
        int pctx_size = 24*1024;
 
-       WARN_ON(!mutex_is_locked(&dev->struct_mutex));
-
+       mutex_lock(&dev->struct_mutex);
        pcbr = I915_READ(VLV_PCBR);
        if (pcbr) {
                /* BIOS set it up already, grab the pre-alloc'd space */
@@ -5275,7 +5272,7 @@ static void valleyview_setup_pctx(struct drm_device *dev)
        pctx = i915_gem_object_create_stolen(dev, pctx_size);
        if (!pctx) {
                DRM_DEBUG("not enough stolen space for PCTX, disabling\n");
-               return;
+               goto out;
        }
 
        pctx_paddr = dev_priv->mm.stolen_base + pctx->stolen->start;
@@ -5284,6 +5281,7 @@ static void valleyview_setup_pctx(struct drm_device *dev)
 out:
        DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR));
        dev_priv->vlv_pctx = pctx;
+       mutex_unlock(&dev->struct_mutex);
 }
 
 static void valleyview_cleanup_pctx(struct drm_device *dev)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre