[PATCH 1/2] drm/i915: Replace vma parking with a clock aging algorithm

Chris Wilson chris at chris-wilson.co.uk
Mon Oct 21 17:32:48 UTC 2019


We cache the user's vma for a brief period of time after they close them
so that if they are immediately reopened we avoid having to unbind and
rebind them. This happens quite frequently for display servers which
only keep a client's frame open for as long as they are copying from it,
and so they open/close every vma about 30 Hz (every other frame for
double buffering).

Our current strategy is to keep the vma alive until the next global idle
point. However this cache should be purely temporal, so switch over from
using the parked notifier to using its own clock based aging algorithm:
if the closed vma is not reused within 2 clock ticks, it is destroyed.

Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_pm.c   |  1 -
 drivers/gpu/drm/i915/gt/intel_gt.c       |  3 --
 drivers/gpu/drm/i915/gt/intel_gt_types.h |  3 --
 drivers/gpu/drm/i915/i915_debugfs.c      |  2 +
 drivers/gpu/drm/i915/i915_drv.c          |  4 +-
 drivers/gpu/drm/i915/i915_drv.h          |  1 +
 drivers/gpu/drm/i915/i915_vma.c          | 51 +++++++++++++++++-------
 drivers/gpu/drm/i915/i915_vma.h          | 12 +++++-
 8 files changed, 53 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
index 7987b54fb1f5..2aa7e9be088f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
@@ -23,7 +23,6 @@ static int pm_notifier(struct notifier_block *nb,
 		break;
 
 	case INTEL_GT_PARK:
-		i915_vma_parked(i915);
 		break;
 	}
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 1c4b6c9642ad..84a1f589fdcc 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -19,9 +19,6 @@ void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915)
 
 	spin_lock_init(&gt->irq_lock);
 
-	INIT_LIST_HEAD(&gt->closed_vma);
-	spin_lock_init(&gt->closed_lock);
-
 	intel_gt_init_hangcheck(gt);
 	intel_gt_init_reset(gt);
 	intel_gt_init_requests(gt);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index ae4aaf75ac78..467e4c60d70c 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -65,9 +65,6 @@ struct intel_gt {
 	struct intel_wakeref wakeref;
 	atomic_t user_wakeref;
 
-	struct list_head closed_vma;
-	spinlock_t closed_lock; /* guards the list of closed_vma */
-
 	struct intel_hangcheck hangcheck;
 	struct intel_reset reset;
 
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index ada57eee914a..c15001d6ee8f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3676,6 +3676,8 @@ i915_drop_caches_set(void *data, u64 val)
 		ret = intel_gt_pm_wait_for_idle(gt);
 		if (ret)
 			return ret;
+
+		i915_vma_clock_flush(&i915->vma_clock);
 	}
 
 	if (val & DROP_RESET_ACTIVE && intel_gt_terminally_wedged(gt))
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 21b36a6143b9..5345e371bed2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -517,8 +517,8 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
 
 	intel_wopcm_init_early(&dev_priv->wopcm);
 
+	i915_vma_clock_init_early(&dev_priv->vma_clock);
 	intel_gt_init_early(&dev_priv->gt, dev_priv);
-
 	i915_gem_init_early(dev_priv);
 
 	/* This must be called before any calls to HAS_PCH_* */
