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

[Chris Wilson]

Quoting Chris Wilson (2017-11-20 16:45:44)
> 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. :)

Michel said, on irc, that he didn't mind who took authorship, so I gave
it back to him for doing the work, and pushed. Thanks for the really
nasty bug fix,
-Chris