[Intel-gfx] [PATCH v3 21/28] drm/i915: Remove the now redundant 'obj->ring'

[Daniel Vetter]

On Fri, Nov 28, 2014 at 05:49:26PM +0000, John Harrison wrote:
> On 26/11/2014 13:43, Daniel Vetter wrote:
> >On Mon, Nov 24, 2014 at 06:49:43PM +0000, John.C.Harrison at Intel.com wrote:
> >>From: John Harrison <John.C.Harrison at Intel.com>
> >>
> >>The ring member of the object structure was always updated with the
> >>last_read_seqno member. Thus with the conversion to last_read_req, obj->ring is
> >>now a direct copy of obj->last_read_req->ring. This makes it somewhat redundant
> >>and potentially misleading (especially as there was no comment to explain its
> >>purpose).
> >>
> >>This checkin removes the redundant field. Many uses were simply testing for
> >>non-null to see if the object is active on the GPU. Some of these have been
> >>converted to check 'obj->active' instead. Others (where the last_read_req is
> >>about to be used anyway) have been changed to check obj->last_read_req. The rest
> >>simply pull the ring out from the request structure and proceed as before.
> >>
> >>For: VIZ-4377
> >>Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> >>Reviewed-by: Thomas Daniel <Thomas.Daniel at intel.com>
> >Ok merged up to this for now. I'd like to settle things a bit first (and
> >also figure out what to do with the trace_irq stuff).
> >
> >Thanks for patches&review,
> >Daniel
> 
> Now that the 3.19 pull request has gone, are you going to continue merging
> these patches? Or is there something else you particularly want to wait for?

Well I'm still not convinced that those patches are what we want. For
android (and a few other things) we really want to support struct fence,
and that already provides all the necessary code to cache the completion
state. So I don't see why we have to do a detour just to get caching into
place if for the long-term plan we'll need to rip all that code out again
anyway. And the scheduler without fence/syncpt integration doesn't make a
lot of sense on Android I think.

Now if there's a seriuos performance issue with requests without this then
maybe this detour makes sense. But I don't see any benchmark data attached
to justify the patches from that pov.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch