[igt-dev] [PATCH 3/7] lib/panfrost: Add a helper to create a job loop

[Boris Brezillon]

On Mon, 21 Jun 2021 15:09:28 +0100
Steven Price <steven.price at arm.com> wrote:

> On 21/06/2021 14:35, Alyssa Rosenzweig wrote:
> > I don't see how this test works.
> >   
> >> +        struct mali_payload_set_value payload = {
> >> +                .unknown = 3,
> >> +        };  
> > 
> > 0x3 is the selector for "zero".
> >   
> >> +        payload.out = header.next_job_64 = submit->submit_bo->offset + ALIGN(sizeof(header) + sizeof(payload), 64);  
> > 
> > So you are writing 0 to the next_job_64 field, which ends the job chain
> > prematurely.
> > 
> > Perhaps you meant to use an "immediate 64" selector to write the address
> > to jump to? If so, that will be Bifrost only, since the "immediate 64"
> > selector is new in Midgard. 
> > 
> > Upon a second reading, maybe the idea is to ping-pong the jobs
> > statically? I.e. two jobs that have next_job pointed to one another,
> > a job barrier and prefetching disabled, with the content irrelevant. If
> > so, the `out` value can be the same for both and allocate upfront with
> > the payload so the logic is clearer. Even better, I think you could use
> > NULL jobs for the same purpose.
> >   
> 
> I think this is overwriting the status field on the next job. The
> hardware checks that job header to make sure the status field is zero
> before executing (precisely to avoid such a loop happening by accident).
> So two (or more) jobs where the next-job pointers are in a loop works as
> long as you clear the status fields.

Exactly.

> 
> As a side-note: this is very much in the not-really-supported area of
> the hardware. The architecture doesn't prevent the GPU from reading
> ahead in the job chain and rejecting a job before the write-value
> happens.

Good to know. I thought the barrier and no_prefetch flags would prevent
that from happening.