Re: [Mesa-dev] [PATCH 09/13] r300g, r600g, radeonsi: add support for ARB_buffer_storage
Is there a way to do the HDP flush at the bottom of the pipe? Can this be fixed in the kernel? I'm not 100% sure, but I don't think so. As far as I know the only thing you can do on the bottom of the pipe is writing a fence/timestamp value or signaling a semaphore (the different EOP events). For now, it would be safer to use GTT for all persistent mappings. I'll update the patch. That sounds like a good idea anyway. Persistent mappings seems to make only sense with stuff that is accessed allot by the CPU and VRAM isn't a good choice for that. Christian. Am 02.02.2014 19:29, schrieb Marek Olšák: Is there a way to do the HDP flush at the bottom of the pipe? Can this be fixed in the kernel? For now, it would be safer to use GTT for all persistent mappings. I'll update the patch. Marek On Sat, Feb 1, 2014 at 11:58 AM, Christian König deathsim...@vodafone.de wrote: Am 31.01.2014 21:36, schrieb Alex Deucher: On Fri, Jan 31, 2014 at 7:05 AM, Marek Olšák mar...@gmail.com wrote: I think we always flush the HDP cache after (before?) command submission. The kernel flushes the HDP cache in the fence command sequence. But we do this at the top of the pipe instead of the bottom and then silently assume that between the top and the bottom the CPU isn't accessing this VRAM buffer. At least theoretically this can lead to a problem if the CPU wants to read a buffer value that's in the same cache line as a value the GPU writes. For example if a stupid application tries something like while(!value_written_by_GPU_in_VRAM); this won't work correctly. Christian. Alex This patch adds nothing new to the drivers - we have always had persistent buffer mappings for all buffers and it has always worked. The only thing this does is that persistent mappings are now also supported by Gallium and OpenGL. If there is a missing cache flush somewhere, the new ARB_buffer_storage piglit test will show it. Marek On Fri, Jan 31, 2014 at 2:04 AM, Michel Dänzer mic...@daenzer.net wrote: On Don, 2014-01-30 at 23:46 +0100, Fredrik Höglund wrote: On Thursday 30 January 2014, Marek Olšák wrote: From: Marek Olšák marek.ol...@amd.com All GTT memory mappings are coherent and therefore can be persistent. As we discussed on IRC, I think there should be a comment somewhere explaining that VRAM mappings are uncached, so the memory_barrier implementations don't need to do anything for those. VRAM is mapped uncacheable by the CPU, but there is an HDP cache which must be flushed to ensure coherency between the CPU and GPU. So I suspect memory_barrier actually needs to flush the HDP cache for VRAM. I'm wondering about GTT mappings on AGP as well. I think we're using CPU write-combining for those, so we probably need to flush the write-combining buffers? -- 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/13] r300g, r600g, radeonsi: add support for ARB_buffer_storage
Is there a way to do the HDP flush at the bottom of the pipe? Can this be fixed in the kernel? For now, it would be safer to use GTT for all persistent mappings. I'll update the patch. Marek On Sat, Feb 1, 2014 at 11:58 AM, Christian König deathsim...@vodafone.de wrote: Am 31.01.2014 21:36, schrieb Alex Deucher: On Fri, Jan 31, 2014 at 7:05 AM, Marek Olšák mar...@gmail.com wrote: I think we always flush the HDP cache after (before?) command submission. The kernel flushes the HDP cache in the fence command sequence. But we do this at the top of the pipe instead of the bottom and then silently assume that between the top and the bottom the CPU isn't accessing this VRAM buffer. At least theoretically this can lead to a problem if the CPU wants to read a buffer value that's in the same cache line as a value the GPU writes. For example if a stupid application tries something like while(!value_written_by_GPU_in_VRAM); this won't work correctly. Christian. Alex This patch adds nothing new to the drivers - we have always had persistent buffer mappings for all buffers and it has always worked. The only thing this does is that persistent mappings are now also supported by Gallium and OpenGL. If there is a missing cache flush somewhere, the new ARB_buffer_storage piglit test will show it. Marek On Fri, Jan 31, 2014 at 2:04 AM, Michel Dänzer mic...@daenzer.net wrote: On Don, 2014-01-30 at 23:46 +0100, Fredrik Höglund wrote: On Thursday 30 January 2014, Marek Olšák wrote: From: Marek Olšák marek.ol...@amd.com All GTT memory mappings are coherent and therefore can be persistent. As we discussed on IRC, I think there should be a comment somewhere explaining that VRAM mappings are uncached, so the memory_barrier implementations don't need to do anything for those. VRAM is mapped uncacheable by the CPU, but there is an HDP cache which must be flushed to ensure coherency between the CPU and GPU. So I suspect memory_barrier actually needs to flush the HDP cache for VRAM. I'm wondering about GTT mappings on AGP as well. I think we're using CPU write-combining for those, so we probably need to flush the write-combining buffers? -- 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/13] r300g, r600g, radeonsi: add support for ARB_buffer_storage
Am 31.01.2014 21:36, schrieb Alex Deucher: On Fri, Jan 31, 2014 at 7:05 AM, Marek Olšák mar...@gmail.com wrote: I think we always flush the HDP cache after (before?) command submission. The kernel flushes the HDP cache in the fence command sequence. But we do this at the top of the pipe instead of the bottom and then silently assume that between the top and the bottom the CPU isn't accessing this VRAM buffer. At least theoretically this can lead to a problem if the CPU wants to read a buffer value that's in the same cache line as a value the GPU writes. For example if a stupid application tries something like while(!value_written_by_GPU_in_VRAM); this won't work correctly. Christian. Alex This patch adds nothing new to the drivers - we have always had persistent buffer mappings for all buffers and it has always worked. The only thing this does is that persistent mappings are now also supported by Gallium and OpenGL. If there is a missing cache flush somewhere, the new ARB_buffer_storage piglit test will show it. Marek On Fri, Jan 31, 2014 at 2:04 AM, Michel Dänzer mic...@daenzer.net wrote: On Don, 2014-01-30 at 23:46 +0100, Fredrik Höglund wrote: On Thursday 30 January 2014, Marek Olšák wrote: From: Marek Olšák marek.ol...@amd.com All GTT memory mappings are coherent and therefore can be persistent. As we discussed on IRC, I think there should be a comment somewhere explaining that VRAM mappings are uncached, so the memory_barrier implementations don't need to do anything for those. VRAM is mapped uncacheable by the CPU, but there is an HDP cache which must be flushed to ensure coherency between the CPU and GPU. So I suspect memory_barrier actually needs to flush the HDP cache for VRAM. I'm wondering about GTT mappings on AGP as well. I think we're using CPU write-combining for those, so we probably need to flush the write-combining buffers? -- 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/13] r300g, r600g, radeonsi: add support for ARB_buffer_storage
I think we always flush the HDP cache after (before?) command submission. This patch adds nothing new to the drivers - we have always had persistent buffer mappings for all buffers and it has always worked. The only thing this does is that persistent mappings are now also supported by Gallium and OpenGL. If there is a missing cache flush somewhere, the new ARB_buffer_storage piglit test will show it. Marek On Fri, Jan 31, 2014 at 2:04 AM, Michel Dänzer mic...@daenzer.net wrote: On Don, 2014-01-30 at 23:46 +0100, Fredrik Höglund wrote: On Thursday 30 January 2014, Marek Olšák wrote: From: Marek Olšák marek.ol...@amd.com All GTT memory mappings are coherent and therefore can be persistent. As we discussed on IRC, I think there should be a comment somewhere explaining that VRAM mappings are uncached, so the memory_barrier implementations don't need to do anything for those. VRAM is mapped uncacheable by the CPU, but there is an HDP cache which must be flushed to ensure coherency between the CPU and GPU. So I suspect memory_barrier actually needs to flush the HDP cache for VRAM. I'm wondering about GTT mappings on AGP as well. I think we're using CPU write-combining for those, so we probably need to flush the write-combining buffers? -- 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 09/13] r300g, r600g, radeonsi: add support for ARB_buffer_storage
On Fri, Jan 31, 2014 at 7:05 AM, Marek Olšák mar...@gmail.com wrote: I think we always flush the HDP cache after (before?) command submission. The kernel flushes the HDP cache in the fence command sequence. Alex This patch adds nothing new to the drivers - we have always had persistent buffer mappings for all buffers and it has always worked. The only thing this does is that persistent mappings are now also supported by Gallium and OpenGL. If there is a missing cache flush somewhere, the new ARB_buffer_storage piglit test will show it. Marek On Fri, Jan 31, 2014 at 2:04 AM, Michel Dänzer mic...@daenzer.net wrote: On Don, 2014-01-30 at 23:46 +0100, Fredrik Höglund wrote: On Thursday 30 January 2014, Marek Olšák wrote: From: Marek Olšák marek.ol...@amd.com All GTT memory mappings are coherent and therefore can be persistent. As we discussed on IRC, I think there should be a comment somewhere explaining that VRAM mappings are uncached, so the memory_barrier implementations don't need to do anything for those. VRAM is mapped uncacheable by the CPU, but there is an HDP cache which must be flushed to ensure coherency between the CPU and GPU. So I suspect memory_barrier actually needs to flush the HDP cache for VRAM. I'm wondering about GTT mappings on AGP as well. I think we're using CPU write-combining for those, so we probably need to flush the write-combining buffers? -- 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/13] r300g, r600g, radeonsi: add support for ARB_buffer_storage
On Thursday 30 January 2014, Marek Olšák wrote:
From: Marek Olšák marek.ol...@amd.com
All GTT memory mappings are coherent and therefore can be persistent.
As we discussed on IRC, I think there should be a comment somewhere
explaining that VRAM mappings are uncached, so the memory_barrier
implementations don't need to do anything for those.
Aside from that, this patch is:
Reviewed-by: Fredrik Höglund fred...@kde.org
---
src/gallium/drivers/r300/r300_screen.c | 1 +
src/gallium/drivers/r300/r300_state.c | 5 +
src/gallium/drivers/r600/r600_pipe.c| 1 +
src/gallium/drivers/radeon/r600_buffer_common.c | 4
src/gallium/drivers/radeon/r600_pipe_common.c | 5 +
src/gallium/drivers/radeonsi/si_pipe.c | 1 +
6 files changed, 17 insertions(+)
diff --git a/src/gallium/drivers/r300/r300_screen.c
b/src/gallium/drivers/r300/r300_screen.c
index 4b790af..a3ca829 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -106,6 +106,7 @@ static int r300_get_param(struct pipe_screen* pscreen,
enum pipe_cap param)
case PIPE_CAP_USER_INDEX_BUFFERS:
case PIPE_CAP_USER_CONSTANT_BUFFERS:
case PIPE_CAP_PREFER_BLIT_BASED_TEXTURE_TRANSFER:
+case PIPE_CAP_BUFFER_TRANSFER_PERSISTENT_COHERENT:
return 1;
case PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT:
diff --git a/src/gallium/drivers/r300/r300_state.c
b/src/gallium/drivers/r300/r300_state.c
index 048672c..5472263 100644
--- a/src/gallium/drivers/r300/r300_state.c
+++ b/src/gallium/drivers/r300/r300_state.c
@@ -2129,6 +2129,10 @@ static void r300_texture_barrier(struct pipe_context
*pipe)
r300_mark_atom_dirty(r300, r300-texture_cache_inval);
}
+static void r300_memory_barrier(struct pipe_context *pipe, unsigned flags)
+{
+}
+
void r300_init_state_functions(struct r300_context* r300)
{
r300-context.create_blend_state = r300_create_blend_state;
@@ -2189,4 +2193,5 @@ void r300_init_state_functions(struct r300_context*
r300)
r300-context.delete_vs_state = r300_delete_vs_state;
r300-context.texture_barrier = r300_texture_barrier;
+r300-context.memory_barrier = r300_memory_barrier;
}
diff --git a/src/gallium/drivers/r600/r600_pipe.c
b/src/gallium/drivers/r600/r600_pipe.c
index 49521e0..3210b2c 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -354,6 +354,7 @@ static int r600_get_param(struct pipe_screen* pscreen,
enum pipe_cap param)
case PIPE_CAP_PREFER_BLIT_BASED_TEXTURE_TRANSFER:
case PIPE_CAP_QUERY_PIPELINE_STATISTICS:
case PIPE_CAP_TEXTURE_MULTISAMPLE:
+case PIPE_CAP_BUFFER_TRANSFER_PERSISTENT_COHERENT:
return 1;
case PIPE_CAP_TGSI_TEXCOORD:
diff --git a/src/gallium/drivers/radeon/r600_buffer_common.c
b/src/gallium/drivers/radeon/r600_buffer_common.c
index d29671e..3fff3cf 100644
--- a/src/gallium/drivers/radeon/r600_buffer_common.c
+++ b/src/gallium/drivers/radeon/r600_buffer_common.c
@@ -103,6 +103,10 @@ bool r600_init_resource(struct r600_common_screen
*rscreen,
{
uint32_t initial_domain, domains;
+ /* Use GTT for coherent mappings. */
+ if (res-b.b.flags PIPE_RESOURCE_FLAG_TRANSFER_COHERENT)
+ usage = PIPE_USAGE_STAGING;
+
switch(usage) {
case PIPE_USAGE_STAGING:
/* Staging resources participate in transfers, i.e. are used
diff --git a/src/gallium/drivers/radeon/r600_pipe_common.c
b/src/gallium/drivers/radeon/r600_pipe_common.c
index 98164f0..307bb47 100644
--- a/src/gallium/drivers/radeon/r600_pipe_common.c
+++ b/src/gallium/drivers/radeon/r600_pipe_common.c
@@ -39,6 +39,10 @@
* pipe_context
*/
+static void r600_memory_barrier(struct pipe_context *ctx, unsigned flags)
+{
+}
+
bool r600_common_context_init(struct r600_common_context *rctx,
struct r600_common_screen *rscreen)
{
@@ -56,6 +60,7 @@ bool r600_common_context_init(struct r600_common_context
*rctx,
rctx-b.transfer_flush_region = u_default_transfer_flush_region;
rctx-b.transfer_unmap = u_transfer_unmap_vtbl;
rctx-b.transfer_inline_write = u_default_transfer_inline_write;
+rctx-b.memory_barrier = r600_memory_barrier;
r600_streamout_init(rctx);
r600_query_init(rctx);
diff --git a/src/gallium/drivers/radeonsi/si_pipe.c
b/src/gallium/drivers/radeonsi/si_pipe.c
index 14dfd30..54a6107 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -252,6 +252,7 @@ static int si_get_param(struct pipe_screen* pscreen, enum
pipe_cap param)
case PIPE_CAP_TEXTURE_BUFFER_OBJECTS:
case PIPE_CAP_TGSI_VS_LAYER:
case PIPE_CAP_QUERY_PIPELINE_STATISTICS:
+ case PIPE_CAP_BUFFER_TRANSFER_PERSISTENT_COHERENT:
return 1;
case
Re: [Mesa-dev] [PATCH 09/13] r300g, r600g, radeonsi: add support for ARB_buffer_storage
On Don, 2014-01-30 at 23:46 +0100, Fredrik Höglund wrote: On Thursday 30 January 2014, Marek Olšák wrote: From: Marek Olšák marek.ol...@amd.com All GTT memory mappings are coherent and therefore can be persistent. As we discussed on IRC, I think there should be a comment somewhere explaining that VRAM mappings are uncached, so the memory_barrier implementations don't need to do anything for those. VRAM is mapped uncacheable by the CPU, but there is an HDP cache which must be flushed to ensure coherency between the CPU and GPU. So I suspect memory_barrier actually needs to flush the HDP cache for VRAM. I'm wondering about GTT mappings on AGP as well. I think we're using CPU write-combining for those, so we probably need to flush the write-combining buffers? -- 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
[Mesa-dev] [PATCH 09/13] r300g, r600g, radeonsi: add support for ARB_buffer_storage
From: Marek Olšák marek.ol...@amd.com
All GTT memory mappings are coherent and therefore can be persistent.
---
src/gallium/drivers/r300/r300_screen.c | 1 +
src/gallium/drivers/r300/r300_state.c | 5 +
src/gallium/drivers/r600/r600_pipe.c| 1 +
src/gallium/drivers/radeon/r600_buffer_common.c | 4
src/gallium/drivers/radeon/r600_pipe_common.c | 5 +
src/gallium/drivers/radeonsi/si_pipe.c | 1 +
6 files changed, 17 insertions(+)
diff --git a/src/gallium/drivers/r300/r300_screen.c
b/src/gallium/drivers/r300/r300_screen.c
index 4b790af..a3ca829 100644
--- a/src/gallium/drivers/r300/r300_screen.c
+++ b/src/gallium/drivers/r300/r300_screen.c
@@ -106,6 +106,7 @@ static int r300_get_param(struct pipe_screen* pscreen, enum
pipe_cap param)
case PIPE_CAP_USER_INDEX_BUFFERS:
case PIPE_CAP_USER_CONSTANT_BUFFERS:
case PIPE_CAP_PREFER_BLIT_BASED_TEXTURE_TRANSFER:
+case PIPE_CAP_BUFFER_TRANSFER_PERSISTENT_COHERENT:
return 1;
case PIPE_CAP_MIN_MAP_BUFFER_ALIGNMENT:
diff --git a/src/gallium/drivers/r300/r300_state.c
b/src/gallium/drivers/r300/r300_state.c
index 048672c..5472263 100644
--- a/src/gallium/drivers/r300/r300_state.c
+++ b/src/gallium/drivers/r300/r300_state.c
@@ -2129,6 +2129,10 @@ static void r300_texture_barrier(struct pipe_context
*pipe)
r300_mark_atom_dirty(r300, r300-texture_cache_inval);
}
+static void r300_memory_barrier(struct pipe_context *pipe, unsigned flags)
+{
+}
+
void r300_init_state_functions(struct r300_context* r300)
{
r300-context.create_blend_state = r300_create_blend_state;
@@ -2189,4 +2193,5 @@ void r300_init_state_functions(struct r300_context* r300)
r300-context.delete_vs_state = r300_delete_vs_state;
r300-context.texture_barrier = r300_texture_barrier;
+r300-context.memory_barrier = r300_memory_barrier;
}
diff --git a/src/gallium/drivers/r600/r600_pipe.c
b/src/gallium/drivers/r600/r600_pipe.c
index 49521e0..3210b2c 100644
--- a/src/gallium/drivers/r600/r600_pipe.c
+++ b/src/gallium/drivers/r600/r600_pipe.c
@@ -354,6 +354,7 @@ static int r600_get_param(struct pipe_screen* pscreen, enum
pipe_cap param)
case PIPE_CAP_PREFER_BLIT_BASED_TEXTURE_TRANSFER:
case PIPE_CAP_QUERY_PIPELINE_STATISTICS:
case PIPE_CAP_TEXTURE_MULTISAMPLE:
+case PIPE_CAP_BUFFER_TRANSFER_PERSISTENT_COHERENT:
return 1;
case PIPE_CAP_TGSI_TEXCOORD:
diff --git a/src/gallium/drivers/radeon/r600_buffer_common.c
b/src/gallium/drivers/radeon/r600_buffer_common.c
index d29671e..3fff3cf 100644
--- a/src/gallium/drivers/radeon/r600_buffer_common.c
+++ b/src/gallium/drivers/radeon/r600_buffer_common.c
@@ -103,6 +103,10 @@ bool r600_init_resource(struct r600_common_screen *rscreen,
{
uint32_t initial_domain, domains;
+ /* Use GTT for coherent mappings. */
+ if (res-b.b.flags PIPE_RESOURCE_FLAG_TRANSFER_COHERENT)
+ usage = PIPE_USAGE_STAGING;
+
switch(usage) {
case PIPE_USAGE_STAGING:
/* Staging resources participate in transfers, i.e. are used
diff --git a/src/gallium/drivers/radeon/r600_pipe_common.c
b/src/gallium/drivers/radeon/r600_pipe_common.c
index 98164f0..307bb47 100644
--- a/src/gallium/drivers/radeon/r600_pipe_common.c
+++ b/src/gallium/drivers/radeon/r600_pipe_common.c
@@ -39,6 +39,10 @@
* pipe_context
*/
+static void r600_memory_barrier(struct pipe_context *ctx, unsigned flags)
+{
+}
+
bool r600_common_context_init(struct r600_common_context *rctx,
struct r600_common_screen *rscreen)
{
@@ -56,6 +60,7 @@ bool r600_common_context_init(struct r600_common_context
*rctx,
rctx-b.transfer_flush_region = u_default_transfer_flush_region;
rctx-b.transfer_unmap = u_transfer_unmap_vtbl;
rctx-b.transfer_inline_write = u_default_transfer_inline_write;
+rctx-b.memory_barrier = r600_memory_barrier;
r600_streamout_init(rctx);
r600_query_init(rctx);
diff --git a/src/gallium/drivers/radeonsi/si_pipe.c
b/src/gallium/drivers/radeonsi/si_pipe.c
index 14dfd30..54a6107 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.c
+++ b/src/gallium/drivers/radeonsi/si_pipe.c
@@ -252,6 +252,7 @@ static int si_get_param(struct pipe_screen* pscreen, enum
pipe_cap param)
case PIPE_CAP_TEXTURE_BUFFER_OBJECTS:
case PIPE_CAP_TGSI_VS_LAYER:
case PIPE_CAP_QUERY_PIPELINE_STATISTICS:
+ case PIPE_CAP_BUFFER_TRANSFER_PERSISTENT_COHERENT:
return 1;
case PIPE_CAP_TEXTURE_MULTISAMPLE:
--
1.8.3.2
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
