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@xxxxxxxxx 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@xxxxxxxxxxxxxxxxxx>
Date: Fri Jul 20 12:41:02 2012 +0100
drm/i915: Remove the defunct flushing list
and
commit cc889e0f6ce6a63c62db17d702ecfed86d58083f
Author: Daniel Vetter <daniel.vetter@xxxxxxxx>
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
