Re: [PATCH v5 06/35] drm/i915: Start of GPU scheduler

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 19/02/2016 13:03, Joonas Lahtinen wrote:
Hi,

Now the code is in reviewable chunks, excellent!

I've added my comments below. A few repeats from last round, but now
with more questions about the logic itself.

On to, 2016-02-18 at 14:26 +0000, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>

Initial creation of scheduler source files. Note that this patch
implements most of the scheduler functionality but does not hook it in
to the driver yet. It also leaves the scheduler code in 'pass through'
mode so that even when it is hooked in, it will not actually do very
much. This allows the hooks to be added one at a time in bite size
chunks and only when the scheduler is finally enabled at the end does
anything start happening.

The general theory of operation is that when batch buffers are
submitted to the driver, the execbuffer() code packages up all the
information required to execute the batch buffer at a later time. This
package is given over to the scheduler which adds it to an internal
node list. The scheduler also scans the list of objects associated
with the batch buffer and compares them against the objects already in
use by other buffers in the node list. If matches are found then the
new batch buffer node is marked as being dependent upon the matching
node. The same is done for the context object. The scheduler also
bumps up the priority of such matching nodes on the grounds that the
more dependencies a given batch buffer has the more important it is
likely to be.

The scheduler aims to have a given (tuneable) number of batch buffers
in flight on the hardware at any given time. If fewer than this are
currently executing when a new node is queued, then the node is passed
straight through to the submit function. Otherwise it is simply added
to the queue and the driver returns back to user land.

The scheduler is notified when each batch buffer completes and updates
its internal tracking accordingly. At the end of the completion
interrupt processing, if any scheduler tracked batches were processed,
the scheduler's deferred worker thread is woken up. This can do more
involved processing such as actually removing completed nodes from the
queue and freeing up the resources associated with them (internal
memory allocations, DRM object references, context reference, etc.).
The work handler also checks the in flight count and calls the
submission code if a new slot has appeared.

When the scheduler's submit code is called, it scans the queued node
list for the highest priority node that has no unmet dependencies.
Note that the dependency calculation is complex as it must take
inter-ring dependencies and potential preemptions into account. Note
also that in the future this will be extended to include external
dependencies such as the Android Native Sync file descriptors and/or
the linux dma-buff synchronisation scheme.

If a suitable node is found then it is sent to execbuff_final() for
submission to the hardware. The in flight count is then re-checked and
a new node popped from the list if appropriate. All nodes that are not
submitted have their priority bumped. This ensures that low priority
tasks do not get starved out by busy higher priority ones - everything
will eventually get its turn to run.

Note that this patch does not implement pre-emptive scheduling. Only
basic scheduling by re-ordering batch buffer submission is currently
implemented. Pre-emption of actively executing batch buffers comes in
the next patch series.

v2: Changed priority levels to +/-1023 due to feedback from Chris
Wilson.

Removed redundant index from scheduler node.

Changed time stamps to use jiffies instead of raw monotonic. This
provides lower resolution but improved compatibility with other i915
code.

Major re-write of completion tracking code due to struct fence
conversion. The scheduler no longer has it's own private IRQ handler
but just lets the existing request code handle completion events.
Instead, the scheduler now hooks into the request notify code to be
told when a request has completed.

Reduced driver mutex locking scope. Removal of scheduler nodes no
longer grabs the mutex lock.

v3: Refactor of dependency generation to make the code more readable.
Also added in read-read optimisation support - i.e., don't treat a
shared read-only buffer as being a dependency.

Allowed the killing of queued nodes rather than only flying ones.

v4: Updated the commit message to better reflect the current state of
the code. Downgraded some BUG_ONs to WARN_ONs. Used the correct array
memory allocator function (kmalloc_array instead of kmalloc).
Corrected the format of some comments. Wrapped some lines differently
to keep the style checker happy.

Fixed a WARN_ON when killing nodes. The dependency removal code checks
that nodes being destroyed do not have any oustanding dependencies
(which would imply they should not have been executed yet). In the
case of nodes being destroyed, e.g. due to context banning, then this
might well be the case - they have not been executed and do indeed
have outstanding dependencies.

Re-instated the code to disble interrupts when not in use. The
underlying problem causing broken IRQ reference counts seems to have
been fixed now.

v5: Shuffled various functions around to remove forward declarations
as apparently these are frowned upon. Removed lots of white space as
apparently having easy to read code is also frowned upon. Split the
direct submission scheduler bypass code out into a separate function.
Squashed down the i915_scheduler.c sections of various patches into
this patch. Thus the later patches simply hook in existing code into
various parts of the driver rather than adding the code as well. Added
documentation to various functions. Re-worked the submit function in
terms of mutex locking, error handling and exit paths. Split the
delayed work handler function in half. Made use of the kernel 'clamp'
macro. [Joonas Lahtinen]

Added runtime PM calls as these must be done at the top level before
acquiring the driver mutex lock. [Chris Wilson]

Removed some obsolete debug code that had been forgotten about.

Moved more clean up code into the 'i915_gem_scheduler_clean_node()'
function rather than replicating it in mutliple places.

Used lighter weight spinlocks.

For: VIZ-1587
Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/Makefile         |   1 +
  drivers/gpu/drm/i915/i915_drv.h       |   6 +
  drivers/gpu/drm/i915/i915_gem.c       |   5 +
  drivers/gpu/drm/i915/i915_scheduler.c | 874 ++++++++++++++++++++++++++++++++++
  drivers/gpu/drm/i915/i915_scheduler.h |  95 ++++
  5 files changed, 981 insertions(+)
  create mode 100644 drivers/gpu/drm/i915/i915_scheduler.c
  create mode 100644 drivers/gpu/drm/i915/i915_scheduler.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 15398c5..79cb38b 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -10,6 +10,7 @@ ccflags-y := -Werror
  i915-y := i915_drv.o \
  	  i915_irq.o \
  	  i915_params.o \
