Re: [Mesa-dev] [PATCH 3/3] etnaviv: Add sampler TS support

2017-11-10 Thread Wladimir J. van der Laan
On Fri, Nov 10, 2017 at 07:42:12AM +0100, Wladimir J. van der Laan wrote:
> Hello Lucas,
> 
> On Thu, Nov 09, 2017 at 06:15:51PM +0100, Lucas Stach wrote:
> > Hi Wladimir!
> 
> > > etna_resource_needs_flush is only called from two places - here, and
> > > in resource_flush, where it also determines whether to do a
> > > resolve-to-self, but before presenting the image. There it also only
> > > makes sense to do if the resource has at least a valid TS.
> > 
> > Yes, this makes sense.
> > 
> > Also I've just tested this and I've seen some intermittent missing
> > shadow tiles in the glmark2 shadow demo. Probably you are now missing
> > the TS cache flush we would normally do before blitting the shadow
> > image with the RS.
> 
> Thanks for testing!
> 
> I'll see if I can reproduce it. 

I was unable to reproduce it locally.

Can you please test if the below patch solves it?

Regards,
Wladimir

commit c26944fc3441e2516d4e4d17540b0f504cdcaf97
Author: Wladimir J. van der Laan 
Date:   Fri Nov 10 09:48:19 2017 +

etnaviv: Flush TS cache before changing TS configuration

This is to make sure that the TS is properly flushed to memory before
rendering to a new surface starts.

Signed-off-by: Wladimir J. van der Laan 

diff --git a/src/gallium/drivers/etnaviv/etnaviv_emit.c 
b/src/gallium/drivers/etnaviv/etnaviv_emit.c
index d313af6..b6b06e3 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_emit.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_emit.c
@@ -322,6 +322,11 @@ etna_emit_state(struct etna_context *ctx)
   etna_stall(stream, SYNC_RECIPIENT_RA, SYNC_RECIPIENT_PE);
}
 
+   /* Flush TS cache before changing TS configuration. */
+   if (unlikely(dirty & ETNA_DIRTY_TS)) {
+  etna_set_state(stream, VIVS_TS_FLUSH_CACHE, VIVS_TS_FLUSH_CACHE_FLUSH);
+   }
+
/* If MULTI_SAMPLE_CONFIG.MSAA_SAMPLES changed, clobber affected shader
 * state to make sure it is always rewritten. */
if (unlikely(dirty & (ETNA_DIRTY_FRAMEBUFFER))) {
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/3] etnaviv: Add sampler TS support

2017-11-09 Thread Wladimir J. van der Laan
Hello Lucas,

On Thu, Nov 09, 2017 at 06:15:51PM +0100, Lucas Stach wrote:
> Hi Wladimir!

> > etna_resource_needs_flush is only called from two places - here, and
> > in resource_flush, where it also determines whether to do a
> > resolve-to-self, but before presenting the image. There it also only
> > makes sense to do if the resource has at least a valid TS.
> 
> Yes, this makes sense.
> 
> Also I've just tested this and I've seen some intermittent missing
> shadow tiles in the glmark2 shadow demo. Probably you are now missing
> the TS cache flush we would normally do before blitting the shadow
> image with the RS.

Thanks for testing!

I'll see if I can reproduce it. 

Good point about the TS, I expect that needs to be flushed before the texture
can be rendered from, so that sampler TS sees a consistent TS. 

Regards,
Wladimir
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/3] etnaviv: Add sampler TS support

2017-11-09 Thread Lucas Stach
Hi Wladimir!

