<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jan 8, 2019 at 11:01 PM Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk">chris@chris-wilson.co.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Kenneth Graunke (2019-01-08 20:17:01)<br>
> On Tuesday, January 8, 2019 3:11:37 AM PST Chris Wilson wrote:<br>
> > Quoting Lionel Landwerlin (2019-01-08 11:03:26)<br>
> > > Hi Andrii,<br>
> > > <br>
> > > Although I think what these patches do makes sense, I think it's missing <br>
> > > the bigger picture.<br>
> > > There is a lot more state that gets lost if we have to revert all of the <br>
> > > emitted commands.<br>
> > > A quick look at brw_upload_pipeline_state() shows all of the programs <br>
> > > could be invalid as well, or the number of samples, etc...<br>
> > > <br>
> > > To me it seems like we need a much larger state save/restore.<br>
> > > <br>
> > > Ken: what do you think?<br>
> > <br>
> > How about not rolling back? If we accept it as currently broken, and just<br>
> > demand the kernel provide logical contexts for all i965 devices (just ack<br>
> > some patches!), and then just flush the batch (possibly with a dummy 3D<br>
> > prim if you want to be sure the 3D state is loaded) and rely on the<br>
> > context preserving state across batches.<br>
> > -Chris<br>
> <br>
> I'm not sure precisely what you're proposing, but every scenario I can<br>
> think of is breaking in some subtle way.<br>
> <br>
> Option 1: Submit batch at last 3DPRIMITIVE, re-call emit in a new batch.<br>
> <br>
> This is what we do today; it doesn't work since brw_upload_render_state<br>
> implicitly sets CPU-side fields for "current state of the GPU", which<br>
> become inaccurate at rollback, so the next state upload would do the<br>
> wrong thing.  We'd need to roll all those back as well, or only commit<br>
> them when we finish uploading state.  Atoms also flag other new atoms,<br>
> and that's not getting rolled back, but extra flagging is harmless.<br>
> <br>
> Fixing this means disentangling some of our code, saving and restoring<br>
> more values, and so on.  Maybe dropping a few CPU micro-optimizations.<br>
> <br>
> Option 2: Submit batch at last 3DPRIMITIVE (A), memcpy commands for<br>
>           for failing primitive (B) into a new batch.<br>
> <br>
> This doesn't work either.  Let's say (A) issued a 3DSTATE_INDEX_BUFFER<br>
> command with a pointer to the index buffer.  (B) didn't change index<br>
> buffer state, so it doesn't emit one, intending to reuse the old value.<br>
<br>
This with all relocs with BRW_NEW_BATCH, and iirc the rule is that they<br>
are already tied into that BRW_NEW_BATCH flag. Except you don't need to<br>
memcpy, since you already have the 3DSTATE equivalent to the cpu state<br>
tracker in the batch, emit that, and it will be recorded in the context<br>
for the next batch. (You just don't tell the kernel about the<br>
buffers/relocs that cause the aperture exhaustion and it won't do<br>
anything about them. The GPU may load stale addresses, but never uses<br>
them, exactly like the context state on the next batch anyway.)<br>
<br>
It's not a great argument, but the argument is that there's less you<br>
need to fixup, than having to remember to every single comparison in<br>
order to rollback the cpu state tracker. <br>
-Chris<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br></blockquote><div><br><div>So, have we a plan as to it?</div><div>This patch was sent 1 year ago and I almost forgot it.</div><div>Maybe it was fixed by somebody and this patch can be just rejected/closed)</div></div><div>- Andrii <br></div></div></div>