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.
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.
That aside I think even for RFC the commit message really should be there,
since that's what I need to be able to efficiently review patches. If the
series is littered with FIXME comments all over that's ok (that's what an
rfc is for really), but the justifications and tricky points should all be
there. So I highly recommend to self-review patch series (using the review
training slides) even for an rfc to make sure there's not too many bubbles
in the pipeline.
Presuming there's no dragon hidden in any of the open issues I'll sign up
someone else for the in-depth review.
Cheers, Daniel
On 06/10/2014 15:15, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>
Work in progress for replacing seqno usage with requst structures.
There is a general feeling that it is better to move away from using a simple
integer 'seqno' value to track batch buffer completion. Instead, the request
structure should be used. That provides for much more flexibility going
forwards. Especially which things like a GPU scheduler (which can re-order batch
buffers and hence seqnos after submission to the hardware), Android sync points
and other such features which potentially make seqno usage more and more
complex.
The current set of patches do most of the seqno to request structure conversion.
There are still a couple of direct seqno comparisons in the semaphore code. The
final conversion of a seqno test into a 'completed' flag inside the request
structure is still do to as well. Along with whatever changes are required to
maintain such a flag.
The patches are being posted now to make sure that the various people involved
agree that it is heading in the right direction.
[Patches against drm-intel-nightly tree fetched 11/09/2014]
John Harrison (21):
Bug: missing i915_seqno_passed() call?
drm/i915: Remove redundant parameter to i915_gem_object_wait_rendering__tail()
drm/i915: Ensure OLS & PLR are always in sync
drm/i915: Add reference count to request structure
drm/i915: Add helper functions to aid seqno -> request transition
drm/i915: Replace last_[rwf]_seqno with last_[rwf]_req
drm/i915: Ensure requests stick around during waits
drm/i915: Remove 'outstanding_lazy_seqno'
drm/i915: Make 'i915_gem_check_olr' actually check by request not seqno
drm/i915: Convert 'last_flip_req' to be a request not a seqno
drm/i915: Convert i915_wait_seqno to i915_wait_request
drm/i915: Convert 'i915_add_request' to take a request not a seqno
drm/i915: Convert mmio_flip::seqno to struct request
drm/i915: Convert 'flip_queued_seqno' into 'flip_queued_request'
drm/i915: Convert most 'i915_seqno_passed' calls into 'i915_gem_request_completed'
drm/i915: Convert __wait_seqno() to __wait_request()
drm/i915: Convert trace functions from seqno to request
drm/i915: Convert 'trace_irq' to use requests rather than seqnos
drm/i915: Convert semaphores to handle requests not seqnos
drm/i915: Convert 'ring_idle()' to use requests not seqnos
drm/i915: Remove 'obj->ring'
drivers/gpu/drm/i915/i915_debugfs.c | 23 +--
drivers/gpu/drm/i915/i915_drv.h | 64 +++++--
drivers/gpu/drm/i915/i915_gem.c | 265 ++++++++++++++--------------
drivers/gpu/drm/i915/i915_gem_context.c | 12 +-
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 +-
drivers/gpu/drm/i915/i915_gem_gtt.h | 4 +-
drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +-
drivers/gpu/drm/i915/i915_gpu_error.c | 19 +-
drivers/gpu/drm/i915/i915_irq.c | 12 +-
drivers/gpu/drm/i915/i915_trace.h | 47 ++---
drivers/gpu/drm/i915/intel_display.c | 47 ++---
drivers/gpu/drm/i915/intel_drv.h | 4 +-
drivers/gpu/drm/i915/intel_lrc.c | 49 ++---
drivers/gpu/drm/i915/intel_overlay.c | 17 +-
drivers/gpu/drm/i915/intel_ringbuffer.c | 88 +++++----
drivers/gpu/drm/i915/intel_ringbuffer.h | 24 ++-
16 files changed, 392 insertions(+), 293 deletions(-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