+	  i915_scheduler.o \
            i915_suspend.o \
  	  i915_sysfs.o \
  	  intel_csr.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f4487b9..03add1a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1703,6 +1703,8 @@ struct i915_execbuffer_params {
  	struct drm_i915_gem_request     *request;
  };
+struct i915_scheduler;
+
  /* used in computing the new watermarks state */
  struct intel_wm_config {
  	unsigned int num_pipes_active;
@@ -1968,6 +1970,8 @@ struct drm_i915_private {
struct i915_runtime_pm pm; + struct i915_scheduler *scheduler;
+
I think we should have i915.enable_scheduler parameter (just like
i915.enable_execlists) and make this a data member, not pointer.

Then we can go forward and have i915.enable_scheduler_preempt and so on
to control things at boot time.
There are. The enable_scheduler comes in as patch #21. Including it from the start means modifying i915_params.[ch] in this patch as well. And then you are getting into one big patch that contains everything and is unreviewable due to its sheer size. The enable_preemption parameter comes in with the pre-emption code. No point in adding that until there is something for it to control.



  	/* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
  	struct {
  		int (*execbuf_submit)(struct i915_execbuffer_params *params,
@@ -2290,6 +2294,8 @@ struct drm_i915_gem_request {
  	/** process identifier submitting this request */
  	struct pid *pid;
+ struct i915_scheduler_queue_entry *scheduler_qe;
+
Funny whitespace.

  	/**
  	 * The ELSP only accepts two elements at a time, so we queue
  	 * context/tail pairs on a given queue (ring->execlist_queue) until the
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index dfe43ea..7d9aa24 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -32,6 +32,7 @@
  #include "i915_vgpu.h"
  #include "i915_trace.h"
  #include "intel_drv.h"
+#include "i915_scheduler.h"
  #include
  #include
  #include
@@ -5315,6 +5316,10 @@ int i915_gem_init(struct drm_device *dev)
  	 */
  	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+ ret = i915_scheduler_init(dev);
+	if (ret)
+		goto out_unlock;
+
  	ret = i915_gem_init_userptr(dev);
  	if (ret)
  		goto out_unlock;
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
new file mode 100644
index 0000000..fc23ee7
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -0,0 +1,874 @@
+/*
+ * Copyright (c) 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "i915_drv.h"
+#include "intel_drv.h"
+#include "i915_scheduler.h"
+
+/**
+ * i915_scheduler_is_enabled - Returns true if the scheduler is enabled.
+ * @dev: DRM device
+ */
+bool i915_scheduler_is_enabled(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	return dev_priv->scheduler != NULL;
+}
i915.enable_scheduler would be used instead.
It is when it arrives.


+
+/**
+ * i915_scheduler_init - Initialise the scheduler.
+ * @dev: DRM device
+ * Returns zero on success or -ENOMEM if memory allocations fail.
+ */
+int i915_scheduler_init(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct i915_scheduler *scheduler = dev_priv->scheduler;
+	int r;
+
This protection -->

+	if (scheduler)
+		return 0;
+
+	scheduler = kzalloc(sizeof(*scheduler), GFP_KERNEL);
+	if (!scheduler)
+		return -ENOMEM;
+
Is not needed if we have data member and i915.enable_scheduler.
Making the scheduler structure a object rather than a pointer basically means moving the whole of i915_scheduler.h into i915_drv.h. There is too much interconnectedness between the various structures. I would say that keeping the code nicely partitioned and readable is worth the one dynamic allocation at driver load time.


+	spin_lock_init(&scheduler->lock);
+
+	for (r = 0; r < I915_NUM_RINGS; r++)
+		INIT_LIST_HEAD(&scheduler->node_queue[r]);
+
+	/* Default tuning values: */
+	scheduler->priority_level_min     = -1023;
+	scheduler->priority_level_max     = 1023;
+	scheduler->priority_level_bump    = 50;
+	scheduler->priority_level_preempt = 900;
+	scheduler->min_flying             = 2;
+
+	dev_priv->scheduler = scheduler;
+
+	return 0;
+}
+
+/*
+ * Add a popped node back in to the queue. For example, because the ring was
+ * hung when execfinal() was called and thus the ring submission needs to be
+ * retried later.
+ */
+static void i915_scheduler_node_requeue(struct i915_scheduler_queue_entry *node)
+{
+	WARN_ON(!node);
Rather remove this completely as it is an internal function, or use
BUG_ON because next line will straight lead to OOPS after warning. I'll
pass mentioning rest of the obvious WARN_ON vs BUG_ON's.
Yeah, most of the WARN/BUG_ONs are probably unnecessary now. They were scattered liberally around during initial development when strange and unexpected things could happen. Now the the code paths are pretty well established they aren't really much use. I'll reduce them down to just the ones that still make sense.



+	WARN_ON(!I915_SQS_IS_FLYING(node));
+
+	/* Seqno will be reassigned on relaunch */
+	node->params.request->seqno = 0;
+	node->status = i915_sqs_queued;
+}
+
+/*
+ * Give up on a node completely. For example, because it is causing the
+ * ring to hang or is using some resource that no longer exists.
+ */
+static void i915_scheduler_node_kill(struct i915_scheduler_queue_entry *node)
+{
+	WARN_ON(!node);
+	WARN_ON(I915_SQS_IS_COMPLETE(node));
+
+	node->status = i915_sqs_dead;
+}
+
+/* Mark a node as in flight on the hardware. */
As for documentation, from below, I only assume scheduler lock must be
held and node's can be only manipulated when scheduler lock is held?
It's good to add a comment describing the expected locking. Or maybe
add assert_scheduler_lock_held() helper and sprinkle it around the
functions similar to assert_rpm_wakelock_held(), which is self-
documenting?

+static int i915_scheduler_node_fly(struct i915_scheduler_queue_entry *node)
+{
+	struct drm_i915_private *dev_priv = node->params.dev->dev_private;
If node is NULL, this would already OOPS, I do not think it's
unreasonable to expect node not to be NULL. The below WARN_ON check is
never reached.

+	struct i915_scheduler *scheduler = dev_priv->scheduler;
+	struct intel_engine_cs *ring;
+
+	WARN_ON(!scheduler);
+	WARN_ON(!node);
+	WARN_ON(node->status != i915_sqs_popped);
+
+	ring = node->params.ring;
+
This and similar assignments can be squashed to declaration line, as
this is basically an alias.

+	/*
+	 * Add the node (which should currently be in state popped) to the
+	 * front of the queue. This ensure that flying nodes are always held
+	 * in hardware submission order.
+	 */
+	list_add(&node->link, &scheduler->node_queue[ring->id]);
+
+	node->status = i915_sqs_flying;
+
+	if (!(scheduler->flags[ring->id] & i915_sf_interrupts_enabled)) {
+		bool success = true;
+
+		success = ring->irq_get(ring);
+		if (success)
+			scheduler->flags[ring->id] |= i915_sf_interrupts_enabled;
+		else
+			return -EINVAL;
Shouldn't the node be cleaned up from node_queue in case of an error?
Not really. The batch buffer will still be executed regardless of whether interrupts could be enabled or not. The return code is actually ignored so this should really be a void function, I guess. Without interrupts then completion might take a while to be noticed but stuff should still basically work.



Also, I think above could be written much more compact form (because it
looks like something where more logic will be added later). It makes it
easier to write and visualize the error paths when reading if there are
no nested if's.

I won't mention about the error paths of functions below, I expect the
following guidline to be adhered;

	if (scheduler->flags[ring->id] & I915_SF_INT_ENABLED)
		goto out;

	if (!ring->irq_get(ring))
		goto err_irq;

	if (!...)
		goto err_foo;

	scheduler->flags[ring->id] |= I915_SF_INT_ENABLED;
out:
	return 0;
err_foo:
	foobar();
err_irq:
	list_remove()
	return -EINVAL;

+	}
+
+	return 0;
+}
+
+static uint32_t i915_scheduler_count_flying(struct i915_scheduler *scheduler,
+					    struct intel_engine_cs *ring)
+{
+	struct i915_scheduler_queue_entry *node;
+	uint32_t flying = 0;
+
+	list_for_each_entry(node, &scheduler->node_queue[ring->id], link)
+1 for the for_each_scheduler_node(...)

+		if (I915_SQS_IS_FLYING(node))
+			flying++;
+
+	return flying;
+}
+
+static void i915_scheduler_priority_bump_clear(struct i915_scheduler *scheduler)
+{
+	struct i915_scheduler_queue_entry *node;
+	int i;
+
+	/*
+	 * Ensure circular dependencies don't cause problems and that a bump
+	 * by object usage only bumps each using buffer once:
+	 */
+	for (i = 0; i < I915_NUM_RINGS; i++) {
+		list_for_each_entry(node, &scheduler->node_queue[i], link)
+			node->bumped = false;
+	}
+}
+
+static int i915_scheduler_priority_bump(struct i915_scheduler *scheduler,
+				struct i915_scheduler_queue_entry *target,
Is there specific reason why struct i915_scheduler_queue_entry are not
referred to just as "node" but "qe" and here something else, do "node"
and "qe" have a special semantic meaning?
Not sure what you mean. The use of 'node' is because almost everything an i915_scheduler_queue_entry object is being used it is as a node in a list that is being iterated over. The only 'qe' variable is in 'i915_scheduler_queue_execbuffer[_bypass]' and there it is because the qe is being passed in from outside and is copied to a local 'node' that is then added to the list. I think the use of 'target' here is largely historical. The subsequent array search was originally a list search and hence had a 'node' that was used as the iterator. Thus the qe passed in was called something else to avoid conflicts and target seemed appropriate.

+				uint32_t bump)
+{
+	uint32_t new_priority;
+	int i, count;
+
+	if (target->priority >= scheduler->priority_level_max)
+		return 1;
So if one node reaches maximum priority the dependencies are expected
to be maxed out already?
Yes. Dependencies should always be of equal or higher priority to the dependee. When a new dependency is created, the priority of the dependee is added on to ensure that something of high priority can never be stuck waiting on something of low priority. Also, it means that a batch buffer that lots of other buffers are waiting on will be bumped to higher and higher priority the more waiters there are and thus is more likely to be executed sooner and free them all up.


+
+	if (target->bumped)
+		return 0;
+
+	new_priority = target->priority + bump;
+	if ((new_priority <= target->priority) ||
+	    (new_priority > scheduler->priority_level_max))
+		target->priority = scheduler->priority_level_max;
+	else
+		target->priority = new_priority;
+
+	count = 1;
+	target->bumped = true;
+
+	for (i = 0; i < target->num_deps; i++) {
+		if (!target->dep_list[i])
+			continue;
+
+		if (target->dep_list[i]->bumped)
+			continue;
+
+		count += i915_scheduler_priority_bump(scheduler,
+						      target->dep_list[i],
+						      bump);
+	}
+
+	return count;
+}
+
+/*
+ * Nodes are considered valid dependencies if they are queued on any ring or
+ * if they are in flight on a different ring. In flight on the same ring is no
+ * longer interesting for non-premptive nodes as the ring serialises execution.
+ * For pre-empting nodes, all in flight dependencies are valid as they must not
+ * be jumped by the act of pre-empting.
+ *
+ * Anything that is neither queued nor flying is uninteresting.
+ */
+static inline bool i915_scheduler_is_dependency_valid(
+			struct i915_scheduler_queue_entry *node, uint32_t idx)
+{
+	struct i915_scheduler_queue_entry *dep;
+
+	dep = node->dep_list[idx];
+	if (!dep)
+		return false;
+
+	if (I915_SQS_IS_QUEUED(dep))
+		return true;
+
+	if (I915_SQS_IS_FLYING(dep)) {
+		if (node->params.ring != dep->params.ring)
+			return true;
+	}
+
+	return false;
+}
+
+static int i915_scheduler_pop_from_queue_locked(struct intel_engine_cs *ring,
+				struct i915_scheduler_queue_entry **pop_node)
+{
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
+	struct i915_scheduler *scheduler = dev_priv->scheduler;
+	struct i915_scheduler_queue_entry *best = NULL;
+	struct i915_scheduler_queue_entry *node;
+	int ret;
+	int i;
+	bool any_queued = false;
+	bool has_local, has_remote, only_remote = false;
+
+	*pop_node = NULL;
+	ret = -ENODATA;
+
+	list_for_each_entry(node, &scheduler->node_queue[ring->id], link) {
+		if (!I915_SQS_IS_QUEUED(node))
+			continue;
+		any_queued = true;
+
+		has_local  = false;
+		has_remote = false;
+		for (i = 0; i < node->num_deps; i++) {
+			if (!i915_scheduler_is_dependency_valid(node, i))
+				continue;
+
+			if (node->dep_list[i]->params.ring == node->params.ring)
+				has_local = true;
+			else
+				has_remote = true;
+		}
+
+		if (has_remote && !has_local)
+			only_remote = true;
+
+		if (!has_local && !has_remote) {
+			if (!best ||
+			    (node->priority > best->priority))
+				best = node;
+		}
+	}
+
+	if (best) {
+		list_del(&best->link);
+
+		INIT_LIST_HEAD(&best->link);
+		best->status  = i915_sqs_popped;
+
+		ret = 0;
+	} else {
+		/* Can only get here if:
+		 * (a) there are no buffers in the queue
+		 * (b) all queued buffers are dependent on other buffers
+		 *     e.g. on a buffer that is in flight on a different ring
+		 */
+		if (only_remote) {
+			/* The only dependent buffers are on another ring. */
+			ret = -EAGAIN;
+		} else if (any_queued) {
+			/* It seems that something has gone horribly wrong! */
+			DRM_ERROR("Broken dependency tracking on ring %d!\n",
+				  (int) ring->id);
Would this condition be worthy WARN_ONCE(), because it will keep
repeating as the situation will persist?

+		}
+	}
+
+	*pop_node = best;
+	return ret;
+}
+
+/*
+ * NB: The driver mutex lock must be held before calling this function. It is
+ * only really required during the actual back end submission call. However,
+ * attempting to acquire a mutex while holding a spin lock is a Bad Idea.
+ * And releasing the one before acquiring the other leads to other code
+ * being run and interfering.
+ */
+static int i915_scheduler_submit(struct intel_engine_cs *ring)
+{
+	struct drm_device *dev = ring->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct i915_scheduler *scheduler = dev_priv->scheduler;
+	struct i915_scheduler_queue_entry *node;
+	int ret, count = 0, flying;
+
+	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
+
+	spin_lock_irq(&scheduler->lock);
+
+	WARN_ON(scheduler->flags[ring->id] & i915_sf_submitting);
+	scheduler->flags[ring->id] |= i915_sf_submitting;
+
+	/* First time around, complain if anything unexpected occurs: */
+	ret = i915_scheduler_pop_from_queue_locked(ring, &node);
+	if (ret)
+		goto error;
+
+	do {
+		WARN_ON(!node);
+		WARN_ON(node->params.ring != ring);
+		WARN_ON(node->status != i915_sqs_popped);
+		count++;
+
+		/*
+		 * The call to pop above will have removed the node from the
+		 * list. So add it back in and mark it as in flight.
+		 */
+		i915_scheduler_node_fly(node);
+
+		spin_unlock_irq(&scheduler->lock);
+		ret = dev_priv->gt.execbuf_final(&node->params);
+		spin_lock_irq(&scheduler->lock);
+
+		/*
+		 * Handle failed submission but first check that the
+		 * watchdog/reset code has not nuked the node while we
+		 * weren't looking:
+		 */
+		if (ret && (node->status != i915_sqs_dead)) {
+			bool requeue = true;
+
+			/*
+			 * Oh dear! Either the node is broken or the ring is
+			 * busy. So need to kill the node or requeue it and try
+			 * again later as appropriate.
+			 */
+
+			switch (-ret) {
+			case ENODEV:
+			case ENOENT:
+				/* Fatal errors. Kill the node. */
+				requeue = false;
+				i915_scheduler_node_kill(node);
+			break;
These break indents still.

+
+			case EAGAIN:
+			case EBUSY:
+			case EIO:
+			case ENOMEM:
+			case ERESTARTSYS:
+			case EINTR:
+				/* Supposedly recoverable errors. */
+			break;
+
+			default:
+				/*
+				 * Assume the error is recoverable and hope
+				 * for the best.
+				 */
+				MISSING_CASE(-ret);
+			break;
+			}
+
+			if (requeue) {
+				i915_scheduler_node_requeue(node);
+				/*
+				 * No point spinning if the ring is currently
+				 * unavailable so just give up and come back
+				 * later.
+				 */
+				break;
+			}
+		}
+
+		/* Keep launching until the sky is sufficiently full. */
+		flying = i915_scheduler_count_flying(scheduler, ring);
+		if (flying >= scheduler->min_flying)
+			break;
+
+		/* Grab another node and go round again... */
+		ret = i915_scheduler_pop_from_queue_locked(ring, &node);
+	} while (ret == 0);
+
+	/* Don't complain about not being able to submit extra entries */
+	if (ret == -ENODATA)
+		ret = 0;
+
+	/*
+	 * Bump the priority of everything that was not submitted to prevent
+	 * starvation of low priority tasks by a spamming high priority task.
+	 */
This, this calls for an I-G-T test to make sure we're effective.
Yeah, IGTs are under development.

+	i915_scheduler_priority_bump_clear(scheduler);
+	list_for_each_entry(node, &scheduler->node_queue[ring->id], link) {
+		if (!I915_SQS_IS_QUEUED(node))
+			continue;
+
+		i915_scheduler_priority_bump(scheduler, node,
+					     scheduler->priority_level_bump);
+	}
+
+	/* On success, return the number of buffers submitted. */
+	if (ret == 0)
+		ret = count;
+
+error:
+	scheduler->flags[ring->id] &= ~i915_sf_submitting;
+	spin_unlock_irq(&scheduler->lock);
+	return ret;
+}
+
+static void i915_generate_dependencies(struct i915_scheduler *scheduler,
+				       struct i915_scheduler_queue_entry *node,
+				       uint32_t ring)
+{
+	struct i915_scheduler_obj_entry *this, *that;
+	struct i915_scheduler_queue_entry *test;
+	int i, j;
+	bool found;
+
+	list_for_each_entry(test, &scheduler->node_queue[ring], link) {
+		if (I915_SQS_IS_COMPLETE(test))
+			continue;
+
+		/*
+		 * Batches on the same ring for the same
+		 * context must be kept in order.
+		 */
+		found = (node->params.ctx == test->params.ctx) &&
+			(node->params.ring == test->params.ring);
+
+		/*
+		 * Batches working on the same objects must
+		 * be kept in order.
+		 */
+		for (i = 0; (i < node->num_objs) && !found; i++) {
+			this = node->saved_objects + i;
node->objs and node->num_objs would be a better naming match. num_objs
and saved_objects makes me think we have a bug here, indexing wrong
array.

+
+			for (j = 0; j < test->num_objs; j++) {
+				that = test->saved_objects + j;
+
+				if (this->obj != that->obj)
+					continue;
+
+				/* Only need to worry about writes */
+				if (this->read_only && that->read_only)
+					continue;
+
+				found = true;
+				break;
+			}
+		}
+
+		if (found) {
+			node->dep_list[node->num_deps] = test;
+			node->num_deps++;
+		}
+	}
+}
+
+static int i915_scheduler_queue_execbuffer_bypass(struct i915_scheduler_queue_entry *qe)
+{
+	struct drm_i915_private *dev_priv = qe->params.dev->dev_private;
+	struct i915_scheduler *scheduler = dev_priv->scheduler;
+	int ret;
+
+	scheduler->flags[qe->params.ring->id] |= i915_sf_submitting;
+	ret = dev_priv->gt.execbuf_final(&qe->params);
+	scheduler->flags[qe->params.ring->id] &= ~i915_sf_submitting;
+
The above code makes me think of locking again, maybe document at top
of this function.
No need to acquire the spinlock at this point. If the scheduler is in bypass mode then there is no internal state to protect. The driver mutex lock will be held because there is only one code path to get here which is the execbuff IOCTL. The whole point of bypass mode is that it is a single contiguous execution path from IOCTL entry all the way through to hardware submission. A mutex check could be added but it seems unnecessary given the limited calling scope.


+	/*
+	 * Don't do any clean up on failure because the caller will
+	 * do it all anyway.
+	 */
+	if (ret)
+		return ret;
+
+	/* Need to release any resources held by the node: */
+	i915_scheduler_clean_node(qe);
+
+	return 0;
+}
+
+/**
+ * i915_scheduler_queue_execbuffer - Submit a batch buffer request to the
+ * scheduler.
+ * @qe: The batch buffer request to be queued.
+ * The expectation is the qe passed in is a local stack variable. This
+ * function will copy its contents into a freshly allocated list node. The
+ * new node takes ownership of said contents so the original qe should simply
+ * be discarded and not cleaned up (i.e. don't free memory it points to or
+ * dereference objects it holds). The node is added to the scheduler's queue
+ * and the batch buffer will be submitted to the hardware at some future
+ * point in time (which may be immediately, before returning or may be quite
+ * a lot later).
+ */
+int i915_scheduler_queue_execbuffer(struct i915_scheduler_queue_entry *qe)
+{
+	struct drm_i915_private *dev_priv = qe->params.dev->dev_private;
+	struct i915_scheduler *scheduler = dev_priv->scheduler;
+	struct intel_engine_cs *ring = qe->params.ring;
+	struct i915_scheduler_queue_entry *node;
+	struct i915_scheduler_queue_entry *test;
+	bool not_flying;
+	int i, r;
+	int incomplete = 0;
+
+	WARN_ON(!scheduler);
+
+	if (1/*!i915.enable_scheduler*/)
+		return i915_scheduler_queue_execbuffer_bypass(qe);
+
I'm thinking, should we branch already before calling a function named
i915_scheduler_queue_execbuffer if scheduler is disabled?
That would require putting if(scheduler) logic in random points throughout the driver. Doing it this way, the driver as a whole has a single execution path which is via the scheduler. It is then up to the scheduler itself to decide whether to run in bypass mode, simple re-ordering mode, full pre-emption mode or any other mode we feel a need to add.


+	node = kmalloc(sizeof(*node), GFP_KERNEL);
+	if (!node)
+		return -ENOMEM;
+
+	*node = *qe;
Now I read the added documentation for the function, maybe we should at
least zero out qe to avoid future problems?
The execbuff success path hands the qe contents over and pretty much immediately drops all the way back out of the IOCTL. The failure path does the full cleanup of de-allocating all the qe stuff it had previously allocated. IMO there doesn't seem to be much need to zero out the structure at this point.


+	INIT_LIST_HEAD(&node->link);
+	node->status = i915_sqs_queued;
+	node->stamp  = jiffies;
+	i915_gem_request_reference(node->params.request);
+
+	WARN_ON(node->params.request->scheduler_qe);
+	node->params.request->scheduler_qe = node;
+
+	/* Need to determine the number of incomplete entries in the list as
+	 * that will be the maximum size of the dependency list.
+	 *
+	 * Note that the allocation must not be made with the spinlock acquired
+	 * as kmalloc can sleep. However, the unlock/relock is safe because no
+	 * new entries can be queued up during the unlock as the i915 driver
+	 * mutex is still held. Entries could be removed from the list but that
+	 * just means the dep_list will be over-allocated which is fine.
+	 */
+	spin_lock_irq(&scheduler->lock);
-->

+	for (r = 0; r < I915_NUM_RINGS; r++) {
+		list_for_each_entry(test, &scheduler->node_queue[r], link) {
+			if (I915_SQS_IS_COMPLETE(test))
+				continue;
+
+			incomplete++;
+		}
+	}
+
<-- This counting structure, I still think it would be good idea to
make it a static helper.

+	/* Temporarily unlock to allocate memory: */
+	spin_unlock_irq(&scheduler->lock);
+	if (incomplete) {
+		node->dep_list = kmalloc_array(incomplete,
+					       sizeof(*node->dep_list),
+					       GFP_KERNEL);
+		if (!node->dep_list) {
+			kfree(node);
+			return -ENOMEM;
+		}
+	} else
+		node->dep_list = NULL;
+
+	spin_lock_irq(&scheduler->lock);
+	node->num_deps = 0;
+
+	if (node->dep_list) {
+		for (r = 0; r < I915_NUM_RINGS; r++)
+			i915_generate_dependencies(scheduler, node, r);
+
+		WARN_ON(node->num_deps > incomplete);
+	}
+
+	node->priority = clamp(node->priority,
+			       scheduler->priority_level_min,
+			       scheduler->priority_level_max);
+
+	if ((node->priority > 0) && node->num_deps) {
+		i915_scheduler_priority_bump_clear(scheduler);
+
+		for (i = 0; i < node->num_deps; i++)
+			i915_scheduler_priority_bump(scheduler,
+					node->dep_list[i], node->priority);
If node is posted with maximum priority to begin with, wouldn't the
priority propagation stop after first level dependencies due to the
check in the beginning of priority_bump function?
The recursion will stop whenever it encounters a node already at maximum priority. However, it is not starting from the newly submitted node but from that node's dependencies. If they are already at max priority then their dependencies must also be and so on down the chain and no further processing is required. On the other hand, if they are not then they will be bumped and their dependencies checked and so on down the chain.

+
+	list_add_tail(&node->link, &scheduler->node_queue[ring->id]);
+
+	not_flying = i915_scheduler_count_flying(scheduler, ring) <
+						 scheduler->min_flying;
+
+	spin_unlock_irq(&scheduler->lock);
+
+	if (not_flying)
+		i915_scheduler_submit(ring);
+
+	return 0;
+}
+
+/**
+ * i915_scheduler_notify_request - Notify the scheduler that the given
+ * request has completed on the hardware.
+ * @req: Request structure which has completed
+ * @preempt: Did it complete pre-emptively?
+ * A sequence number has popped out of the hardware and the request handling
+ * code has mapped it back to a request and will mark that request complete.
+ * It also calls this function to notify the scheduler about the completion
+ * so the scheduler's node can be updated appropriately.
+ * Returns true if the request is scheduler managed, false if not. The return
+ * value is combined for all freshly completed requests and if any were true
+ * then i915_scheduler_wakeup() is called so the scheduler can do further
+ * processing (submit more work) at the end.
+ */
+bool i915_scheduler_notify_request(struct drm_i915_gem_request *req)
+{
+	struct drm_i915_private *dev_priv = to_i915(req->ring->dev);
+	struct i915_scheduler *scheduler = dev_priv->scheduler;
+	struct i915_scheduler_queue_entry *node = req->scheduler_qe;
+	unsigned long flags;
+
+	if (!node)
+		return false;
+
+	spin_lock_irqsave(&scheduler->lock, flags);
+
+	WARN_ON(!I915_SQS_IS_FLYING(node));
+
+	/* Node was in flight so mark it as complete. */
+	if (req->cancelled)
+		node->status = i915_sqs_dead;
+	else
+		node->status = i915_sqs_complete;
+
+	spin_unlock_irqrestore(&scheduler->lock, flags);
+
+	return true;
+}
+
+static int i915_scheduler_remove_dependent(struct i915_scheduler *scheduler,
+				struct i915_scheduler_queue_entry *remove)
+{
+	struct i915_scheduler_queue_entry *node;
+	int i, r;
+	int count = 0;
+
+	/*
+	 * Ensure that a node is not being removed which is still dependent
+	 * upon other (not completed) work. If that happens, it implies
+	 * something has gone very wrong with the dependency tracking! Note
+	 * that there is no need to worry if this node has been explicitly
+	 * killed for some reason - it might be being killed before it got
+	 * sent to the hardware.
+	 */
+	if (remove->status != i915_sqs_dead) {
+		for (i = 0; i < remove->num_deps; i++)
+			if ((remove->dep_list[i]) &&
+			    (!I915_SQS_IS_COMPLETE(remove->dep_list[i])))
+				count++;
+		WARN_ON(count);
+	}
+
+	/*
+	 * Remove this node from the dependency lists of any other node which
+	 * might be waiting on it.
+	 */
+	for (r = 0; r < I915_NUM_RINGS; r++) {
+		list_for_each_entry(node, &scheduler->node_queue[r], link) {
+			for (i = 0; i < node->num_deps; i++) {
+				if (node->dep_list[i] != remove)
+					continue;
+
+				node->dep_list[i] = NULL;
+			}
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * i915_scheduler_wakeup - wake the scheduler's worker thread
+ * @dev: DRM device
+ * Called at the end of seqno interrupt processing if any request has
+ * completed that corresponds to a scheduler node.
+ */
+void i915_scheduler_wakeup(struct drm_device *dev)
+{
+	/* XXX: Need to call i915_scheduler_remove() via work handler. */
+}
+
+/**
+ * i915_scheduler_clean_node - free up any allocations/references
+ * associated with the given scheduler queue entry.
+ * @node: Queue entry structure which is complete
+ * After a give batch buffer completes on the hardware, all the information
+ * required to resubmit it is no longer required. However, the node entry
+ * itself might still be required for tracking purposes for a while longer.
+ * This function should be called as soon as the node is known to be complete
+ * so that these resources may be freed even though the node itself might
+ * hang around.
+ */
+void i915_scheduler_clean_node(struct i915_scheduler_queue_entry *node)
+{
+	if (!I915_SQS_IS_COMPLETE(node)) {
+		WARN(!node->params.request->cancelled,
+		     "Cleaning active node: %d!\n", node->status);
+		return;
+	}
+
+	if (node->params.batch_obj) {
+		/*
+		 * The batch buffer must be unpinned before it is unreferenced
+		 * otherwise the unpin fails with a missing vma!?
+		 */
+		if (node->params.dispatch_flags & I915_DISPATCH_SECURE)
+			i915_gem_execbuff_release_batch_obj(node->params.batch_obj);
+
This code seems little bit out of place.
Not sure where else you could put it. The code itself is just a copy of the existing clean up code that was already in the execbuff code path. The cleanup can no longer be done within the execbuff call due to the batch buffer execution being arbitrarily delayed by the scheduler. Thus it is down to the scheduler to do all the necessary cleanup when it knows that the batch buffer is no longer required.


+		node->params.batch_obj = NULL;
+	}
+
+	/* And anything else owned by the node: */
+	if (node->params.cliprects) {
+		kfree(node->params.cliprects);
+		node->params.cliprects = NULL;
+	}
+}
+
The below function is quite hacky, I understood this will be mitigated
by per-ctx sequence numbering in the future?
To be honest, I think it is already obsolete due to the previous work of converting from seqno tests everywhere to testing request structures. The point of the code below was to cope with seqno values popping out of the hardware in random order due to the re-ordering of batch buffers prior to execution. Whereas, the struct request conversion allowed the seqno to be late allocated and thus kept in order.

Unfortunately, pre-emption confuses matters again. Although the intention is that seqno values will still be kept in order. It's just that a given request might have multiple different seqno values assigned to it throughout its life. The pre-emption code hasn't quite settled down yet though, especially with interesting new hardware planned for future chips. Thus I don't really want to rip out all of this code quite yet just in case we do still need it.

As you say, per-ctx seqnos would definitely have an effect here as well. So yes, long term the intention is to remove all this nastyness. Short term, it seems best to keep it around until proven unnecessary.


+static bool i915_scheduler_remove(struct i915_scheduler *scheduler,
+				  struct intel_engine_cs *ring,
+				  struct list_head *remove)
+{
+	struct i915_scheduler_queue_entry *node, *node_next;
+	int flying = 0, queued = 0;
+	bool do_submit;
+	uint32_t min_seqno;
+
+	spin_lock_irq(&scheduler->lock);
+
+	/*
+	 * In the case where the system is idle, starting 'min_seqno' from a big
+	 * number will cause all nodes to be removed as they are now back to
+	 * being in-order. However, this will be a problem if the last one to
+	 * complete was actually out-of-order as the ring seqno value will be
+	 * lower than one or more completed buffers. Thus code looking for the
+	 * completion of said buffers will wait forever.
+	 * Instead, use the hardware seqno as the starting point. This means
+	 * that some buffers might be kept around even in a completely idle
+	 * system but it should guarantee that no-one ever gets confused when
+	 * waiting for buffer completion.
+	 */
+	min_seqno = ring->get_seqno(ring, true);
+
+	list_for_each_entry(node, &scheduler->node_queue[ring->id], link) {
+		if (I915_SQS_IS_QUEUED(node))
+			queued++;
+		else if (I915_SQS_IS_FLYING(node))
+			flying++;
+		else if (I915_SQS_IS_COMPLETE(node))
+			continue;
+
+		if (node->params.request->seqno == 0)
+			continue;
+
+		if (!i915_seqno_passed(node->params.request->seqno, min_seqno))
+			min_seqno = node->params.request->seqno;
+	}
+
+	INIT_LIST_HEAD(remove);
+	list_for_each_entry_safe(node, node_next, &scheduler->node_queue[ring->id], link) {
+		/*
+		 * Only remove completed nodes which have a lower seqno than
+		 * all pending nodes. While there is the possibility of the
+		 * ring's seqno counting backwards, all higher buffers must
+		 * be remembered so that the 'i915_seqno_passed()' test can
+		 * report that they have in fact passed.
+		 *
+		 * NB: This is not true for 'dead' nodes. The GPU reset causes
+		 * the software seqno to restart from its initial value. Thus
+		 * the dead nodes must be removed even though their seqno values
+		 * are potentially vastly greater than the current ring seqno.
+		 */
+		if (!I915_SQS_IS_COMPLETE(node))
+			continue;
+
+		if (node->status != i915_sqs_dead) {
+			if (i915_seqno_passed(node->params.request->seqno, min_seqno) &&
+			    (node->params.request->seqno != min_seqno))
+				continue;
+		}
+
+		list_del(&node->link);
+		list_add(&node->link, remove);
+
+		/* Strip the dependency info while the mutex is still locked */
+		i915_scheduler_remove_dependent(scheduler, node);
+
+		continue;
+	}
+
+	/*
+	 * Release the interrupt reference count if there are no longer any
+	 * nodes to worry about.
+	 */
+	if (!flying && !queued &&
+	    (scheduler->flags[ring->id] & i915_sf_interrupts_enabled)) {
+		ring->irq_put(ring);
+		scheduler->flags[ring->id] &= ~i915_sf_interrupts_enabled;
Maybe have this piece of code as a helper? To enable interrupts and
disable them, maybe even a reference count?
Not sure how much you could abstract out into a helper other than the two lines of irq_get|put + flag set|clear. The flying|queued test is particular to this one instance. Also, there is no need to reference count as the irq code does that already.


+	}
+
+	/* Launch more packets now? */
+	do_submit = (queued > 0) && (flying < scheduler->min_flying);
+
+	spin_unlock_irq(&scheduler->lock);
+
+	return do_submit;
+}
+
+void i915_scheduler_process_work(struct intel_engine_cs *ring)
+{
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
+	struct i915_scheduler *scheduler = dev_priv->scheduler;
+	struct i915_scheduler_queue_entry *node;
+	bool do_submit;
+	struct list_head remove;
+
+	if (list_empty(&scheduler->node_queue[ring->id]))
+		return;
+
+	/* Remove completed nodes. */
+	do_submit = i915_scheduler_remove(scheduler, ring, &remove);
+
+	if (!do_submit && list_empty(&remove))
+		return;
+
+	/* Need to grab the pm lock outside of the mutex lock */
+	if (do_submit)
+		intel_runtime_pm_get(dev_priv);
+
+	mutex_lock(&ring->dev->struct_mutex);
+
+	if (do_submit)
+		i915_scheduler_submit(ring);
+
+	while (!list_empty(&remove)) {
+		node = list_first_entry(&remove, typeof(*node), link);
+		list_del(&node->link);
+
+		/* Free up all the DRM references */
+		i915_scheduler_clean_node(node);
+
+		/* And anything else owned by the node: */
+		node->params.request->scheduler_qe = NULL;
+		i915_gem_request_unreference(node->params.request);
+		kfree(node->dep_list);
+		kfree(node);
Maybe have a separate helper for freeing a node, would make this much
cleaner.
There is a helper for freeing up the contents of the node - see i915_scheduler_clean_node() above. However, that function is called from multiple places and this is the only instance where the node itself is actually freed. The other callers either need to keep the node around for a while longer (i.e. until this piece of code can run) or don't own the node because it is stack allocated (in the bypass case). So the 'anything else' section cannot be moved into the existing clean function and any secondary helper would be single usage and only four lines long. It doesn't seem worth it.


+	}
+
+	mutex_unlock(&ring->dev->struct_mutex);
+
+	if (do_submit)
+		intel_runtime_pm_put(dev_priv);
+}
diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h
new file mode 100644
index 0000000..415fec8
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_scheduler.h
@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef _I915_SCHEDULER_H_
+#define _I915_SCHEDULER_H_
+
+enum i915_scheduler_queue_status {
+	/* Limbo: */
+	i915_sqs_none = 0,
+	/* Not yet submitted to hardware: */
+	i915_sqs_queued,
+	/* Popped from queue, ready to fly: */
+	i915_sqs_popped,
+	/* Sent to hardware for processing: */
+	i915_sqs_flying,
+	/* Finished processing on the hardware: */
+	i915_sqs_complete,
+	/* Killed by watchdog or catastrophic submission failure: */
+	i915_sqs_dead,
+	/* Limit value for use with arrays/loops */
+	i915_sqs_MAX
+};
+
To uppercase.
Grrr. Linux kernel style guide is daft.


+#define I915_SQS_IS_QUEUED(node)	(((node)->status == i915_sqs_queued))
+#define I915_SQS_IS_FLYING(node)	(((node)->status == i915_sqs_flying))
+#define I915_SQS_IS_COMPLETE(node)	(((node)->status == i915_sqs_complete) || \
+					 ((node)->status == i915_sqs_dead))
+
+struct i915_scheduler_obj_entry {
+	struct drm_i915_gem_object          *obj;
+	bool                                read_only;
+};
+
+struct i915_scheduler_queue_entry {
+	struct i915_execbuffer_params       params;
+	/* -1023 = lowest priority, 0 = default, 1023 = highest */
+	int32_t                             priority;
+	struct i915_scheduler_obj_entry     *saved_objects;
+	int                                 num_objs;
+	bool                                bumped;
+	struct i915_scheduler_queue_entry   **dep_list;
+	int                                 num_deps;
+	enum i915_scheduler_queue_status    status;
+	unsigned long                       stamp;
+	struct list_head                    link;
Above whitespace is still incorrect and I could really use comments.

+};
+
+struct i915_scheduler {
+	struct list_head    node_queue[I915_NUM_RINGS];
+	uint32_t            flags[I915_NUM_RINGS];
+	spinlock_t          lock;
+
+	/* Tuning parameters: */
+	int32_t             priority_level_min;
+	int32_t             priority_level_max;
+	int32_t             priority_level_bump;
+	int32_t             priority_level_preempt;
+	uint32_t            min_flying;
+};
+
+/* Flag bits for i915_scheduler::flags */
+enum {
+	i915_sf_interrupts_enabled  = (1 << 0),
+	i915_sf_submitting          = (1 << 1),
+};
These must be I915_UPPERCASE_SOMETHING by convention.

+
+bool i915_scheduler_is_enabled(struct drm_device *dev);
+int i915_scheduler_init(struct drm_device *dev);
+void i915_scheduler_clean_node(struct i915_scheduler_queue_entry *node);
+int i915_scheduler_queue_execbuffer(struct i915_scheduler_queue_entry *qe);
+bool i915_scheduler_notify_request(struct drm_i915_gem_request *req);
+void i915_scheduler_wakeup(struct drm_device *dev);
+
+#endif  /* _I915_SCHEDULER_H_ */

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





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux