Re: [Mesa-dev] [PATCH 1/2] r600g, radeonsi: query the buffer domain from the kernel for DRI2 buffers

2014-02-05 Thread Christian König 

Am 05.02.2014 00:01, schrieb Marek Olšák:

From: Marek Olšák marek.ol...@amd.com

Better then guessing it.

Yeah we have had this query for a long time...


Sounds reasonable to me.

Both patches are: Reviewed-by: Christian König christian.koe...@amd.com


---
  src/gallium/drivers/radeon/r600_texture.c |  2 +-
  src/gallium/winsys/radeon/drm/radeon_drm_bo.c | 23 +++
  src/gallium/winsys/radeon/drm/radeon_winsys.h |  5 +
  3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeon/r600_texture.c 
b/src/gallium/drivers/radeon/r600_texture.c
index 878b26f..aa4e8ea 100644
--- a/src/gallium/drivers/radeon/r600_texture.c
+++ b/src/gallium/drivers/radeon/r600_texture.c
@@ -627,7 +627,7 @@ r600_texture_create_object(struct pipe_screen *screen,
} else {
resource-buf = buf;
resource-cs_buf = rscreen-ws-buffer_get_cs_handle(buf);
-   resource-domains = RADEON_DOMAIN_GTT | RADEON_DOMAIN_VRAM;
+   resource-domains = rscreen-ws-buffer_get_current_domain(buf);
}
  
  	if (rtex-cmask.size) {

diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c 
b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
index 2ac060b..7c59f26 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
@@ -201,6 +201,28 @@ static boolean radeon_bo_is_busy(struct pb_buffer *_buf,
  }
  }
  
+static enum radeon_bo_domain radeon_bo_get_current_domain(struct pb_buffer *_buf)

+{
+struct radeon_bo *bo = get_radeon_bo(_buf);
+struct drm_radeon_gem_busy args;
+
+memset(args, 0, sizeof(args));
+args.handle = bo-handle;
+
+drmCommandWriteRead(bo-rws-fd, DRM_RADEON_GEM_BUSY,
+args, sizeof(args));
+
+/* Zero domains the driver doesn't understand. */
+args.domain = ~(RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT);
+
+/* If no domain is set, we must set something... */
+if (!args.domain)
+args.domain = RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT;
+
+/* GEM domains and winsys domains are defined the same. */
+return args.domain;
+}
+
  static uint64_t radeon_bomgr_find_va(struct radeon_bomgr *mgr, uint64_t size, 
uint64_t alignment)
  {
  struct radeon_bo_va_hole *hole, *n;
@@ -1089,4 +,5 @@ void radeon_bomgr_init_functions(struct radeon_drm_winsys 
*ws)
  ws-base.buffer_from_handle = radeon_winsys_bo_from_handle;
  ws-base.buffer_get_handle = radeon_winsys_bo_get_handle;
  ws-base.buffer_get_virtual_address = radeon_winsys_bo_va;
+ws-base.buffer_get_current_domain = radeon_bo_get_current_domain;
  }
diff --git a/src/gallium/winsys/radeon/drm/radeon_winsys.h 
b/src/gallium/winsys/radeon/drm/radeon_winsys.h
index 55f60d3..fb942c0 100644
--- a/src/gallium/winsys/radeon/drm/radeon_winsys.h
+++ b/src/gallium/winsys/radeon/drm/radeon_winsys.h
@@ -376,6 +376,11 @@ struct radeon_winsys {
   */
  uint64_t (*buffer_get_virtual_address)(struct radeon_winsys_cs_handle 
*buf);
  
+/**

+ * Query the current placement of the buffer from the memory manager.
+ */
+enum radeon_bo_domain (*buffer_get_current_domain)(struct pb_buffer *buf);
+
  
/**
   * Command submission.
   *


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

Re: [Mesa-dev] [PATCH 1/2] r600g, radeonsi: query the buffer domain from the kernel for DRI2 buffers

2014-02-05 Thread Christian König 

Am 05.02.2014 08:37, schrieb Michel Dänzer:

On Mit, 2014-02-05 at 00:01 +0100, Marek Olšák wrote:

From: Marek Olšák marek.ol...@amd.com

Better then guessing it.

Yeah we have had this query for a long time...

[...]


diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c 
b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
index 2ac060b..7c59f26 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
@@ -201,6 +201,28 @@ static boolean radeon_bo_is_busy(struct pb_buffer *_buf,
  }
  }
  
+static enum radeon_bo_domain radeon_bo_get_current_domain(struct pb_buffer *_buf)

+{
+struct radeon_bo *bo = get_radeon_bo(_buf);
+struct drm_radeon_gem_busy args;
+
+memset(args, 0, sizeof(args));
+args.handle = bo-handle;
+
+drmCommandWriteRead(bo-rws-fd, DRM_RADEON_GEM_BUSY,
+args, sizeof(args));
+
+/* Zero domains the driver doesn't understand. */
+args.domain = ~(RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT);
+
+/* If no domain is set, we must set something... */
+if (!args.domain)
+args.domain = RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT;
+
+/* GEM domains and winsys domains are defined the same. */
+return args.domain;
+}

