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

[Chris Wilson]

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