Re: [Mesa-dev] [PATCH 4/4] radeonsi: use SDMA for uploading data through const_uploader
For the _rev2_ version (Patchwork Mesa) series is
Tested-by: Dieter Nützel
on Polaris 20
UH+UV working flawlessly, now.
No 'measurable' speed decrease. - GREAT!
Blender, FreeCAD, glmark2 all fine.
But I had to have rebased part 4 (see attachment).
Dieter
Am 07.02.2019 02:22, schrieb Marek Olšák:
From: Marek Olšák
---
src/gallium/drivers/radeonsi/si_buffer.c | 56 ++--
src/gallium/drivers/radeonsi/si_dma_cs.c | 19
src/gallium/drivers/radeonsi/si_gfx_cs.c | 42 +++---
src/gallium/drivers/radeonsi/si_pipe.c | 23 ++
src/gallium/drivers/radeonsi/si_pipe.h | 17 +++
5 files changed, 131 insertions(+), 26 deletions(-)
diff --git a/src/gallium/drivers/radeonsi/si_buffer.c
b/src/gallium/drivers/radeonsi/si_buffer.c
index c01118ce96a..3f8db7cf4f0 100644
--- a/src/gallium/drivers/radeonsi/si_buffer.c
+++ b/src/gallium/drivers/radeonsi/si_buffer.c
@@ -433,21 +433,29 @@ static void *si_buffer_transfer_map(struct
pipe_context *ctx,
if (si_invalidate_buffer(sctx, buf)) {
/* At this point, the buffer is always idle. */
usage |= PIPE_TRANSFER_UNSYNCHRONIZED;
} else {
/* Fall back to a temporary buffer. */
usage |= PIPE_TRANSFER_DISCARD_RANGE;
}
}
- if ((usage & PIPE_TRANSFER_DISCARD_RANGE) &&
+ if (usage & PIPE_TRANSFER_FLUSH_EXPLICIT &&
+ buf->b.b.flags & SI_RESOURCE_FLAG_UPLOAD_FLUSH_EXPLICIT_VIA_SDMA)
{
+ usage &= ~(PIPE_TRANSFER_UNSYNCHRONIZED |
+ PIPE_TRANSFER_PERSISTENT);
+ usage |= PIPE_TRANSFER_DISCARD_RANGE;
+ force_discard_range = true;
+ }
+
+ if (usage & PIPE_TRANSFER_DISCARD_RANGE &&
((!(usage & (PIPE_TRANSFER_UNSYNCHRONIZED |
PIPE_TRANSFER_PERSISTENT))) ||
(buf->flags & RADEON_FLAG_SPARSE))) {
assert(usage & PIPE_TRANSFER_WRITE);
/* Check if mapping this buffer would cause waiting for the GPU.
*/
if (buf->flags & RADEON_FLAG_SPARSE ||
force_discard_range ||
si_rings_is_buffer_referenced(sctx, buf->buf,
RADEON_USAGE_READWRITE) ||
@@ -514,32 +522,72 @@ static void *si_buffer_transfer_map(struct
pipe_context *ctx,
data += box->x;
return si_buffer_get_transfer(ctx, resource, usage, box,
ptransfer, data, NULL, 0);
}
static void si_buffer_do_flush_region(struct pipe_context *ctx,
struct pipe_transfer *transfer,
const struct pipe_box *box)
{
+ struct si_context *sctx = (struct si_context*)ctx;
struct si_transfer *stransfer = (struct si_transfer*)transfer;
struct si_resource *buf = si_resource(transfer->resource);
if (stransfer->staging) {
unsigned src_offset = stransfer->offset +
transfer->box.x % SI_MAP_BUFFER_ALIGNMENT
+
(box->x - transfer->box.x);
+ if (buf->b.b.flags &
SI_RESOURCE_FLAG_UPLOAD_FLUSH_EXPLICIT_VIA_SDMA) {
+ /* This should be true for all uploaders. */
+ assert(transfer->box.x == 0);
+
+ /* Find a previous upload and extend its range. The last
+* upload is likely to be at the end of the list.
+*/
+ for (int i = sctx->num_sdma_uploads - 1; i >= 0; i--) {
+ struct si_sdma_upload *up =
&sctx->sdma_uploads[i];
+
+ if (up->dst != buf)
+ continue;
+
+ assert(up->src == stransfer->staging);
+ assert(box->x > up->dst_offset);
+ up->size = box->x + box->width - up->dst_offset;
+ return;
+ }
+
+ /* Enlarge the array if it's full. */
+ if (sctx->num_sdma_uploads == sctx->max_sdma_uploads) {
+ unsigned size;
+
+ sctx->max_sdma_uploads += 4;
+ size = sctx->max_sdma_uploads *
sizeof(sctx->sdma_uploads[0]);
+ sctx->sdma_uploads =
realloc(sctx->sdma_uploads, size);
+ }
+
+ /* Add a new upload. */
+ struct si_sdma_upload *up =
+ &sctx->sdma_uploads[sctx->num_sdma_uploads++];
+ up->dst = up->src = NULL;
+ si_resource_reference(&up->dst, buf);
+ si_resource_reference(&up->src, stransfer->staging);
+
Re: [Mesa-dev] [PATCH 4/4] radeonsi: use SDMA for uploading data through const_uploader
I'll remove the env var.
Additionally, I'm amending this:
diff --git a/src/gallium/drivers/radeonsi/si_buffer.c
b/src/gallium/drivers/radeonsi/si_buffer.c
index 3f8db7cf4f0..4936eb5a5b1 100644
--- a/src/gallium/drivers/radeonsi/si_buffer.c
+++ b/src/gallium/drivers/radeonsi/si_buffer.c
@@ -461,10 +461,20 @@ static void *si_buffer_transfer_map(struct
pipe_context *ctx,
si_rings_is_buffer_referenced(sctx, buf->buf,
RADEON_USAGE_READWRITE) ||
!sctx->ws->buffer_wait(buf->buf, 0,
RADEON_USAGE_READWRITE)) {
/* Do a wait-free write-only transfer using a
temporary buffer. */
- unsigned offset;
+ struct u_upload_mgr *uploader;
struct si_resource *staging = NULL;
+ unsigned offset;
+
+ /* If we are not called from the driver thread, we
have
+* to use the uploader from u_threaded_context,
which is
+* local to the calling thread.
+*/
+ if (usage & TC_TRANSFER_MAP_THREADED_UNSYNC)
+ uploader = sctx->tc->base.stream_uploader;
+ else
+ uploader = sctx->b.stream_uploader;
- u_upload_alloc(ctx->stream_uploader, 0,
+ u_upload_alloc(uploader, 0,
box->width + (box->x %
SI_MAP_BUFFER_ALIGNMENT),
sctx->screen->info.tcc_cache_line_size,
&offset, (struct
pipe_resource**)&staging,
Marek
On Mon, Feb 11, 2019 at 4:38 AM Nicolai Hähnle wrote:
> On 07.02.19 02:22, Marek Olšák wrote:
> > + bool use_sdma_upload = sscreen->info.has_dedicated_vram &&
> sctx->dma_cs && debug_get_bool_option("SDMA", true);
>
> Could you please namespace the environment variable, e.g. RADEONSI_SDMA?
>
> Apart from that, series is
>
> Reviewed-by: Nicolai Hähnle
>
>
> > + sctx->b.const_uploader = u_upload_create(&sctx->b, 256 * 1024,
> > + 0, PIPE_USAGE_DEFAULT,
> > + SI_RESOURCE_FLAG_32BIT |
> > + (use_sdma_upload ?
> > +
> SI_RESOURCE_FLAG_UPLOAD_FLUSH_EXPLICIT_VIA_SDMA :
> > +
> (sscreen->cpdma_prefetch_writes_memory ?
> > +0 :
> SI_RESOURCE_FLAG_READ_ONLY)));
> > + if (!sctx->b.const_uploader)
> > + goto fail;
> > +
> > + if (use_sdma_upload)
> > + u_upload_enable_flush_explicit(sctx->b.const_uploader);
> > +
> > si_init_buffer_functions(sctx);
> > si_init_clear_functions(sctx);
> > si_init_blit_functions(sctx);
> > si_init_compute_functions(sctx);
> > si_init_compute_blit_functions(sctx);
> > si_init_debug_functions(sctx);
> > si_init_msaa_functions(sctx);
> > si_init_streamout_functions(sctx);
> >
> > if (sscreen->info.has_hw_decode) {
> > diff --git a/src/gallium/drivers/radeonsi/si_pipe.h
> b/src/gallium/drivers/radeonsi/si_pipe.h
> > index b01d5744752..b208bdeb848 100644
> > --- a/src/gallium/drivers/radeonsi/si_pipe.h
> > +++ b/src/gallium/drivers/radeonsi/si_pipe.h
> > @@ -103,20 +103,22 @@
> > #define SI_MAX_VARIABLE_THREADS_PER_BLOCK 1024
> >
> > #define SI_RESOURCE_FLAG_TRANSFER (PIPE_RESOURCE_FLAG_DRV_PRIV << 0)
> > #define SI_RESOURCE_FLAG_FLUSHED_DEPTH
> (PIPE_RESOURCE_FLAG_DRV_PRIV << 1)
> > #define SI_RESOURCE_FLAG_FORCE_MSAA_TILING
> (PIPE_RESOURCE_FLAG_DRV_PRIV << 2)
> > #define SI_RESOURCE_FLAG_DISABLE_DCC
> (PIPE_RESOURCE_FLAG_DRV_PRIV << 3)
> > #define SI_RESOURCE_FLAG_UNMAPPABLE (PIPE_RESOURCE_FLAG_DRV_PRIV << 4)
> > #define SI_RESOURCE_FLAG_READ_ONLY (PIPE_RESOURCE_FLAG_DRV_PRIV << 5)
> > #define SI_RESOURCE_FLAG_32BIT
> (PIPE_RESOURCE_FLAG_DRV_PRIV << 6)
> > #define SI_RESOURCE_FLAG_CLEAR
> (PIPE_RESOURCE_FLAG_DRV_PRIV << 7)
> > +/* For const_uploader, upload data via GTT and copy to VRAM on context
> flush via SDMA. */
> > +#define SI_RESOURCE_FLAG_UPLOAD_FLUSH_EXPLICIT_VIA_SDMA
> (PIPE_RESOURCE_FLAG_DRV_PRIV << 8)
> >
> > enum si_clear_code
> > {
> > DCC_CLEAR_COLOR_ = 0x,
> > DCC_CLEAR_COLOR_0001 = 0x40404040,
> > DCC_CLEAR_COLOR_1110 = 0x80808080,
> > DCC_CLEAR_COLOR_ = 0xC0C0C0C0,
> > DCC_CLEAR_COLOR_REG= 0x20202020,
> > DCC_UNCOMPRESSED = 0x,
> > };
> > @@ -769,20 +771,28 @@ struct si_saved_cs {
> > struct si_context *ctx;
> > struct radeon_saved_cs gfx;
> > struct si_resource *trace_buf;
> > unsignedtrace_id;
> >
> > unsignedgfx_last_dw;
> > boolflushed;
> > int64_t time_flush;
> > };
> >
> > +struct si_sdma_upload {
> > + struct si_
Re: [Mesa-dev] [PATCH 4/4] radeonsi: use SDMA for uploading data through const_uploader
On 07.02.19 02:22, Marek Olšák wrote:
+ bool use_sdma_upload = sscreen->info.has_dedicated_vram && sctx->dma_cs &&
debug_get_bool_option("SDMA", true);
Could you please namespace the environment variable, e.g. RADEONSI_SDMA?
Apart from that, series is
Reviewed-by: Nicolai Hähnle
+ sctx->b.const_uploader = u_upload_create(&sctx->b, 256 * 1024,
+0, PIPE_USAGE_DEFAULT,
+SI_RESOURCE_FLAG_32BIT |
+(use_sdma_upload ?
+
SI_RESOURCE_FLAG_UPLOAD_FLUSH_EXPLICIT_VIA_SDMA :
+
(sscreen->cpdma_prefetch_writes_memory ?
+ 0 :
SI_RESOURCE_FLAG_READ_ONLY)));
+ if (!sctx->b.const_uploader)
+ goto fail;
+
+ if (use_sdma_upload)
+ u_upload_enable_flush_explicit(sctx->b.const_uploader);
+
si_init_buffer_functions(sctx);
si_init_clear_functions(sctx);
si_init_blit_functions(sctx);
si_init_compute_functions(sctx);
si_init_compute_blit_functions(sctx);
si_init_debug_functions(sctx);
si_init_msaa_functions(sctx);
si_init_streamout_functions(sctx);
if (sscreen->info.has_hw_decode) {
diff --git a/src/gallium/drivers/radeonsi/si_pipe.h
b/src/gallium/drivers/radeonsi/si_pipe.h
index b01d5744752..b208bdeb848 100644
--- a/src/gallium/drivers/radeonsi/si_pipe.h
+++ b/src/gallium/drivers/radeonsi/si_pipe.h
@@ -103,20 +103,22 @@
#define SI_MAX_VARIABLE_THREADS_PER_BLOCK 1024
#define SI_RESOURCE_FLAG_TRANSFER (PIPE_RESOURCE_FLAG_DRV_PRIV << 0)
#define SI_RESOURCE_FLAG_FLUSHED_DEPTH(PIPE_RESOURCE_FLAG_DRV_PRIV <<
1)
#define SI_RESOURCE_FLAG_FORCE_MSAA_TILING (PIPE_RESOURCE_FLAG_DRV_PRIV << 2)
#define SI_RESOURCE_FLAG_DISABLE_DCC (PIPE_RESOURCE_FLAG_DRV_PRIV << 3)
#define SI_RESOURCE_FLAG_UNMAPPABLE (PIPE_RESOURCE_FLAG_DRV_PRIV << 4)
#define SI_RESOURCE_FLAG_READ_ONLY(PIPE_RESOURCE_FLAG_DRV_PRIV << 5)
#define SI_RESOURCE_FLAG_32BIT(PIPE_RESOURCE_FLAG_DRV_PRIV <<
6)
#define SI_RESOURCE_FLAG_CLEAR(PIPE_RESOURCE_FLAG_DRV_PRIV <<
7)
+/* For const_uploader, upload data via GTT and copy to VRAM on context flush
via SDMA. */
+#define SI_RESOURCE_FLAG_UPLOAD_FLUSH_EXPLICIT_VIA_SDMA
(PIPE_RESOURCE_FLAG_DRV_PRIV << 8)
enum si_clear_code
{
DCC_CLEAR_COLOR_ = 0x,
DCC_CLEAR_COLOR_0001 = 0x40404040,
DCC_CLEAR_COLOR_1110 = 0x80808080,
DCC_CLEAR_COLOR_ = 0xC0C0C0C0,
DCC_CLEAR_COLOR_REG= 0x20202020,
DCC_UNCOMPRESSED = 0x,
};
@@ -769,20 +771,28 @@ struct si_saved_cs {
struct si_context *ctx;
struct radeon_saved_cs gfx;
struct si_resource *trace_buf;
unsignedtrace_id;
unsigned gfx_last_dw;
boolflushed;
int64_t time_flush;
};
+struct si_sdma_upload {
+ struct si_resource *dst;
+ struct si_resource *src;
+ unsignedsrc_offset;
+ unsigneddst_offset;
+ unsignedsize;
+};
+
struct si_context {
struct pipe_context b; /* base class */
enum radeon_family family;
enum chip_class chip_class;
struct radeon_winsys *ws;
struct radeon_winsys_ctx*ctx;
struct radeon_cmdbuf*gfx_cs;
struct radeon_cmdbuf*dma_cs;
@@ -1074,20 +1084,26 @@ struct si_context {
int num_perfect_occlusion_queries;
struct list_headactive_queries;
unsignednum_cs_dw_queries_suspend;
/* Render condition. */
struct pipe_query *render_cond;
unsignedrender_cond_mode;
boolrender_cond_invert;
boolrender_cond_force_off; /* for u_blitter
*/
+ /* For uploading data via GTT and copy to VRAM on context flush via SDMA. */
+ boolsdma_uploads_in_progress;
+ struct si_sdma_upload *sdma_uploads;
+ unsignednum_sdma_uploads;
+ unsignedmax_sdma_uploads;
+
/* Statistics gathering for the DCC enablement heuristic. It can't be
* in si_texture because si_texture can be shared by multiple
* contexts. This is for back buffers only. We shouldn't get too many
* of those.
*
* X11 DRI3 rotates among a finite set of back buffers. They should
* all fit in this array. If they don't, separate DCC migh
Re: [Mesa-dev] [PATCH 4/4] radeonsi: use SDMA for uploading data through const_uploader
On Thu, Feb 7, 2019 at 2:47 PM Axel Davy wrote:
>
> On 07/02/2019 02:22, Marek Olšák wrote:
> >
> > + bool use_sdma_upload = sscreen->info.has_dedicated_vram &&
> > sctx->dma_cs && debug_get_bool_option("SDMA", true);
> > + sctx->b.const_uploader = u_upload_create(&sctx->b, 256 * 1024,
> > + 0, PIPE_USAGE_DEFAULT,
> > + SI_RESOURCE_FLAG_32BIT |
> > + (use_sdma_upload ?
> > +
> > SI_RESOURCE_FLAG_UPLOAD_FLUSH_EXPLICIT_VIA_SDMA :
> > +
> > (sscreen->cpdma_prefetch_writes_memory ?
> > +0 :
> > SI_RESOURCE_FLAG_READ_ONLY)));
> > + if (!sctx->b.const_uploader)
> > + goto fail;
> > +
> > + if (use_sdma_upload)
> > + u_upload_enable_flush_explicit(sctx->b.const_uploader);
> > +
>
>
> I see that APU are not affected by the change.
>
> Are they affected by the issue this patch aims to fix though ? If so,
> wouldn't it make sense to switch to PIPE_USAGE_STREAM for APUs ?
They are not affected. "VRAM" on APUs is just carved out system
memory so the driver can access it directly. No need to go through
the PCI BAR interface. All of "VRAM" is visible to the CPU on APUs.
Alex
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/4] radeonsi: use SDMA for uploading data through const_uploader
On 07/02/2019 02:22, Marek Olšák wrote:
+ bool use_sdma_upload = sscreen->info.has_dedicated_vram && sctx->dma_cs && debug_get_bool_option("SDMA", true);
+ sctx->b.const_uploader = u_upload_create(&sctx->b, 256 * 1024,
+0, PIPE_USAGE_DEFAULT,
+SI_RESOURCE_FLAG_32BIT |
+(use_sdma_upload ?
+
SI_RESOURCE_FLAG_UPLOAD_FLUSH_EXPLICIT_VIA_SDMA :
+
(sscreen->cpdma_prefetch_writes_memory ?
+ 0 :
SI_RESOURCE_FLAG_READ_ONLY)));
+ if (!sctx->b.const_uploader)
+ goto fail;
+
+ if (use_sdma_upload)
+ u_upload_enable_flush_explicit(sctx->b.const_uploader);
+
I see that APU are not affected by the change.
Are they affected by the issue this patch aims to fix though ? If so,
wouldn't it make sense to switch to PIPE_USAGE_STREAM for APUs ?
Axel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/4] radeonsi: use SDMA for uploading data through const_uploader
From: Marek Olšák
---
src/gallium/drivers/radeonsi/si_buffer.c | 56 ++--
src/gallium/drivers/radeonsi/si_dma_cs.c | 19
src/gallium/drivers/radeonsi/si_gfx_cs.c | 42 +++---
src/gallium/drivers/radeonsi/si_pipe.c | 23 ++
src/gallium/drivers/radeonsi/si_pipe.h | 17 +++
5 files changed, 131 insertions(+), 26 deletions(-)
diff --git a/src/gallium/drivers/radeonsi/si_buffer.c
b/src/gallium/drivers/radeonsi/si_buffer.c
index c01118ce96a..3f8db7cf4f0 100644
--- a/src/gallium/drivers/radeonsi/si_buffer.c
+++ b/src/gallium/drivers/radeonsi/si_buffer.c
@@ -433,21 +433,29 @@ static void *si_buffer_transfer_map(struct pipe_context
*ctx,
if (si_invalidate_buffer(sctx, buf)) {
/* At this point, the buffer is always idle. */
usage |= PIPE_TRANSFER_UNSYNCHRONIZED;
} else {
/* Fall back to a temporary buffer. */
usage |= PIPE_TRANSFER_DISCARD_RANGE;
}
}
- if ((usage & PIPE_TRANSFER_DISCARD_RANGE) &&
+ if (usage & PIPE_TRANSFER_FLUSH_EXPLICIT &&
+ buf->b.b.flags & SI_RESOURCE_FLAG_UPLOAD_FLUSH_EXPLICIT_VIA_SDMA) {
+ usage &= ~(PIPE_TRANSFER_UNSYNCHRONIZED |
+ PIPE_TRANSFER_PERSISTENT);
+ usage |= PIPE_TRANSFER_DISCARD_RANGE;
+ force_discard_range = true;
+ }
+
+ if (usage & PIPE_TRANSFER_DISCARD_RANGE &&
((!(usage & (PIPE_TRANSFER_UNSYNCHRONIZED |
PIPE_TRANSFER_PERSISTENT))) ||
(buf->flags & RADEON_FLAG_SPARSE))) {
assert(usage & PIPE_TRANSFER_WRITE);
/* Check if mapping this buffer would cause waiting for the GPU.
*/
if (buf->flags & RADEON_FLAG_SPARSE ||
force_discard_range ||
si_rings_is_buffer_referenced(sctx, buf->buf,
RADEON_USAGE_READWRITE) ||
@@ -514,32 +522,72 @@ static void *si_buffer_transfer_map(struct pipe_context
*ctx,
data += box->x;
return si_buffer_get_transfer(ctx, resource, usage, box,
ptransfer, data, NULL, 0);
}
static void si_buffer_do_flush_region(struct pipe_context *ctx,
struct pipe_transfer *transfer,
const struct pipe_box *box)
{
+ struct si_context *sctx = (struct si_context*)ctx;
struct si_transfer *stransfer = (struct si_transfer*)transfer;
struct si_resource *buf = si_resource(transfer->resource);
if (stransfer->staging) {
unsigned src_offset = stransfer->offset +
transfer->box.x % SI_MAP_BUFFER_ALIGNMENT
+
(box->x - transfer->box.x);
+ if (buf->b.b.flags &
SI_RESOURCE_FLAG_UPLOAD_FLUSH_EXPLICIT_VIA_SDMA) {
+ /* This should be true for all uploaders. */
+ assert(transfer->box.x == 0);
+
+ /* Find a previous upload and extend its range. The last
+* upload is likely to be at the end of the list.
+*/
+ for (int i = sctx->num_sdma_uploads - 1; i >= 0; i--) {
+ struct si_sdma_upload *up =
&sctx->sdma_uploads[i];
+
+ if (up->dst != buf)
+ continue;
+
+ assert(up->src == stransfer->staging);
+ assert(box->x > up->dst_offset);
+ up->size = box->x + box->width - up->dst_offset;
+ return;
+ }
+
+ /* Enlarge the array if it's full. */
+ if (sctx->num_sdma_uploads == sctx->max_sdma_uploads) {
+ unsigned size;
+
+ sctx->max_sdma_uploads += 4;
+ size = sctx->max_sdma_uploads *
sizeof(sctx->sdma_uploads[0]);
+ sctx->sdma_uploads =
realloc(sctx->sdma_uploads, size);
+ }
+
+ /* Add a new upload. */
+ struct si_sdma_upload *up =
+ &sctx->sdma_uploads[sctx->num_sdma_uploads++];
+ up->dst = up->src = NULL;
+ si_resource_reference(&up->dst, buf);
+ si_resource_reference(&up->src, stransfer->staging);
+ up->dst_offset = box->x;
+ up->src_offset = src_offset;
+ up->size = box->width;
+ return;
+ }
+
/* Copy the staging buffer into the original one. */
-