The problem with this is that DRM_RADEON_GEM_BUSY doesn't say where the
BO is supposed to be, but where it happens to be right now. E.g. it
could return GTT for a BO that's supposed to be in VRAM but was evicted
(or couldn't get into VRAM in the first place).


But isn't that exactly what we want here?

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

Re: [Mesa-dev] [PATCH 1/2] r600g, radeonsi: query the buffer domain from the kernel for DRI2 buffers

2014-02-05 Thread Marek Olšák 
I know, but do you have a better idea? If the buffer isn't evicted, we
should get the correct placement. The code also respects placement of
Prime buffers, which should be in GTT. This is as close to the ideal
solution as it can possibly be. I don't think it can be any better
with our kernel interface.

Marek

On Wed, Feb 5, 2014 at 8:37 AM, Michel Dänzer mic...@daenzer.net wrote:
 On Mit, 2014-02-05 at 00:01 +0100, Marek Olšák wrote:
 From: Marek Olšák marek.ol...@amd.com

 Better then guessing it.

 Yeah we have had this query for a long time...

 [...]

 diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c 
 b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
 index 2ac060b..7c59f26 100644
 --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
 +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
 @@ -201,6 +201,28 @@ static boolean radeon_bo_is_busy(struct pb_buffer *_buf,
  }
  }

 +static enum radeon_bo_domain radeon_bo_get_current_domain(struct pb_buffer 
 *_buf)
 +{
 +struct radeon_bo *bo = get_radeon_bo(_buf);
 +struct drm_radeon_gem_busy args;
 +
 +memset(args, 0, sizeof(args));
 +args.handle = bo-handle;
 +
 +drmCommandWriteRead(bo-rws-fd, DRM_RADEON_GEM_BUSY,
 +args, sizeof(args));
 +
 +/* Zero domains the driver doesn't understand. */
 +args.domain = ~(RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT);
 +
 +/* If no domain is set, we must set something... */
 +if (!args.domain)
 +args.domain = RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT;
 +
 +/* GEM domains and winsys domains are defined the same. */
 +return args.domain;
 +}

 The problem with this is that DRM_RADEON_GEM_BUSY doesn't say where the
 BO is supposed to be, but where it happens to be right now. E.g. it
 could return GTT for a BO that's supposed to be in VRAM but was evicted
 (or couldn't get into VRAM in the first place).


 --
 Earthling Michel Dänzer|  http://www.amd.com
 Libre software enthusiast  |Mesa and X developer

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

Re: [Mesa-dev] [PATCH 1/2] r600g, radeonsi: query the buffer domain from the kernel for DRI2 buffers

2014-02-05 Thread Michel Dänzer 
On Mit, 2014-02-05 at 10:13 +0100, Christian König wrote:
 Am 05.02.2014 08:37, schrieb Michel Dänzer:
  On Mit, 2014-02-05 at 00:01 +0100, Marek Olšák wrote:
 
  diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c 
  b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
  index 2ac060b..7c59f26 100644
  --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
  +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
  @@ -201,6 +201,28 @@ static boolean radeon_bo_is_busy(struct pb_buffer 
  *_buf,
}
}

  +static enum radeon_bo_domain radeon_bo_get_current_domain(struct 
  pb_buffer *_buf)
  +{
  +struct radeon_bo *bo = get_radeon_bo(_buf);
  +struct drm_radeon_gem_busy args;
  +
  +memset(args, 0, sizeof(args));
  +args.handle = bo-handle;
  +
  +drmCommandWriteRead(bo-rws-fd, DRM_RADEON_GEM_BUSY,
  +args, sizeof(args));
  +
  +/* Zero domains the driver doesn't understand. */
  +args.domain = ~(RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT);
  +
  +/* If no domain is set, we must set something... */
  +if (!args.domain)
  +args.domain = RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT;
  +
  +/* GEM domains and winsys domains are defined the same. */
  +return args.domain;
  +}
  The problem with this is that DRM_RADEON_GEM_BUSY doesn't say where the
  BO is supposed to be, but where it happens to be right now. E.g. it
  could return GTT for a BO that's supposed to be in VRAM but was evicted
  (or couldn't get into VRAM in the first place).
 
 But isn't that exactly what we want here?

Not really. E.g. consider an app's DRI2 front buffer (i.e. normally the
backing pixmap of an X11 window): If the BO happens to be in GTT when
DRM_RADEON_GEM_BUSY is called from the compositor, the compositor will
force the BO to GTT every time it uses the BO for rendering. But the
app's DRI2 buffer swap (or any other accelerated rendering to the window
in the X server) will force the BO to VRAM. BO ping pong.


