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

[Michel Dänzer]

On 16.07.2014 00:07, Marek Olšák wrote:
> 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.

Exactly, except in that case border colours 4,5,6,7 won't work properly
either, because their values are written to the old buffer, because the
border_color_table pointer isn't updated when reallocating the BO.


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

Sounds good, are you going to give this a shot?


-- 
Earthling Michel Dänzer            |                  http://www.amd.com
Libre software enthusiast          |                Mesa and X developer