[Mesa-dev] [PATCH] radeonsi: add si_init_descriptor_list() helper

[Nicolai Hähnle]

On 31.03.2017 16:16, Samuel Pitoiset wrote:
> This will be used by bindless to initialize the descriptor for
> both samplers and images.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>

I'd prefer to see these patches in a larger context, to be honest.

As for this particular change, "init" to me implies also state/metadata 
initialization. As a stand-alone function, I think the new function 
should be called something else, perhaps si_fill_descriptors -- because 
that's what it does -- and the null_descriptor parameter should be 
renamed (to just descriptor) and made non-optional, so that the 
NULL-check happens in the caller.

This way, the function would make more sense to me as a stand-alone unit 
that does one thing.

Thanks,
Nicolai


> ---
>  src/gallium/drivers/radeonsi/si_descriptors.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c
> index d106351c85..84da830c11 100644
> --- a/src/gallium/drivers/radeonsi/si_descriptors.c
> +++ b/src/gallium/drivers/radeonsi/si_descriptors.c
> @@ -96,6 +96,21 @@ static uint32_t null_image_descriptor[8] = {
>  	 * descriptor */
>  };
>
> +static void si_init_descriptor_list(uint32_t *desc_list,
> +				    unsigned element_dw_size,
> +				    unsigned num_elements,
> +				    const uint32_t *null_descriptor)
> +{
> +	int i;
> +
> +	/* Initialize the array to NULL descriptors if the element size is 8. */
> +	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, 8 * 4);
> +	}
> +}
> +
>  static void si_init_descriptors(struct si_descriptors *desc,
>  				unsigned shader_userdata_index,
>  				unsigned element_dw_size,
> @@ -103,8 +118,6 @@ static void si_init_descriptors(struct si_descriptors *desc,
>  				const uint32_t *null_descriptor,
>  				unsigned *ce_offset)
>  {
> -	int i;
> -
>  	assert(num_elements <= sizeof(desc->dirty_mask)*8);
>
>  	desc->list = CALLOC(num_elements, element_dw_size * 4);
> @@ -121,13 +134,8 @@ static void si_init_descriptors(struct si_descriptors *desc,
>  		*ce_offset += align(element_dw_size * num_elements * 4, 32);
>  	}
>
> -	/* Initialize the array to NULL descriptors if the element size is 8. */
> -	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,
> -			       8 * 4);
> -	}
> +	si_init_descriptor_list(desc->list, element_dw_size, num_elements,
> +				null_descriptor);
>  }
>
>  static void si_release_descriptors(struct si_descriptors *desc)
>


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.