[Mesa-dev] [PATCH v2 3/4] etnaviv: hook-up etc2 patching

[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 <christian.gmeiner at gmail.com>
> ---
>  .../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(&rsc->texture, NULL);
>     pipe_resource_reference(&rsc->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 = &rsc->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 = &rsc->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 lines of "The ETC2 patching operates
in-place on the resource, so the resource will get written even on
read-only transfers".

Regards,
Lucas

> +      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 {
>        unsigned divSizeX = util_format_get_blockwidth(format);