drm/i915: Make closing request flush mandatory
authorChris Wilson <chris@chris-wilson.co.uk>
Tue, 12 Jun 2018 10:51:35 +0000 (11:51 +0100)
committerChris Wilson <chris@chris-wilson.co.uk>
Thu, 14 Jun 2018 07:16:12 +0000 (08:16 +0100)
For symmetry, simplicity and ensuring the request is always truly idle
upon its completion, always emit the closing flush prior to emitting the
request breadcrumb. Previously, we would only emit the flush if we had
started a user batch, but this just leaves all the other paths open to
speculation (do they affect the GPU caches or not?) With mm switching, a
key requirement is that the GPU is flushed and invalidated before hand,
so for absolute safety, we want that closing flush be mandatory.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180612105135.4459-1-chris@chris-wilson.co.uk
12 files changed:
drivers/gpu/drm/i915/i915_gem.c
drivers/gpu/drm/i915/i915_gem_context.c
drivers/gpu/drm/i915/i915_gem_execbuffer.c
drivers/gpu/drm/i915/i915_request.c
drivers/gpu/drm/i915/i915_request.h
drivers/gpu/drm/i915/selftests/huge_pages.c
drivers/gpu/drm/i915/selftests/i915_gem_coherency.c
drivers/gpu/drm/i915/selftests/i915_gem_context.c
drivers/gpu/drm/i915/selftests/i915_request.c
drivers/gpu/drm/i915/selftests/intel_hangcheck.c
drivers/gpu/drm/i915/selftests/intel_lrc.c
drivers/gpu/drm/i915/selftests/intel_workarounds.c

index 93efd92362dbc73c6d8504e2894675cf380afa23..8dd4d35655afc9b2aa8565bfe7dfd21611a6e22f 100644 (file)
@@ -3213,7 +3213,7 @@ void i915_gem_reset(struct drm_i915_private *dev_priv,
                        rq = i915_request_alloc(engine,
                                                dev_priv->kernel_context);
                        if (!IS_ERR(rq))
-                               __i915_request_add(rq, false);
+                               i915_request_add(rq);
                }
        }
 
@@ -5332,7 +5332,7 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
                if (engine->init_context)
                        err = engine->init_context(rq);
 
-               __i915_request_add(rq, true);
+               i915_request_add(rq);
                if (err)
                        goto err_active;
        }
index b2c7ac1b074d42f54fbfae9bc64c06787b5b0226..ef6ea4bcd7733a75c784b0e7c9d1903b5df0e2ce 100644 (file)
@@ -700,14 +700,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915)
                        i915_timeline_sync_set(rq->timeline, &prev->fence);
                }
 
-               /*
-                * Force a flush after the switch to ensure that all rendering
-                * and operations prior to switching to the kernel context hits
-                * memory. This should be guaranteed by the previous request,
-                * but an extra layer of paranoia before we declare the system
-                * idle (on suspend etc) is advisable!
-                */
-               __i915_request_add(rq, true);
+               i915_request_add(rq);
        }
 
        return 0;
index 2d2eb3075960c4103386a7d5be7e55cfde8087fe..60dc2a865f5f960ce0ca46d89fa4e8eb66a0d652 100644 (file)
@@ -921,7 +921,7 @@ static void reloc_gpu_flush(struct reloc_cache *cache)
        i915_gem_object_unpin_map(cache->rq->batch->obj);
        i915_gem_chipset_flush(cache->rq->i915);
 
-       __i915_request_add(cache->rq, true);
+       i915_request_add(cache->rq);
        cache->rq = NULL;
 }
 
@@ -2438,7 +2438,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
        trace_i915_request_queue(eb.request, eb.batch_flags);
        err = eb_submit(&eb);
 err_request:
-       __i915_request_add(eb.request, err == 0);
+       i915_request_add(eb.request);
        add_to_client(eb.request, file);
 
        if (fences)
