Lift moving the timeline to/from the active_list on enter/exit in order
to shorten the active tracking span in comparison to the existing
pin/unpin.

Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Reviewed-by: Matthew Auld <matthew.a...@intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_pm.c        |  1 -
 drivers/gpu/drm/i915/gt/intel_context.c       |  2 +
 drivers/gpu/drm/i915/gt/intel_engine_pm.c     |  2 +
 drivers/gpu/drm/i915/gt/intel_lrc.c           |  4 +
 drivers/gpu/drm/i915/gt/intel_timeline.c      | 98 +++++++------------
 drivers/gpu/drm/i915/gt/intel_timeline.h      |  3 +-
 .../gpu/drm/i915/gt/intel_timeline_types.h    | 18 ++++
 drivers/gpu/drm/i915/gt/selftest_timeline.c   |  2 -
 8 files changed, 64 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c 
b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
index 17e3618241c5..92e53c25424c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
@@ -37,7 +37,6 @@ static void i915_gem_park(struct drm_i915_private *i915)
        for_each_engine(engine, i915, id)
                call_idle_barriers(engine); /* cleanup after wedging */
 
-       intel_timelines_park(i915);
        i915_vma_parked(i915);
 
        i915_globals_park();
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c 
b/drivers/gpu/drm/i915/gt/intel_context.c
index 77833f1558a9..9114953bf920 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -280,10 +280,12 @@ int __init i915_global_context_init(void)
 void intel_context_enter_engine(struct intel_context *ce)
 {
        intel_engine_pm_get(ce->engine);
+       intel_timeline_enter(ce->timeline);
 }
 
 void intel_context_exit_engine(struct intel_context *ce)
 {
+       intel_timeline_exit(ce->timeline);
        intel_engine_pm_put(ce->engine);
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c 
b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 49ad02c3720f..f3f0109f9e22 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -66,6 +66,8 @@ static bool switch_to_kernel_context(struct intel_engine_cs 
*engine)
                /* Context switch failed, hope for the best! Maybe reset? */
                return true;
 
+       intel_timeline_enter(rq->timeline);
+
        /* Check again on the next retirement. */
        engine->wakeref_serial = engine->serial + 1;
        i915_request_add_active_barriers(rq);
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index a5d9b902d6e3..b4c2662dbc75 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -3306,6 +3306,8 @@ static void virtual_context_enter(struct intel_context 
*ce)
 
        for (n = 0; n < ve->num_siblings; n++)
                intel_engine_pm_get(ve->siblings[n]);
+
+       intel_timeline_enter(ce->timeline);
 }
 
 static void virtual_context_exit(struct intel_context *ce)
@@ -3313,6 +3315,8 @@ static void virtual_context_exit(struct intel_context *ce)
        struct virtual_engine *ve = container_of(ce, typeof(*ve), context);
        unsigned int n;
 
+       intel_timeline_exit(ce->timeline);
+
        for (n = 0; n < ve->num_siblings; n++)
                intel_engine_pm_put(ve->siblings[n]);
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.c 
b/drivers/gpu/drm/i915/gt/intel_timeline.c
index 6daa9eb59e19..4af0b9801d91 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.c
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.c
@@ -278,64 +278,11 @@ void intel_timelines_init(struct drm_i915_private *i915)
        timelines_init(&i915->gt);
 }
 
-static void timeline_add_to_active(struct intel_timeline *tl)
-{
-       struct intel_gt_timelines *gt = &tl->gt->timelines;
-
-       mutex_lock(&gt->mutex);
-       list_add(&tl->link, &gt->active_list);
-       mutex_unlock(&gt->mutex);
-}
-
-static void timeline_remove_from_active(struct intel_timeline *tl)
-{
-       struct intel_gt_timelines *gt = &tl->gt->timelines;
-
-       mutex_lock(&gt->mutex);
-       list_del(&tl->link);
-       mutex_unlock(&gt->mutex);
-}
-
-static void timelines_park(struct intel_gt *gt)
-{
-       struct intel_gt_timelines *timelines = &gt->timelines;
-       struct intel_timeline *timeline;
-
-       mutex_lock(&timelines->mutex);
-       list_for_each_entry(timeline, &timelines->active_list, link) {
-               /*
-                * All known fences are completed so we can scrap
-                * the current sync point tracking and start afresh,
-                * any attempt to wait upon a previous sync point
-                * will be skipped as the fence was signaled.
-                */
-               i915_syncmap_free(&timeline->sync);
-       }
-       mutex_unlock(&timelines->mutex);
-}
-
-/**
- * intel_timelines_park - called when the driver idles
- * @i915: the drm_i915_private device
- *
- * When the driver is completely idle, we know that all of our sync points
- * have been signaled and our tracking is then entirely redundant. Any request
- * to wait upon an older sync point will be completed instantly as we know
- * the fence is signaled and therefore we will not even look them up in the
- * sync point map.
- */
-void intel_timelines_park(struct drm_i915_private *i915)
-{
-       timelines_park(&i915->gt);
-}
-
 void intel_timeline_fini(struct intel_timeline *timeline)
 {
        GEM_BUG_ON(timeline->pin_count);
        GEM_BUG_ON(!list_empty(&timeline->requests));
 
-       i915_syncmap_free(&timeline->sync);
-
        if (timeline->hwsp_cacheline)
                cacheline_free(timeline->hwsp_cacheline);
        else
@@ -370,6 +317,7 @@ int intel_timeline_pin(struct intel_timeline *tl)
        if (tl->pin_count++)
                return 0;
        GEM_BUG_ON(!tl->pin_count);
+       GEM_BUG_ON(tl->active_count);
 
        err = i915_vma_pin(tl->hwsp_ggtt, 0, 0, PIN_GLOBAL | PIN_HIGH);
        if (err)
@@ -380,7 +328,6 @@ int intel_timeline_pin(struct intel_timeline *tl)
                offset_in_page(tl->hwsp_offset);
 
        cacheline_acquire(tl->hwsp_cacheline);
-       timeline_add_to_active(tl);
 
        return 0;
 
@@ -389,6 +336,40 @@ int intel_timeline_pin(struct intel_timeline *tl)
        return err;
 }
 
