Quoting Mika Kuoppala (2020-02-11 15:54:07) > Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > > > Quoting Mika Kuoppala (2020-02-11 15:23:24) > >> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > >> > + /* Kick the tasklet to process the error */ > >> > + intel_engine_flush_submission(engine); > >> > >> This pattern of usage in selftests makes me want to add mb(); to > >> intel_engine_flush_submission(), as it does not seem to do it > >> implicitly. > >> > >> We want to flush submission and observe the effects, thus > >> it seems like a good place. > > > > Hmm. From the cpu perspective the memory barrier would be on the other > > side in clear_bit_unlock(), and flush_submission does unlock_wait. > > > > But, then, we have to factor in that we have to communicate with an > > external client the GPU, so perhaps an explicit memory barrier... > > > > We certainly do perform the memory barrier in order to set the GPU in > > motion, but have not relied on them for observing effects (aside from > > the CSB ringbuf). > > > > I don't see a strong argument that adding a 'mb/rmb' here will make any > > difference, I don't see what we are pairing with from the GPU > > perspective. But maybe there is? > > I don't have a strong argument from gpu side. But what if the > flush only does the nonatomic wait and returns, and our > CPU has read the state up front for the next comparison. > > Or now thinking it, the saving grace is that if we don't need > to flush in here, the tasklet has finished and finish has > the barrier? That's how we synchronize with the tasklet, yes. So by this point (as we expect to have a request ready and scheduled the tasklet) we know that the tasklet has completed. So we know we've *initiated* some action on the GPU. Most of the side-effects from doing so are without barriers. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