Am Mittwoch, den 08.11.2017, 03:41 +0100 schrieb Wladimir:
> > +/* Return true if a resource has a TS, and it is valid for at least one 
> > level */
> > +static bool
> > +etna_resource_has_valid_ts(struct pipe_resource *prsc)
> > +{
> > +   struct etna_resource *rsc = etna_resource(prsc);
> > +
> > +   if (!rsc->ts_bo)
> > +  return false;
> > +
> > +   for (int level = 0; level <= rsc->base.last_level; level++)
> > +  if (rsc->levels[level].ts_valid)
> > + return true;
> > +   return false;
> > +}
> >  static void
> > -etna_update_sampler_source(struct pipe_sampler_view *view)
> > +etna_update_sampler_source(struct etna_context *ctx, struct 
> > pipe_sampler_view *view, int num)
> >  {
> > struct etna_resource *base = etna_resource(view->texture);
> > struct etna_resource *to = base, *from = base;
> > +   bool enable_sampler_ts = false;
> > 
> > if (base->external && 
> > etna_resource_newer(etna_resource(base->external), base))
> >    from = etna_resource(base->external);
> > @@ -128,12 +199,19 @@ etna_update_sampler_source(struct pipe_sampler_view 
> > *view)
> >    etna_copy_resource(view->context, &to->base, &from->base, 0,
> >   view->texture->last_level);
> >    to->seqno = from->seqno;
> > -   } else if ((to == from) && etna_resource_needs_flush(to)) {
> > -  /* Resolve TS if needed, remove when adding sampler TS */
> > -  etna_copy_resource(view->context, &to->base, &from->base, 0,
> > - view->texture->last_level);
> > -  to->flush_seqno = from->seqno;
> > +   } else if ((to == from) &&
> > + etna_resource_needs_flush(to) &&
> > + etna_resource_has_valid_ts(&to->base)) {
> 
> I just realized - would it maybe make sense to roll the call to
> etna_resource_has_valid_ts into etna_resource_needs_flush?
> 
> etna_resource_needs_flush is only called from two places - here, and
> in resource_flush, where it also determines whether to do a
> resolve-to-self, but before presenting the image. There it also only
> makes sense to do if the resource has at least a valid TS.

Yes, this makes sense.

Also I've just tested this and I've seen some intermittent missing
shadow tiles in the glmark2 shadow demo. Probably you are now missing
the TS cache flush we would normally do before blitting the shadow
image with the RS.

Regards,
Lucas
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/3] etnaviv: Add sampler TS support

2017-11-07 Thread Wladimir
> +/* Return true if a resource has a TS, and it is valid for at least one 
> level */
> +static bool
> +etna_resource_has_valid_ts(struct pipe_resource *prsc)
> +{
> +   struct etna_resource *rsc = etna_resource(prsc);
> +
> +   if (!rsc->ts_bo)
> +  return false;
> +
> +   for (int level = 0; level <= rsc->base.last_level; level++)
> +  if (rsc->levels[level].ts_valid)
> + return true;
> +   return false;
> +}

