[PATCH v3 5/7] drm/panfrost: Add a new ioctl to submit batches

[Steven Price]

On 02/07/2021 19:11, Boris Brezillon wrote:
> On Fri, 2 Jul 2021 12:49:55 -0400
> Alyssa Rosenzweig <alyssa at collabora.com> wrote:
> 
>>>> ```  
>>>>>  #define PANFROST_BO_REF_EXCLUSIVE	0x1
>>>>> +#define PANFROST_BO_REF_NO_IMPLICIT_DEP	0x2    
>>>> ```
>>>>
>>>> This seems logically backwards. NO_IMPLICIT_DEP makes sense if we're
>>>> trying to keep backwards compatibility, but here you're crafting a new
>>>> interface totally from scratch. If anything, isn't BO_REF_IMPLICIT_DEP
>>>> the flag you'd want?  
>>>
>>> AFAICT, all other drivers make the no-implicit-dep an opt-in, and I
>>> didn't want to do things differently in panfrost. But if that's really
>>> an issue, I can make it an opt-out.  
>>
>> I don't have strong feelings either way. I was just under the
>> impressions other drivers did this for b/w compat reasons which don't
>> apply here.
> 
> Okay, I think I'll keep it like that unless there's a strong reason to
> make no-implicit dep the default. It's safer to oversync than the skip
> the synchronization, so it does feel like something the user should
> explicitly enable.

I don't have strong feelings - ultimately the number of projects caring
about the uABI is so limited the "default" is pretty irrelevant (it's
not as if we are needing to guide random developers who are new to the
interface). But a conservative default seems sensible.

>>
>>>> Hmm. I'm not /opposed/ and I know kbase uses strides but it seems like
>>>> somewhat unwarranted complexity, and there is a combinatoric explosion
>>>> here (if jobs, bo refs, and syncobj refs use 3 different versions, as
>>>> this encoding permits... as opposed to just specifying a UABI version or
>>>> something like that)  
>>>
>>> Sounds like a good idea. I'll add a version field and map that
>>> to a <job_stride,bo_ref_stride,syncobj_ref_stride> tuple.  
>>
>> Cc Steven, does this make sense?
> 
> I have this approach working, and I must admit I prefer it to the
> per-object stride field passed to the submit struct.
> 

There are benefits both ways:

 * Version number: easier to think about, and less worries about
combinatorial explosion of possible options to test.

 * Explicit structure sizes/strides: much harder to accidentally forgot
to change, clients 'naturally' move to newer versions just with recompiling.

For now I'd be tempted to go for the version number, but I suspect we
should also ensure there's a generic 'flags' field in there. That would
allow us to introduce new features/behaviours in a way which can be
backported more easily if necessary.

The main benefit of structure sizes/strides is if you can break binary
backwards compatibility after a few years - because source compatibility
can easily be maintained while dropping the code for the shorter/older
structs. But Linux tries to maintain binary compatibility so this isn't
so relevant.

Steve