[Mesa-dev] [PATCH v2 08/12] radeonsi: Allocate chunks of CE ram.

[Marek Olšák]

On Mon, Apr 18, 2016 at 12:19 AM, Bas Nieuwenhuizen
<bas at basnieuwenhuizen.nl> wrote:
> On Mon, Apr 18, 2016 at 12:13 AM, Marek Olšák <maraeo at gmail.com> wrote:
>> On Sun, Apr 17, 2016 at 1:43 AM, Bas Nieuwenhuizen
>> <bas at basnieuwenhuizen.nl> wrote:
>>> v2: Use 32 byte alignment.
>>>
>>> Signed-off-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
>>> ---
>>>  src/gallium/drivers/radeonsi/si_descriptors.c | 30 +++++++++++++++++++--------
>>>  src/gallium/drivers/radeonsi/si_state.h       |  3 +++
>>>  2 files changed, 24 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c
>>> index 7fc1461..a937973 100644
>>> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
>>> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
>>> @@ -98,7 +98,8 @@ static void si_init_descriptors(struct si_descriptors *desc,
>>>                                 unsigned shader_userdata_index,
>>>                                 unsigned element_dw_size,
>>>                                 unsigned num_elements,
>>> -                               const uint32_t *null_descriptor)
>>> +                               const uint32_t *null_descriptor,
>>> +                               unsigned *ce_offset)
>>>  {
>>>         int i;
>>>
>>> @@ -109,6 +110,10 @@ static void si_init_descriptors(struct si_descriptors *desc,
>>>         desc->num_elements = num_elements;
>>>         desc->list_dirty = true; /* upload the list before the next draw */
>>>         desc->shader_userdata_offset = shader_userdata_index * 4;
>>> +       desc->ce_offset = *ce_offset;
>>> +
>>> +       /* make sure that ce_offset stays 32 byte aligned */
>>> +       *ce_offset += align(element_dw_size * num_elements * 4, 32);
>>
>> Please define SI_GLOBAL_L2_CACHE_LINE_SIZE 64 in si_pipe.h and use that.
>>
>> Aligning the offset is indeed a good idea, but aligning to the cache
>> line size should be even better.
>
> The 32-byte alignment is a restriction of the LOAD_CONST_RAM packet
> the wants size and offset to be 32 byte aligned.
>
> Note that this is alignment within the CE ram, not the aligment of the
> memory loaded and stored. Does the L2 cache line size really matter
> here? I would expect the alignment that the uploader allocation gives
> to be more important.

OK, good point. The patch is:

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek