[Mesa-dev] [PATCH 1/6] radeonsi: move sampler descriptors from IB to memory

[Marek Olšák]

On Tue, Jul 15, 2014 at 11:53 AM, Michel Dänzer <michel at daenzer.net> wrote:
> On 13.07.2014 01:35, Marek Olšák wrote:
>>
>> Border colors have been broken if texturing from multiple shader stages is
>> used. This patch doesn't change that.
>
> [...]
>
>> +/* Upload border colors and update the pointers in resource descriptors.
>> + * There can only be 4096 border colors per context.
>> + *
>> + * XXX: This is broken if sampler states are bound to multiple shader stages,
>> + *      because TA_BC_BASE_ADDR is shared by all of them and we overwrite it
>> + *      for stages which were set earlier. This is also broken for
>> + *      fine-grained sampler state updates.
>> + */
>
> I don't think that's accurate, as the BO for storing the border colours
> is per-context, not per-shader-stage.

Ah yes. The problem only occurs when the BO is reallocated. Consider this:

set_sampler_states(SHADER_VERTEX)
  // This sets TA_BC_BASE_ADDR and sets the border color pointers
  // in the sampler descriptors. The pointers are relative to the base address.

set_sampler_states(SHADER_FRAGMENT)
  // If the buffer is reallocated, TA_BC_BASE_ADDR is changed.
  // All border color pointers for fragment sampler states are set and valid.
  // All border color pointers for vertex sampler states are now invalid,
  // because TA_BC_BASE_ADDR has been changed.

The reallocation can also happen halfway through setting up border
colors, e.g. you set border colors 0,1,2,3, then you have to
reallocate, and then you set border colors 4,5,6,7, so the first four
border color pointers end up being incorrect, because the previous
buffer has been thrown away.

>
> AFAICS the only thing missing is flushing the command stream when we
> need to allocate a new BO for that. Might also be a good idea to always
> allocate a new BO for this after a flush.

I don't think so. I don't understand why a flush would be needed here.
We don't have to flush the command stream or allocate a new BO after a
flush, because the BO is mapped without synchronization. The only
thing we need to do is to flush the correct read caches after the
upload, but that's usually not needed if you upload to memory which
hasn't been used before.

I think the proper solution would be to update all border colors for
all bound sampler states again when the buffer is reallocated. Also,
to prevent frequent reallocations, we can check if the current border
color pointer in a sampler state is still valid and if it is, we can
skip the upload.

Marek