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

[Marek Olšák]

On Wed, Jul 16, 2014 at 10:17 AM, Michel Dänzer <michel at daenzer.net> wrote:
> 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?

Yes, but not in this patch. For now, I'll just change the comment to
"this code is very broken".

Marek