Re: [Mesa-dev] [PATCH v2 3/4] etnaviv: hook-up etc2 patching
Hi Lucas
Am Mi., 27. Feb. 2019 um 10:19 Uhr schrieb Lucas Stach :
>
> Am Dienstag, den 26.02.2019, 19:15 +0100 schrieb Christian Gmeiner:
> > Changes v1 -> v2:
> > - Avoid the GPU sampling from the resource that gets mutated by the the
> >transfer map by setting DRM_ETNA_PREP_WRITE.
> >
> > > Signed-off-by: Christian Gmeiner
> > ---
> > .../drivers/etnaviv/etnaviv_resource.c| 3 +
> > .../drivers/etnaviv/etnaviv_resource.h| 5 ++
> > .../drivers/etnaviv/etnaviv_transfer.c| 55 +++
> > 3 files changed, 63 insertions(+)
> >
> > diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.c
> > b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> > index db5ead4d0ba..45d5f69169e 100644
> > --- a/src/gallium/drivers/etnaviv/etnaviv_resource.c
> > +++ b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> > @@ -478,6 +478,9 @@ etna_resource_destroy(struct pipe_screen *pscreen,
> > struct pipe_resource *prsc)
> > pipe_resource_reference(>texture, NULL);
> > pipe_resource_reference(>external, NULL);
> >
> > + for (unsigned i = 0; i < ETNA_NUM_LOD; i++)
> > + FREE(rsc->levels[i].patch_offsets);
> > +
> > FREE(rsc);
> > }
> >
> > diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.h
> > b/src/gallium/drivers/etnaviv/etnaviv_resource.h
> > index 75aa80b3d7a..c45ff7586d1 100644
> > --- a/src/gallium/drivers/etnaviv/etnaviv_resource.h
> > +++ b/src/gallium/drivers/etnaviv/etnaviv_resource.h
> > @@ -33,6 +33,7 @@
> > #include "util/list.h"
> >
> > struct pipe_screen;
> > +struct util_dynarray;
> >
> > struct etna_resource_level {
> > unsigned width, padded_width; /* in pixels */
> > @@ -47,6 +48,10 @@ struct etna_resource_level {
> > uint32_t ts_size;
> > uint32_t clear_value; /* clear value of resource level (mainly for TS)
> > */
> > bool ts_valid;
> > +
> > + /* keep track if we have done some per block patching */
> > + bool patched;
> > + struct util_dynarray *patch_offsets;
> > };
> >
> > enum etna_resource_addressing_mode {
> > diff --git a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> > b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> > index 01da393d211..9a83c481dce 100644
> > --- a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> > +++ b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> > @@ -28,6 +28,7 @@
> > #include "etnaviv_clear_blit.h"
> > #include "etnaviv_context.h"
> > #include "etnaviv_debug.h"
> > +#include "etnaviv_etc2.h"
> > #include "etnaviv_screen.h"
> >
> > #include "pipe/p_defines.h"
> > @@ -57,6 +58,46 @@ etna_compute_offset(enum pipe_format format, const
> > struct pipe_box *box,
> > util_format_get_blocksize(format);
> > }
> >
> > +static void etna_patch_data(void *buffer, const struct pipe_transfer
> > *ptrans)
> > +{
> > + struct pipe_resource *prsc = ptrans->resource;
> > + struct etna_resource *rsc = etna_resource(prsc);
> > + struct etna_resource_level *level = >levels[ptrans->level];
> > +
> > + if (!etna_etc2_needs_patching(prsc))
>
> Maybe likely() here?
>
okay.
> > + return;
> > +
> > + if (level->patched)
> > + return;
> > +
> > + /* do have the offsets of blocks to patch? */
> > + if (!level->patch_offsets) {
> > + level->patch_offsets = CALLOC_STRUCT(util_dynarray);
> > +
> > + etna_etc2_calculate_blocks(buffer, ptrans->stride,
> > + ptrans->box.width,
> > ptrans->box.height,
> > + prsc->format,
> > level->patch_offsets);
> > + }
> > +
> > + etna_etc2_patch(buffer, level->patch_offsets);
> > +
> > + level->patched = true;
> > +}
> > +
> > +static void etna_unpatch_data(void *buffer, const struct pipe_transfer
> > *ptrans)
> > +{
> > + struct pipe_resource *prsc = ptrans->resource;
> > + struct etna_resource *rsc = etna_resource(prsc);
> > + struct etna_resource_level *level = >levels[ptrans->level];
> > +
> > + if (!level->patched)
> > + return;
> > +
> > + etna_etc2_patch(buffer, level->patch_offsets);
> > +
> > + level->patched = false;
> > +}
> > +
> > static void
> > etna_transfer_unmap(struct pipe_context *pctx, struct pipe_transfer
> > *ptrans)
> > {
> > @@ -119,6 +160,10 @@ etna_transfer_unmap(struct pipe_context *pctx, struct
> > pipe_transfer *ptrans)
> >}
> > }
> >
> > + /* We need to have the patched data ready for the GPU. */
> > + if (rsc->layout == ETNA_LAYOUT_LINEAR)
>
> I don't like the LAYOUT_LINEAR checks here, as GC3000 actually has a
> feature bit TEX_COMPRESSION_SUPERTILED, so I don't think the assertion
> that all compressed formats are linear will hold in the future and it
> seems like a minor optimization.
>
Makes sense - will remove it in v3.
> > + etna_patch_data(trans->mapped, ptrans);
> > +
> > /*
> > * Transfers without a temporary are only pulled into the CPU domain if
> > they
> > * are not mapped unsynchronized. If they are, must push them
Re: [Mesa-dev] [PATCH v2 3/4] etnaviv: hook-up etc2 patching
Am Dienstag, den 26.02.2019, 19:15 +0100 schrieb Christian Gmeiner:
> Changes v1 -> v2:
> - Avoid the GPU sampling from the resource that gets mutated by the the
> transfer map by setting DRM_ETNA_PREP_WRITE.
>
> > Signed-off-by: Christian Gmeiner
> ---
> .../drivers/etnaviv/etnaviv_resource.c| 3 +
> .../drivers/etnaviv/etnaviv_resource.h| 5 ++
> .../drivers/etnaviv/etnaviv_transfer.c| 55 +++
> 3 files changed, 63 insertions(+)
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.c
> b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> index db5ead4d0ba..45d5f69169e 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_resource.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_resource.c
> @@ -478,6 +478,9 @@ etna_resource_destroy(struct pipe_screen *pscreen, struct
> pipe_resource *prsc)
> pipe_resource_reference(>texture, NULL);
> pipe_resource_reference(>external, NULL);
>
> + for (unsigned i = 0; i < ETNA_NUM_LOD; i++)
> + FREE(rsc->levels[i].patch_offsets);
> +
> FREE(rsc);
> }
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.h
> b/src/gallium/drivers/etnaviv/etnaviv_resource.h
> index 75aa80b3d7a..c45ff7586d1 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_resource.h
> +++ b/src/gallium/drivers/etnaviv/etnaviv_resource.h
> @@ -33,6 +33,7 @@
> #include "util/list.h"
>
> struct pipe_screen;
> +struct util_dynarray;
>
> struct etna_resource_level {
> unsigned width, padded_width; /* in pixels */
> @@ -47,6 +48,10 @@ struct etna_resource_level {
> uint32_t ts_size;
> uint32_t clear_value; /* clear value of resource level (mainly for TS) */
> bool ts_valid;
> +
> + /* keep track if we have done some per block patching */
> + bool patched;
> + struct util_dynarray *patch_offsets;
> };
>
> enum etna_resource_addressing_mode {
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> index 01da393d211..9a83c481dce 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
> @@ -28,6 +28,7 @@
> #include "etnaviv_clear_blit.h"
> #include "etnaviv_context.h"
> #include "etnaviv_debug.h"
> +#include "etnaviv_etc2.h"
> #include "etnaviv_screen.h"
>
> #include "pipe/p_defines.h"
> @@ -57,6 +58,46 @@ etna_compute_offset(enum pipe_format format, const struct
> pipe_box *box,
> util_format_get_blocksize(format);
> }
>
> +static void etna_patch_data(void *buffer, const struct pipe_transfer *ptrans)
> +{
> + struct pipe_resource *prsc = ptrans->resource;
> + struct etna_resource *rsc = etna_resource(prsc);
> + struct etna_resource_level *level = >levels[ptrans->level];
> +
> + if (!etna_etc2_needs_patching(prsc))
Maybe likely() here?
> + return;
> +
> + if (level->patched)
> + return;
> +
> + /* do have the offsets of blocks to patch? */
> + if (!level->patch_offsets) {
> + level->patch_offsets = CALLOC_STRUCT(util_dynarray);
> +
> + etna_etc2_calculate_blocks(buffer, ptrans->stride,
> + ptrans->box.width,
> ptrans->box.height,
> + prsc->format, level->patch_offsets);
> + }
> +
> + etna_etc2_patch(buffer, level->patch_offsets);
> +
> + level->patched = true;
> +}
> +
> +static void etna_unpatch_data(void *buffer, const struct pipe_transfer
> *ptrans)
> +{
> + struct pipe_resource *prsc = ptrans->resource;
> + struct etna_resource *rsc = etna_resource(prsc);
> + struct etna_resource_level *level = >levels[ptrans->level];
> +
> + if (!level->patched)
> + return;
> +
> + etna_etc2_patch(buffer, level->patch_offsets);
> +
> + level->patched = false;
> +}
> +
> static void
> etna_transfer_unmap(struct pipe_context *pctx, struct pipe_transfer *ptrans)
> {
> @@ -119,6 +160,10 @@ etna_transfer_unmap(struct pipe_context *pctx, struct
> pipe_transfer *ptrans)
> }
> }
>
> + /* We need to have the patched data ready for the GPU. */
> + if (rsc->layout == ETNA_LAYOUT_LINEAR)
I don't like the LAYOUT_LINEAR checks here, as GC3000 actually has a
feature bit TEX_COMPRESSION_SUPERTILED, so I don't think the assertion
that all compressed formats are linear will hold in the future and it
seems like a minor optimization.
> + etna_patch_data(trans->mapped, ptrans);
> +
> /*
> * Transfers without a temporary are only pulled into the CPU domain if
> they
> * are not mapped unsynchronized. If they are, must push them back into
> GPU
> @@ -321,6 +366,12 @@ etna_transfer_map(struct pipe_context *pctx, struct
> pipe_resource *prsc,
> if (usage & PIPE_TRANSFER_WRITE)
> prep_flags |= DRM_ETNA_PREP_WRITE;
>
> + /* avoid the GPU sampling from the resource that gets mutated */
This doesn't really explain to the reader of this function what happens
here. Maybe something along the
[Mesa-dev] [PATCH v2 3/4] etnaviv: hook-up etc2 patching
Changes v1 -> v2:
- Avoid the GPU sampling from the resource that gets mutated by the the
transfer map by setting DRM_ETNA_PREP_WRITE.
Signed-off-by: Christian Gmeiner
---
.../drivers/etnaviv/etnaviv_resource.c| 3 +
.../drivers/etnaviv/etnaviv_resource.h| 5 ++
.../drivers/etnaviv/etnaviv_transfer.c| 55 +++
3 files changed, 63 insertions(+)
diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.c
b/src/gallium/drivers/etnaviv/etnaviv_resource.c
index db5ead4d0ba..45d5f69169e 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_resource.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_resource.c
@@ -478,6 +478,9 @@ etna_resource_destroy(struct pipe_screen *pscreen, struct
pipe_resource *prsc)
pipe_resource_reference(>texture, NULL);
pipe_resource_reference(>external, NULL);
+ for (unsigned i = 0; i < ETNA_NUM_LOD; i++)
+ FREE(rsc->levels[i].patch_offsets);
+
FREE(rsc);
}
diff --git a/src/gallium/drivers/etnaviv/etnaviv_resource.h
b/src/gallium/drivers/etnaviv/etnaviv_resource.h
index 75aa80b3d7a..c45ff7586d1 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_resource.h
+++ b/src/gallium/drivers/etnaviv/etnaviv_resource.h
@@ -33,6 +33,7 @@
#include "util/list.h"
struct pipe_screen;
+struct util_dynarray;
struct etna_resource_level {
unsigned width, padded_width; /* in pixels */
@@ -47,6 +48,10 @@ struct etna_resource_level {
uint32_t ts_size;
uint32_t clear_value; /* clear value of resource level (mainly for TS) */
bool ts_valid;
+
+ /* keep track if we have done some per block patching */
+ bool patched;
+ struct util_dynarray *patch_offsets;
};
enum etna_resource_addressing_mode {
diff --git a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
index 01da393d211..9a83c481dce 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_transfer.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_transfer.c
@@ -28,6 +28,7 @@
#include "etnaviv_clear_blit.h"
#include "etnaviv_context.h"
#include "etnaviv_debug.h"
+#include "etnaviv_etc2.h"
#include "etnaviv_screen.h"
#include "pipe/p_defines.h"
@@ -57,6 +58,46 @@ etna_compute_offset(enum pipe_format format, const struct
pipe_box *box,
util_format_get_blocksize(format);
}
+static void etna_patch_data(void *buffer, const struct pipe_transfer *ptrans)
+{
+ struct pipe_resource *prsc = ptrans->resource;
+ struct etna_resource *rsc = etna_resource(prsc);
+ struct etna_resource_level *level = >levels[ptrans->level];
+
+ if (!etna_etc2_needs_patching(prsc))
+ return;
+
+ if (level->patched)
+ return;
+
+ /* do have the offsets of blocks to patch? */
+ if (!level->patch_offsets) {
+ level->patch_offsets = CALLOC_STRUCT(util_dynarray);
+
+ etna_etc2_calculate_blocks(buffer, ptrans->stride,
+ ptrans->box.width, ptrans->box.height,
+ prsc->format, level->patch_offsets);
+ }
+
+ etna_etc2_patch(buffer, level->patch_offsets);
+
+ level->patched = true;
+}
+
+static void etna_unpatch_data(void *buffer, const struct pipe_transfer *ptrans)
+{
+ struct pipe_resource *prsc = ptrans->resource;
+ struct etna_resource *rsc = etna_resource(prsc);
+ struct etna_resource_level *level = >levels[ptrans->level];
+
+ if (!level->patched)
+ return;
+
+ etna_etc2_patch(buffer, level->patch_offsets);
+
+ level->patched = false;
+}
+
static void
etna_transfer_unmap(struct pipe_context *pctx, struct pipe_transfer *ptrans)
{
@@ -119,6 +160,10 @@ etna_transfer_unmap(struct pipe_context *pctx, struct
pipe_transfer *ptrans)
}
}
+ /* We need to have the patched data ready for the GPU. */
+ if (rsc->layout == ETNA_LAYOUT_LINEAR)
+ etna_patch_data(trans->mapped, ptrans);
+
/*
* Transfers without a temporary are only pulled into the CPU domain if they
* are not mapped unsynchronized. If they are, must push them back into GPU
@@ -321,6 +366,12 @@ etna_transfer_map(struct pipe_context *pctx, struct
pipe_resource *prsc,
if (usage & PIPE_TRANSFER_WRITE)
prep_flags |= DRM_ETNA_PREP_WRITE;
+ /* avoid the GPU sampling from the resource that gets mutated */
+ if (rsc->layout == ETNA_LAYOUT_LINEAR &&
+ (usage & PIPE_TRANSFER_READ) &&
+ etna_etc2_needs_patching(prsc))
+ prep_flags |= DRM_ETNA_PREP_WRITE;
+
if (etna_bo_cpu_prep(rsc->bo, prep_flags))
goto fail_prep;
}
@@ -340,6 +391,10 @@ etna_transfer_map(struct pipe_context *pctx, struct
pipe_resource *prsc,
etna_compute_offset(prsc->format, box, res_level->stride,
res_level->layer_stride);
+ /* We need to have the unpatched data ready for the gfx stack. */
+ if (usage & PIPE_TRANSFER_READ)
+ etna_unpatch_data(trans->mapped, ptrans);
+
return trans->mapped;
} else {
