On Mon, Jun 08, 2015 at 07:00:49PM +0100, Chris Wilson wrote: > On Mon, Jun 08, 2015 at 08:34:51PM +0300, Ville Syrjälä wrote: > > On Mon, Jun 08, 2015 at 06:12:47PM +0100, Chris Wilson wrote: > > > On Mon, Jun 08, 2015 at 06:08:00PM +0100, Dave Gordon wrote: > > > > On 08/06/15 17:28, Imre Deak wrote: > > > > > By running igt/store_dword_loop_render on BXT we can hit a coherency > > > > > problem where the seqno written at GPU command completion time is not > > > > > seen by the CPU. This results in __i915_wait_request seeing the stale > > > > > seqno and not completing the request (not considering the lost > > > > > interrupt/GPU reset mechanism). I also verified that this isn't a case > > > > > of a lost interrupt, or that the command didn't complete somehow: when > > > > > the coherency issue occured I read the seqno via an uncached GTT mapping > > > > > too. While the cached version of the seqno still showed the stale value > > > > > the one read via the uncached mapping was the correct one. > > > > > > > > > > Work around this issue by clflushing the corresponding CPU cacheline > > > > > following any store of the seqno and preceding any reading of it. When > > > > > reading it do this only when the caller expects a coherent view. > > > > > > > > > > Testcase: igt/store_dword_loop_render > > > > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > > > > > > > > Not necessarily a cure for this, but BSpec says of MI_STORE_DATA_IMM > > > > (and MI_STORE_DATA_INDEX): > > > > > > > > This command simply initiates the write operation with > > > > command execution proceeding normally. Although the write > > > > operation is guaranteed to complete "eventually", there is > > > > no mechanism to synchronize command execution with the > > > > completion (or even initiation) of these operations. > > > > > > > > So shouldn't we use MI_FLUSH_DW or PIPE_CONTROL to update the seqno in > > > > the HWSP instead? > > > > > > iirc there is also no guarrantee for when the post-sync write op is > > > completed for a FLUSH_DW/PIPE_CONTROL either. I'd be happy to be > > > corrected! > > > > My reading of the spec suggests that something like this could work: > > PIPE_CONTROL w/ PIPE_CONTROL_QW_WRITE > > PIPE_CONTROL w/ PIPE_CONTROL_NOTIFY | PIPE_CONTROL_FLUSH_ENABLE > > Absolutely sure? The issue is not the completion of the PIPE_CONTROL, > but of ensure that the value has been written to memory and the CPU > cache snooped. I don't remember there being anything as clear as the > gen2-5 statement that all writes are coherent before the interrupt is > raised. > > We can hit the issue that the seqno writes aren't coherent before the > interrupt with our current method - I have seen it with hsw, so this is > definitely something worth improving. What I get from the spec is: - The post-sync operation is started after previous and current flushes have completed - The flush enable bit causes the CS to wait until all previous post-sync operations have completed, which hopefully means the store is visible to everyone - The notify interrupt is signalled after the current sync operation has completed, which I hope means the flush enable stall has also finished (and if not a three PIPE_CONTROL seqence could be used instead) So no, I'm not absolutely sure by any means. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
