Re: [Mesa-dev] [PATCH 8/8] gallium/radeon: do not reallocate user memory buffers

2016-01-13 Thread Marek Olšák 
Patches 3-8:

Reviewed-by: Marek Olšák 

Marek

On Tue, Jan 12, 2016 at 5:06 PM, Nicolai Hähnle  wrote:
> From: Nicolai Hähnle 
>
> The whole point of AMD_pinned_memory is that applications don't have to map
> buffers via OpenGL - but they're still allowed to, so make sure we don't break
> the link between buffer object and user memory unless explicitly instructed
> to.
> ---
>  src/gallium/drivers/radeon/r600_buffer_common.c | 31 
> ++---
>  src/gallium/drivers/radeon/radeon_winsys.h  |  8 +++
>  src/gallium/winsys/amdgpu/drm/amdgpu_bo.c   |  6 +
>  src/gallium/winsys/radeon/drm/radeon_drm_bo.c   |  6 +
>  4 files changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/src/gallium/drivers/radeon/r600_buffer_common.c 
> b/src/gallium/drivers/radeon/r600_buffer_common.c
> index 09755e0..6592c5b 100644
> --- a/src/gallium/drivers/radeon/r600_buffer_common.c
> +++ b/src/gallium/drivers/radeon/r600_buffer_common.c
> @@ -209,11 +209,15 @@ static void r600_buffer_destroy(struct pipe_screen 
> *screen,
> FREE(rbuffer);
>  }
>
> -void r600_invalidate_resource(struct pipe_context *ctx,
> - struct pipe_resource *resource)
> +static bool
> +r600_do_invalidate_resource(struct r600_common_context *rctx,
> +   struct r600_resource *rbuffer)
>  {
> -   struct r600_common_context *rctx = (struct r600_common_context*)ctx;
> -struct r600_resource *rbuffer = r600_resource(resource);
> +   /* In AMD_pinned_memory, the user pointer association only gets
> +* broken when the buffer is explicitly re-allocated.
> +*/
> +   if (rctx->ws->buffer_is_user_ptr(rbuffer->buf))
> +   return false;
>
> /* Check if mapping this buffer would cause waiting for the GPU. */
> if (r600_rings_is_buffer_referenced(rctx, rbuffer->buf, 
> RADEON_USAGE_READWRITE) ||
> @@ -222,6 +226,17 @@ void r600_invalidate_resource(struct pipe_context *ctx,
> } else {
> util_range_set_empty(>valid_buffer_range);
> }
> +
> +   return true;
> +}
> +
> +void r600_invalidate_resource(struct pipe_context *ctx,
> + struct pipe_resource *resource)
> +{
> +   struct r600_common_context *rctx = (struct r600_common_context*)ctx;
> +   struct r600_resource *rbuffer = r600_resource(resource);
> +
> +   (void)r600_do_invalidate_resource(rctx, rbuffer);
>  }
>
>  static void *r600_buffer_get_transfer(struct pipe_context *ctx,
> @@ -291,10 +306,10 @@ static void *r600_buffer_transfer_map(struct 
> pipe_context *ctx,
> !(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {
> assert(usage & PIPE_TRANSFER_WRITE);
>
> -   r600_invalidate_resource(ctx, resource);
> -
> -   /* At this point, the buffer is always idle. */
> -   usage |= PIPE_TRANSFER_UNSYNCHRONIZED;
> +   if (r600_do_invalidate_resource(rctx, rbuffer)) {
> +   /* At this point, the buffer is always idle. */
> +   usage |= PIPE_TRANSFER_UNSYNCHRONIZED;
> +   }
> }
> else if ((usage & PIPE_TRANSFER_DISCARD_RANGE) &&
>  !(usage & PIPE_TRANSFER_UNSYNCHRONIZED) &&
> diff --git a/src/gallium/drivers/radeon/radeon_winsys.h 
> b/src/gallium/drivers/radeon/radeon_winsys.h
> index 4af6a18..ad30474 100644
> --- a/src/gallium/drivers/radeon/radeon_winsys.h
> +++ b/src/gallium/drivers/radeon/radeon_winsys.h
> @@ -530,6 +530,14 @@ struct radeon_winsys {
>   void *pointer, unsigned size);
>
>  /**
> + * Whether the buffer was created from a user pointer.
> + *
> + * \param buf   A winsys buffer object
> + * \return  whether \p buf was created via buffer_from_ptr
> + */
> +bool (*buffer_is_user_ptr)(struct pb_buffer *buf);
> +
> +/**
>   * Get a winsys handle from a winsys buffer. The internal structure
>   * of the handle is platform-specific and only a winsys should access it.
>   *
> diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c 
> b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
> index a844773..82c803b 100644
> --- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
> +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
> @@ -686,6 +686,11 @@ error:
>  return NULL;
>  }
>
> +static bool amdgpu_bo_is_user_ptr(struct pb_buffer *buf)
> +{
> +   return ((struct amdgpu_winsys_bo*)buf)->user_ptr != NULL;
> +}
> +
>  static uint64_t amdgpu_bo_get_va(struct pb_buffer *buf)
>  {
> return ((struct amdgpu_winsys_bo*)buf)->va;
> @@ -701,6 +706,7 @@ void amdgpu_bo_init_functions(struct amdgpu_winsys *ws)
> ws->base.buffer_create = amdgpu_bo_create;
> ws->base.buffer_from_handle = amdgpu_bo_from_handle;
> ws->base.buffer_from_ptr = amdgpu_bo_from_ptr;
> +   ws->base.buffer_is_user_ptr =

[Mesa-dev] [PATCH 8/8] gallium/radeon: do not reallocate user memory buffers

2016-01-12 Thread Nicolai Hähnle 
From: Nicolai Hähnle 

The whole point of AMD_pinned_memory is that applications don't have to map
buffers via OpenGL - but they're still allowed to, so make sure we don't break
the link between buffer object and user memory unless explicitly instructed
to.
---
 src/gallium/drivers/radeon/r600_buffer_common.c | 31 ++---
 src/gallium/drivers/radeon/radeon_winsys.h  |  8 +++
 src/gallium/winsys/amdgpu/drm/amdgpu_bo.c   |  6 +
 src/gallium/winsys/radeon/drm/radeon_drm_bo.c   |  6 +
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/src/gallium/drivers/radeon/r600_buffer_common.c 
b/src/gallium/drivers/radeon/r600_buffer_common.c
index 09755e0..6592c5b 100644
--- a/src/gallium/drivers/radeon/r600_buffer_common.c
+++ b/src/gallium/drivers/radeon/r600_buffer_common.c
@@ -209,11 +209,15 @@ static void r600_buffer_destroy(struct pipe_screen 
*screen,
FREE(rbuffer);
 }
 
-void r600_invalidate_resource(struct pipe_context *ctx,
- struct pipe_resource *resource)
+static bool
+r600_do_invalidate_resource(struct r600_common_context *rctx,
+   struct r600_resource *rbuffer)
 {
-   struct r600_common_context *rctx = (struct r600_common_context*)ctx;
-struct r600_resource *rbuffer = r600_resource(resource);
+   /* In AMD_pinned_memory, the user pointer association only gets
+* broken when the buffer is explicitly re-allocated.
+*/
+   if (rctx->ws->buffer_is_user_ptr(rbuffer->buf))
+   return false;
 
/* Check if mapping this buffer would cause waiting for the GPU. */
if (r600_rings_is_buffer_referenced(rctx, rbuffer->buf, 
RADEON_USAGE_READWRITE) ||
@@ -222,6 +226,17 @@ void r600_invalidate_resource(struct pipe_context *ctx,
} else {
util_range_set_empty(>valid_buffer_range);
}
+
+   return true;
+}
+
+void r600_invalidate_resource(struct pipe_context *ctx,
+ struct pipe_resource *resource)
+{
+   struct r600_common_context *rctx = (struct r600_common_context*)ctx;
+   struct r600_resource *rbuffer = r600_resource(resource);
+
+   (void)r600_do_invalidate_resource(rctx, rbuffer);
 }
 
 static void *r600_buffer_get_transfer(struct pipe_context *ctx,
@@ -291,10 +306,10 @@ static void *r600_buffer_transfer_map(struct pipe_context 
*ctx,
!(usage & PIPE_TRANSFER_UNSYNCHRONIZED)) {
assert(usage & PIPE_TRANSFER_WRITE);
 
-   r600_invalidate_resource(ctx, resource);
-
-   /* At this point, the buffer is always idle. */
-   usage |= PIPE_TRANSFER_UNSYNCHRONIZED;
+   if (r600_do_invalidate_resource(rctx, rbuffer)) {
+   /* At this point, the buffer is always idle. */
+   usage |= PIPE_TRANSFER_UNSYNCHRONIZED;
+   }
}
else if ((usage & PIPE_TRANSFER_DISCARD_RANGE) &&
 !(usage & PIPE_TRANSFER_UNSYNCHRONIZED) &&
diff --git a/src/gallium/drivers/radeon/radeon_winsys.h 
b/src/gallium/drivers/radeon/radeon_winsys.h
index 4af6a18..ad30474 100644
--- a/src/gallium/drivers/radeon/radeon_winsys.h
+++ b/src/gallium/drivers/radeon/radeon_winsys.h
@@ -530,6 +530,14 @@ struct radeon_winsys {
  void *pointer, unsigned size);
 
 /**
+ * Whether the buffer was created from a user pointer.
+ *
+ * \param buf   A winsys buffer object
+ * \return  whether \p buf was created via buffer_from_ptr
+ */
+bool (*buffer_is_user_ptr)(struct pb_buffer *buf);
+
+/**
  * Get a winsys handle from a winsys buffer. The internal structure
  * of the handle is platform-specific and only a winsys should access it.
  *
diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c 
b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
index a844773..82c803b 100644
--- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
+++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c
@@ -686,6 +686,11 @@ error:
 return NULL;
 }
 
+static bool amdgpu_bo_is_user_ptr(struct pb_buffer *buf)
+{
+   return ((struct amdgpu_winsys_bo*)buf)->user_ptr != NULL;
+}
+
 static uint64_t amdgpu_bo_get_va(struct pb_buffer *buf)
 {
return ((struct amdgpu_winsys_bo*)buf)->va;
@@ -701,6 +706,7 @@ void amdgpu_bo_init_functions(struct amdgpu_winsys *ws)
ws->base.buffer_create = amdgpu_bo_create;
ws->base.buffer_from_handle = amdgpu_bo_from_handle;
ws->base.buffer_from_ptr = amdgpu_bo_from_ptr;
+   ws->base.buffer_is_user_ptr = amdgpu_bo_is_user_ptr;
ws->base.buffer_get_handle = amdgpu_bo_get_handle;
ws->base.buffer_get_virtual_address = amdgpu_bo_get_va;
ws->base.buffer_get_initial_domain = amdgpu_bo_get_initial_domain;
diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c 
b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
index ee61e54..2196b7b 100644
---