[PATCH 00/15] [RFC] Forced throttling/scheduling

[PATCH 00/15] [RFC] Forced throttling/scheduling

[Ben Widawsky]

This code is currently living on my personal git repo:
git://people.freedesktop.org/~bwidawsk/drm-intel forced_throttling

Since these are RFC, I haven't spent too much time cleaning things up.
Feel free to comment on typos, comments you feel are missing, etc. I
also haven't spent any time running the standard kernel tests, kmemleak
and such - I intend to do so while these are reviewed here.

There are two main "scheduler" types implemented in this patch. What I
refer to as a fairness scheduler, and a batch scheduler. The fairness
scheduler is currently implemented on batch granularity but that is not
guaranteed going forward. The batch scheduler is a way to set batch
limits per pid. Refer to the relevant patch for more details.

It is my opinion that the fairness scheduler isn't terribly interesting
except to prevent badly written, or malicious apps. For example, a 3d
app which queues up a ton of work but never calls glxSwapBuffer.

The batch scheduler is also somewhat uninteresting as the values it uses
require proper tuning and will vary from system to system, and even then
depending on what's currently running. But like the fairness one, this
too has its applications.

Most comments and feedback are welcome.


Ben Widawsky (13):
  drm/i915: relative_constants_mode race fix
  drm/i915: drop lock support for  i915_wait_request
  drm/i915: remove mm structure from file_priv
  drm/i915: Keep track of drm_file in file_priv
  drm/i915: Keep track of request counts
  drm/i915: fairness
  drm/i915: Keep track of open i915 clients
  drm/i915: debugfs entry for i915 clients
  drm/i915: debugfs entries for scheduler params
  drm/i915: infrastructure to support scheduler types
  drm/i915: get/set scheduler type from debugfs
  drm/i915: Implement batch scheduler
  drm/i915: Add handling for batch parameters in debugfs

Daniel Vetter (2):
  drm/i915: refactor debugfs open function
  drm/i915: refactor debugfs create functions

 drivers/gpu/drm/i915/i915_debugfs.c        |  343 +++++++++++++++++++++++-----
 drivers/gpu/drm/i915/i915_dma.c            |   18 ++-
 drivers/gpu/drm/i915/i915_drv.c            |    5 +
 drivers/gpu/drm/i915/i915_drv.h            |   41 +++-
 drivers/gpu/drm/i915/i915_gem.c            |   90 ++++++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  194 +++++++++++++---
 drivers/gpu/drm/i915/intel_overlay.c       |    4 +-
 7 files changed, 568 insertions(+), 127 deletions(-)

-- 
1.7.7.3

[PATCH 01/15] drm/i915: refactor debugfs open function

[Ben Widawsky]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Only forcewake has an open with special semantics, the other r/w
debugfs only assign the file private pointer.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   26 +++++---------------------
 1 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f2e0207..027681a 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1320,8 +1320,8 @@ static int i915_gen6_forcewake_count_info(struct seq_file *m, void *data)
 }
 
 static int