-- 
Earthling Michel Dänzer|  http://www.amd.com
Libre software enthusiast  |Mesa and X developer

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

Re: [Mesa-dev] [PATCH 1/2] r600g, radeonsi: query the buffer domain from the kernel for DRI2 buffers

2014-02-05 Thread Christian König 

Am 05.02.2014 10:57, schrieb Michel Dänzer:

On Mit, 2014-02-05 at 10:13 +0100, Christian König wrote:

Am 05.02.2014 08:37, schrieb Michel Dänzer:

On Mit, 2014-02-05 at 00:01 +0100, Marek Olšák wrote:

diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c 
b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
index 2ac060b..7c59f26 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
@@ -201,6 +201,28 @@ static boolean radeon_bo_is_busy(struct pb_buffer *_buf,
   }
   }
   
+static enum radeon_bo_domain radeon_bo_get_current_domain(struct pb_buffer *_buf)

+{
+struct radeon_bo *bo = get_radeon_bo(_buf);
+struct drm_radeon_gem_busy args;
+
+memset(args, 0, sizeof(args));
+args.handle = bo-handle;
+
+drmCommandWriteRead(bo-rws-fd, DRM_RADEON_GEM_BUSY,
+args, sizeof(args));
+
+/* Zero domains the driver doesn't understand. */
+args.domain = ~(RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT);
+
+/* If no domain is set, we must set something... */
+if (!args.domain)
+args.domain = RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT;
+
+/* GEM domains and winsys domains are defined the same. */
+return args.domain;
+}

The problem with this is that DRM_RADEON_GEM_BUSY doesn't say where the
BO is supposed to be, but where it happens to be right now. E.g. it
could return GTT for a BO that's supposed to be in VRAM but was evicted
(or couldn't get into VRAM in the first place).

But isn't that exactly what we want here?

Not really. E.g. consider an app's DRI2 front buffer (i.e. normally the
backing pixmap of an X11 window): If the BO happens to be in GTT when
DRM_RADEON_GEM_BUSY is called from the compositor, the compositor will
force the BO to GTT every time it uses the BO for rendering. But the
app's DRI2 buffer swap (or any other accelerated rendering to the window
in the X server) will force the BO to VRAM. BO ping pong.


Oh, yeah that makes sense. Instead of defining where the buffer should 
be on command submission we should rather define how badly we want to 
move it around. No good idea either how to handle this without a kernel 
interface rework.


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

Re: [Mesa-dev] [PATCH 1/2] r600g, radeonsi: query the buffer domain from the kernel for DRI2 buffers

2014-02-04 Thread Michel Dänzer 
On Mit, 2014-02-05 at 00:01 +0100, Marek Olšák wrote:
 From: Marek Olšák marek.ol...@amd.com
 
 Better then guessing it.
 
 Yeah we have had this query for a long time...

[...]

 diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c 
 b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
 index 2ac060b..7c59f26 100644
 --- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
 +++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
 @@ -201,6 +201,28 @@ static boolean radeon_bo_is_busy(struct pb_buffer *_buf,
  }
  }
  
 +static enum radeon_bo_domain radeon_bo_get_current_domain(struct pb_buffer 
 *_buf)
 +{
 +struct radeon_bo *bo = get_radeon_bo(_buf);
 +struct drm_radeon_gem_busy args;
 +
 +memset(args, 0, sizeof(args));
 +args.handle = bo-handle;
 +
 +drmCommandWriteRead(bo-rws-fd, DRM_RADEON_GEM_BUSY,
 +args, sizeof(args));
 +
 +/* Zero domains the driver doesn't understand. */
 +args.domain = ~(RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT);
 +
 +/* If no domain is set, we must set something... */
 +if (!args.domain)
 +args.domain = RADEON_GEM_DOMAIN_VRAM | RADEON_GEM_DOMAIN_GTT;
 +
 +/* GEM domains and winsys domains are defined the same. */
 +return args.domain;
 +}

The problem with this is that DRM_RADEON_GEM_BUSY doesn't say where the
BO is supposed to be, but where it happens to be right now. E.g. it
could return GTT for a BO that's supposed to be in VRAM but was evicted
(or couldn't get into VRAM in the first place).


-- 
Earthling Michel Dänzer|  http://www.amd.com
Libre software enthusiast  |Mesa and X developer

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