[Intel-gfx] [PATCH] drm/i915: Update write_domains on active list after flush.
Daniel Vetter
daniel at ffwll.ch
Tue Feb 9 12:47:01 CET 2010
Hi Eric,
Can you please merge this patch for -rc8 and add the following to the
sob-section (gathered from various emails):
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Tested-by: Adam Lantos <hege at playma.org>
Cc: stable at kernel.org
This fixes real-world BUG_ONs with fence register starved hw (i8xx and
i915G/M without execbuf2).
Patch as-is applies to .32-stable, but also works on latest -linus. It has
some conflicts with my fixlets patch-queue (due to code-refactoring),
therefore the rebase.
Thanks, Daniel
On Sun, Feb 07, 2010 at 04:20:18PM +0100, Daniel Vetter wrote:
> Before changing the status of a buffer with a pending write we will await
> upon a new flush for that buffer. So we can take advantage of any flushes
> posted whilst the buffer is active and pending processing by the GPU, by
> clearing its write_domain and updating its last_rendering_seqno -- thus
> saving a potential flush in deep queues and improves flushing behaviour
> upon eviction for both GTT space and fences.
>
> In order to reduce the time spent searching the active list for matching
> write_domains, we move those to a separate list whose elements are
> the buffers belong to the active/flushing list with pending writes.
>
> Orignal patch by Chris Wilson <chris at chris-wilson.co.uk>, forward-ported
> by me.
>
> In addition to better performance, this also fixes a real bug. Before
> this changes, i915_gem_evict_everything didn't work as advertised. When
> the gpu was actually busy and processing request, the flush and subsequent
> wait would not move active and dirty buffers to the inactive list, but
> just to the flushing list. Which triggered the BUG_ON at the end of this
> function. With the more tight dirty buffer tracking, all currently busy and
> dirty buffers get moved to the inactive list by one i915_gem_flush operation.
>
> I've left the BUG_ON I've used to prove this in there.
>
> References:
> Bug 25911 - 2.10.0 causes kernel oops and system hangs
> http://bugs.freedesktop.org/show_bug.cgi?id=25911
>
> Bug 26101 - [i915] xf86-video-intel 2.10.0 (and git) triggers kernel oops
> within seconds after login
> http://bugs.freedesktop.org/show_bug.cgi?id=26101
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>
> Same patch rebased to apply to .32-stable (and linus mainlin, too).
> Compile-tested only.
>
> drivers/gpu/drm/i915/i915_drv.h | 11 +++++++++++
> drivers/gpu/drm/i915/i915_gem.c | 23 +++++++++++++++++++----
> 2 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 791fded..2c031cf 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -467,6 +467,15 @@ typedef struct drm_i915_private {
> struct list_head flushing_list;
>
> /**
> + * List of objects currently pending a GPU write flush.
> + *
> + * All elements on this list will belong to either the
> + * active_list or flushing_list, last_rendering_seqno can
> + * be used to differentiate between the two elements.
> + */
> + struct list_head gpu_write_list;
> +
> + /**
> * LRU list of objects which are not in the ringbuffer and
> * are ready to unbind, but are still in the GTT.
> *
> @@ -558,6 +567,8 @@ struct drm_i915_gem_object {
>
> /** This object's place on the active/flushing/inactive lists */
> struct list_head list;
> + /** This object's place on GPU write list */
> + struct list_head gpu_write_list;
>
> /** This object's place on the fenced object LRU */
> struct list_head fence_list;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index df2c625..b2e0742 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1568,6 +1568,8 @@ i915_gem_object_move_to_inactive(struct drm_gem_object *obj)
> else
> list_move_tail(&obj_priv->list, &dev_priv->mm.inactive_list);
>
> + BUG_ON(!list_empty(&obj_priv->gpu_write_list));
> +
> obj_priv->last_rendering_seqno = 0;
> if (obj_priv->active) {
> obj_priv->active = 0;
> @@ -1638,7 +1640,8 @@ i915_add_request(struct drm_device *dev, struct drm_file *file_priv,
> struct drm_i915_gem_object *obj_priv, *next;
>
> list_for_each_entry_safe(obj_priv, next,
> - &dev_priv->mm.flushing_list, list) {
> + &dev_priv->mm.gpu_write_list,
> + gpu_write_list) {
> struct drm_gem_object *obj = obj_priv->obj;
>
> if ((obj->write_domain & flush_domains) ==
> @@ -1646,6 +1649,7 @@ i915_add_request(struct drm_device *dev, struct drm_file *file_priv,
> uint32_t old_write_domain = obj->write_domain;
>
> obj->write_domain = 0;
> + list_del_init(&obj_priv->gpu_write_list);
> i915_gem_object_move_to_active(obj, seqno);
>
> trace_i915_gem_object_change_domain(obj,
> @@ -2089,8 +2093,8 @@ static int
> i915_gem_evict_everything(struct drm_device *dev)
> {
> drm_i915_private_t *dev_priv = dev->dev_private;
> - uint32_t seqno;
> int ret;
> + uint32_t seqno;
> bool lists_empty;
>
> spin_lock(&dev_priv->mm.active_list_lock);
> @@ -2112,6 +2116,8 @@ i915_gem_evict_everything(struct drm_device *dev)
> if (ret)
> return ret;
>
> + BUG_ON(!list_empty(&dev_priv->mm.flushing_list));
> +
> ret = i915_gem_evict_from_inactive_list(dev);
> if (ret)
> return ret;
> @@ -2710,7 +2716,7 @@ i915_gem_object_flush_gpu_write_domain(struct drm_gem_object *obj)
> old_write_domain = obj->write_domain;
> i915_gem_flush(dev, 0, obj->write_domain);
> seqno = i915_add_request(dev, NULL, obj->write_domain);
> - obj->write_domain = 0;
> + BUG_ON(obj->write_domain);
> i915_gem_object_move_to_active(obj, seqno);
>
> trace_i915_gem_object_change_domain(obj,
> @@ -3730,16 +3736,23 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
> i915_gem_flush(dev,
> dev->invalidate_domains,
> dev->flush_domains);
> - if (dev->flush_domains)
> + if (dev->flush_domains & I915_GEM_GPU_DOMAINS)
> (void)i915_add_request(dev, file_priv,
> dev->flush_domains);
> }
>
> for (i = 0; i < args->buffer_count; i++) {
> struct drm_gem_object *obj = object_list[i];
> + struct drm_i915_gem_object *obj_priv = obj->driver_private;
> uint32_t old_write_domain = obj->write_domain;
>
> obj->write_domain = obj->pending_write_domain;
> + if (obj->write_domain)
> + list_move_tail(&obj_priv->gpu_write_list,
> + &dev_priv->mm.gpu_write_list);
> + else
> + list_del_init(&obj_priv->gpu_write_list);
> +
> trace_i915_gem_object_change_domain(obj,
> obj->read_domains,
> old_write_domain);
> @@ -4132,6 +4145,7 @@ int i915_gem_init_object(struct drm_gem_object *obj)
> obj_priv->obj = obj;
> obj_priv->fence_reg = I915_FENCE_REG_NONE;
> INIT_LIST_HEAD(&obj_priv->list);
> + INIT_LIST_HEAD(&obj_priv->gpu_write_list);
> INIT_LIST_HEAD(&obj_priv->fence_list);
> obj_priv->madv = I915_MADV_WILLNEED;
>
> @@ -4583,6 +4597,7 @@ i915_gem_load(struct drm_device *dev)
> spin_lock_init(&dev_priv->mm.active_list_lock);
> INIT_LIST_HEAD(&dev_priv->mm.active_list);
> INIT_LIST_HEAD(&dev_priv->mm.flushing_list);
> + INIT_LIST_HEAD(&dev_priv->mm.gpu_write_list);
> INIT_LIST_HEAD(&dev_priv->mm.inactive_list);
> INIT_LIST_HEAD(&dev_priv->mm.request_list);
> INIT_LIST_HEAD(&dev_priv->mm.fence_list);
> --
> 1.6.6.1
>
--
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48
More information about the Intel-gfx
mailing list