-i915_wedged_open(struct inode *inode,
-		 struct file *filp)
+i915_debugfs_common_open(struct inode *inode,
+			 struct file *filp)
 {
 	filp->private_data = inode->i_private;
 	return 0;
@@ -1377,20 +1377,12 @@ i915_wedged_write(struct file *filp,
 
 static const struct file_operations i915_wedged_fops = {
 	.owner = THIS_MODULE,
-	.open = i915_wedged_open,
+	.open = i915_debugfs_common_open,
 	.read = i915_wedged_read,
 	.write = i915_wedged_write,
 	.llseek = default_llseek,
 };
 
-static int
-i915_max_freq_open(struct inode *inode,
-		   struct file *filp)
-{
-	filp->private_data = inode->i_private;
-	return 0;
-}
-
 static ssize_t
 i915_max_freq_read(struct file *filp,
 		   char __user *ubuf,
@@ -1447,20 +1439,12 @@ i915_max_freq_write(struct file *filp,
 
 static const struct file_operations i915_max_freq_fops = {
 	.owner = THIS_MODULE,
-	.open = i915_max_freq_open,
+	.open = i915_debugfs_common_open,
 	.read = i915_max_freq_read,
 	.write = i915_max_freq_write,
 	.llseek = default_llseek,
 };
 
-static int
-i915_cache_sharing_open(struct inode *inode,
-		   struct file *filp)
-{
-	filp->private_data = inode->i_private;
-	return 0;
-}
-
 static ssize_t
 i915_cache_sharing_read(struct file *filp,
 		   char __user *ubuf,
@@ -1526,7 +1510,7 @@ i915_cache_sharing_write(struct file *filp,
 
 static const struct file_operations i915_cache_sharing_fops = {
 	.owner = THIS_MODULE,
-	.open = i915_cache_sharing_open,
+	.open = i915_debugfs_common_open,
 	.read = i915_cache_sharing_read,
 	.write = i915_cache_sharing_write,
 	.llseek = default_llseek,
-- 
1.7.7.3

[PATCH 02/15] drm/i915: refactor debugfs create functions

[Ben Widawsky]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

All r/w debugfs files are created equal.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   53 ++++++++++------------------------
 1 files changed, 16 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 027681a..261aa7c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1539,21 +1539,6 @@ drm_add_fake_info_node(struct drm_minor *minor,
 	return 0;
 }
 
-static int i915_wedged_create(struct dentry *root, struct drm_minor *minor)
-{
-	struct drm_device *dev = minor->dev;
-	struct dentry *ent;
-
-	ent = debugfs_create_file("i915_wedged",
-				  S_IRUGO | S_IWUSR,
-				  root, dev,
-				  &i915_wedged_fops);
-	if (IS_ERR(ent))
-		return PTR_ERR(ent);
-
-	return drm_add_fake_info_node(minor, ent, &i915_wedged_fops);
-}
-
 static int i915_forcewake_open(struct inode *inode, struct file *file)
 {
 	struct drm_device *dev = inode->i_private;
@@ -1615,34 +1600,22 @@ static int i915_forcewake_create(struct dentry *root, struct drm_minor *minor)
 	return drm_add_fake_info_node(minor, ent, &i915_forcewake_fops);
 }
 
-static int i915_max_freq_create(struct dentry *root, struct drm_minor *minor)
-{
-	struct drm_device *dev = minor->dev;
-	struct dentry *ent;
-
-	ent = debugfs_create_file("i915_max_freq",
-				  S_IRUGO | S_IWUSR,
-				  root, dev,
-				  &i915_max_freq_fops);
-	if (IS_ERR(ent))
-		return PTR_ERR(ent);
-
-	return drm_add_fake_info_node(minor, ent, &i915_max_freq_fops);
-}
-
-static int i915_cache_sharing_create(struct dentry *root, struct drm_minor *minor)
+static int i915_debugfs_create(struct dentry *root,
+			       struct drm_minor *minor,
+			       const char *name,
+			       const struct file_operations *fops)
 {
 	struct drm_device *dev = minor->dev;
 	struct dentry *ent;
 
-	ent = debugfs_create_file("i915_cache_sharing",
+	ent = debugfs_create_file(name,
 				  S_IRUGO | S_IWUSR,
 				  root, dev,
-				  &i915_cache_sharing_fops);
+				  fops);
 	if (IS_ERR(ent))
 		return PTR_ERR(ent);
 
-	return drm_add_fake_info_node(minor, ent, &i915_cache_sharing_fops);
+	return drm_add_fake_info_node(minor, ent, fops);
 }
 
 static struct drm_info_list i915_debugfs_list[] = {
@@ -1691,17 +1664,23 @@ int i915_debugfs_init(struct drm_minor *minor)
 {
 	int ret;
 
-	ret = i915_wedged_create(minor->debugfs_root, minor);
+	ret = i915_debugfs_create(minor->debugfs_root, minor,
+				  "i915_wedged",
+				  &i915_wedged_fops);
 	if (ret)
 		return ret;
 
 	ret = i915_forcewake_create(minor->debugfs_root, minor);
 	if (ret)
 		return ret;
-	ret = i915_max_freq_create(minor->debugfs_root, minor);
+	ret = i915_debugfs_create(minor->debugfs_root, minor,
+				  "i915_max_freq",
+				  &i915_max_freq_fops);
 	if (ret)
 		return ret;
-	ret = i915_cache_sharing_create(minor->debugfs_root, minor);
+	ret = i915_debugfs_create(minor->debugfs_root, minor,
+				  "i915_cache_sharing",
+				  &i915_cache_sharing_fops);
 	if (ret)
 		return ret;
 
-- 
1.7.7.3

[PATCH 03/15] drm/i915: relative_constants_mode race fix

[Ben Widawsky]

After my refactoring, Chris noticed that we had a bug.

dev_priv keeps track of the current addressing mode that gets set at
execbuffer time. Unfortunately the existing code was doing this before
acquiring struct_mutex which leaves a race with another thread also
doing an execbuffer. If that wasn't bad enough, relocate_slow drops
struct_mutex which opens a much more likely error where another thread
comes in and modifies the state while relocate_slow is being slow.

The solution here is to just defer setting this state until we
absolutely need it, and we know we'll have struct_mutex for the
remainder of our code path.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   67 ++++++++++++++--------------
 1 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3693e83..1d66c24 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1003,39 +1003,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		return -EINVAL;
 	}
 
-	mode = args->flags & I915_EXEC_CONSTANTS_MASK;
-	switch (mode) {
-	case I915_EXEC_CONSTANTS_REL_GENERAL:
-	case I915_EXEC_CONSTANTS_ABSOLUTE:
-	case I915_EXEC_CONSTANTS_REL_SURFACE:
-		if (ring == &dev_priv->ring[RCS] &&
-		    mode != dev_priv->relative_constants_mode) {
-			if (INTEL_INFO(dev)->gen < 4)
-				return -EINVAL;
-
-			if (INTEL_INFO(dev)->gen > 5 &&
-			    mode == I915_EXEC_CONSTANTS_REL_SURFACE)
-				return -EINVAL;
-
-			ret = intel_ring_begin(ring, 4);
-			if (ret)
-				return ret;
-
-			intel_ring_emit(ring, MI_NOOP);
-			intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
-			intel_ring_emit(ring, INSTPM);
-			intel_ring_emit(ring,
-					I915_EXEC_CONSTANTS_MASK << 16 | mode);
-			intel_ring_advance(ring);
-
-			dev_priv->relative_constants_mode = mode;
-		}
-		break;
-	default:
-		DRM_ERROR("execbuf with unknown constants: %d\n", mode);
-		return -EINVAL;
-	}
-
 	if (args->buffer_count < 1) {
 		DRM_ERROR("execbuf with %d buffers\n", args->buffer_count);
 		return -EINVAL;
@@ -1159,6 +1126,40 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		}
 	}
 
+	mode = args->flags & I915_EXEC_CONSTANTS_MASK;
+	switch (mode) {
+	case I915_EXEC_CONSTANTS_REL_GENERAL:
+	case I915_EXEC_CONSTANTS_ABSOLUTE:
+	case I915_EXEC_CONSTANTS_REL_SURFACE:
+		if (ring == &dev_priv->ring[RCS] &&
+		    mode != dev_priv->relative_constants_mode) {
+			if (INTEL_INFO(dev)->gen < 4)
+				return -EINVAL;
+
+			if (INTEL_INFO(dev)->gen > 5 &&
+			    mode == I915_EXEC_CONSTANTS_REL_SURFACE)
+				return -EINVAL;
+
+			ret = intel_ring_begin(ring, 4);
+			if (ret)
+				goto err;
+
+			intel_ring_emit(ring, MI_NOOP);
+			intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+			intel_ring_emit(ring, INSTPM);
+			intel_ring_emit(ring,
+					I915_EXEC_CONSTANTS_MASK << 16 | mode);
+			intel_ring_advance(ring);
+
+			dev_priv->relative_constants_mode = mode;
+		}
+		break;
+	default:
+		DRM_ERROR("execbuf with unknown constants: %d\n", mode);
+		ret = -EINVAL;
+		goto err;
+	}
+
 	trace_i915_gem_ring_dispatch(ring, seqno);
 
 	exec_start = batch_obj->gtt_offset + args->batch_start_offset;
-- 
1.7.7.3

[PATCH 04/15] drm/i915: drop lock support for i915_wait_request

[Ben Widawsky]

Provide a way for callers to instruct the wait request to drop
struct_mutex before the actually wait. This gives an opportunity for GPU
clients to submit independent work.

While it's tempting to make i915_wait_request always drop the lock and
enforce the caller to check their specific criteria for a sane
environment in turns out to be quite difficult to implement in most
cases as almost any state can change out from underneath one the lock is
dropped, and things such as domains aren't easy to figure out whether
they've changed (they could have been modified, and then changed back to
the original flags).

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h      |    3 +-
 drivers/gpu/drm/i915/i915_gem.c      |   42 +++++++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_overlay.c |    4 +-
 3 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d2da91f..9d9d160 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1187,7 +1187,8 @@ int __must_check i915_add_request(struct intel_ring_buffer *ring,
 				  struct drm_file *file,
 				  struct drm_i915_gem_request *request);
 int __must_check i915_wait_request(struct intel_ring_buffer *ring,
-				   uint32_t seqno);
+				   uint32_t seqno,
+				   bool drop_mutex);
 int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);
 int __must_check
 i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ed0b68f..9c743ae 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1943,13 +1943,15 @@ i915_gem_retire_work_handler(struct work_struct *work)
  */
 int
 i915_wait_request(struct intel_ring_buffer *ring,
-		  uint32_t seqno)
+		  uint32_t seqno,
+		  bool drop_mutex)
 {
 	drm_i915_private_t *dev_priv = ring->dev->dev_private;
 	u32 ier;
 	int ret = 0;
 
 	BUG_ON(seqno == 0);
+	BUG_ON(!mutex_is_locked(&ring->dev->struct_mutex));
 
 	if (atomic_read(&dev_priv->mm.wedged)) {
 		struct completion *x = &dev_priv->error_completion;
@@ -1994,8 +1996,12 @@ i915_wait_request(struct intel_ring_buffer *ring,
 
 		trace_i915_gem_request_wait_begin(ring, seqno);
 
-		ring->waiting_seqno = seqno;
+		if (!drop_mutex)
+			ring->waiting_seqno = seqno;
 		if (ring->irq_get(ring)) {
+			if (drop_mutex)
+				mutex_unlock(&ring->dev->struct_mutex);
+
 			if (dev_priv->mm.interruptible)
 				ret = wait_event_interruptible(ring->irq_queue,
 							       i915_seqno_passed(ring->get_seqno(ring), seqno)
@@ -2005,12 +2011,23 @@ i915_wait_request(struct intel_ring_buffer *ring,
 					   i915_seqno_passed(ring->get_seqno(ring), seqno)
 					   || atomic_read(&dev_priv->mm.wedged));
 
+			if (drop_mutex)
+				mutex_lock(&ring->dev->struct_mutex);
+
 			ring->irq_put(ring);
-		} else if (wait_for(i915_seqno_passed(ring->get_seqno(ring),
-						      seqno) ||
-				    atomic_read(&dev_priv->mm.wedged), 3000))
-			ret = -EBUSY;
-		ring->waiting_seqno = 0;
+		} else {
+			if (drop_mutex)
+				mutex_unlock(&ring->dev->struct_mutex);
+			if (wait_for(i915_seqno_passed(ring->get_seqno(ring),
+						       seqno) ||
+				     atomic_read(&dev_priv->mm.wedged), 3000))
+				ret = -EBUSY;
+			if (drop_mutex)
+				mutex_lock(&ring->dev->struct_mutex);
+		}
+
+		if (!drop_mutex)
+			ring->waiting_seqno = 0;
 
 		trace_i915_gem_request_wait_end(ring, seqno);
 	}
@@ -2051,7 +2068,8 @@ i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj)
 	 * it.
 	 */
 	if (obj->active) {
-		ret = i915_wait_request(obj->ring, obj->last_rendering_seqno);
+		ret = i915_wait_request(obj->ring, obj->last_rendering_seqno,
+					false);
 		if (ret)
 			return ret;
 	}
@@ -2186,7 +2204,7 @@ static int i915_ring_idle(struct intel_ring_buffer *ring)
 			return ret;
 	}
 
-	return i915_wait_request(ring, i915_gem_next_request_seqno(ring));
+	return i915_wait_request(ring, i915_gem_next_request_seqno(ring), false);
 }
 
 int
@@ -2400,7 +2418,8 @@ i915_gem_object_flush_fence(struct drm_i915_gem_object *obj,
 		if (!ring_passed_seqno(obj->last_fenced_ring,
 				       obj->last_fenced_seqno)) {
 			ret = i915_wait_request(obj->last_fenced_ring,
-						obj->last_fenced_seqno);
+						obj->last_fenced_seqno,
+						false);
 			if (ret)
 				return ret;
 		}
@@ -2541,7 +2560,8 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj,
 				if (!ring_passed_seqno(obj->last_fenced_ring,
 						       reg->setup_seqno)) {
 					ret = i915_wait_request(obj->last_fenced_ring,
-								reg->setup_seqno);
+								reg->setup_seqno,
+								false);
 					if (ret)
 						return ret;
 				}
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index cdf17d4..8f27f2b 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -227,7 +227,7 @@ static int intel_overlay_do_wait_request(struct intel_overlay *overlay,
 	}
 	overlay->last_flip_req = request->seqno;
 	overlay->flip_tail = tail;
-	ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
+	ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req, false);
 	if (ret)
 		return ret;
 
