Re: [PATCH] drm/i915: Record the tail at each request and use it to estimate the head

[Chris Wilson <chris@chris-wilson.co.uk>]

On Wed, 8 Feb 2012 16:56:30 -0200, Eugeni Dodonov <eugeni@dodonov.net> wrote:
> On Wed, Feb 8, 2012 at 16:06, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > +       /* Record the position of the start of the request so that
> > +        * should we detect the updated seqno part-way through the
> > +        * GPU processing the request, we never over-estimate the
> > +        * position of the head.
> > +        */
> > +       request_ring_position = intel_ring_get_tail(ring);
> >
> Perhaps a stupid question within the bikeshedding spirit, but why do we
> need intel_ring_get_tail()? Wouldn't just:
> 
> request_ring_position = ring->tail;
> 
> be self-explainable? Or some additional logic could be involved there at
> some point?

It is to try and preserve the illusion of a ringbuffer abstraction; the
position of the request within the ring should be an internal detail of
the ring but we are leaking it for our purposes. The use of a function there
serves as a reminder of that leak (and easier to find than ring->tail) in
the hope that someone will clean up the entire ringbuffer API and make the
issue moot.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre