Re: [PATCH 7/8] drm/i915: Grab execlist spinlock to avoid post-reset concurrency issues.

[Tomas Elf <tomas.elf@intel.com>]

On 09/10/2015 08:45, Chris Wilson wrote:
> On Thu, Oct 08, 2015 at 07:31:39PM +0100, Tomas Elf wrote:
>> Grab execlist lock when cleaning up execlist queues after GPU reset to avoid
>> concurrency problems between the context event interrupt handler and the reset
>> path immediately following a GPU reset.
>
> Hmm, my tree just does:
>
> static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
>                                          struct intel_engine_cs *ring)
> {
>          /*
>           * Clear the execlists queue up before freeing the requests, as those
>           * are the ones that keep the context and ringbuffer backing objects
>           * pinned in place.
>           */
>          if (i915.enable_execlists) {
>                  spin_lock_irq(&ring->execlist_lock);
>                  list_splice_tail_init(&ring->execlist_queue,
>                                        &ring->execlist_completed);
>                  memset(&ring->execlist_port, 0, sizeof(ring->execlist_port));
>                  spin_unlock_irq(&ring->execlist_lock);
>
>                  intel_execlists_retire_requests(ring);
>          }
>
>
> Oh, that's right another unreviewed patch from several months ago.
>
> For your patch, you should not take the spinlock unconditionally as it is
> only initialised for execlists, and you can use the simpler form of irq
> locking as above.

Those are fair points. Mind pointing me to your patch so that I can have 
a look at it?

Thanks,
Tomas

> -Chris
>

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