@@ -448,7 +448,7 @@ static int intel_overlay_recover_from_interrupt(struct intel_overlay *overlay)
 	if (overlay->last_flip_req == 0)
 		return 0;
 
-	ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req);
+	ret = i915_wait_request(LP_RING(dev_priv), overlay->last_flip_req, false);
 	if (ret)
 		return ret;
 
-- 
1.7.7.3

[PATCH 05/15] drm/i915: remove mm structure from file_priv

[Ben Widawsky]

We will be using file_priv for scheduler things, and request list which
is useful for us isn't only mm related.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_dma.c |    4 ++--
 drivers/gpu/drm/i915/i915_drv.h |    7 +++----
 drivers/gpu/drm/i915/i915_gem.c |   24 ++++++++++++------------
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 2eac955..43131bf 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2202,8 +2202,8 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
 
 	file->driver_priv = file_priv;
 
-	spin_lock_init(&file_priv->mm.lock);
-	INIT_LIST_HEAD(&file_priv->mm.request_list);
+	spin_lock_init(&file_priv->lock);
+	INIT_LIST_HEAD(&file_priv->request_list);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9d9d160..fe06613 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -917,10 +917,9 @@ struct drm_i915_gem_request {
 };
 
 struct drm_i915_file_private {
-	struct {
-		struct spinlock lock;
-		struct list_head request_list;
-	} mm;
+	struct spinlock lock;
+
+	struct list_head request_list;
 };
 
 #define INTEL_INFO(dev)	(((struct drm_i915_private *) (dev)->dev_private)->info)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9c743ae..c90c4b4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1674,11 +1674,11 @@ i915_add_request(struct intel_ring_buffer *ring,
 	if (file) {
 		struct drm_i915_file_private *file_priv = file->driver_priv;
 
-		spin_lock(&file_priv->mm.lock);
+		spin_lock(&file_priv->lock);
 		request->file_priv = file_priv;
 		list_add_tail(&request->client_list,
-			      &file_priv->mm.request_list);
-		spin_unlock(&file_priv->mm.lock);
+			      &file_priv->request_list);
+		spin_unlock(&file_priv->lock);
 	}
 
 	ring->outstanding_lazy_request = false;
@@ -1704,12 +1704,12 @@ i915_gem_request_remove_from_client(struct drm_i915_gem_request *request)
 	if (!file_priv)
 		return;
 
-	spin_lock(&file_priv->mm.lock);
+	spin_lock(&file_priv->lock);
 	if (request->file_priv) {
 		list_del(&request->client_list);
 		request->file_priv = NULL;
 	}
-	spin_unlock(&file_priv->mm.lock);
+	spin_unlock(&file_priv->lock);
 }
 
 static void i915_gem_reset_ring_lists(struct drm_i915_private *dev_priv,
@@ -3301,15 +3301,15 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file)
 	if (atomic_read(&dev_priv->mm.wedged))
 		return -EIO;
 
-	spin_lock(&file_priv->mm.lock);
-	list_for_each_entry(request, &file_priv->mm.request_list, client_list) {
+	spin_lock(&file_priv->lock);
+	list_for_each_entry(request, &file_priv->request_list, client_list) {
 		if (time_after_eq(request->emitted_jiffies, recent_enough))
 			break;
 
 		ring = request->ring;
 		seqno = request->seqno;
 	}
-	spin_unlock(&file_priv->mm.lock);
+	spin_unlock(&file_priv->lock);
 
 	if (seqno == 0)
 		return 0;
@@ -4131,17 +4131,17 @@ void i915_gem_release(struct drm_device *dev, struct drm_file *file)
 	 * later retire_requests won't dereference our soon-to-be-gone
 	 * file_priv.
 	 */
-	spin_lock(&file_priv->mm.lock);
-	while (!list_empty(&file_priv->mm.request_list)) {
+	spin_lock(&file_priv->lock);
+	while (!list_empty(&file_priv->request_list)) {
 		struct drm_i915_gem_request *request;
 
-		request = list_first_entry(&file_priv->mm.request_list,
+		request = list_first_entry(&file_priv->request_list,
 					   struct drm_i915_gem_request,
 					   client_list);
 		list_del(&request->client_list);
 		request->file_priv = NULL;
 	}
-	spin_unlock(&file_priv->mm.lock);
+	spin_unlock(&file_priv->lock);
 }
 
 static int
-- 
1.7.7.3

[PATCH 06/15] drm/i915: Keep track of drm_file in file_priv

[Ben Widawsky]

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_dma.c |    1 +
 drivers/gpu/drm/i915/i915_drv.h |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 43131bf..697db7b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2201,6 +2201,7 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
 		return -ENOMEM;
 
 	file->driver_priv = file_priv;
+	file_priv->drm_file = file;
 
 	spin_lock_init(&file_priv->lock);
 	INIT_LIST_HEAD(&file_priv->request_list);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fe06613..b0ce5c8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -917,6 +917,7 @@ struct drm_i915_gem_request {
 };
 
 struct drm_i915_file_private {
+	struct drm_file *drm_file;
 	struct spinlock lock;
 
 	struct list_head request_list;
-- 
1.7.7.3

[PATCH 07/15] drm/i915: Keep track of request counts

[Ben Widawsky]

There is already a list of requests outstanding for a given client.
Keeping a count is easy, and will give a quick way to determine when a
particular client has gotten too greedy.

For now a client is uniquely identified by its file descriptor. This is a
limitation in the design which is acceptable for now.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_dma.c |    1 +
 drivers/gpu/drm/i915/i915_drv.h |    1 +
 drivers/gpu/drm/i915/i915_gem.c |   10 ++++++++++
 3 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 697db7b..d1c9a6f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2205,6 +2205,7 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
 
 	spin_lock_init(&file_priv->lock);
 	INIT_LIST_HEAD(&file_priv->request_list);
+	file_priv->outstanding_requests = 0;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b0ce5c8..406279c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -921,6 +921,7 @@ struct drm_i915_file_private {
 	struct spinlock lock;
 
 	struct list_head request_list;
+	int outstanding_requests;
 };
 
 #define INTEL_INFO(dev)	(((struct drm_i915_private *) (dev)->dev_private)->info)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c90c4b4..ee9a77a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1678,6 +1678,7 @@ i915_add_request(struct intel_ring_buffer *ring,
 		request->file_priv = file_priv;
 		list_add_tail(&request->client_list,
 			      &file_priv->request_list);
+		file_priv->outstanding_requests++;
 		spin_unlock(&file_priv->lock);
 	}
 
@@ -1706,6 +1707,10 @@ i915_gem_request_remove_from_client(struct drm_i915_gem_request *request)
 
 	spin_lock(&file_priv->lock);
 	if (request->file_priv) {
+		file_priv->outstanding_requests--;
+		if (WARN_ON(file_priv->outstanding_requests < 0)) {
+			file_priv->outstanding_requests = 0;
+		}
 		list_del(&request->client_list);
 		request->file_priv = NULL;
 	}
@@ -4139,8 +4144,13 @@ void i915_gem_release(struct drm_device *dev, struct drm_file *file)
 					   struct drm_i915_gem_request,
 					   client_list);
 		list_del(&request->client_list);
+		file_priv->outstanding_requests--;
 		request->file_priv = NULL;
 	}
+	if (file_priv->outstanding_requests != 0) {
+		DRM_ERROR("Count was %d\n", file_priv->outstanding_requests);
+	}
+	file_priv->outstanding_requests = -1;
 	spin_unlock(&file_priv->lock);
 }
 
-- 
1.7.7.3

[PATCH 08/15] drm/i915: fairness

[Ben Widawsky]

We now have enough information to be able to block apps which are
getting greedy.

We don't have HW support for preempting batches, therefore the finest
granularity for which we can schedule is the batch buffer. We can block
clients at the time they submit if they've gone too many batches ahead.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_dma.c            |    1 +
 drivers/gpu/drm/i915/i915_drv.c            |    5 ++
 drivers/gpu/drm/i915/i915_drv.h            |   16 ++++
 drivers/gpu/drm/i915/i915_gem.c            |    4 +
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  109 +++++++++++++++++++++++++++-
 5 files changed, 131 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index d1c9a6f..942c5c2 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2206,6 +2206,7 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
 	spin_lock_init(&file_priv->lock);
 	INIT_LIST_HEAD(&file_priv->request_list);
 	file_priv->outstanding_requests = 0;
+	file_priv->forced_throttles = 0;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 548e04b..79653b6 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -102,6 +102,11 @@ MODULE_PARM_DESC(enable_hangcheck,
 		"WARNING: Disabling this can cause system wide hangs. "
 		"(default: true)");
 
