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

2017-06-13 Thread 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 
Reviewed-by: Nicolai Hähnle  (v1)
Reviewed-by: Marek Olšák  (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.



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-06-13 Thread James Legg 
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 
> Reviewed-by: Nicolai Hähnle  (v1)
> Reviewed-by: Marek Olšák  (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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2017-06-09 Thread Samuel Pitoiset 
v2: - rename st_bound_handle to st_bound_handles

Signed-off-by: Samuel Pitoiset 
Reviewed-by: Nicolai Hähnle  (v1)
Reviewed-by: Marek Olšák  (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_context.c 
b/src/mesa/state_tracker/st_context.c
index b99a53b3c7..80811f6cf4 100644
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -291,6 +291,8 @@ st_destroy_context_priv(struct st_context *st, bool 
destroy_pipe)
st_destroy_drawtex(st);
st_destroy_perfmon(st);
st_destroy_pbo_helpers(st);
+   st_destroy_bound_texture_handles(st);
+   st_destroy_bound_image_handles(st);
 
for (shader = 0; shader < ARRAY_SIZE(st->state.sampler_views); shader++) {
   for (i = 0; i < ARRAY_SIZE(st->state.sampler_views[0]); i++) {
diff --git a/src/mesa/state_tracker/st_context.h 
b/src/mesa/state_tracker/st_context.h
index 770477a9bb..f8e9bf96c5 100644
--- a/src/mesa/state_tracker/st_context.h
+++ b/src/mesa/state_tracker/st_context.h
@@ -79,6 +79,12 @@ struct st_bitmap_cache
ubyte *buffer;
 };
 
+struct st_bound_handles
+{
+   unsigned num_handles;
+   uint64_t *handles;
+};
+
 struct st_context
 {
struct st_context_iface iface;
@@ -274,6 +280,11 @@ struct st_context
struct st_perf_monitor_group *perfmon;
 
enum pipe_reset_status reset_status;
+
+   /* Array of bound texture/image handles which are resident in the context.
+*/
+   struct st_bound_handles bound_texture_handles[PIPE_SHADER_TYPES];
+   struct st_bound_handles bound_image_handles[PIPE_SHADER_TYPES];
 };
 
 
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;
+}
+
+
+/**
+ * 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;
+}
+
+
+/**
+ * Destroy all bound image handles in the context.
+ */
+void
+st_destroy_bound_image_handles(struct st_context *st)
+{
+   unsigned i;
+
+   for (i = 0; i < PIPE_SHADER_TYPES; i++) {
+  st_destroy_bound_image_handles_per_stage(st, i);
+   }
+}
+
 
 /**
  * Create a texture handle from a texture unit.
diff --git a/src/mesa/state_tracker/st_texture.h 
b/src/mesa/state_tracker/st_texture.h
index 7f8a0cb841..b97814cb16 100644
--- a/src/mesa/state_tracker/st_texture.h
+++ b/src/mesa/state_tracker/st_texture.h
@@ -250,6 +250,11 @@ st_texture_image_copy(struct pipe_context *pipe,
 extern struct pipe_resource *
 st_create_color_map_texture(struct gl_context *ctx);
 
+void
+st_destroy_bound_texture_handles(struct st_context *st);
+
+void
+st_destroy_bound_image_handles(struct st_context *st);
 
 bool
 st_etc_fallback(struct st_context *st, struct gl_texture_image *texImage);
-- 
2.13.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev