Commit b1e3177b authored by Chris Wilson's avatar Chris Wilson
Browse files

drm/i915: Coordinate i915_active with its own mutex



Forgo the struct_mutex serialisation for i915_active, and interpose its
own mutex handling for active/retire.

This is a multi-layered sleight-of-hand. First, we had to ensure that no
active/retire callbacks accidentally inverted the mutex ordering rules,
nor assumed that they were themselves serialised by struct_mutex. More
challenging though, is the rule over updating elements of the active
rbtree. Instead of the whole i915_active now being serialised by
struct_mutex, allocations/rotations of the tree are serialised by the
i915_active.mutex and individual nodes are serialised by the caller
using the i915_timeline.mutex (we need to use nested spinlocks to
interact with the dma_fence callback lists).

The pain point here is that instead of a single mutex around execbuf, we
now have to take a mutex for active tracker (one for each vma, context,
etc) and a couple of spinlocks for each fence update. The improvement in
fine grained locking allowing for multiple concurrent clients
(eventually!) should be worth it in typical loads.

v2: Add some comments that barely elucidate anything :(

Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191004134015.13204-6-chris@chris-wilson.co.uk
parent 274cbf20
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -257,7 +257,7 @@ intel_frontbuffer_get(struct drm_i915_gem_object *obj)
	front->obj = obj;
	kref_init(&front->ref);
	atomic_set(&front->bits, 0);
	i915_active_init(i915, &front->write,
	i915_active_init(&front->write,
			 frontbuffer_active,
			 i915_active_may_sleep(frontbuffer_retire));

+1 −2
Original line number Diff line number Diff line
@@ -1360,8 +1360,7 @@ void intel_overlay_setup(struct drm_i915_private *dev_priv)
	overlay->contrast = 75;
	overlay->saturation = 146;

	i915_active_init(dev_priv,
			 &overlay->last_flip,
	i915_active_init(&overlay->last_flip,
			 NULL, intel_overlay_last_flip_retire);

	ret = get_registers(overlay, OVERLAY_NEEDS_PHYSICAL(dev_priv));
+1 −3
Original line number Diff line number Diff line
@@ -868,20 +868,18 @@ static int context_barrier_task(struct i915_gem_context *ctx,
				void (*task)(void *data),
				void *data)
{
	struct drm_i915_private *i915 = ctx->i915;
	struct context_barrier_task *cb;
	struct i915_gem_engines_iter it;
	struct intel_context *ce;
	int err = 0;

	lockdep_assert_held(&i915->drm.struct_mutex);
	GEM_BUG_ON(!task);

	cb = kmalloc(sizeof(*cb), GFP_KERNEL);
	if (!cb)
		return -ENOMEM;

	i915_active_init(i915, &cb->base, NULL, cb_retire);
	i915_active_init(&cb->base, NULL, cb_retire);
	err = i915_active_acquire(&cb->base);
	if (err) {
		kfree(cb);
+1 −0
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@
#define __I915_GEM_OBJECT_TYPES_H__

#include <drm/drm_gem.h>
#include <uapi/drm/i915_drm.h>

#include "i915_active.h"
#include "i915_selftest.h"
+3 −6
Original line number Diff line number Diff line
@@ -16,14 +16,11 @@ static void call_idle_barriers(struct intel_engine_cs *engine)
	struct llist_node *node, *next;

	llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks)) {
		struct i915_active_request *active =
		struct dma_fence_cb *cb =
			container_of((struct list_head *)node,
				     typeof(*active), link);
				     typeof(*cb), node);

		INIT_LIST_HEAD(&active->link);
		RCU_INIT_POINTER(active->request, NULL);

		active->retire(active, NULL);
		cb->func(NULL, cb);
	}
}

Loading