>  static void
> -etna_update_sampler_source(struct pipe_sampler_view *view)
> +etna_update_sampler_source(struct etna_context *ctx, struct 
> pipe_sampler_view *view, int num)
>  {
> struct etna_resource *base = etna_resource(view->texture);
> struct etna_resource *to = base, *from = base;
> +   bool enable_sampler_ts = false;
>
> if (base->external && etna_resource_newer(etna_resource(base->external), 
> base))
>from = etna_resource(base->external);
> @@ -128,12 +199,19 @@ etna_update_sampler_source(struct pipe_sampler_view 
> *view)
>etna_copy_resource(view->context, &to->base, &from->base, 0,
>   view->texture->last_level);
>to->seqno = from->seqno;
> -   } else if ((to == from) && etna_resource_needs_flush(to)) {
> -  /* Resolve TS if needed, remove when adding sampler TS */
> -  etna_copy_resource(view->context, &to->base, &from->base, 0,
> - view->texture->last_level);
> -  to->flush_seqno = from->seqno;
> +   } else if ((to == from) &&
> + etna_resource_needs_flush(to) &&
> + etna_resource_has_valid_ts(&to->base)) {

I just realized - would it maybe make sense to roll the call to
etna_resource_has_valid_ts into etna_resource_needs_flush?

etna_resource_needs_flush is only called from two places - here, and
in resource_flush, where it also determines whether to do a
resolve-to-self, but before presenting the image. There it also only
makes sense to do if the resource has at least a valid TS.

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


Re: [Mesa-dev] [PATCH 3/3] etnaviv: Add sampler TS support

2017-11-07 Thread Christian Gmeiner
2017-11-07 17:43 GMT+01:00 Wladimir J. van der Laan :
> Sampler TS is an hardware optimization that can be used when rendering
> to textures. After rendering to a resource with TS enabled, the
> texture unit can use this to bypass lookups to empty tiles. This also
> means a resolve-in-place can be avoided to flush the TS.
>
> This commit is also an optimization when not using sampler TS, as
> resolve-in-place will now be skipped if a resource has no (valid) TS.
>
> Signed-off-by: Wladimir J. van der Laan 

Reviewed-by: Christian Gmeiner 

> ---
>  src/gallium/drivers/etnaviv/etnaviv_emit.c| 26 
>  src/gallium/drivers/etnaviv/etnaviv_texture.c | 92 
> +--
>  src/gallium/drivers/etnaviv/etnaviv_texture.h |  5 ++
>  3 files changed, 116 insertions(+), 7 deletions(-)
>
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_emit.c 
> b/src/gallium/drivers/etnaviv/etnaviv_emit.c
> index d313af6..bd2a570 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_emit.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_emit.c
> @@ -627,6 +627,32 @@ etna_emit_state(struct etna_context *ctx)
>/*01668*/ EMIT_STATE_RELOC(TS_DEPTH_SURFACE_BASE, 
> &ctx->framebuffer.TS_DEPTH_SURFACE_BASE);
>/*0166C*/ EMIT_STATE(TS_DEPTH_CLEAR_VALUE, 
> ctx->framebuffer.TS_DEPTH_CLEAR_VALUE);
> }
> +   if (unlikely(dirty & ETNA_DIRTY_SAMPLER_VIEWS)) {
> +  for (int x = 0; x < VIVS_TS_SAMPLER__LEN; ++x) {
> + if ((1 << x) & active_samplers) {
> +struct etna_sampler_view *sv = 
> etna_sampler_view(ctx->sampler_view[x]);
> +/*01720*/ EMIT_STATE(TS_SAMPLER_CONFIG(x), 
> sv->TS_SAMPLER_CONFIG);
> + }
> +  }
> +  for (int x = 0; x < VIVS_TS_SAMPLER__LEN; ++x) {
> + if ((1 << x) & active_samplers) {
> +struct etna_sampler_view *sv = 
> etna_sampler_view(ctx->sampler_view[x]);
> +/*01740*/ EMIT_STATE_RELOC(TS_SAMPLER_STATUS_BASE(x), 
> &sv->TS_SAMPLER_STATUS_BASE);
> + }
> +  }
> +  for (int x = 0; x < VIVS_TS_SAMPLER__LEN; ++x) {
> + if ((1 << x) & active_samplers) {
> +struct etna_sampler_view *sv = 
> etna_sampler_view(ctx->sampler_view[x]);
> +/*01760*/ EMIT_STATE(TS_SAMPLER_CLEAR_VALUE(x), 
> sv->TS_SAMPLER_CLEAR_VALUE);
> + }
> +  }
> +  for (int x = 0; x < VIVS_TS_SAMPLER__LEN; ++x) {
> + if ((1 << x) & active_samplers) {
> +struct etna_sampler_view *sv = 
> etna_sampler_view(ctx->sampler_view[x]);
> +/*01780*/ EMIT_STATE(TS_SAMPLER_CLEAR_VALUE2(x), 
> sv->TS_SAMPLER_CLEAR_VALUE2);
> + }
> +  }
> +   }
> if (unlikely(dirty & (ETNA_DIRTY_SAMPLER_VIEWS | ETNA_DIRTY_SAMPLERS))) {
>for (int x = 0; x < VIVS_TE_SAMPLER__LEN; ++x) {
>   uint32_t val = 0; /* 0 == sampler inactive */
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_texture.c 
> b/src/gallium/drivers/etnaviv/etnaviv_texture.c
> index 34529c6..3d5e88b 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_texture.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_texture.c
> @@ -112,11 +112,82 @@ etna_delete_sampler_state(struct pipe_context *pctx, 
> void *ss)
> FREE(ss);
>  }
>
> +/* Return true if a resource has a TS, and it is valid for at least one 
> level */
> +static bool
> +etna_resource_has_valid_ts(struct pipe_resource *prsc)
> +{
> +   struct etna_resource *rsc = etna_resource(prsc);
> +
> +   if (!rsc->ts_bo)
> +  return false;
> +
> +   for (int level = 0; level <= rsc->base.last_level; level++)
> +  if (rsc->levels[level].ts_valid)
> + return true;
> +   return false;
> +}
> +
> +/* Return true if the GPU can use sampler TS with this sampler view.
> + * Sampler TS is an optimization used when rendering to textures, where
> + * a resolve-in-place can be avoided when rendering has left a (valid) TS.
> + */
> +static bool
> +etna_can_use_sampler_ts(struct etna_context *ctx, struct pipe_sampler_view 
> *view, int num)
> +{
> +/* Can use sampler TS when:
> + * - the hardware supports sampler TS.
> + * - the sampler view will be bound to sampler  + *   HALTI5 adds a mapping from sampler to sampler TS unit, but this is 
> AFAIK
> + *   absent on earlier models.
> + * - it is a texture, not a buffer.
> + * - the sampler view has a supported format for sampler TS.
> + * - the sampler will have one LOD, and it happens to be level 0.
> + *   (it is not sure if the hw supports it for other levels, but 
> available
> + *   state strongly suggests only one at a time).
> + * - the resource TS is valid for level 0.
> + */
> +   struct etna_resource *rsc = etna_resource(view->texture);
> +   struct etna_screen *screen = etna_screen(rsc->base.screen);
> +   return VIV_FEATURE(screen, chipMinorFeatures2, TEXTURE_TILED_READ) &&
> +  num < VIVS_TS_SAMPLER__LEN &&
> +  rsc->base.target != PIPE_BUFFER &&
> +  translate_ts_sampler_format(rsc->base.format) != ETNA_NO_MATCH &&

[Mesa-dev] [PATCH 3/3] etnaviv: Add sampler TS support

2017-11-07 Thread Wladimir J. van der Laan
Sampler TS is an hardware optimization that can be used when rendering
to textures. After rendering to a resource with TS enabled, the
texture unit can use this to bypass lookups to empty tiles. This also
means a resolve-in-place can be avoided to flush the TS.

This commit is also an optimization when not using sampler TS, as
resolve-in-place will now be skipped if a resource has no (valid) TS.

Signed-off-by: Wladimir J. van der Laan 
---
 src/gallium/drivers/etnaviv/etnaviv_emit.c| 26 
 src/gallium/drivers/etnaviv/etnaviv_texture.c | 92 +--
 src/gallium/drivers/etnaviv/etnaviv_texture.h |  5 ++
 3 files changed, 116 insertions(+), 7 deletions(-)

diff --git a/src/gallium/drivers/etnaviv/etnaviv_emit.c 
b/src/gallium/drivers/etnaviv/etnaviv_emit.c
index d313af6..bd2a570 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_emit.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_emit.c
@@ -627,6 +627,32 @@ etna_emit_state(struct etna_context *ctx)
   /*01668*/ EMIT_STATE_RELOC(TS_DEPTH_SURFACE_BASE, 
&ctx->framebuffer.TS_DEPTH_SURFACE_BASE);
   /*0166C*/ EMIT_STATE(TS_DEPTH_CLEAR_VALUE, 
ctx->framebuffer.TS_DEPTH_CLEAR_VALUE);
}
+   if (unlikely(dirty & ETNA_DIRTY_SAMPLER_VIEWS)) {
+  for (int x = 0; x < VIVS_TS_SAMPLER__LEN; ++x) {
+ if ((1 << x) & active_samplers) {
+struct etna_sampler_view *sv = 
etna_sampler_view(ctx->sampler_view[x]);
+/*01720*/ EMIT_STATE(TS_SAMPLER_CONFIG(x), sv->TS_SAMPLER_CONFIG);
+ }
+  }
+  for (int x = 0; x < VIVS_TS_SAMPLER__LEN; ++x) {
+ if ((1 << x) & active_samplers) {
+struct etna_sampler_view *sv = 
etna_sampler_view(ctx->sampler_view[x]);
+/*01740*/ EMIT_STATE_RELOC(TS_SAMPLER_STATUS_BASE(x), 
&sv->TS_SAMPLER_STATUS_BASE);
+ }
+  }
+  for (int x = 0; x < VIVS_TS_SAMPLER__LEN; ++x) {
+ if ((1 << x) & active_samplers) {
+struct etna_sampler_view *sv = 
etna_sampler_view(ctx->sampler_view[x]);
+/*01760*/ EMIT_STATE(TS_SAMPLER_CLEAR_VALUE(x), 
sv->TS_SAMPLER_CLEAR_VALUE);
+ }
+  }
+  for (int x = 0; x < VIVS_TS_SAMPLER__LEN; ++x) {
+ if ((1 << x) & active_samplers) {
+struct etna_sampler_view *sv = 
etna_sampler_view(ctx->sampler_view[x]);
+/*01780*/ EMIT_STATE(TS_SAMPLER_CLEAR_VALUE2(x), 
sv->TS_SAMPLER_CLEAR_VALUE2);
+ }
+  }
+   }
if (unlikely(dirty & (ETNA_DIRTY_SAMPLER_VIEWS | ETNA_DIRTY_SAMPLERS))) {
   for (int x = 0; x < VIVS_TE_SAMPLER__LEN; ++x) {
  uint32_t val = 0; /* 0 == sampler inactive */
diff --git a/src/gallium/drivers/etnaviv/etnaviv_texture.c 
b/src/gallium/drivers/etnaviv/etnaviv_texture.c
index 34529c6..3d5e88b 100644
--- a/src/gallium/drivers/etnaviv/etnaviv_texture.c
+++ b/src/gallium/drivers/etnaviv/etnaviv_texture.c
@@ -112,11 +112,82 @@ etna_delete_sampler_state(struct pipe_context *pctx, void 
*ss)
FREE(ss);
 }
 
+/* Return true if a resource has a TS, and it is valid for at least one level 
*/
+static bool
+etna_resource_has_valid_ts(struct pipe_resource *prsc)
+{
+   struct etna_resource *rsc = etna_resource(prsc);
+
+   if (!rsc->ts_bo)
+  return false;
+
+   for (int level = 0; level <= rsc->base.last_level; level++)
+  if (rsc->levels[level].ts_valid)
+ return true;
+   return false;
+}
+
+/* Return true if the GPU can use sampler TS with this sampler view.
+ * Sampler TS is an optimization used when rendering to textures, where
+ * a resolve-in-place can be avoided when rendering has left a (valid) TS.
+ */
+static bool
+etna_can_use_sampler_ts(struct etna_context *ctx, struct pipe_sampler_view 
*view, int num)
+{
+/* Can use sampler TS when:
+ * - the hardware supports sampler TS.
+ * - the sampler view will be bound to sampler texture);
+   struct etna_screen *screen = etna_screen(rsc->base.screen);
+   return VIV_FEATURE(screen, chipMinorFeatures2, TEXTURE_TILED_READ) &&
+  num < VIVS_TS_SAMPLER__LEN &&
+  rsc->base.target != PIPE_BUFFER &&
+  translate_ts_sampler_format(rsc->base.format) != ETNA_NO_MATCH &&
+  view->u.tex.first_level == 0 && MIN2(view->u.tex.last_level, 
rsc->base.last_level) == 0 &&
+  rsc->levels[0].ts_valid;
+}
+
+static void
+etna_configure_sampler_ts(struct etna_context *ctx, struct pipe_sampler_view 
*pview, bool enable)
+{
+   struct etna_sampler_view *view = etna_sampler_view(pview);
+   if (enable) {
+  struct etna_resource *rsc = etna_resource(view->base.texture);
+  struct etna_resource_level *lev = &rsc->levels[0];
+  assert(rsc->ts_bo && lev->ts_valid);
+
+  view->TE_SAMPLER_CONFIG1 |= VIVS_TE_SAMPLER_CONFIG1_USE_TS;
+  view->TS_SAMPLER_CONFIG =
+ VIVS_TS_SAMPLER_CONFIG_ENABLE(1) |
+ 
VIVS_TS_SAMPLER_CONFIG_FORMAT(translate_ts_sampler_format(rsc->base.format));
+  view->TS_SAMPLER_CLEAR_VALUE = lev->clear_value;
+  view->TS_SAMPLER_CLEAR_VALUE2 = lev->clear_value; /* To ha