[Intel-gfx] [PATCH 52/53] drm/i915: Remove the now obsolete 'outstanding_lazy_request'

[Daniel Vetter]

On Fri, Mar 13, 2015 at 01:32:38PM +0000, John Harrison wrote:
> On 10/03/2015 10:18, Daniel Vetter wrote:
> >On Mon, Mar 09, 2015 at 11:51:26PM +0000, Tomas Elf wrote:
> >>On 19/02/2015 17:18, John.C.Harrison at Intel.com wrote:
> >>>diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> >>>index 3b0261f..d2c6427 100644
> >>>--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> >>>+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> >>>@@ -258,10 +258,6 @@ struct  intel_engine_cs {
> >>>  	 */
> >>>  	struct list_head request_list;
> >>>
> >>>-	/**
> >>>-	 * Do we have some not yet emitted requests outstanding?
> >>>-	 */
> >>>-	struct drm_i915_gem_request *outstanding_lazy_request;
> >>>  	bool gpu_caches_dirty;
> >>>  	bool fbc_dirty;
> >>>
> >>>
> >>Since we're removing the i915_add_request from i915_check_olr and thereby
> >>removing the OLR emission the following comment at
> >>i915_gem_object_flush_active is no longer valid:
> >>
> >>         /**
> >>          * Ensures that an object will eventually get non-busy by flushing
> >>any required
> >>          * write domains, emitting any outstanding lazy request and retiring
> >>and
> >>          * completed requests.
> >>          */
> >>          static int
> >>          i915_gem_object_flush_active(struct drm_i915_gem_object *obj)
> >>          {
> >>                 struct intel_engine_cs *ring;
> >>                 int ret;
> >>
> >>                 if (obj->active) {
> >>                         ring =
> >>i915_gem_request_get_ring(obj->last_read_req);
> >>                         ret = i915_gem_check_olr(obj->last_read_req);
> >Nice catch! On top of that the int return value is no longer needed, so a
> >follow-up patch should simplify it to void.
> >-Daniel
> 
> The comment also talks about flushing write domains but there is no obvious
> flush call. All it does is the check_olr (which is now a no-op) and the
> retire (which won't do anything unless the request has already completed).
> So is there any need for this function at all anymore? Or should it just be
> removed and replaced with a call to retire instead?

That's historical baggage - way back we've done lazy cache-flushing, which
mean on readback you might have needed to flush caches first. Massive case
of premature optimization, the comment is just plainly outdated. See

commit 65ce3027415d4dc9ee18ef0a135214b4fb76730b
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Jul 20 12:41:02 2012 +0100

    drm/i915: Remove the defunct flushing list

and

commit cc889e0f6ce6a63c62db17d702ecfed86d58083f
Author: Daniel Vetter <daniel.vetter at ffwll.ch>
Date:   Wed Jun 13 20:45:19 2012 +0200

    drm/i915: disable flushing_list/gpu_write_list

You can savely remove the comment (maybe with the above commits referenced
in the commit message).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch