[Mesa-dev] [PATCH 06/20] radeonsi: implement set_shader_images

[Nicolai Hähnle]

On 17.03.2016 18:13, Marek Olšák wrote:
> On Tue, Mar 15, 2016 at 7:28 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>
>> Whether DCC is disabled depends on the access flags with which the image
>> is bound: image_load supports DCC, but store and atomic don't.
>> ---
>>   src/gallium/drivers/radeonsi/si_descriptors.c | 206 ++++++++++++++++++++++++--
>>   src/gallium/drivers/radeonsi/si_pipe.h        |   7 +
>>   src/gallium/drivers/radeonsi/si_shader.c      |   8 +-
>>   src/gallium/drivers/radeonsi/si_shader.h      |   4 +-
>>   src/gallium/drivers/radeonsi/si_state.c       |  39 +++--
>>   src/gallium/drivers/radeonsi/si_state.h       |  19 +++
>>   6 files changed, 254 insertions(+), 29 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c
>> index d12b3e6..37e49e5 100644
>> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
>> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
>> @@ -64,7 +64,8 @@
>>   #include "util/u_upload_mgr.h"
>>
>>
>> -/* NULL image and buffer descriptor.
>> +/* NULL image and buffer descriptor for textures (alpha = 1) and images
>> + * (alpha = 0).
>>    *
>>    * For images, all fields must be zero except for the swizzle, which
>>    * supports arbitrary combinations of 0s and 1s. The texture type must be
>> @@ -74,7 +75,7 @@
>>    *
>>    * This is the only reason why the buffer descriptor must be in words [4:7].
>>    */
>> -static uint32_t null_descriptor[8] = {
>> +static uint32_t null_texture_descriptor[8] = {
>>          0,
>>          0,
>>          0,
>> @@ -84,10 +85,20 @@ static uint32_t null_descriptor[8] = {
>>           * descriptor */
>>   };
>>
>> +static uint32_t null_image_descriptor[8] = {
>> +       0,
>> +       0,
>> +       0,
>> +       S_008F1C_TYPE(V_008F1C_SQ_RSRC_IMG_1D)
>> +       /* the rest must contain zeros, which is also used by the buffer
>> +        * descriptor */
>> +};
>> +
>>   static void si_init_descriptors(struct si_descriptors *desc,
>>                                  unsigned shader_userdata_index,
>>                                  unsigned element_dw_size,
>> -                               unsigned num_elements)
>> +                               unsigned num_elements,
>> +                               const uint32_t *null_descriptor)
>>   {
>>          int i;
>>
>> @@ -100,10 +111,12 @@ static void si_init_descriptors(struct si_descriptors *desc,
>>          desc->shader_userdata_offset = shader_userdata_index * 4;
>>
>>          /* Initialize the array to NULL descriptors if the element size is 8. */
>> -       if (element_dw_size % 8 == 0)
>> +       if (null_descriptor) {
>> +               assert(element_dw_size % 8 == 0);
>>                  for (i = 0; i < num_elements * element_dw_size / 8; i++)
>> -                       memcpy(desc->list + i*8, null_descriptor,
>> -                              sizeof(null_descriptor));
>> +                       memcpy(desc->list + i * 8, null_descriptor,
>> +                              8 * 4);
>> +       }
>>   }
>>
>>   static void si_release_descriptors(struct si_descriptors *desc)
>> @@ -210,7 +223,7 @@ static void si_set_sampler_view(struct si_context *sctx,
>>                  } else {
>>                          /* Disable FMASK and bind sampler state in [12:15]. */
>>                          memcpy(views->desc.list + slot*16 + 8,
>> -                              null_descriptor, 4*4);
>> +                              null_texture_descriptor, 4*4);
>>
>>                          if (views->sampler_states[slot])
>>                                  memcpy(views->desc.list + slot*16 + 12,
>> @@ -220,9 +233,9 @@ static void si_set_sampler_view(struct si_context *sctx,
>>                  views->desc.enabled_mask |= 1llu << slot;
>>          } else {
>>                  pipe_sampler_view_reference(&views->views[slot], NULL);
>> -               memcpy(views->desc.list + slot*16, null_descriptor, 8*4);
>> +               memcpy(views->desc.list + slot*16, null_texture_descriptor, 8*4);
>>                  /* Only clear the lower dwords of FMASK. */
>> -               memcpy(views->desc.list + slot*16 + 8, null_descriptor, 4*4);
>> +               memcpy(views->desc.list + slot*16 + 8, null_texture_descriptor, 4*4);
>>                  views->desc.enabled_mask &= ~(1llu << slot);
>>          }
>>
>> @@ -301,6 +314,160 @@ si_samplers_update_compressed_colortex_mask(struct si_textures_info *samplers)
>>          }
>>   }
>>
>> +/* IMAGE VIEWS */
>> +
>> +static void
>> +si_release_image_views(struct si_images_info *images)
>> +{
>> +       unsigned i;
>> +
>> +       for (i = 0; i < SI_NUM_IMAGES; ++i) {
>> +               struct pipe_image_view *view = &images->views[i];
>> +
>> +               pipe_resource_reference(&view->resource, NULL);
>> +       }
>> +
>> +       si_release_descriptors(&images->desc);
>> +}
>> +
>> +static void
>> +si_image_views_begin_new_cs(struct si_context *sctx, struct si_images_info *images)
>> +{
>> +       uint64_t mask = images->desc.enabled_mask & ((1llu << SI_NUM_IMAGES) - 1);
>
> Is this masking really needed? It suggests that enabled_mask can have
> non-zero upper bits, which shouldn't happen.

No, it's not needed. It's a left-over from when I thought I would 
implement MSAA textures, which this current series doesn't do yet (the 
state tracker bits for that aren't here yet either, see 
GL_MAX_IMAGE_SAMPLES). I'll remove it.

Thanks,
Nicolai

> Marek
>