+void intel_timeline_enter(struct intel_timeline *tl)
+{
+       struct intel_gt_timelines *timelines = &tl->gt->timelines;
+
+       GEM_BUG_ON(!tl->pin_count);
+       if (tl->active_count++)
+               return;
+       GEM_BUG_ON(!tl->active_count); /* overflow? */
+
+       mutex_lock(&timelines->mutex);
+       list_add(&tl->link, &timelines->active_list);
+       mutex_unlock(&timelines->mutex);
+}
+
+void intel_timeline_exit(struct intel_timeline *tl)
+{
+       struct intel_gt_timelines *timelines = &tl->gt->timelines;
+
+       GEM_BUG_ON(!tl->active_count);
+       if (--tl->active_count)
+               return;
+
+       mutex_lock(&timelines->mutex);
+       list_del(&tl->link);
+       mutex_unlock(&timelines->mutex);
+
+       /*
+        * Since this timeline is idle, all bariers upon which we were waiting
+        * must also be complete and so we can discard the last used barriers
+        * without loss of information.
+        */
+       i915_syncmap_free(&tl->sync);
+}
+
 static u32 timeline_advance(struct intel_timeline *tl)
 {
        GEM_BUG_ON(!tl->pin_count);
@@ -546,16 +527,9 @@ void intel_timeline_unpin(struct intel_timeline *tl)
        if (--tl->pin_count)
                return;
 
-       timeline_remove_from_active(tl);
+       GEM_BUG_ON(tl->active_count);
        cacheline_release(tl->hwsp_cacheline);
 
-       /*
-        * Since this timeline is idle, all bariers upon which we were waiting
-        * must also be complete and so we can discard the last used barriers
-        * without loss of information.
-        */
-       i915_syncmap_free(&tl->sync);
-
        __i915_vma_unpin(tl->hwsp_ggtt);
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline.h 
b/drivers/gpu/drm/i915/gt/intel_timeline.h
index e08cebf64833..f583af1ba18d 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline.h
+++ b/drivers/gpu/drm/i915/gt/intel_timeline.h
@@ -77,9 +77,11 @@ static inline bool intel_timeline_sync_is_later(struct 
intel_timeline *tl,
 }
 
 int intel_timeline_pin(struct intel_timeline *tl);
+void intel_timeline_enter(struct intel_timeline *tl);
 int intel_timeline_get_seqno(struct intel_timeline *tl,
                             struct i915_request *rq,
                             u32 *seqno);
+void intel_timeline_exit(struct intel_timeline *tl);
 void intel_timeline_unpin(struct intel_timeline *tl);
 
 int intel_timeline_read_hwsp(struct i915_request *from,
@@ -87,7 +89,6 @@ int intel_timeline_read_hwsp(struct i915_request *from,
                             u32 *hwsp_offset);
 
 void intel_timelines_init(struct drm_i915_private *i915);
-void intel_timelines_park(struct drm_i915_private *i915);
 void intel_timelines_fini(struct drm_i915_private *i915);
 
 #endif
diff --git a/drivers/gpu/drm/i915/gt/intel_timeline_types.h 
b/drivers/gpu/drm/i915/gt/intel_timeline_types.h
index 9a71aea7a338..b1a9f0c54bf0 100644
--- a/drivers/gpu/drm/i915/gt/intel_timeline_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_timeline_types.h
@@ -25,7 +25,25 @@ struct intel_timeline {
 
        struct mutex mutex; /* protects the flow of requests */
 
+       /*
+        * pin_count and active_count track essentially the same thing:
+        * How many requests are in flight or may be under construction.
+        *
+        * We need two distinct counters so that we can assign different
+        * lifetimes to the events for different use-cases. For example,
+        * we want to permanently keep the timeline pinned for the kernel
+        * context so that we can issue requests at any time without having
+        * to acquire space in the GGTT. However, we want to keep tracking
+        * the activity (to be able to detect when we become idle) along that
+        * permanently pinned timeline and so end up requiring two counters.
+        *
+        * Note that the active_count is protected by the intel_timeline.mutex,
+        * but the pin_count is protected by a combination of serialisation
+        * from the intel_context caller plus internal atomicity.
+        */
        unsigned int pin_count;
+       unsigned int active_count;
+
        const u32 *hwsp_seqno;
        struct i915_vma *hwsp_ggtt;
        u32 hwsp_offset;
diff --git a/drivers/gpu/drm/i915/gt/selftest_timeline.c 
b/drivers/gpu/drm/i915/gt/selftest_timeline.c
index f0a840030382..d54113697745 100644
--- a/drivers/gpu/drm/i915/gt/selftest_timeline.c
+++ b/drivers/gpu/drm/i915/gt/selftest_timeline.c
@@ -816,8 +816,6 @@ static int live_hwsp_recycle(void *arg)
 
                        if (err)
                                goto out;
-
-                       intel_timelines_park(i915); /* Encourage recycling! */
                } while (!__igt_timeout(end_time, NULL));
        }
 
-- 
2.23.0.rc1

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

Reply via email to