<div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, Jan 8, 2019 at 6:16 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 andrey simiklit (2019-01-08 16:00:45)<br>
> On Tue, Jan 8, 2019 at 1:11 PM Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>> wrote:<br>
> <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>
> <br>
> Could you please provide a link to the patches you talking about?<br>
<br>
I just need an ack for the kernel patches to enable context support.<br>
<br>
> At the moment as far as I understood the rollback system<br>
> helps to reduce quantity of the flush operations in some lucky cases.<br>
> When there isn't enough space for a batch aperture,<br>
> we will rollback the batch to the saved state,<br>
> when limit (means aperture_threshold) wasn't reached by it.<br>
> <br>
> pseudo code (simple variant, without error handling):<br>
>    save batch state here;<br>
>    do<br>
>    {<br>
>        add primitives to the batch;<br>
>        if the batch size limit is exceeded  <br>
>        {<br>
>            rollback to the saved batch state;<br>
>            flush the batch;<br>
>        }<br>
>        else<br>
>        {<br>
>            break;<br>
>        }<br>
>    } while(true);<br>
> <br>
> Are you suggesting to flush the batch every time without waiting for a nearly<br>
> full filling of it?<br>
> Like this:<br>
>    add primitives to batch;<br>
>    flush batch;<br>
<br>
The suggestion was just thinking about if we detect that this primitive<br>
would exceed the aperture, instead of rolling back the batch to before<br>
the primitive, unroll the objects/relocs (so that we don't trigger<br>
ENOSPC from the kernel) but keep the 3DSTATE without the final PRIM and<br>
submit that.<br>
<br>
Basically it's all about removing no_batch_wrap.<br>
<br>
Quite icky. All it saves is having to track the incidental details like<br>
URB, but you still have to re-emit all the surface state (but that's<br>
already a given as it is stored alongside the batch). However, that's<br>
all you have to remember.<br>
-Chris<br></blockquote><div><br></div><div>Thanks a lot for clarifications.</div><div>-Andrii<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
_______________________________________________<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></div></div>