[Intel-gfx] [PATCH v2 01/14] drm/i915: Keep a global seqno per-engine

[Chris Wilson]

On Thu, Feb 16, 2017 at 08:10:07AM +0000, Tvrtko Ursulin wrote:
> 
> On 15/02/2017 21:49, Chris Wilson wrote:
> >On Wed, Feb 15, 2017 at 05:05:40PM +0000, Tvrtko Ursulin wrote:
> >>
> >>On 14/02/2017 09:54, Chris Wilson wrote:
> >>>Replace the global device seqno with one for each engine, and account
> >>>for in-flight seqno on each separately. This is consistent with
> >>>dma-fence as each timeline has separate fence-contexts for each engine
> >>>and a seqno is only ordered within a fence-context (i.e.  seqno do not
> >>>need to be ordered wrt to other engines, just ordered within a single
> >>>engine). This is required to enable request rewinding for preemption on
> >>>individual engines (we have to rewind the global seqno to avoid
> >>>overflow, and we do not have to rewind all engines just to preempt one.)
> >>>
> >>>Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>>---
> >>>drivers/gpu/drm/i915/i915_debugfs.c      |  5 +--
> >>>drivers/gpu/drm/i915/i915_gem_request.c  | 68 +++++++++++++++-----------------
> >>>drivers/gpu/drm/i915/i915_gem_request.h  |  8 +---
> >>>drivers/gpu/drm/i915/i915_gem_timeline.h |  4 +-
> >>>drivers/gpu/drm/i915/intel_breadcrumbs.c | 33 +++++++---------
> >>>drivers/gpu/drm/i915/intel_engine_cs.c   |  2 -
> >>>drivers/gpu/drm/i915/intel_ringbuffer.h  |  4 +-
> >>>7 files changed, 52 insertions(+), 72 deletions(-)
> >>>
> >>>diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> >>>index cda957c674ee..9b636962cab6 100644
> >>>--- a/drivers/gpu/drm/i915/i915_debugfs.c
> >>>+++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >>>@@ -1080,10 +1080,7 @@ static const struct file_operations i915_error_state_fops = {
> >>>static int
> >>>i915_next_seqno_get(void *data, u64 *val)
> >>>{
> >>>-	struct drm_i915_private *dev_priv = data;
> >>>-
> >>>-	*val = 1 + atomic_read(&dev_priv->gt.global_timeline.seqno);
> >>>-	return 0;
> >>>+	return -ENODEV;
> >>
> >>I assume reason for leaving this function in this state appears in a
> >>later patch? gt.global_timeline stays around for something else?
> >
> >There's no longer a single global seqno, so we tell userspace (igt) it can't
> >have it.
> 
> I missed this is debugfs and that we even have this facility. Does
> the exact errno matters here? Thinking of just dropping the vfunc
> entirely and letting the core return an error. After looking it up
> it seems it would be -EACCES.

The errno doesn't matter. ENODEV is my dirty habit.

It's used by just one test gem_seqno_wrap, which sets a value and then
expects the read to match after a batch. I incorporated the
seqno-wrapping check into gem_exec_whisper, which checks the
synchronisation and ordering of requests between engines across the
wrap. It has been very useful for detecting bugs.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre