[Mesa-dev] [PATCH] radeonsi: add flexible shader descriptor management and use it for sampler views

[Christian König]

Am 15.08.2013 09:33, schrieb Michel Dänzer:
> On Don, 2013-08-15 at 05:25 +0200, Marek Olšák wrote:
>> (This should be applied before MSAA, which will need to be rebased.)
>>
>> It moves all sampler view descriptors to a buffer.
>> It supports partial resource updates and it can also unbind resources
>> (required for FMASK texturing).
>>
>> The buffer contains all sampler view descriptors for one shader stage,
>> represented as an array. On top of that, there are N arrays in the buffer,
>> which are used to emulate context registers as implemented by the previous
>> ASICs (each array is a context).
>>
>> This uses the RCU synchronization approach to avoid read-after-write hazards
>> as discussed in the thread:
>> "radeonsi: add FMASK texture binding slots and resource setup"
>>
>> CP DMA is used to clear the descriptors at context initialization and to copy
>> the descriptors from one context to the next.
>>
>> IMPORTANT:
>>    128 resource contexts are needed, 64 doesn't work.
> Hmm, I suppose this might depend on the specific GPU?

That looks like the SQ is indeed capable of handling more request at once.

Well there has to be an upper limit to this, we just have to figure it 
out from the docs.

>>    If I set SH_KCACHE_ACTION_ENA before every draw call, only 2 contexts are needed.
> But that hurts performance?

Yeah, probably allot. Sounds like a complete pipeline drain to me.

Don't fully understand it either, but I think the commands from the IB 
are read through the KCACHE as well. So when you flush it the CP 
completely stops until that command is complete.

>
> The patch looks good to me, just a minor comment:

Also have some more comments on this, but going to write a separate mail 
on it.

Christian.

>
>> +/* Emit a CP DMA packet to do a copy from one buffer to another.
>> + * The size must fit in bits [20:0]. Notes:
>> + *
>> + * 1) Set sync to true if you want the 3D engine to wait until CP DMA is done.
>> + *
>> + * 2) Set raw_hazard_wait to true if the source data was used as a destination
>> + *    in a previous CP DMA packet. It's for preventing a read-after-write hazard
>> + *    between two CP DMA packets.
>> + */
>> +static void si_emit_cp_dma_copy_buffer(struct r600_context *rctx,
>> +				       uint64_t dst_va, uint64_t src_va,
>> +				       unsigned size,
>> +				       bool sync, bool raw_hazard_wait)
> [...]
>
>> +	/* Copy the descriptors to a new context slot. */
>> +	si_emit_cp_dma_copy_buffer(rctx,
>> +				   va_base + new_context_id * desc->context_size,
>> +				   va_base + desc->current_context_id * desc->context_size,
>> +				   desc->context_size, true, false);
> It's hard to remember / guess the meaning of such boolean parameters, so
> it might be better to use self-explanatory flags instead.
>
>