> -----Original Message----- > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel > Vetter > Sent: Monday, November 03, 2014 3:33 PM > To: Daniel, Thomas > Cc: intel-gfx@lists.freedesktop.org; shuang...@linux.intel.com > Subject: Re: [Intel-gfx] [PATCH 1/4] drm/i915/bdw: Clean up execlist queue > items in retire_work > > On Wed, Oct 29, 2014 at 09:52:50AM +0000, Thomas Daniel wrote: > > No longer create a work item to clean each execlist queue item. > > Instead, move retired execlist requests to a queue and clean up the > > items during retire_requests. > > > > v2: Fix legacy ring path broken during overzealous cleanup > > > > v3: Update idle detection to take execlists queue into account > > > > Issue: VIZ-4274 > > Signed-off-by: Thomas Daniel <thomas.dan...@intel.com> > > --- > > drivers/gpu/drm/i915/i915_gem.c | 4 +++ > > drivers/gpu/drm/i915/intel_lrc.c | 52 > > ++++++++++++++++++----------- > -- > > drivers/gpu/drm/i915/intel_lrc.h | 2 +- > > drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + > > 4 files changed, 36 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c index 827edb5..df28202 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -2718,6 +2718,10 @@ i915_gem_retire_requests(struct drm_device > *dev) > > for_each_ring(ring, dev_priv, i) { > > i915_gem_retire_requests_ring(ring); > > idle &= list_empty(&ring->request_list); > > + if (i915.enable_execlists) { > > + idle &= list_empty(&ring->execlist_queue); > > + intel_execlists_retire_requests(ring); > > This needs to be the other way round I think - we care about idleness after > all > the currently processed stuff is retired, not before. Otherwise we might > notice the busy->idle transition one invocation too late. I thought for a while about this. The GPU will be idle when the execlist_queues are empty. Intel_execlists_retire_requests() cleans up requests which have already finished so it is more conservative (in terms of CPU idleness) to check the queue beforehand. I thought this would be more desirable than potentially reporting idleness early... Intel_execlists_retire_requests() can not affect the state of the queue. And there is no point checking the execlist_retired_req_list because execlists_retire_requests() always empties it.
Thomas. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx