[Mesa-dev] [PATCH 1/2] i965: add missing rollback of URB requirements

[andrey simiklit]

On Tue, Jan 8, 2019 at 6:16 PM Chris Wilson <chris at chris-wilson.co.uk>
wrote:

> Quoting andrey simiklit (2019-01-08 16:00:45)
> > On Tue, Jan 8, 2019 at 1:11 PM Chris Wilson <chris at chris-wilson.co.uk>
> wrote:
> >
> >     Quoting Lionel Landwerlin (2019-01-08 11:03:26)
> >     > Hi Andrii,
> >     >
> >     > Although I think what these patches do makes sense, I think it's
> missing
> >     > the bigger picture.
> >     > There is a lot more state that gets lost if we have to revert all
> of the
> >     > emitted commands.
> >     > A quick look at brw_upload_pipeline_state() shows all of the
> programs
> >     > could be invalid as well, or the number of samples, etc...
> >     >
> >     > To me it seems like we need a much larger state save/restore.
> >     >
> >     > Ken: what do you think?
> >
> >     How about not rolling back? If we accept it as currently broken, and
> just
> >     demand the kernel provide logical contexts for all i965 devices
> (just ack
> >     some patches!), and then just flush the batch (possibly with a dummy
> 3D
> >     prim if you want to be sure the 3D state is loaded) and rely on the
> >     context preserving state across batches.
> >     -Chris
> >
> >
> > Could you please provide a link to the patches you talking about?
>
> I just need an ack for the kernel patches to enable context support.
>
> > At the moment as far as I understood the rollback system
> > helps to reduce quantity of the flush operations in some lucky cases.
> > When there isn't enough space for a batch aperture,
> > we will rollback the batch to the saved state,
> > when limit (means aperture_threshold) wasn't reached by it.
> >
> > pseudo code (simple variant, without error handling):
> >    save batch state here;
> >    do
> >    {
> >        add primitives to the batch;
> >        if the batch size limit is exceeded
> >        {
> >            rollback to the saved batch state;
> >            flush the batch;
> >        }
> >        else
> >        {
> >            break;
> >        }
> >    } while(true);
> >
> > Are you suggesting to flush the batch every time without waiting for a
> nearly
> > full filling of it?
> > Like this:
> >    add primitives to batch;
> >    flush batch;
>
> The suggestion was just thinking about if we detect that this primitive
> would exceed the aperture, instead of rolling back the batch to before
> the primitive, unroll the objects/relocs (so that we don't trigger
> ENOSPC from the kernel) but keep the 3DSTATE without the final PRIM and
> submit that.
>
> Basically it's all about removing no_batch_wrap.
>
> Quite icky. All it saves is having to track the incidental details like
> URB, but you still have to re-emit all the surface state (but that's
> already a given as it is stored alongside the batch). However, that's
> all you have to remember.
> -Chris
>

Thanks a lot for clarifications.
-Andrii


> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190108/e32716b0/attachment.html>