On Mon, Oct 20, 2014 at 11:19:28AM +0100, John Harrison wrote: > On 19/10/2014 15:21, Daniel Vetter wrote: > >On Fri, Oct 10, 2014 at 01:03:17PM +0100, John Harrison wrote: > >>I have just posted an updated subset of the patch series. Note that one > >>patch has been inserted in the middle and the first one has been dropped. > >>The correct sequence is now: > >> > >> 01 drm/i915: Remove redundant parameter to > >> i915_gem_object_wait_rendering__tail() > >> 02 drm/i915: Ensure OLS & PLR are always in sync > >> 03 drm/i915: Add reference count to request structure > >> 04 drm/i915: Add helper functions to aid seqno -> request transition > >> 05 drm/i915: Replace last_[rwf]_seqno with last_[rwf]_req > >> 06 drm/i915: Ensure requests stick around during waits > >> 07 drm/i915: Remove 'outstanding_lazy_seqno' > >> 08 drm/i915: Make 'i915_gem_check_olr' actually check by request > >> not seqno > >> 09 drm/i915: Convert 'last_flip_req' to be a request not a seqno > >> 10 drm/i915: Convert i915_wait_seqno to i915_wait_request > >> 11 drm/i915: Convert 'i915_add_request' to take a request not a seqno > >> 12 drm/i915: Convert mmio_flip::seqno to struct request > >> 13 drm/i915: Convert 'flip_queued_seqno' into 'flip_queued_request' > >> 14 drm/i915: Connect requests to rings at creation not submission > >> 15 drm/i915: Convert most 'i915_seqno_passed' calls into > >> 'i915_gem_request_completed' > >> 16 drm/i915: Convert __wait_seqno() to __wait_request() > >> 17 drm/i915: Convert trace functions from seqno to request > >> 18 drm/i915: Convert 'trace_irq' to use requests rather than seqnos > >> 19 drm/i915: Convert semaphores to handle requests not seqnos > >> 20 drm/i915: Convert 'ring_idle()' to use requests not seqnos > >> 21 drm/i915: Remove 'obj->ring' > >> 22 drm/i915: Cache request completion status > >> 23 drm/i915: Zero fill the request structure > >> 24 drm/i915: Defer seqno allocation until actual hardware > >> submission time > >> > >> > >>The whole set in its latest and greatest form has also been uploaded to the > >>drm-private git as 'topic/seqno-request'. > >Ok, read through the entire pile and looks good from a high level I think. > >Review summary is really just "please less BUG_ON and more commit > >message". I think even the few funky things can probably just be explained > >away with a good commit message. > A few of the BUG_ONs disappear again along the way and others are just > converting existing BUG_ONs from seqnos to requests. Yeah, there's a bunch of preexisting ones that I've let slip through. But I've screamed at a BUG_ON that killed my machine once too often in the past few months, so I've started to be really strict about them. If they're just temporary then a WARN_ON should be about as informative, or just drop them since if a pointer is NULL you'll blow up anyway a few instructions later with an Oops. > The minimal messaging was because the intention was to get something posted > as soon as possible in order to be reviewed as soon as possible if only from > a 'is this what you had in mind' point of view. I didn't realise you were > going to be out of office for so long. There didn't seem much value in > writing reams of description only to be told a day later that I've > misunderstood your design spec and gone off in completely the wrong > direction. Oh, I guess then I've looked too closely at it ;-) From the high-level point it's pretty much what I expected, so lgtm. -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
