[Intel-gfx] [PATCH 4/4] drm/i915/execlists: Delay writing to ELSP until HW has processed the previous write

[Chris Wilson]

Quoting Michel Thierry (2017-11-20 16:17:01)
> On 11/20/2017 4:34 AM, Chris Wilson wrote:
> > The hardware needs some time to process the information received in the
> > ExecList Submission Port, and expects us to not write anything more until
> > it has 'acknowledged' this new submission by sending an IDLE_ACTIVE or
> > PREEMPTED CSB event.
> > 
> > If we do not follow this, the driver could write new data into the ELSP
> > before HW had finishing fetching the previous one, putting us in
> > 'undefined behaviour' space.
> > 
> > This seems to be the problem causing the spurious PREEMPTED & COMPLETE
> > events after a COMPLETE like the one below:
> > 
> > [] vcs0: sw rd pointer = 2, hw wr pointer = 0, current 'head' = 3.
> > [] vcs0:  Execlist CSB[0]: 0x00000018 _ 0x00000007
> > [] vcs0:  Execlist CSB[1]: 0x00000001 _ 0x00000000
> > [] vcs0:  Execlist CSB[2]: 0x00000018 _ 0x00000007  <<< COMPLETE
> > [] vcs0:  Execlist CSB[3]: 0x00000012 _ 0x00000007  <<< PREEMPTED & COMPLETE
> > [] vcs0:  Execlist CSB[4]: 0x00008002 _ 0x00000006
> > [] vcs0:  Execlist CSB[5]: 0x00000014 _ 0x00000006
> > 
> > The ELSP writes that lead to this CSB sequence show that the HW hadn't
> > started executing the previous execlist (the one with only ctx 0x6) by the
> > time the new one was submitted; this is a bit more clear in the data
> > show in the EXECLIST_STATUS register at the time of the ELSP write.
> > 
> > [] vcs0: ELSP[0] = 0x0_0        [execlist1] - status_reg = 0x0_302
> > [] vcs0: ELSP[1] = 0x6_fedb2119 [execlist0] - status_reg = 0x0_8302
> > 
> > [] vcs0: ELSP[2] = 0x7_fedaf119 [execlist1] - status_reg = 0x0_8308
> > [] vcs0: ELSP[3] = 0x6_fedb2119 [execlist0] - status_reg = 0x7_8308
> > 
> > Note that having to wait for this ack does not disable lite-restores,
> > although it may reduce their numbers.
> > 
> > v2: Rewrote Michel's patch, his digging and his fix, my spelling.
> > v3: Reorder to ack early to allow preemption
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102035
> > Suggested-by: Michel Thierry <michel.thierry at intel.com>
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Michel Thierry <michel.thierry at intel.com>
> 
> Isn't it nice to come back after the weekend and see everything is ok?
> 
> Reviewed-by: Michel Thierry <michel.thierry at intel.com>

Do you want to do a quick review-author swap? You take authorship and
I'll take reviewership? It is basically your patch. :)
-Chris