On Mon, Dec 01, 2014 at 12:44:12PM +0000, John Harrison wrote: > On 28/11/2014 18:06, Daniel Vetter wrote: > >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@xxxxxxxxx wrote: > >>>>From: John Harrison <John.C.Harrison@xxxxxxxxx> > >>>> > >>>>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@xxxxxxxxx> > >>>>Reviewed-by: Thomas Daniel <Thomas.Daniel@xxxxxxxxx> > >>>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 > > Firstly, not all the missing patches are about performance caching. For > example, the kmalloc vs kzalloc patch is simply better code all round. The > tracing patch is also a general purpose driver improvement and makes > debugging/analysing the code easier. No concern with those patches from my side except that they're after the caching change. I can cherry-pick them but it looks a bit like that might backfire if I also don't pick the request state caching. > Re the caching. My argument is that the request implementation, as it > stands, is a significant step backwards over the original seqno version. In > the original code, someone had evidently seen fit to make the completion > test a minimal inline function. In the half merged request version, it is a > function call with register reading and list traversal. As the cached > version is already written, tested and reviewed, I do not see a reason to > discard it in order to write a completely new version at some unspecified > point in the future that may or may not be quite a large piece of work. > > Also, the scheduler is already ported on top of the full request patch set > and is running in the GMin Android code base with native sync included. > Having to rewrite the underlying code yet again is going to be a significant > delay in something that needs to be shipped last month. Delaying that short > term work for a long term goal is really undesirable at this point. The plan > has always been to rework for long term awesomeness (such as dmabuf sync > integration) after something basic is implemented and shipped. > > Do you have time for a phone call or meeting to discuss this? Yeah, probably easier to discuss in a mtg. Jon Ewins should be there too I think. -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
