[Intel-gfx] [PATCH 6/6] drm/i915/execlists: Read the context-status HEAD from the HWSP

[Chris Wilson]

Quoting Mika Kuoppala (2017-09-13 15:12:42)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > The engine also provides a mirror of the CSB write pointer in the HWSP,
> > but not of our read pointer. To take advantage of this we need to
> > remember where we read up to on the last interrupt and continue off from
> > there. This poses a problem following a reset, as we don't know where
> > the hw will start writing from, and due to the use of power contexts we
> > cannot perform that query during the reset itself. So we continue the
> > current modus operandi of delaying the first read of the context-status
> > read/write pointers until after the first interrupt. With this we should
> > now have eliminated all uncached mmio reads in handling the
> > context-status interrupt, though we still have the uncached mmio writes
> > for submitting new work, and many uncached mmio reads in the global
> > interrupt handler itself. Still a step in the right direction towards
> > reducing our resubmit latency, although it appears lost in the noise!
> >
> > v2: Cannonlake moved the CSB write index
> > v3: Include the sw/hwsp state in debugfs/i915_engine_info
> > v4: Also revert to using CSB mmio for GVT-g
> > v5: Prevent the compiler reloading tail (Mika)
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Michel Thierry <michel.thierry at intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> > Cc: Zhenyu Wang <zhenyuw at linux.intel.com>
> > Cc: Zhi Wang <zhi.a.wang at intel.com>
> > Acked-by: Michel Thierry <michel.thierry at intel.com>

> > @@ -569,9 +568,19 @@ static void intel_lrc_irq_handler(unsigned long data)
> >                * is set and we do a new loop.
> >                */
> >               __clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
> > -             head = readl(csb_mmio);
> > -             tail = GEN8_CSB_WRITE_PTR(head);
> > -             head = GEN8_CSB_READ_PTR(head);
> > +             if (unlikely(engine->csb_head == -1)) { /* following a reset */
> 
> Was going to suggest using the same csb_use_mmio flag for this
> but that would not gain much when looking at the read ptr write
> further down.

Right, it has to cover the post-reset fixup of state as well.
 
> Reviewed-by: Mika Kuoppala <mika.kuoppala at intel.com>

(Minor fixup for logical flip of csb_use_mmio).

Pushed this after a few months of waiting in the wings. Thanks all,
-Chris