]> git.proxmox.com Git - mirror_ubuntu-bionic-kernel.git/blobdiff - drivers/gpu/drm/i915/i915_gem.c
drm/i915: Assert that the active request hasn't been signaled
[mirror_ubuntu-bionic-kernel.git] / drivers / gpu / drm / i915 / i915_gem.c
index c8689892a89fc8218ca43ec1be7ffc9724bee5e0..48922ff454e6b02c194d6755e789fdb52de1fed9 100644 (file)
@@ -2009,8 +2009,16 @@ void i915_gem_runtime_suspend(struct drm_i915_private *dev_priv)
        for (i = 0; i < dev_priv->num_fence_regs; i++) {
                struct drm_i915_fence_reg *reg = &dev_priv->fence_regs[i];
 
-               if (WARN_ON(reg->pin_count))
-                       continue;
+               /* Ideally we want to assert that the fence register is not
+                * live at this point (i.e. that no piece of code will be
+                * trying to write through fence + GTT, as that both violates
+                * our tracking of activity and associated locking/barriers,
+                * but also is illegal given that the hw is powered down).
+                *
+                * Previously we used reg->pin_count as a "liveness" indicator.
+                * That is not sufficient, and we need a more fine-grained
+                * tool if we want to have a sanity check here.
+                */
 
                if (!reg->vma)
                        continue;
@@ -2603,6 +2611,8 @@ i915_gem_find_active_request(struct intel_engine_cs *engine)
                        continue;
 
                GEM_BUG_ON(request->engine != engine);
+               GEM_BUG_ON(test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
+                                   &request->fence.flags));
                return request;
        }
 
@@ -2633,8 +2643,20 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
        for_each_engine(engine, dev_priv, id) {
                struct drm_i915_gem_request *request;
 
+               /* Prevent request submission to the hardware until we have
+                * completed the reset in i915_gem_reset_finish(). If a request
+                * is completed by one engine, it may then queue a request
+                * to a second via its engine->irq_tasklet *just* as we are
+                * calling engine->init_hw() and also writing the ELSP.
+                * Turning off the engine->irq_tasklet until the reset is over
+                * prevents the race.
+                */
+               tasklet_disable(&engine->irq_tasklet);
                tasklet_kill(&engine->irq_tasklet);
 
+               if (engine->irq_seqno_barrier)
+                       engine->irq_seqno_barrier(engine);
+
                if (engine_stalled(engine)) {
                        request = i915_gem_find_active_request(engine);
                        if (request && request->fence.error == -EIO)
@@ -2731,28 +2753,21 @@ static void i915_gem_reset_engine(struct intel_engine_cs *engine)
 {
        struct drm_i915_gem_request *request;
 
-       if (engine->irq_seqno_barrier)
-               engine->irq_seqno_barrier(engine);
-
        request = i915_gem_find_active_request(engine);
-       if (!request)
-               return;
+       if (request && i915_gem_reset_request(request)) {
+               DRM_DEBUG_DRIVER("resetting %s to restart from tail of request 0x%x\n",
+                                engine->name, request->global_seqno);
 
-       if (!i915_gem_reset_request(request))
-               return;
-
-       DRM_DEBUG_DRIVER("resetting %s to restart from tail of request 0x%x\n",
-                        engine->name, request->global_seqno);
+               /* If this context is now banned, skip all pending requests. */
+               if (i915_gem_context_is_banned(request->ctx))
+                       engine_skip_context(request);
+       }
 
        /* Setup the CS to resume from the breadcrumb of the hung request */
        engine->reset_hw(engine, request);
-
-       /* If this context is now banned, skip all of its pending requests. */
-       if (i915_gem_context_is_banned(request->ctx))
-               engine_skip_context(request);
 }
 
-void i915_gem_reset_finish(struct drm_i915_private *dev_priv)
+void i915_gem_reset(struct drm_i915_private *dev_priv)
 {
        struct intel_engine_cs *engine;
        enum intel_engine_id id;
@@ -2774,6 +2789,17 @@ void i915_gem_reset_finish(struct drm_i915_private *dev_priv)
        }
 }
 
+void i915_gem_reset_finish(struct drm_i915_private *dev_priv)
+{
+       struct intel_engine_cs *engine;
+       enum intel_engine_id id;
+
+       lockdep_assert_held(&dev_priv->drm.struct_mutex);
+
+       for_each_engine(engine, dev_priv, id)
+               tasklet_enable(&engine->irq_tasklet);
+}
+
 static void nop_submit_request(struct drm_i915_gem_request *request)
 {
        dma_fence_set_error(&request->fence, -EIO);
@@ -3517,7 +3543,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
        vma->display_alignment = max_t(u64, vma->display_alignment, alignment);
 
        /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */
-       if (obj->cache_dirty) {
+       if (obj->cache_dirty || obj->base.write_domain == I915_GEM_DOMAIN_CPU) {
                i915_gem_clflush_object(obj, true);
                intel_fb_obj_flush(obj, false, ORIGIN_DIRTYFB);
        }
@@ -4199,6 +4225,23 @@ static void assert_kernel_context_is_current(struct drm_i915_private *dev_priv)
                           !i915_gem_context_is_kernel(engine->last_retired_context));
 }
 
+void i915_gem_sanitize(struct drm_i915_private *i915)
+{
+       /*
+        * If we inherit context state from the BIOS or earlier occupants
+        * of the GPU, the GPU may be in an inconsistent state when we
+        * try to take over. The only way to remove the earlier state
+        * is by resetting. However, resetting on earlier gen is tricky as
+        * it may impact the display and we are uncertain about the stability
+        * of the reset, so we only reset recent machines with logical
+        * context support (that must be reset to remove any stray contexts).
+        */
+       if (HAS_HW_CONTEXTS(i915)) {
+               int reset = intel_gpu_reset(i915, ALL_ENGINES);
+               WARN_ON(reset && reset != -ENODEV);
+       }
+}
+
 int i915_gem_suspend(struct drm_i915_private *dev_priv)
 {
        struct drm_device *dev = &dev_priv->drm;
@@ -4269,10 +4312,7 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
         * machines is a good idea, we don't - just in case it leaves the
         * machine in an unusable condition.
         */
-       if (HAS_HW_CONTEXTS(dev_priv)) {
-               int reset = intel_gpu_reset(dev_priv, ALL_ENGINES);
-               WARN_ON(reset && reset != -ENODEV);
-       }
+       i915_gem_sanitize(dev_priv);
 
        return 0;
 
@@ -4347,11 +4387,24 @@ static void init_unused_rings(struct drm_i915_private *dev_priv)
        }
 }
 