@@ -555,6 +555,8 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv)
  */
 static void i915_driver_late_release(struct drm_i915_private *dev_priv)
 {
+	i915_vma_clock_flush(&dev_priv->vma_clock);
+
 	intel_irq_fini(dev_priv);
 	intel_power_domains_cleanup(dev_priv);
 	i915_gem_cleanup_early(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8882c0908c3b..d891d8e205af 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1332,6 +1332,7 @@ struct drm_i915_private {
 	struct intel_runtime_pm runtime_pm;
 
 	struct i915_perf perf;
+	struct i915_vma_clock vma_clock;
 
 	/* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
 	struct intel_gt gt;
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index e90c4d0af8fd..536340dd1e92 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -945,7 +945,7 @@ int i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 
 void i915_vma_close(struct i915_vma *vma)
 {
-	struct drm_i915_private *i915 = vma->vm->i915;
+	struct i915_vma_clock *clock = &vma->vm->i915->vma_clock;
 	unsigned long flags;
 
 	GEM_BUG_ON(i915_vma_is_closed(vma));
@@ -962,18 +962,20 @@ void i915_vma_close(struct i915_vma *vma)
 	 * causing us to rebind the VMA once more. This ends up being a lot
 	 * of wasted work for the steady state.
 	 */
-	spin_lock_irqsave(&i915->gt.closed_lock, flags);
-	list_add(&vma->closed_link, &i915->gt.closed_vma);
-	spin_unlock_irqrestore(&i915->gt.closed_lock, flags);
+	spin_lock_irqsave(&clock->lock, flags);
+	list_add(&vma->closed_link, &clock->age[0]);
+	if (!timer_pending(&clock->timer))
+		mod_timer(&clock->timer, round_jiffies_up_relative(HZ));
+	spin_unlock_irqrestore(&clock->lock, flags);
 }
 
 static void __i915_vma_remove_closed(struct i915_vma *vma)
 {
-	struct drm_i915_private *i915 = vma->vm->i915;
+	struct i915_vma_clock *clock = &vma->vm->i915->vma_clock;
 
-	spin_lock_irq(&i915->gt.closed_lock);
+	spin_lock_irq(&clock->lock);
 	list_del_init(&vma->closed_link);
-	spin_unlock_irq(&i915->gt.closed_lock);
+	spin_unlock_irq(&clock->lock);
 }
 
 void i915_vma_reopen(struct i915_vma *vma)
@@ -1009,12 +1011,13 @@ void i915_vma_destroy(struct i915_vma *vma)
 	i915_vma_free(vma);
 }
 
-void i915_vma_parked(struct drm_i915_private *i915)
+static void i915_vma_clock(struct timer_list *t)
 {
+	struct i915_vma_clock *clock = from_timer(clock, t, timer);
 	struct i915_vma *vma, *next;
 
-	spin_lock_irq(&i915->gt.closed_lock);
-	list_for_each_entry_safe(vma, next, &i915->gt.closed_vma, closed_link) {
+	spin_lock_irq(&clock->lock);
+	list_for_each_entry_safe(vma, next, &clock->age[1], closed_link) {
 		struct drm_i915_gem_object *obj = vma->obj;
 		struct i915_address_space *vm = vma->vm;
 
@@ -1028,7 +1031,7 @@ void i915_vma_parked(struct drm_i915_private *i915)
 			obj = NULL;
 		}
 
-		spin_unlock_irq(&i915->gt.closed_lock);
+		spin_unlock_irq(&clock->lock);
 
 		if (obj) {
 			i915_vma_destroy(vma);
@@ -1038,11 +1041,14 @@ void i915_vma_parked(struct drm_i915_private *i915)
 		i915_vm_close(vm);
 
 		/* Restart after dropping lock */
-		spin_lock_irq(&i915->gt.closed_lock);
-		next = list_first_entry(&i915->gt.closed_vma,
+		spin_lock_irq(&clock->lock);
+		next = list_first_entry(&clock->age[1],
 					typeof(*next), closed_link);
 	}
-	spin_unlock_irq(&i915->gt.closed_lock);
+	list_splice_tail_init(&clock->age[0], &clock->age[1]);
+	if (!list_empty(&clock->age[1]))
+		mod_timer(&clock->timer, round_jiffies_up_relative(HZ));
+	spin_unlock_irq(&clock->lock);
 }
 
 static void __i915_vma_iounmap(struct i915_vma *vma)
@@ -1224,6 +1230,23 @@ void i915_vma_make_purgeable(struct i915_vma *vma)
 	i915_gem_object_make_purgeable(vma->obj);
 }
 
+void i915_vma_clock_init_early(struct i915_vma_clock *clock)
+{
+	spin_lock_init(&clock->lock);
+	INIT_LIST_HEAD(&clock->age[0]);
+	INIT_LIST_HEAD(&clock->age[1]);
+
+	timer_setup(&clock->timer, i915_vma_clock, 0);
+}
+
+void i915_vma_clock_flush(struct i915_vma_clock *clock)
+{
+	do {
+		if (del_timer_sync(&clock->timer))
+			i915_vma_clock(&clock->timer);
+	} while (timer_pending(&clock->timer));
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftests/i915_vma.c"
 #endif
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 858908e3d1cc..5679c518cd2b 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -39,6 +39,7 @@
 
 enum i915_cache_level;
 
+
 /**
  * DOC: Virtual Memory Address
  *
@@ -462,8 +463,6 @@ i915_vma_unpin_fence(struct i915_vma *vma)
 		__i915_vma_unpin_fence(vma);
 }
 
-void i915_vma_parked(struct drm_i915_private *i915);
-
 #define for_each_until(cond) if (cond) break; else
 
 /**
@@ -492,4 +491,13 @@ static inline int i915_vma_sync(struct i915_vma *vma)
 	return i915_active_wait(&vma->active);
 }
 
+struct i915_vma_clock {
+	spinlock_t lock;
+	struct list_head age[2];
+	struct timer_list timer;
+};
+
+void i915_vma_clock_init_early(struct i915_vma_clock *clock);
+void i915_vma_clock_flush(struct i915_vma_clock *clock);
+
 #endif
-- 
2.24.0.rc0



More information about the Intel-gfx-trybot mailing list