Having recently fixed a few ordering/lifetime bugs in this area, it
seems worthwhile putting a few more checks and warnings here, to provide
early detection of any future code changes that cause problems;
especially as we hope to rework the pinning/refcounting of the kernel
context soon.

Specifically, we want to check that teardown_status_page is called
*before* the underlying storage is freed, and that by the time
intel_logical_ring_cleanup() is called, the status page mapping have
already been released. We'll also take care to undo the status page
kmapping only if it has been set up.

Signed-off-by: Dave Gordon <david.s.gor...@intel.com>
Cc: Mika Kuoppala <mika.kuopp...@linux.intel.com>
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lrc.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 95ba8ec..b4deaca 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2003,6 +2003,9 @@ void intel_logical_ring_cleanup(struct intel_engine_cs 
*ring)
        i915_cmd_parser_fini_ring(ring);
        i915_gem_batch_pool_fini(&ring->batch_pool);
 
+       /* Status page should have gone already */
+       WARN_ON(ring->status_page.page_addr);
+       WARN_ON(ring->status_page.obj);
 
        ring->disable_lite_restore_wa = false;
        ring->ctx_desc_template = 0;
@@ -2446,6 +2449,10 @@ void intel_lr_context_free(struct intel_context *ctx)
                        continue;
 
                if (ctx == ctx->i915->kernel_context) {
+                       /*
+                        * The HWSP is part of the kernel context
+                        * object in LRC mode, so tear it down *now*
+                        */
                        lrc_teardown_hardware_status_page(ringbuf->ring);
                        intel_unpin_ringbuffer_obj(ringbuf);
                        i915_gem_object_ggtt_unpin(ctx_obj);
@@ -2517,12 +2524,19 @@ static void lrc_setup_hardware_status_page(struct 
intel_engine_cs *ring)
        POSTING_READ(RING_HWS_PGA(ring->mmio_base));
 }
 
+/* This should be called *before* the default context is destroyed */
 static void lrc_teardown_hardware_status_page(struct intel_engine_cs *ring)
 {
-       if (ring->status_page.obj) {
+       struct drm_i915_gem_object *dctx_obj = ring->status_page.obj;
+
+       WARN_ON(!dctx_obj);
+
+       if (ring->status_page.page_addr) {
                kunmap(kmap_to_page(ring->status_page.page_addr));
-               ring->status_page.obj = NULL;
+               ring->status_page.page_addr = NULL;
        }
+
+       ring->status_page.obj = NULL;
 }
 
 /**
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to