+int i915_scheduler __read_mostly = 0;
+module_param_named(scheduler, i915_scheduler, int, 0444);
+MODULE_PARM_DESC(scheduler,
+		"GPU workload scheduling. (default: off)");
+
 static struct drm_driver driver;
 extern int intel_agp_enabled;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 406279c..2917e54 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -734,6 +734,17 @@ typedef struct drm_i915_private {
 	struct drm_property *force_audio_property;
 
 	atomic_t forcewake_count;
+
+	/* For the foreseeable future, we will only have 2 real scheduler
+	 * types, and both are really similar. If the count grows past that,
+	 * we'd want to abstract this better.
+	 */
+	struct {
+		/* Point at which we consider blocking */
+		unsigned high_watermark;
+		/* Point when we consider unblocking */
+		unsigned low_watermark;
+	} scheduler;
 } drm_i915_private_t;
 
 enum i915_cache_level {
@@ -922,6 +933,7 @@ struct drm_i915_file_private {
 
 	struct list_head request_list;
 	int outstanding_requests;
+	int forced_throttles;
 };
 
 #define INTEL_INFO(dev)	(((struct drm_i915_private *) (dev)->dev_private)->info)
@@ -1006,6 +1018,10 @@ extern int i915_vbt_sdvo_panel_type __read_mostly;
 extern unsigned int i915_enable_rc6 __read_mostly;
 extern unsigned int i915_enable_fbc __read_mostly;
 extern bool i915_enable_hangcheck __read_mostly;
+extern int i915_scheduler __read_mostly ;
+extern unsigned int i915_sched_hog_threshold;
+extern unsigned int i915_sched_low_watermark;
+
 
 extern int i915_suspend(struct drm_device *dev, pm_message_t state);
 extern int i915_resume(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ee9a77a..2a2f0cd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3942,6 +3942,10 @@ i915_gem_load(struct drm_device *dev)
 	dev_priv->mm.inactive_shrinker.shrink = i915_gem_inactive_shrink;
 	dev_priv->mm.inactive_shrinker.seeks = DEFAULT_SEEKS;
 	register_shrinker(&dev_priv->mm.inactive_shrinker);
+
+	/* Set some decent defaults for the scheudler */
+	dev_priv->scheduler.high_watermark = 50;
+	dev_priv->scheduler.low_watermark = 5;
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1d66c24..744ff9b 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -953,6 +953,87 @@ i915_gem_execbuffer_retire_commands(struct drm_device *dev,
 	}
 }
 
+enum {
+	I915_RESCHED,
+	I915_OUT,
+	I915_DONE,
+	I915_ERROR
+};
+
+static int
+i915_schedule(struct drm_device *dev, struct intel_ring_buffer *ring, struct drm_file *file)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_i915_file_private *file_priv = file->driver_priv;
+	struct drm_i915_gem_request *request = NULL;
+	u32 seqno = 0;
+	int ret, i = 0;
+
+#define BLOCK_CONDITION ( \
+	i915_scheduler && \
+	(file_priv->outstanding_requests > dev_priv->scheduler.high_watermark))
+#define OUT_CONDITION ( \
+	(atomic_read(&dev_priv->mm.wedged)) || \
+	(dev_priv->mm.suspended))
+#define RUNNABLE ( \
+	file_priv->outstanding_requests < dev_priv->scheduler.low_watermark)
+
+	spin_lock(&file_priv->lock);
+	if (!BLOCK_CONDITION) {
+		spin_unlock(&file_priv->lock);
+		return I915_DONE;
+	}
+
+	/* Check if our fd was released while spinning*/
+	if (file_priv->outstanding_requests == -1) {
+		spin_unlock(&file_priv->lock);
+		return I915_OUT;
+	}
+
+	/* Check if we're now able to run */
+	if (RUNNABLE) {
+		spin_unlock(&file_priv->lock);
+		return I915_DONE;
+	}
+
+	list_for_each_entry(request, &file_priv->request_list, client_list) {
+		if (i == dev_priv->scheduler.low_watermark) {
+			seqno = request->seqno;
+			break;
+		}
+		i++;
+	}
+	spin_unlock(&file_priv->lock);
+
+	BUG_ON(seqno == 0 || i < dev_priv->scheduler.low_watermark);
+
+	ret = i915_wait_request(ring, seqno, true);
+	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
+
+	/* Check if our fd was released while spinning*/
+	spin_lock(&file_priv->lock);
+	if (file_priv->outstanding_requests == -1) {
+		DRM_INFO("File was closed while waiting\n");
+		spin_unlock(&file_priv->lock);
+		return I915_OUT;
+	} else {
+		file_priv->forced_throttles++;
+		spin_unlock(&file_priv->lock);
+	}
+
+	if (ret == -ERESTARTSYS || (ret == 0 && OUT_CONDITION))
+		return I915_OUT;
+	else if (ret)
+		return I915_OUT;
+	else
+		return I915_RESCHED;
+
+#undef RUNNABLE
+#undef BLOCK_CONDITION
+#undef OUT_CONDITION
+		return I915_ERROR;
+}
+
 static int
 i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		       struct drm_file *file,
@@ -961,7 +1042,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct list_head objects;
-	struct eb_objects *eb;
+	struct eb_objects *eb = NULL;
 	struct drm_i915_gem_object *batch_obj;
 	struct drm_clip_rect *cliprects = NULL;
 	struct intel_ring_buffer *ring;
@@ -1034,17 +1115,37 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	if (ret)
 		goto pre_mutex_err;
 
+again:
 	if (dev_priv->mm.suspended) {
 		mutex_unlock(&dev->struct_mutex);
 		ret = -EBUSY;
 		goto pre_mutex_err;
 	}
 
