| Message ID | 1406217891-8912-35-git-send-email-thomas.daniel@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 24, 2014 at 05:04:42PM +0100, Thomas Daniel wrote: > From: Oscar Mateo <oscar.mateo@intel.com> > > If we reset a ring after a hang, we have to make sure that we clear > out all queued Execlists requests. > > v2: The ring is, at this point, already being correctly re-programmed > for Execlists, and the hangcheck counters cleared. > > v3: Daniel suggests to drop the "if (execlists)" because the Execlists > queue should be empty in legacy mode (which is true, if we do the > INIT_LIST_HEAD). > > v4: Do the pending intel_runtime_pm_put I don't see a intel_runtime_pm_get() that put() would correspond to. > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 12 ++++++++++++ > drivers/gpu/drm/i915/intel_ringbuffer.c | 1 + > 2 files changed, 13 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 1c83b9c..143cff7 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2567,6 +2567,18 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv, > i915_gem_free_request(request); > } > > + while (!list_empty(&ring->execlist_queue)) { > + struct intel_ctx_submit_request *submit_req; > + > + submit_req = list_first_entry(&ring->execlist_queue, > + struct intel_ctx_submit_request, > + execlist_link); > + list_del(&submit_req->execlist_link); > + intel_runtime_pm_put(dev_priv); > + i915_gem_context_unreference(submit_req->ctx); > + kfree(submit_req); > + } > + > /* These may not have been flush before the reset, do so now */ > kfree(ring->preallocated_lazy_request); > ring->preallocated_lazy_request = NULL; > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 3188403..6e604c9 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1587,6 +1587,7 @@ static int intel_init_ring_buffer(struct drm_device *dev, > ring->dev = dev; > INIT_LIST_HEAD(&ring->active_list); > INIT_LIST_HEAD(&ring->request_list); > + INIT_LIST_HEAD(&ring->execlist_queue); > ringbuf->size = 32 * PAGE_SIZE; > ringbuf->ring = ring; > ringbuf->ctx = ring->default_context; > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Jul 24, 2014 at 05:04:42PM +0100, Thomas Daniel wrote: > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 3188403..6e604c9 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1587,6 +1587,7 @@ static int intel_init_ring_buffer(struct drm_device *dev, > ring->dev = dev; > INIT_LIST_HEAD(&ring->active_list); > INIT_LIST_HEAD(&ring->request_list); > + INIT_LIST_HEAD(&ring->execlist_queue); It's also a bit weird to now have sites where we initialize this list. or I'm missing something?
> -----Original Message----- > From: Lespiau, Damien > Sent: Friday, August 01, 2014 3:42 PM > To: Daniel, Thomas > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 34/43] drm/i915/bdw: Make sure gpu reset > still works with Execlists > > On Thu, Jul 24, 2014 at 05:04:42PM +0100, Thomas Daniel wrote: > > From: Oscar Mateo <oscar.mateo@intel.com> > > > > If we reset a ring after a hang, we have to make sure that we clear > > out all queued Execlists requests. > > > > v2: The ring is, at this point, already being correctly re-programmed > > for Execlists, and the hangcheck counters cleared. > > > > v3: Daniel suggests to drop the "if (execlists)" because the Execlists > > queue should be empty in legacy mode (which is true, if we do the > > INIT_LIST_HEAD). > > > > v4: Do the pending intel_runtime_pm_put > > I don't see a intel_runtime_pm_get() that put() would correspond to. It is in execlists_context_queue() where the request is added to the queue. > > > > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> > > --- > > drivers/gpu/drm/i915/i915_gem.c | 12 ++++++++++++ > > drivers/gpu/drm/i915/intel_ringbuffer.c | 1 + > > 2 files changed, 13 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c index 1c83b9c..143cff7 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -2567,6 +2567,18 @@ static void i915_gem_reset_ring_cleanup(struct > drm_i915_private *dev_priv, > > i915_gem_free_request(request); > > } > > > > + while (!list_empty(&ring->execlist_queue)) { > > + struct intel_ctx_submit_request *submit_req; > > + > > + submit_req = list_first_entry(&ring->execlist_queue, > > + struct intel_ctx_submit_request, > > + execlist_link); > > + list_del(&submit_req->execlist_link); > > + intel_runtime_pm_put(dev_priv); > > + i915_gem_context_unreference(submit_req->ctx); > > + kfree(submit_req); > > + } > > + > > /* These may not have been flush before the reset, do so now */ > > kfree(ring->preallocated_lazy_request); > > ring->preallocated_lazy_request = NULL; diff --git > > a/drivers/gpu/drm/i915/intel_ringbuffer.c > > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index 3188403..6e604c9 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -1587,6 +1587,7 @@ static int intel_init_ring_buffer(struct drm_device > *dev, > > ring->dev = dev; > > INIT_LIST_HEAD(&ring->active_list); > > INIT_LIST_HEAD(&ring->request_list); > > + INIT_LIST_HEAD(&ring->execlist_queue); > > ringbuf->size = 32 * PAGE_SIZE; > > ringbuf->ring = ring; > > ringbuf->ctx = ring->default_context; > > -- > > 1.7.9.5 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> -----Original Message----- > From: Lespiau, Damien > Sent: Friday, August 01, 2014 3:46 PM > To: Daniel, Thomas > Cc: intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH 34/43] drm/i915/bdw: Make sure gpu reset > still works with Execlists > > On Thu, Jul 24, 2014 at 05:04:42PM +0100, Thomas Daniel wrote: > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index 3188403..6e604c9 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -1587,6 +1587,7 @@ static int intel_init_ring_buffer(struct drm_device > *dev, > > ring->dev = dev; > > INIT_LIST_HEAD(&ring->active_list); > > INIT_LIST_HEAD(&ring->request_list); > > + INIT_LIST_HEAD(&ring->execlist_queue); > > It's also a bit weird to now have sites where we initialize this list. > or I'm missing something? The legacy ringbuffer init path now also has to initialize the queue because I915_gem_reset_ring_cleanup() now assumes that the list is always valid, even though it will always be empty in legacy mode. > > -- > Damien
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 1c83b9c..143cff7 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2567,6 +2567,18 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv, i915_gem_free_request(request); } + while (!list_empty(&ring->execlist_queue)) { + struct intel_ctx_submit_request *submit_req; + + submit_req = list_first_entry(&ring->execlist_queue, + struct intel_ctx_submit_request, + execlist_link); + list_del(&submit_req->execlist_link); + intel_runtime_pm_put(dev_priv); + i915_gem_context_unreference(submit_req->ctx); + kfree(submit_req); + } + /* These may not have been flush before the reset, do so now */ kfree(ring->preallocated_lazy_request); ring->preallocated_lazy_request = NULL; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 3188403..6e604c9 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1587,6 +1587,7 @@ static int intel_init_ring_buffer(struct drm_device *dev, ring->dev = dev; INIT_LIST_HEAD(&ring->active_list); INIT_LIST_HEAD(&ring->request_list); + INIT_LIST_HEAD(&ring->execlist_queue); ringbuf->size = 32 * PAGE_SIZE; ringbuf->ring = ring; ringbuf->ctx = ring->default_context;