-int
-i915_gem_init_hw(struct drm_i915_private *dev_priv)
+static int __i915_gem_restart_engines(void *data)
 {
+       struct drm_i915_private *i915 = data;
        struct intel_engine_cs *engine;
        enum intel_engine_id id;
+       int err;
+
+       for_each_engine(engine, i915, id) {
+               err = engine->init_hw(engine);
+               if (err)
+                       return err;
+       }
+
+       return 0;
+}
+
+int i915_gem_init_hw(struct drm_i915_private *dev_priv)
+{
        int ret;
 
        dev_priv->gt.last_init_time = ktime_get();
@@ -4397,11 +4450,9 @@ i915_gem_init_hw(struct drm_i915_private *dev_priv)
        }
 
        /* Need to do basic initialisation of all rings first: */
-       for_each_engine(engine, dev_priv, id) {
-               ret = engine->init_hw(engine);
-               if (ret)
-                       goto out;
-       }
+       ret = __i915_gem_restart_engines(dev_priv);
+       if (ret)
+               goto out;
 
        intel_mocs_init_l3cc_table(dev_priv);
 
@@ -4490,6 +4541,11 @@ out_unlock:
        return ret;
 }
 
+void i915_gem_init_mmio(struct drm_i915_private *i915)
+{
+       i915_gem_sanitize(i915);
+}
+
 void
 i915_gem_cleanup_engines(struct drm_i915_private *dev_priv)
 {
@@ -4605,7 +4661,9 @@ err_out:
 
 void i915_gem_load_cleanup(struct drm_i915_private *dev_priv)
 {
+       i915_gem_drain_freed_objects(dev_priv);
        WARN_ON(!llist_empty(&dev_priv->mm.free_list));
+       WARN_ON(dev_priv->mm.object_count);
 
        mutex_lock(&dev_priv->drm.struct_mutex);
        i915_gem_timeline_fini(&dev_priv->gt.global_timeline);
@@ -4623,14 +4681,10 @@ void i915_gem_load_cleanup(struct drm_i915_private *dev_priv)
 
 int i915_gem_freeze(struct drm_i915_private *dev_priv)
 {
-       intel_runtime_pm_get(dev_priv);
-
        mutex_lock(&dev_priv->drm.struct_mutex);
        i915_gem_shrink_all(dev_priv);
        mutex_unlock(&dev_priv->drm.struct_mutex);
 
-       intel_runtime_pm_put(dev_priv);
-
        return 0;
 }