index 9092f5464c24d7e5914a4485cfb051748bac790b..e1dbb544046fef9c6b10b28a05922f10a155f9dc 100644 (file)
@@ -1018,14 +1018,13 @@ i915_request_await_object(struct i915_request *to,
  * request is not being tracked for completion but the work itself is
  * going to happen on the hardware. This would be a Bad Thing(tm).
  */
-void __i915_request_add(struct i915_request *request, bool flush_caches)
+void i915_request_add(struct i915_request *request)
 {
        struct intel_engine_cs *engine = request->engine;
        struct i915_timeline *timeline = request->timeline;
        struct intel_ring *ring = request->ring;
        struct i915_request *prev;
        u32 *cs;
-       int err;
 
        GEM_TRACE("%s fence %llx:%d\n",
                  engine->name, request->fence.context, request->fence.seqno);
@@ -1046,20 +1045,7 @@ void __i915_request_add(struct i915_request *request, bool flush_caches)
         * know that it is time to use that space up.
         */
        request->reserved_space = 0;
-
-       /*
-        * Emit any outstanding flushes - execbuf can fail to emit the flush
-        * after having emitted the batchbuffer command. Hence we need to fix
-        * things up similar to emitting the lazy request. The difference here
-        * is that the flush _must_ happen before the next request, no matter
-        * what.
-        */
-       if (flush_caches) {
-               err = engine->emit_flush(request, EMIT_FLUSH);
-
-               /* Not allowed to fail! */
-               WARN(err, "engine->emit_flush() failed: %d!\n", err);
-       }
+       engine->emit_flush(request, EMIT_FLUSH);
 
        /*
         * Record the position of the start of the breadcrumb so that
index 0e9aba53d0e423528d2e9759c31118ba9c227aae..7ee220ded9c9aa30ad0d9c775c89c8ef6ac5ee56 100644 (file)
@@ -253,9 +253,7 @@ int i915_request_await_object(struct i915_request *to,
 int i915_request_await_dma_fence(struct i915_request *rq,
                                 struct dma_fence *fence);
 
-void __i915_request_add(struct i915_request *rq, bool flush_caches);
-#define i915_request_add(rq) \
-       __i915_request_add(rq, false)
+void i915_request_add(struct i915_request *rq);
 
 void __i915_request_submit(struct i915_request *request);
 void i915_request_submit(struct i915_request *request);
index 7846ea4a99bc6065546694970f9fd3e7ed9a8e08..fbe4324116d72b42bb401dc9c1f89b9f03ab61d2 100644 (file)
@@ -1003,7 +1003,7 @@ static int gpu_write(struct i915_vma *vma,
        reservation_object_unlock(vma->resv);
 
 err_request:
-       __i915_request_add(rq, err == 0);
+       i915_request_add(rq);
 
        return err;
 }
index 340a98c0c804a5c8203b4130a114ddf0b2ed7b20..a4900091ae3dc85cb269d5cf4ff42e6709d9547a 100644 (file)
@@ -199,7 +199,7 @@ static int gpu_set(struct drm_i915_gem_object *obj,
 
        cs = intel_ring_begin(rq, 4);
        if (IS_ERR(cs)) {
-               __i915_request_add(rq, false);
+               i915_request_add(rq);
                i915_vma_unpin(vma);
                return PTR_ERR(cs);
        }
@@ -229,7 +229,7 @@ static int gpu_set(struct drm_i915_gem_object *obj,
        reservation_object_add_excl_fence(obj->resv, &rq->fence);
        reservation_object_unlock(obj->resv);
 
-       __i915_request_add(rq, true);
+       i915_request_add(rq);
 
        return 0;
 }
index 708e8d721448868ce6d198459b2660fe74eafd96..836f1af8b833c0b32695d0e8bf5c2f92d4df7bdf 100644 (file)
@@ -182,12 +182,12 @@ static int gpu_fill(struct drm_i915_gem_object *obj,
        reservation_object_add_excl_fence(obj->resv, &rq->fence);
        reservation_object_unlock(obj->resv);
 
-       __i915_request_add(rq, true);
+       i915_request_add(rq);
 
        return 0;
 
 err_request:
-       __i915_request_add(rq, false);
+       i915_request_add(rq);
 err_batch:
        i915_vma_unpin(batch);
 err_vma:
index a3a89aadeccb0563ec2dbfd49e0bc2517bc17f02..f5d00332bb31eb70c947f04acce3425ef5dd4218 100644 (file)
@@ -466,7 +466,7 @@ empty_request(struct intel_engine_cs *engine,
                goto out_request;
 
 out_request:
-       __i915_request_add(request, err == 0);
+       i915_request_add(request);
        return err ? ERR_PTR(err) : request;
 }
 
index 390a157b37c3f5e1f52f9e6e859e71e1e574e97e..fe7d3190ebfee5ec91b3fd668bb0bf3bc1b416bd 100644 (file)
@@ -245,7 +245,7 @@ hang_create_request(struct hang *h, struct intel_engine_cs *engine)
 
        err = emit_recurse_batch(h, rq);
        if (err) {
-               __i915_request_add(rq, false);
+               i915_request_add(rq);
                return ERR_PTR(err);
        }
 
@@ -318,7 +318,7 @@ static int igt_hang_sanitycheck(void *arg)
                *h.batch = MI_BATCH_BUFFER_END;
                i915_gem_chipset_flush(i915);
 
-               __i915_request_add(rq, true);
+               i915_request_add(rq);
 
                timeout = i915_request_wait(rq,
                                            I915_WAIT_LOCKED,
@@ -464,7 +464,7 @@ static int __igt_reset_engine(struct drm_i915_private *i915, bool active)
                                }
 
                                i915_request_get(rq);
-                               __i915_request_add(rq, true);
+                               i915_request_add(rq);
                                mutex_unlock(&i915->drm.struct_mutex);
 
                                if (!wait_until_running(&h, rq)) {
@@ -742,7 +742,7 @@ static int __igt_reset_engines(struct drm_i915_private *i915,
                                }
 
                                i915_request_get(rq);
-                               __i915_request_add(rq, true);
+                               i915_request_add(rq);
                                mutex_unlock(&i915->drm.struct_mutex);
 
                                if (!wait_until_running(&h, rq)) {
@@ -942,7 +942,7 @@ static int igt_wait_reset(void *arg)
        }
 
        i915_request_get(rq);
-       __i915_request_add(rq, true);
+       i915_request_add(rq);
 
        if (!wait_until_running(&h, rq)) {
                struct drm_printer p = drm_info_printer(i915->drm.dev);
@@ -1037,7 +1037,7 @@ static int igt_reset_queue(void *arg)
                }
 
                i915_request_get(prev);
-               __i915_request_add(prev, true);
+               i915_request_add(prev);
 
                count = 0;
                do {
@@ -1051,7 +1051,7 @@ static int igt_reset_queue(void *arg)
                        }
 
                        i915_request_get(rq);
-                       __i915_request_add(rq, true);
+                       i915_request_add(rq);
 
                        /*
                         * XXX We don't handle resetting the kernel context
@@ -1184,7 +1184,7 @@ static int igt_handle_error(void *arg)
        }
 
        i915_request_get(rq);
-       __i915_request_add(rq, true);
+       i915_request_add(rq);
 
        if (!wait_until_running(&h, rq)) {
                struct drm_printer p = drm_info_printer(i915->drm.dev);
index 0b6da08c8caec6c53f3060a55d4a7ec4d1715dfd..ea27c7cfbf9685fac867538e24c3acd27cb85a33 100644 (file)
@@ -155,7 +155,7 @@ spinner_create_request(struct spinner *spin,
 
        err = emit_recurse_batch(spin, rq, arbitration_command);
        if (err) {
-               __i915_request_add(rq, false);
+               i915_request_add(rq);
                return ERR_PTR(err);
        }
 
index f1cfb0fb6bea42f75ad09c03d7a771a3f1e50b52..e1ea2d2bedd2f0e9280a1e7b560b37b558e846bf 100644 (file)
@@ -75,7 +75,7 @@ read_nonprivs(struct i915_gem_context *ctx, struct intel_engine_cs *engine)
        i915_gem_object_get(result);
        i915_gem_object_set_active_reference(result);
 
-       __i915_request_add(rq, true);
+       i915_request_add(rq);
        i915_vma_unpin(vma);
 
        return result;