<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jun 15, 2017 at 9:32 AM, Chris Wilson <span dir="ltr"><<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Quoting Jason Ekstrand (2017-06-15 16:58:13)<br>
<span class="">> On Thu, Jun 15, 2017 at 4:15 AM, Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk">chris@chris-wilson.co.uk</a>> wrote:<br>
><br>
>     Quoting Kenneth Graunke (2017-06-14 21:44:45)<br>
</span><span class="">>     > If Chris is right, and what we're really seeing is that MI_SET_CONTEXT<br>
>     > needs additional flushing, it probably makes sense to fix the kernel.<br>
>     > If it's really fast clear related, then we should do it in Mesa.<br>
><br>
>     If I'm right, it's more of a userspace problem because you have to<br>
>     insert a pipeline stall before STATE_BASE_ADDRESS when switching between<br>
>     blorp/normal and back again, in the same batch. That the MI_SET_CONTEXT<br>
>     may be restoring the dirty GPU state from the previous batch just means<br>
>     that<br>
>     you have to think of batches as being one long continuous batch.<br>
>     -Chris<br>
><br>
><br>
>  Given that, I doubt your explanation is correct.  Right now, we should be<br>
> correct under the "long continuous batch" assumption and we're hanging.  So I<br>
> think that either MI_SET_CONTEXT doesn't stall hard enough or we're conflicting<br>
> with another process somehow.<br>
<br>
</span>What I said was too simplistic, as the MI_SET_CONTEXT would be<br>
introducing side-effects (such as the pipeline being active, hmm, unless<br>
it does flush at the end!). What I mean is that if it is<br>
MI_SET_CONTEXT causing the pipeline to be active, you would need to<br>
treat switching operations within the same pipeline equally. That you<br>
would need a pipeline stall after a blorp/hiz not just to ensure the<br>
data is written but to ensure that the STATE_BASE_ADDRESS doesn't trip<br>
up.<br></blockquote><div><br></div><div>Right.  It's entirely possible that MI_SET_CONTEXT could trip of STATE_BASE_ADDRESS or that simple dirty caches can.  We've seen cache flushing issues around STATE_BASE_ADDRESS in Vulkan where we set it multiple times per batch.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Of course, now I said that it would be a side-effect of MI_SET_CONTEXT<br>
causing the state of the GPU pipelines to be different from expectation,<br>
it becomes the kernel responsibility to add the flush. Argh!<br></blockquote><div><br></div><div>Yeah... I don't think it would be a bad thing for the kernel to do a end-of-pipe sync after MI_SET_CONTEXT (or maybe before?) but us just handling it in userspace is probably reasonable.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I'm open to putting it into the kernel, though I'd rather userspace<br>
handled it. We want to keep the kernel out of the loop as much as<br>
possible.<span class="HOEnZb"><font color="#888888"><br></font></span></blockquote><div><br></div><div>I think I agree with you there.  Which is why Ken and I merged the patches yesterday :-) <br></div></div></div></div>