-	eb = eb_create(args->buffer_count);
-	if (eb == NULL) {
+	if (!eb) {
+		eb = eb_create(args->buffer_count);
+		if (eb == NULL) {
+			mutex_unlock(&dev->struct_mutex);
+			ret = -ENOMEM;
+			goto pre_mutex_err;
+		}
+	}
+
+	switch (i915_schedule(dev, ring, file)) {
+	case I915_RESCHED:
+		goto again;
+		break;
+	case I915_OUT:
+		eb_destroy(eb);
 		mutex_unlock(&dev->struct_mutex);
-		ret = -ENOMEM;
+		ret = -EINVAL;
 		goto pre_mutex_err;
+		break;
+	case I915_DONE:
+		break;
+	case I915_ERROR:
+	default:
+		BUG();
 	}
 
 	/* Look up object handles */
-- 
1.7.7.3

[PATCH 09/15] drm/i915: Keep track of open i915 clients

[Ben Widawsky]

DRM core keeps track of open clients, but that may be include clients
which are not relevant to i915. So we keep our own count to help us
track when certain events are interesting.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_dma.c |    9 +++++++++
 drivers/gpu/drm/i915/i915_drv.h |    6 ++++++
 drivers/gpu/drm/i915/i915_gem.c |    7 +++++++
 3 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 942c5c2..df06a7e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2073,6 +2073,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	ips_ping_for_i915_load();
 
+	INIT_LIST_HEAD(&dev_priv->i915_client_list);
+
 	return 0;
 
 out_gem_unload:
@@ -2194,6 +2196,7 @@ int i915_driver_unload(struct drm_device *dev)
 int i915_driver_open(struct drm_device *dev, struct drm_file *file)
 {
 	struct drm_i915_file_private *file_priv;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	DRM_DEBUG_DRIVER("\n");
 	file_priv = kmalloc(sizeof(*file_priv), GFP_KERNEL);
@@ -2205,9 +2208,15 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
 
 	spin_lock_init(&file_priv->lock);
 	INIT_LIST_HEAD(&file_priv->request_list);
+	INIT_LIST_HEAD(&file_priv->client_link);
 	file_priv->outstanding_requests = 0;
 	file_priv->forced_throttles = 0;
 
+	mutex_lock(&dev->struct_mutex);
+	dev_priv->num_clients++;
+	list_add(&file_priv->client_link, &dev_priv->i915_client_list);
+	mutex_unlock(&dev->struct_mutex);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2917e54..3ce03a0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -735,6 +735,10 @@ typedef struct drm_i915_private {
 
 	atomic_t forcewake_count;
 
+	int num_clients;
+	/* There's something already named client_list... too bad */
+	struct list_head i915_client_list;
+
 	/* For the foreseeable future, we will only have 2 real scheduler
 	 * types, and both are really similar. If the count grows past that,
 	 * we'd want to abstract this better.
@@ -934,6 +938,8 @@ struct drm_i915_file_private {
 	struct list_head request_list;
 	int outstanding_requests;
 	int forced_throttles;
+
+	struct list_head client_link;
 };
 
 #define INTEL_INFO(dev)	(((struct drm_i915_private *) (dev)->dev_private)->info)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2a2f0cd..5b7072d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4135,6 +4135,7 @@ i915_gem_phys_pwrite(struct drm_device *dev,
 void i915_gem_release(struct drm_device *dev, struct drm_file *file)
 {
 	struct drm_i915_file_private *file_priv = file->driver_priv;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	/* Clean up our request list when the client is going away, so that
 	 * later retire_requests won't dereference our soon-to-be-gone
@@ -4156,6 +4157,12 @@ void i915_gem_release(struct drm_device *dev, struct drm_file *file)
 	}
 	file_priv->outstanding_requests = -1;
 	spin_unlock(&file_priv->lock);
+
+	mutex_lock(&dev->struct_mutex);
+	--dev_priv->num_clients;
+	list_del(&file_priv->client_link);
+	mutex_unlock(&dev->struct_mutex);
+
 }
 
 static int
-- 
1.7.7.3

[PATCH 10/15] drm/i915: debugfs entry for i915 clients

[Ben Widawsky]

$ cat /sys/kernel/debug/dri/0/i915_clients
PID   CMD
15869  X

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 261aa7c..59b9f22 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1307,6 +1307,33 @@ static int i915_context_status(struct seq_file *m, void *unused)
 	return 0;
 }
 
+static int i915_clients(struct seq_file *m, void *unused)
+{
+	struct drm_info_node *node = (struct drm_info_node *) m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_file_private *temp;
+	int ret;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	seq_printf(m, "PID   CMD\n");
+	list_for_each_entry(temp, &dev_priv->i915_client_list, client_link) {
+		struct pid *p;
+		struct task_struct *tsk;
+		pid_t pid = temp->drm_file->pid;
+		p = find_get_pid(pid);
+		tsk = get_pid_task(p, PIDTYPE_PID);
+		seq_printf(m, "%d  %s\n", pid, tsk->comm);
+		put_pid(p);
+	}
+
+	mutex_unlock(&dev->struct_mutex);
+	return 0;
+}
+
 static int i915_gen6_forcewake_count_info(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
@@ -1657,6 +1684,7 @@ static struct drm_info_list i915_debugfs_list[] = {
 	{"i915_gem_framebuffer", i915_gem_framebuffer_info, 0},
 	{"i915_context_status", i915_context_status, 0},
 	{"i915_gen6_forcewake_count", i915_gen6_forcewake_count_info, 0},
+	{"i915_clients", i915_clients, 0},
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
 
-- 
1.7.7.3

[PATCH 11/15] drm/i915: debugfs entries for scheduler params

[Ben Widawsky]

Using the i915 client list we can easily read and set the various
parameters.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  138 +++++++++++++++++++++++++++++++++++
 1 files changed, 138 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 59b9f22..790875d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1543,6 +1543,136 @@ static const struct file_operations i915_cache_sharing_fops = {
 	.llseek = default_llseek,
 };
 
+static ssize_t
+i915_sched_read(struct file *filp,
+		char __user *ubuf,
+		size_t max,
+		loff_t *ppos)
+{
+	struct drm_device *dev = filp->private_data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_file_private *temp;
+	unsigned high, low;
+	size_t len = 0;
+	char *buf, *buf2;
+	struct {
+		pid_t pid;
+		int outstanding;
+		int forced;
+		int high;
+		int low;
+	} requests[20];
+	int entries, entry_size, i = 0, ret = 0;
+
+	if (!i915_scheduler)
+		return 0;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	high = dev_priv->scheduler.high_watermark;
+	low = dev_priv->scheduler.low_watermark;
+	list_for_each_entry(temp, &dev_priv->i915_client_list, client_link) {
+		struct drm_file *file = temp->drm_file;
+		requests[i].pid = file->pid;
+		spin_lock(&temp->lock);
+		requests[i].outstanding = temp->outstanding_requests;
+		requests[i].forced = temp->forced_throttles;
+		spin_unlock(&temp->lock);
+		i++;
+	}
+	mutex_unlock(&dev->struct_mutex);
+
+	entries = i;
+
+	entry_size = strlen("pid 123456 = XXXX outstanding (XXXXXX forced)\n") + 1;
+	buf2 = drm_malloc_ab(entries, entry_size);
+	memset(buf2, 0, entries * entry_size);
+	for(i = 0; i < entries; i++) {
+		len += sprintf(buf2 + len,
+			       "pid %06d = %06d outstanding (%06d forced)\n",
+				requests[i].pid, requests[i].outstanding,
+				requests[i].forced);
+	}
+
+	buf = kasprintf(GFP_KERNEL, "%s\n"
+				    "Scheduler = %s\n"
+				    "High watermark = %d buffers\n"
+				    "Low watermark = %d buffers\n",
+				     buf2, "fair", high, low);
+
+	len = strlen(buf);
+
+	ret = simple_read_from_buffer(ubuf, max, ppos, buf, len);
+
+	drm_free_large(buf2);
+	kfree(buf);
+	return ret;
+}
+
+static ssize_t
+i915_sched_write(struct file *filp,
+		 const char __user *ubuf,
+		 size_t cnt,
+		 loff_t *ppos)
+{
+	struct drm_device *dev = filp->private_data;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	char *buf, *ptr;
+	uint32_t high, low;
+	int ret;
+
+	buf = drm_malloc_ab(cnt + 1, 1);
+	if (!buf)
+		return -E2BIG;
+
+	ret = copy_from_user(buf, ubuf, cnt);
+	if (ret)
+		goto out;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	low = dev_priv->scheduler.low_watermark;
+	high = dev_priv->scheduler.high_watermark;
+
+	buf[cnt] = 0;
+	ptr = buf;
+
+	while ((ptr != NULL) && *ptr != 0) {
+		if (!strncmp(ptr, "high=", 5))
+			high = simple_strtoul(ptr + 5, NULL, 0);
+		if (!strncmp(ptr, "low=", 4))
+			low = simple_strtoul(ptr + 4, NULL, 0);
+		ptr = strchr(ptr, ',');
+		if (ptr != NULL)
+			ptr += strspn(ptr, ", \t\n");
+	}
+
+	if (high != dev_priv->scheduler.high_watermark ||
+	    low != dev_priv->scheduler.low_watermark) {
+		DRM_INFO("new high = %d\n", high);
+		DRM_INFO("new low = %d\n", low);
+		dev_priv->scheduler.high_watermark = high;
+		dev_priv->scheduler.low_watermark = low;
+	}
+
+	mutex_unlock(&dev->struct_mutex);
+out:
+	drm_free_large(buf);
+	return cnt;
+}
+
+static const struct file_operations i915_scheduler_fops = {
+	.owner = THIS_MODULE,
+	.open = i915_debugfs_common_open,
+	.read = i915_sched_read,
+	.write = i915_sched_write,
+};
+
+
 /* As the drm_debugfs_init() routines are called before dev->dev_private is
  * allocated we need to hook into the minor for release. */
 static int
@@ -1712,6 +1842,12 @@ int i915_debugfs_init(struct drm_minor *minor)
 	if (ret)
 		return ret;
 
+	ret = i915_debugfs_create(minor->debugfs_root, minor,
+				  "i915_scheduler",
+				  &i915_scheduler_fops);
+	if (ret)
+		return ret;
+
 	return drm_debugfs_create_files(i915_debugfs_list,
 					I915_DEBUGFS_ENTRIES,
 					minor->debugfs_root, minor);
@@ -1729,6 +1865,8 @@ void i915_debugfs_cleanup(struct drm_minor *minor)
 				 1, minor);
 	drm_debugfs_remove_files((struct drm_info_list *) &i915_cache_sharing_fops,
 				 1, minor);
+	drm_debugfs_remove_files((struct drm_info_list *) &i915_scheduler_fops,
+				 1, minor);
 }
 
 #endif /* CONFIG_DEBUG_FS */
-- 
1.7.7.3

[PATCH 12/15] drm/i915: infrastructure to support scheduler types

[Ben Widawsky]

As of now there are only two types, fairness, and none. This can be set
using the same module parameter, and will use default values for
whatever type is chosen. Those values should be readable through
debugfs.
---
 drivers/gpu/drm/i915/i915_drv.h            |    4 ++++
 drivers/gpu/drm/i915/i915_gem.c            |    9 ++++++---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |    2 +-
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3ce03a0..a12445b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -744,6 +744,10 @@ typedef struct drm_i915_private {
 	 * we'd want to abstract this better.
 	 */
 	struct {
+		#define I915_SCHEDULER_NONE 0
+		#define I915_SCHEDULER_FAIR 1
+		#define I915_SCHEDULER_INVALID 2
+		unsigned type;
 		/* Point at which we consider blocking */
 		unsigned high_watermark;
 		/* Point when we consider unblocking */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5b7072d..69d4a59 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3943,9 +3943,12 @@ i915_gem_load(struct drm_device *dev)
 	dev_priv->mm.inactive_shrinker.seeks = DEFAULT_SEEKS;
 	register_shrinker(&dev_priv->mm.inactive_shrinker);
 
-	/* Set some decent defaults for the scheudler */
-	dev_priv->scheduler.high_watermark = 50;
-	dev_priv->scheduler.low_watermark = 5;
+	if (i915_scheduler <= I915_SCHEDULER_INVALID) {
+		dev_priv->scheduler.type = i915_scheduler;
+		/* Set some decent defaults for the fair scheduler */
+		dev_priv->scheduler.high_watermark = 50;
+		dev_priv->scheduler.low_watermark = 5;
+	}
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 744ff9b..c2e96be 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -970,7 +970,7 @@ i915_schedule(struct drm_device *dev, struct intel_ring_buffer *ring, struct drm
 	int ret, i = 0;
 
 #define BLOCK_CONDITION ( \
-	i915_scheduler && \
+	(dev_priv->scheduler.type != I915_SCHEDULER_NONE) && \
 	(file_priv->outstanding_requests > dev_priv->scheduler.high_watermark))
 #define OUT_CONDITION ( \
 	(atomic_read(&dev_priv->mm.wedged)) || \
-- 
1.7.7.3

[PATCH 13/15] drm/i915: get/set scheduler type from debugfs

[Ben Widawsky]

Allows users to get and set the type of GPU scheduling done in the i915
driver.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   30 +++++++++++++++++++++++-------
 1 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 790875d..903a004 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1543,6 +1543,9 @@ static const struct file_operations i915_cache_sharing_fops = {
 	.llseek = default_llseek,
 };
 
+static const char *sched_type[I915_SCHEDULER_INVALID] =
+	{"none", "fair"};
+
 static ssize_t
 i915_sched_read(struct file *filp,
 		char __user *ubuf,
@@ -1552,7 +1555,7 @@ i915_sched_read(struct file *filp,
 	struct drm_device *dev = filp->private_data;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_file_private *temp;
-	unsigned high, low;
+	unsigned high, low, type;
 	size_t len = 0;
 	char *buf, *buf2;
 	struct {
@@ -1564,13 +1567,11 @@ i915_sched_read(struct file *filp,
 	} requests[20];
 	int entries, entry_size, i = 0, ret = 0;
 
-	if (!i915_scheduler)
-		return 0;
-
 	ret = mutex_lock_interruptible(&dev->struct_mutex);
 	if (ret)
 		return ret;
 
+	type = dev_priv->scheduler.type;
 	high = dev_priv->scheduler.high_watermark;
 	low = dev_priv->scheduler.low_watermark;
 	list_for_each_entry(temp, &dev_priv->i915_client_list, client_link) {
@@ -1600,7 +1601,7 @@ i915_sched_read(struct file *filp,
 				    "Scheduler = %s\n"
 				    "High watermark = %d buffers\n"
 				    "Low watermark = %d buffers\n",
-				     buf2, "fair", high, low);
+				     buf2, sched_type[type], high, low);
 
 	len = strlen(buf);
 
@@ -1620,7 +1621,7 @@ i915_sched_write(struct file *filp,
 	struct drm_device *dev = filp->private_data;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	char *buf, *ptr;
-	uint32_t high, low;
+	uint32_t high, low, type;
 	int ret;
 
 	buf = drm_malloc_ab(cnt + 1, 1);
@@ -1637,11 +1638,21 @@ i915_sched_write(struct file *filp,
 
 	low = dev_priv->scheduler.low_watermark;
 	high = dev_priv->scheduler.high_watermark;
+	type = dev_priv->scheduler.type;
 
 	buf[cnt] = 0;
 	ptr = buf;
 
 	while ((ptr != NULL) && *ptr != 0) {
+		if (!strncmp(ptr, "type=", 5)) {
+			char *tmp = strchr(ptr, '=');
+			if (tmp != NULL)
+				tmp += strspn(tmp, "= \t\n");
+			if (!strncmp("fair", tmp, 4))
+				type = I915_SCHEDULER_FAIR;
+			if (!strncmp("none", tmp, 4))
+				type = I915_SCHEDULER_NONE;
+		}
 		if (!strncmp(ptr, "high=", 5))
 			high = simple_strtoul(ptr + 5, NULL, 0);
 		if (!strncmp(ptr, "low=", 4))
@@ -1652,11 +1663,16 @@ i915_sched_write(struct file *filp,
 	}
 
 	if (high != dev_priv->scheduler.high_watermark ||
-	    low != dev_priv->scheduler.low_watermark) {
+	    (low != dev_priv->scheduler.low_watermark) ||
+	    (type != dev_priv->scheduler.type)) {
+		if (i915_gpu_idle(dev))
+			DRM_ERROR("Couldn't idle GPU before sched changes\n");
+		DRM_INFO("new type = %d\n", type);
 		DRM_INFO("new high = %d\n", high);
 		DRM_INFO("new low = %d\n", low);
 		dev_priv->scheduler.high_watermark = high;
 		dev_priv->scheduler.low_watermark = low;
+		dev_priv->scheduler.type = type;
 	}
 
 	mutex_unlock(&dev->struct_mutex);
-- 
1.7.7.3

[PATCH 14/15] drm/i915: Implement batch scheduler

[Ben Widawsky]

While the fairness scheduler also currently uses batches to attempt to
keep things fair, that is an implementation detail. A fairness scheduler
should still be fair even if we achieve a better granularity on future
products.  The batch scheduler on the other hand is meant to remain
relevant even in light of newer scheduling metrics.

The batch scheduler introduces a per-client high and low watermark.
These watermarks define when the client will get blocked, and when it
will get unblocked in terms of number of batches submitted.

For example, if we wanted to run X as unthrottled, we could use a
high watermark of -1, and a low watermark of 0. In theory it is also
accomplished by high watermark = low watermark.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_dma.c            |    2 +
 drivers/gpu/drm/i915/i915_drv.h            |    5 +++-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   32 +++++++++++++++++++++------
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index df06a7e..a871b95 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2211,6 +2211,8 @@ int i915_driver_open(struct drm_device *dev, struct drm_file *file)
 	INIT_LIST_HEAD(&file_priv->client_link);
 	file_priv->outstanding_requests = 0;
 	file_priv->forced_throttles = 0;
+	file_priv->high_watermark = 50;
+	file_priv->low_watermark = 5;
 
 	mutex_lock(&dev->struct_mutex);
 	dev_priv->num_clients++;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a12445b..9933e01 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -746,7 +746,8 @@ typedef struct drm_i915_private {
 	struct {
 		#define I915_SCHEDULER_NONE 0
 		#define I915_SCHEDULER_FAIR 1
-		#define I915_SCHEDULER_INVALID 2
+		#define I915_SCHEDULER_BATCH 2
+		#define I915_SCHEDULER_INVALID 3
 		unsigned type;
 		/* Point at which we consider blocking */
 		unsigned high_watermark;
@@ -942,6 +943,8 @@ struct drm_i915_file_private {
 	struct list_head request_list;
 	int outstanding_requests;
 	int forced_throttles;
+	unsigned high_watermark;
+	unsigned low_watermark;
 
 	struct list_head client_link;
 };
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index c2e96be..a24e890 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -968,17 +968,35 @@ i915_schedule(struct drm_device *dev, struct intel_ring_buffer *ring, struct drm
 	struct drm_i915_gem_request *request = NULL;
 	u32 seqno = 0;
 	int ret, i = 0;
+	uint32_t high_watermark = 0, low_watermark = 0;
 
-#define BLOCK_CONDITION ( \
-	(dev_priv->scheduler.type != I915_SCHEDULER_NONE) && \
-	(file_priv->outstanding_requests > dev_priv->scheduler.high_watermark))
+	switch (dev_priv->scheduler.type) {
+	case I915_SCHEDULER_NONE:
+		return I915_DONE;
+	case I915_SCHEDULER_FAIR:
+		high_watermark = dev_priv->scheduler.high_watermark;
+		low_watermark = dev_priv->scheduler.low_watermark;
+		break;
+	case I915_SCHEDULER_BATCH:
+		/* Can't do anything until we hold the spinlock */
+		break;
+	default:
+		WARN_ON(1);
+		return I915_DONE;
+	}
+
+#define BLOCK_CONDITION (file_priv->outstanding_requests > high_watermark)
 #define OUT_CONDITION ( \
 	(atomic_read(&dev_priv->mm.wedged)) || \
 	(dev_priv->mm.suspended))
-#define RUNNABLE ( \
-	file_priv->outstanding_requests < dev_priv->scheduler.low_watermark)
+#define RUNNABLE (file_priv->outstanding_requests < low_watermark)
 
 	spin_lock(&file_priv->lock);
+	if (dev_priv->scheduler.type == I915_SCHEDULER_BATCH) {
+		BUG_ON(high_watermark != 0 || low_watermark != 0);
+		high_watermark = file_priv->high_watermark;
+		low_watermark = file_priv->low_watermark;
+	}
 	if (!BLOCK_CONDITION) {
 		spin_unlock(&file_priv->lock);
 		return I915_DONE;
@@ -997,7 +1015,7 @@ i915_schedule(struct drm_device *dev, struct intel_ring_buffer *ring, struct drm
 	}
 
 	list_for_each_entry(request, &file_priv->request_list, client_list) {
-		if (i == dev_priv->scheduler.low_watermark) {
+		if (i == low_watermark) {
 			seqno = request->seqno;
 			break;
 		}
@@ -1005,7 +1023,7 @@ i915_schedule(struct drm_device *dev, struct intel_ring_buffer *ring, struct drm
 	}
 	spin_unlock(&file_priv->lock);
 
-	BUG_ON(seqno == 0 || i < dev_priv->scheduler.low_watermark);
+	BUG_ON(seqno == 0 || i < low_watermark);
 
 	ret = i915_wait_request(ring, seqno, true);
 	BUG_ON(!mutex_is_locked(&dev->struct_mutex));
-- 
1.7.7.3

[PATCH 15/15] drm/i915: Add handling for batch parameters in debugfs

[Ben Widawsky]

Module parameter only allows default high/low watermarks when batch scheduling.
Here we introduce some new debugfs parsing to allow settings per pid.

This patch is a bit more than just handling batch params as the code turned out
quite ugly as a result. So a bit of cleaning up happens here too, and ideally
it would be split out into another commit.

Here are some usage examples:
Set the globally fair scheduler
echo "type=fair,high=10,low=2" > /sys/kernel/debug/dri/0/i915_scheduler

Turn off scheduling
echo "type=none" > /sys/kernel/debug/dri/0/i915_scheduler

Set high watermark of 10, an low of 2 for pid 16603
echo "pidh=16603-10,pidl=16603,2"> /sys/kernel/debug/dri/0/i915_scheduler

Change scheduler to batch scheduler
echo "type=batch" > /sys/kernel/debug/dri/0/i915_scheduler

Set high watermark of 5, an low of 0 while already running batch sched
echo "pidh=16603-5,pidl=16603,0"> /sys/kernel/debug/dri/0/i915_scheduler

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  152 +++++++++++++++++++++++++++--------
 1 files changed, 117 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 903a004..f4c44fa 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1544,7 +1544,7 @@ static const struct file_operations i915_cache_sharing_fops = {
 };
 
 static const char *sched_type[I915_SCHEDULER_INVALID] =
-	{"none", "fair"};
+	{"none", "fair", "batch"};
 
 static ssize_t
 i915_sched_read(struct file *filp,
@@ -1555,7 +1555,7 @@ i915_sched_read(struct file *filp,
 	struct drm_device *dev = filp->private_data;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_file_private *temp;
-	unsigned high, low, type;
+	unsigned type;
 	size_t len = 0;
 	char *buf, *buf2;
 	struct {
@@ -1572,14 +1572,19 @@ i915_sched_read(struct file *filp,
 		return ret;
 
 	type = dev_priv->scheduler.type;
-	high = dev_priv->scheduler.high_watermark;
-	low = dev_priv->scheduler.low_watermark;
 	list_for_each_entry(temp, &dev_priv->i915_client_list, client_link) {
 		struct drm_file *file = temp->drm_file;
 		requests[i].pid = file->pid;
 		spin_lock(&temp->lock);
 		requests[i].outstanding = temp->outstanding_requests;
 		requests[i].forced = temp->forced_throttles;
+		if (type == I915_SCHEDULER_BATCH) {
+			requests[i].high = temp->high_watermark;
+			requests[i].low = temp->low_watermark;
+		} else {
+			requests[i].high = dev_priv->scheduler.high_watermark;
+			requests[i].low = dev_priv->scheduler.low_watermark;
+		}
 		spin_unlock(&temp->lock);
 		i++;
 	}
@@ -1587,21 +1592,22 @@ i915_sched_read(struct file *filp,
 
 	entries = i;
 
-	entry_size = strlen("pid 123456 = XXXX outstanding (XXXXXX forced)\n") + 1;
+	entry_size = strlen("pid 123456 = XXXX outstanding (XXXXXX forced) "
+			    "h=XXXX l=YYYY\n") + 1;
 	buf2 = drm_malloc_ab(entries, entry_size);
 	memset(buf2, 0, entries * entry_size);
 	for(i = 0; i < entries; i++) {
 		len += sprintf(buf2 + len,
-			       "pid %06d = %06d outstanding (%06d forced)\n",
+			       "pid %06d = %06d outstanding (%06d forced) "
+			       "h=%04d l=%04d\n",
 				requests[i].pid, requests[i].outstanding,
-				requests[i].forced);
+				requests[i].forced, requests[i].high,
+				requests[i].low);
 	}
 
 	buf = kasprintf(GFP_KERNEL, "%s\n"
-				    "Scheduler = %s\n"
-				    "High watermark = %d buffers\n"
-				    "Low watermark = %d buffers\n",
-				     buf2, sched_type[type], high, low);
+				    "Scheduler = %s\n",
+				    buf2, sched_type[type]);
 
 	len = strlen(buf);
 
@@ -1612,6 +1618,49 @@ i915_sched_read(struct file *filp,
 	return ret;
 }
 
+static inline void
+test_and_do_idle(struct drm_device *dev, bool *change)
+{
+	if (*change == false && i915_gpu_idle(dev))
+		DRM_ERROR("Couldn't idle GPU before sched changes\n");
+
+	*change = true;
+}
+
+static void
+set_new_water(struct drm_device *dev, pid_t pid, uint32_t watermark, bool high,
+	      bool *change)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_file_private *temp;
+
+	list_for_each_entry(temp, &dev_priv->i915_client_list, client_link) {
+		struct drm_file *file = temp->drm_file;
+		if (file->pid == pid) {
+			uint32_t old = 0;
+			if (high) {
+				old = temp->high_watermark;
+				if (old != watermark) {
+					temp->high_watermark = watermark;
+					test_and_do_idle(dev, change);
+					DRM_INFO("New high watermark for pid "
+						 "%d = %d\n", pid, watermark);
+				}
+				break;
+			} else {
+				old = temp->low_watermark;
+				if (old != watermark) {
+					temp->low_watermark = watermark;
+					test_and_do_idle(dev, change);
+					DRM_INFO("New low watermark for pid "
+						 "%d = %d\n", pid, watermark);
+				}
+				break;
+			}
+		}
+	}
+}
+
 static ssize_t
 i915_sched_write(struct file *filp,
 		 const char __user *ubuf,
@@ -1622,59 +1671,92 @@ i915_sched_write(struct file *filp,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	char *buf, *ptr;
 	uint32_t high, low, type;
-	int ret;
+	bool change = false;
 
 	buf = drm_malloc_ab(cnt + 1, 1);
 	if (!buf)
 		return -E2BIG;
 
-	ret = copy_from_user(buf, ubuf, cnt);
-	if (ret)
+	if (copy_from_user(buf, ubuf, cnt))
 		goto out;
 
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
-	if (ret)
-		return ret;
+	if (mutex_lock_interruptible(&dev->struct_mutex))
+		goto out;
 
-	low = dev_priv->scheduler.low_watermark;
-	high = dev_priv->scheduler.high_watermark;
 	type = dev_priv->scheduler.type;
 
 	buf[cnt] = 0;
 	ptr = buf;
 
+	/* type=[none, fair, batch]
+	 * pidh=[pid]-[new high watermark]
+	 * pidl=[pid]-[new low watermark]
+	 * low=[new fair low watermark]
+	 * high=[new fair high watermark]
+	 */
 	while ((ptr != NULL) && *ptr != 0) {
 		if (!strncmp(ptr, "type=", 5)) {
 			char *tmp = strchr(ptr, '=');
 			if (tmp != NULL)
 				tmp += strspn(tmp, "= \t\n");
+			else
+				break;
+
 			if (!strncmp("fair", tmp, 4))
 				type = I915_SCHEDULER_FAIR;
+			if (!strncmp("batch", tmp, 5))
+				type = I915_SCHEDULER_BATCH;
 			if (!strncmp("none", tmp, 4))
 				type = I915_SCHEDULER_NONE;
-		}
-		if (!strncmp(ptr, "high=", 5))
+
+			if (dev_priv->scheduler.type != type) {
+				test_and_do_idle(dev, &change);
+				dev_priv->scheduler.type = type;
+				DRM_INFO("New scheduler type = %s\n",
+					 sched_type[type]);
+			}
+		} else if (!strncmp(ptr, "high=", 5)) {
 			high = simple_strtoul(ptr + 5, NULL, 0);
-		if (!strncmp(ptr, "low=", 4))
+			if (dev_priv->scheduler.high_watermark != high) {
+				test_and_do_idle(dev, &change);
+				dev_priv->scheduler.high_watermark = high;
+				DRM_INFO("New high watermark (fair) = %d\n",
+					 high);
+			}
+		} else if (!strncmp(ptr, "low=", 4)) {
 			low = simple_strtoul(ptr + 4, NULL, 0);
+			if (dev_priv->scheduler.low_watermark != low) {
+				test_and_do_idle(dev, &change);
+				dev_priv->scheduler.low_watermark = low;
+				DRM_INFO("New low watermark (fair) = %d\n",
+					 low);
+			}
+		} else if (!strncmp(ptr, "pidh=", 5)) {
+			pid_t pid = simple_strtoul(ptr + 5, NULL, 10);
+			ptr = strchr(ptr, '-');
+			if (ptr != NULL && ptr[1] != 0) {
+				high = simple_strtoul(ptr+1, NULL, 10);
+				set_new_water(dev, pid, high, true, &change);
+			} else if (ptr == NULL)
+				break;
+		} else if (!strncmp(ptr, "pidl=", 5)) {
+			pid_t pid = simple_strtoul(ptr + 5, NULL, 10);
+			ptr = strchr(ptr, '-');
+			if (ptr != NULL && ptr[1] != 0) {
+				low = simple_strtoul(ptr+1, NULL, 10);
+				set_new_water(dev, pid, low, false, &change);
+			} else if (ptr == NULL)
+				break;
+		} else {
+			/* Assume the whole string is busted, and gtfo */
+			DRM_ERROR("Unable to parse command %s\n", ptr);
+			break;
+		}
 		ptr = strchr(ptr, ',');
 		if (ptr != NULL)
 			ptr += strspn(ptr, ", \t\n");
 	}
 
-	if (high != dev_priv->scheduler.high_watermark ||
-	    (low != dev_priv->scheduler.low_watermark) ||
-	    (type != dev_priv->scheduler.type)) {
-		if (i915_gpu_idle(dev))
-			DRM_ERROR("Couldn't idle GPU before sched changes\n");
-		DRM_INFO("new type = %d\n", type);
-		DRM_INFO("new high = %d\n", high);
-		DRM_INFO("new low = %d\n", low);
-		dev_priv->scheduler.high_watermark = high;
-		dev_priv->scheduler.low_watermark = low;
-		dev_priv->scheduler.type = type;
-	}
-
 	mutex_unlock(&dev->struct_mutex);
 out:
 	drm_free_large(buf);
-- 
1.7.7.3

Re: [PATCH 01/15] drm/i915: refactor debugfs open function

[Kenneth Graunke]

On 11/18/2011 06:24 PM, Ben Widawsky wrote:
> From: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Only forcewake has an open with special semantics, the other r/w
> debugfs only assign the file private pointer.
> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

Patches 1 and 2 are
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

[PATCH 03/15] drm/i915: relative_constants_mode race fix

[Ben Widawsky]

After my refactoring, Chris noticed that we had a bug.

dev_priv keeps track of the current addressing mode that gets set at
execbuffer time. Unfortunately the existing code was doing this before
acquiring struct_mutex which leaves a race with another thread also
doing an execbuffer. If that wasn't bad enough, relocate_slow drops
struct_mutex which opens a much more likely error where another thread
comes in and modifies the state while relocate_slow is being slow.

The solution here is to just defer setting this state until we
absolutely need it, and we know we'll have struct_mutex for the
remainder of our code path.

v2: Keith noticed a bug in the original patch.

Cc: Keith Packard <keithp@keithp.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   29 +++++++++++++++------------
 1 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3693e83..be0e3bc 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1016,19 +1016,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 			if (INTEL_INFO(dev)->gen > 5 &&
 			    mode == I915_EXEC_CONSTANTS_REL_SURFACE)
 				return -EINVAL;
-
-			ret = intel_ring_begin(ring, 4);
-			if (ret)
-				return ret;
-
-			intel_ring_emit(ring, MI_NOOP);
-			intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
-			intel_ring_emit(ring, INSTPM);
-			intel_ring_emit(ring,
-					I915_EXEC_CONSTANTS_MASK << 16 | mode);
-			intel_ring_advance(ring);
-
-			dev_priv->relative_constants_mode = mode;
 		}
 		break;
 	default:
@@ -1159,6 +1146,22 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		}
 	}
 
+	if (ring == &dev_priv->ring[RCS] &&
+	    mode != dev_priv->relative_constants_mode) {
+		ret = intel_ring_begin(ring, 4);
+		if (ret)
+				goto err;
+
+		intel_ring_emit(ring, MI_NOOP);
+		intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+		intel_ring_emit(ring, INSTPM);
+		intel_ring_emit(ring,
+				I915_EXEC_CONSTANTS_MASK << 16 | mode);
+		intel_ring_advance(ring);
+
+		dev_priv->relative_constants_mode = mode;
+	}
+
 	trace_i915_gem_ring_dispatch(ring, seqno);
 
 	exec_start = batch_obj->gtt_offset + args->batch_start_offset;
-- 
1.7.7.4

Re: [PATCH 00/15] [RFC] Forced throttling/scheduling

[Eugeni Dodonov]

[-- Attachment #1.1: Type: text/plain, Size: 2123 bytes --]

On Sat, Nov 19, 2011 at 00:24, Ben Widawsky <ben@bwidawsk.net> wrote:

> This code is currently living on my personal git repo:
> git://people.freedesktop.org/~bwidawsk/drm-intel<http://people.freedesktop.org/%7Ebwidawsk/drm-intel>forced_throttling
>
> Since these are RFC, I haven't spent too much time cleaning things up.
> Feel free to comment on typos, comments you feel are missing, etc. I
> also haven't spent any time running the standard kernel tests, kmemleak
> and such - I intend to do so while these are reviewed here.
>
> There are two main "scheduler" types implemented in this patch. What I
> refer to as a fairness scheduler, and a batch scheduler. The fairness
> scheduler is currently implemented on batch granularity but that is not
> guaranteed going forward. The batch scheduler is a way to set batch
> limits per pid. Refer to the relevant patch for more details.
>
> It is my opinion that the fairness scheduler isn't terribly interesting
> except to prevent badly written, or malicious apps. For example, a 3d
> app which queues up a ton of work but never calls glxSwapBuffer.
>
> The batch scheduler is also somewhat uninteresting as the values it uses
> require proper tuning and will vary from system to system, and even then
> depending on what's currently running. But like the fairness one, this
> too has its applications.
>
> Most comments and feedback are welcome.
>

I am not that expert about how GPU scheduling works, but all the patches
make sense to me and I haven't found anything apparently wrong.

I have one feature request for patch 11, which is to list available
schedulers via a debugfs entry. For example:
# cat /sys/kernel/debug/dri/0/i915_available_schedulers
none *fair batch

or list them in the output of i915_scheduler, like:
# cat /sys/kernel/debug/dri/0/i915_scheduler
...
Scheduler: batch
Available schedulers: none fair batch

To see what schedulers are available for usage.

But besides this, for the series:
Tested-by: Eugeni Dodonov <eugeni.dodonov@intel.com>
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 2753 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH 00/15] [RFC] Forced throttling/scheduling

[Ben Widawsky]

On Fri, 18 Nov 2011 18:24:17 -0800
Ben Widawsky <ben@bwidawsk.net> wrote:

> This code is currently living on my personal git repo:
> git://people.freedesktop.org/~bwidawsk/drm-intel forced_throttling
> 
> Since these are RFC, I haven't spent too much time cleaning things up.
> Feel free to comment on typos, comments you feel are missing, etc. I
> also haven't spent any time running the standard kernel tests, kmemleak
> and such - I intend to do so while these are reviewed here.
> 
> There are two main "scheduler" types implemented in this patch. What I
> refer to as a fairness scheduler, and a batch scheduler. The fairness
> scheduler is currently implemented on batch granularity but that is not
> guaranteed going forward. The batch scheduler is a way to set batch
> limits per pid. Refer to the relevant patch for more details.
> 
> It is my opinion that the fairness scheduler isn't terribly interesting
> except to prevent badly written, or malicious apps. For example, a 3d
> app which queues up a ton of work but never calls glxSwapBuffer.
> 
> The batch scheduler is also somewhat uninteresting as the values it uses
> require proper tuning and will vary from system to system, and even then
> depending on what's currently running. But like the fairness one, this
> too has its applications.
> 
> Most comments and feedback are welcome.
> 

I think one of the main gripes with these patches was the need for a
process abstraction (and using debugfs instead of sysfs). With the
context patches that I'm actively working on getting into 3.5, I'm
wondering if it's worth trying to redo this on top of the context
interface?