[Mesa-dev] [PATCH] drirc: add option to disable ARB_draw_indirect

[Nicolai Hähnle]

On 15.12.2017 00:56, Rob Clark wrote:
> On Wed, Dec 6, 2017 at 3:31 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> On 12/05/2017 08:25 AM, Ilia Mirkin wrote:
>>> On Tue, Dec 5, 2017 at 8:18 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>> Hi Rob,
>>>>
>>>> On 5 December 2017 at 12:54, Rob Clark <robdclark at gmail.com> wrote:
>>>>> This is a bit sad/annoying.  But with current GPU firmware (at least on
>>>>> a5xx) we can support both draw-indirect and base-instance.  But we can't
>>>>> support draw-indirect with a non-zero base-instance specified.  So add a
>>>>> driconf option to hide the extension from games that are known to use
>>>>> both.
>>>>>
>>>>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>>>>> ---
>>>>> Tbh, I'm also not really sure what to do when/if we got updated firmware
>>>>> which handled draw-indirect with base-instance, since we'd need to make
>>>>> this option conditional on fw version.  For STK that probably isn't a
>>>>> big deal since it doesn't use draw-indirect in a particularly useful way
>>>>> (the indirect buffer is generated on CPU).
>>>>>
>>>> Couldn't freedreno just return 0 for PIPE_CAP_DRAW_INDIRECT (aka
>>>> disable the extension) as it detects buggy FW?
>>>> This is what radeons have been doing as they encounter iffy firmware or LLVM.
>>>>
>>>> AFAICT freedreno doesn't do GL 4.0 or GLES 3.1 so one should be safe.
>>>
>>> Rob is this -><- close to ES 3.1, so that's not a great option.
>>
>> And I don't suppose there's a way to get updated firmware?  i965 has
>> similar sorts of cases where higher versions are disabled due to missing
>> kernel features.
>>
> 
> so after r/e the instruction set for the CP microcontrollers and
> writing a disassembler and assembler[1], and figuring out how the fw
> handles CP_DRAW_INDIRECT and CP_DRAW_INDX_INDIRECT packets, I've come
> to the conclusion that the issue isn't actually with draw-indirect vs
> base-instance (at least not w/ the fw from my pixel2 which md5sum
> claims is the same as what is in linux-firmware.. it is possible that
> I was using an earlier version of the fw before when I came to this
> conclusion).  On the plus side, the PFP/ME microcontrollers that parse
> the cmdstream are pretty neat and I learned some useful stuff along
> the way.
> 
> But thinking a bit about how stk is using GL_MAP_PERSISTENT_BIT to map
> and update the draw-indirect buffers, it seems to me there are plenty
> of ways this can go wrong w/ tilers (and even more when you throw
> re-ordering into the mix).  Possibly I should disable reordering when
> the indirect buffer is mapped w/ PERSISTENT bit, although for games
> like stk this is probably counter-productive vs just hiding the
> draw-indirect extension.. for games that actually use the GPU to write
> the draw-indirect buffer it shouldn't be a problem.  So I think a
> driconf patch like this probably still ends up being useful in the
> end.

Can you detail a bit what you think could go wrong? I believe that the 
intention of the GL spec is that reordering in tilers should be possible 
at least for buffers that are mapped PERSISTENT but not COHERENT.

You may only have to block reordering if the buffer is mapped both 
PERSISTENT *and* COHERENT -- and even then, reordering is probably possible.

Granted, the spec is unclear as usual when it comes to these memory 
synchronization issues -- the description of the MAP_COHERENT_BIT in 
section 6.2 does not mention WAR hazards (in particular, Write by client 
after Read by server) -- but perhaps that can be fixed.

To go into a bit more detail, what I suspect you're worried about is 
applications doing stuff like:

1. Write to indirect buffer (persistently & coherently mapped)
2. Draw*Indirect
3. Write to the same location in the indirect buffer
4. Draw*Indirect

... but this is bound to fail with "normal" GPUs (like ours) as well. 
Perhaps you have a different scenario in mind?

Cheers,
Nicolai