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

[Boris Brezillon]

On Mon, 21 Jun 2021 09:35:03 -0400
Alyssa Rosenzweig <alyssa at collabora.com> wrote:

> I don't see how this test works.
> 
> > +        struct mali_payload_set_value payload = {
> > +                .unknown = 3,
> > +        };  
> 
> 0x3 is the selector for "zero".

Yep.

> 
> > +        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.

Nope, I'm writing 0 to the next job ->exception_status field, which is
needed if we want the loop to be effective (as soon as the GPU sees a
job that has a non-zero exception_status it triggers an INVALID_DATA
fault).

> 
> 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. 

I really want to write a zero here.

> 
> 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.

If I'm correct, ->out is the address the WRITE_VALUE job writes to, so
with 2 jobs, I really have to pass 2 different addresses there,
otherwise one job won't have its exception_status field reset to 0.
This being said, I wonder if I really need 2 jobs here. I'll try with a
single job pointing to himself and writing 0 to its exception_status
field.


> Even better, I think you could use
> NULL jobs for the same purpose.

NULL jobs won't work, I really need to reset the exception_status,
otherwise it triggers an INVALID_DATA fault.