[Mesa-dev] [PATCH v3 44/63] st/mesa: add infrastructure for storing bound texture/image handles

[Samuel Pitoiset]

On 06/13/2017 04:23 PM, James Legg wrote:
> On Fri, 2017-06-09 at 15:35 +0200, Samuel Pitoiset wrote:
>> v2: - rename st_bound_handle to st_bound_handles
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com> (v1)
>> Reviewed-by: Marek Olšák <marek.olsak at amd.com> (v2)
>> ---
>>   src/mesa/state_tracker/st_context.c |  2 +
>>   src/mesa/state_tracker/st_context.h | 11 ++++++
>>   src/mesa/state_tracker/st_texture.c | 77
>> +++++++++++++++++++++++++++++++++++++
>>   src/mesa/state_tracker/st_texture.h |  5 +++
>>   4 files changed, 95 insertions(+)
> 
>> diff --git a/src/mesa/state_tracker/st_texture.c
>> b/src/mesa/state_tracker/st_texture.c
>> index 7da111f39f..fdd727ec8e 100644
>> --- a/src/mesa/state_tracker/st_texture.c
>> +++ b/src/mesa/state_tracker/st_texture.c
>> @@ -421,6 +421,83 @@ st_create_color_map_texture(struct gl_context
>> *ctx)
>>      return pt;
>>   }
>>   
>> +/**
>> + * Destroy bound texture handles for the given stage.
>> + */
>> +static void
>> +st_destroy_bound_texture_handles_per_stage(struct st_context *st,
>> +                                           enum pipe_shader_type
>> shader)
>> +{
>> +   struct st_bound_handles *bound_handles = &st-
>>> bound_texture_handles[shader];
>> +   struct pipe_context *pipe = st->pipe;
>> +   unsigned i;
>> +
>> +   if (likely(!bound_handles->num_handles))
>> +      return;
>> +
>> +   for (i = 0; i < bound_handles->num_handles; i++) {
>> +      uint64_t handle = bound_handles->handles[i];
>> +
>> +      pipe->make_texture_handle_resident(pipe, handle, false);
>> +      pipe->delete_texture_handle(pipe, handle);
>> +   }
>> +   free(bound_handles->handles);
>> +   bound_handles->num_handles = 0;
>> +}
> 
> Perhaps set bound_handles->handles to NULL here, otherwise the address
> of freed memory can be used by the realloc added in the following
> patch.
> 
>> +
>> +
>> +/**
>> + * Destroy all bound texture handles in the context.
>> + */
>> +void
>> +st_destroy_bound_texture_handles(struct st_context *st)
>> +{
>> +   unsigned i;
>> +
>> +   for (i = 0; i < PIPE_SHADER_TYPES; i++) {
>> +      st_destroy_bound_texture_handles_per_stage(st, i);
>> +   }
>> +}
>> +
>> +
>> +/**
>> + * Destroy bound image handles for the given stage.
>> + */
>> +static void
>> +st_destroy_bound_image_handles_per_stage(struct st_context *st,
>> +                                         enum pipe_shader_type
>> shader)
>> +{
>> +   struct st_bound_handles *bound_handles = &st-
>>> bound_image_handles[shader];
>> +   struct pipe_context *pipe = st->pipe;
>> +   unsigned i;
>> +
>> +   if (likely(!bound_handles->num_handles))
>> +      return;
>> +
>> +   for (i = 0; i < bound_handles->num_handles; i++) {
>> +      uint64_t handle = bound_handles->handles[i];
>> +
>> +      pipe->make_image_handle_resident(pipe, handle, GL_READ_WRITE,
>> false);
>> +      pipe->delete_image_handle(pipe, handle);
>> +   }
>> +   free(bound_handles->handles);
>> +   bound_handles->num_handles = 0;
> 
> Same here.
> 
> Thanks,
> James

Right, that seems better. Thanks for noticing this.

Samuel.
>