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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
