On Mon, Oct 20, 2014 at 08:19:18AM +0100, Chris Wilson wrote: > On Sun, Oct 19, 2014 at 07:15:53PM +0200, Daniel Vetter wrote: > > On Mon, Oct 6, 2014 at 5:17 PM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > > > On Mon, Oct 06, 2014 at 03:15:04PM +0100, John.C.Harrison@xxxxxxxxx wrote: > > >> From: John Harrison <John.C.Harrison@xxxxxxxxx> > > >> > > >> Work in progress for replacing seqno usage with requst structures. > > > > > > You fail to end up with my earlier code. Nak. > > > > Well I've tried to split up your patch into small independent changes > > and failed at that. And given how many changes there are in there I > > simply can't merge your patch as-is. I know that it does seem to fix > > some random hangs, but with massive behaviour changes like the > > read-read stuff in there this could very well just be a random fluke. > > Nope. Try reading it again. The most invasive change is defining the > order of engine/context/ppgtt/ring creation (and doing the setup/enable > split). Everything else is about making a request a transaction and > using that to fix bugs in our state tacking. I guess the setup changes only showed up in the revised and much bigger version. At least I don't remember seeing that in the first iteration I've tried to split up. Or maybe I've been blind. In any case I want to untangle that web too, just with a much more gradual approach. > The fundamental change, that I am going to be a stickler for, is that the > request is the ring access. It defines the context, owner and interface > to both the ring and the engine/scheduler. In doing so it drops the > inconsistent and unmaintainable approach of introducing separate but almost > duplicate code paths inside the core of GEM. So for a clean slate design I agree with you. But given where we are I still think that the duplication with the execlist code in intel_lrc.c is a good approach. It will lead to a lot more ugliness, and there's a good chance that we can't get rid of the final bits in years. But given that execlist absolutely had to get in I really didn't see any other way to merge it, without the inevitable reworks totally wreaking the other parts of gem. It's definitely not a great option, but there's also limits to how much flak I can absorb without crumbling completely. > > But if there's anything amiss in John's work for the plain > > s/seqno/request/ change, or anything else that we absolutely need to > > have then I very much want your opinion on this. But a flat outright > > nak without further content isn't useful to move forward. > > It doesn't reduce the technical debt of execlists, which was the > motivation for the changes. It even brings a breath of sanity to > contexts and ppgtt as well, a massive boon. I agree on that. It should give us a start though to clean things up by shifting a lot tracking execlist does over to requests. Or at least I hope so. And there's still the issue that I really need to avoid another dept pile-up excercise with the scheduler, so for just that part I think we need this. Even when it doesn't address any of the issues with execlist. But I do think this will allow us to at least move into the right direction a bit, so please scream if I'm totally off the tracks. Thanks, 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
