[PATCH] etnaviv: bugfix: Don't do resolve-in-place without valid TS
Lucas Stach
l.stach at pengutronix.de
Wed Nov 1 10:52:55 UTC 2017
Am Mittwoch, den 01.11.2017, 11:17 +0100 schrieb Wladimir J. van der Laan:
> GC3000 resolve-in-place assumes that the TS state is configured.
> If it is not, this will result in MMU errors. This is especially
> apparent when using glGenMipmaps().
>
> Fixes a problem introduced in 78ade659569ee6fe9bd244170956139f19dd8c6c.
>
> > Signed-off-by: Wladimir J. van der Laan <laanwj at gmail.com>
> ---
> src/gallium/drivers/etnaviv/etnaviv_clear_blit.c | 4 ++++
> src/gallium/drivers/etnaviv/etnaviv_emit.c | 4 ++++
> src/gallium/drivers/etnaviv/etnaviv_rs.c | 1 +
> src/gallium/drivers/etnaviv/etnaviv_rs.h | 2 ++
> 4 files changed, 11 insertions(+)
>
> Ooops. This seems like an obvious oversight but I hadn't thought we would get
> into this path at all when there is no TS to "flush".
And that's probably what we should fix. The self-resolve cases on
resource flush and sampler update don't check the TS status, but they
are only useful if there is a valid TS.
With the change you did here we are still wasting bandwidth for a no-op
blit on older cores like GC880 when generating mipmaps.
Regards,
Lucas
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_clear_blit.c b/src/gallium/drivers/etnaviv/etnaviv_clear_blit.c
> index c62287b..8fc7cfc 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_clear_blit.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_clear_blit.c
> @@ -555,6 +555,7 @@ etna_try_rs_blit(struct pipe_context *pctx,
> }
>
> /* Set up color TS to source surface before blit, if needed */
> + bool source_ts_valid = false;
> if (src->levels[blit_info->src.level].ts_size &&
> src->levels[blit_info->src.level].ts_valid) {
> struct etna_reloc reloc;
> @@ -579,6 +580,8 @@ etna_try_rs_blit(struct pipe_context *pctx,
>
> etna_set_state(ctx->stream, VIVS_TS_COLOR_CLEAR_VALUE,
> src->levels[blit_info->src.level].clear_value);
> +
> + source_ts_valid = true;
> } else {
> etna_set_state(ctx->stream, VIVS_TS_MEM_CONFIG, ts_mem_config);
> }
> @@ -593,6 +596,7 @@ etna_try_rs_blit(struct pipe_context *pctx,
> .source_stride = src_lev->stride,
> .source_padded_width = src_lev->padded_width,
> .source_padded_height = src_lev->padded_height,
> + .source_ts_valid = source_ts_valid,
> .dest_format = translate_rs_format(dst_format),
> .dest_tiling = dst->layout,
> .dest = dst->bo,
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_emit.c b/src/gallium/drivers/etnaviv/etnaviv_emit.c
> index 707b1e7..5397aa3 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_emit.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_emit.c
> @@ -171,6 +171,10 @@ etna_submit_rs_state(struct etna_context *ctx,
> struct etna_cmd_stream *stream = ctx->stream;
> struct etna_coalesce coalesce;
>
> + if (cs->RS_KICKER_INPLACE && !cs->source_ts_valid)
> + /* Inplace resolve is no-op if TS is not configured */
> + return;
> +
> ctx->stats.rs_operations++;
>
> if (cs->RS_KICKER_INPLACE) {
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_rs.c b/src/gallium/drivers/etnaviv/etnaviv_rs.c
> index c9072c2..60c2c39 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_rs.c
> +++ b/src/gallium/drivers/etnaviv/etnaviv_rs.c
> @@ -133,6 +133,7 @@ etna_compile_rs_state(struct etna_context *ctx, struct compiled_rs_state *cs,
> /* Total number of tiles (same as for autodisable) */
> cs->RS_KICKER_INPLACE = rs->source_padded_width * rs->source_padded_height / 16;
> }
> + cs->source_ts_valid = rs->source_ts_valid;
> }
>
> void
> diff --git a/src/gallium/drivers/etnaviv/etnaviv_rs.h b/src/gallium/drivers/etnaviv/etnaviv_rs.h
> index 171d3fa..41a5960 100644
> --- a/src/gallium/drivers/etnaviv/etnaviv_rs.h
> +++ b/src/gallium/drivers/etnaviv/etnaviv_rs.h
> @@ -33,6 +33,7 @@
> struct rs_state {
> uint8_t downsample_x : 1; /* Downsample in x direction */
> uint8_t downsample_y : 1; /* Downsample in y direction */
> + uint8_t source_ts_valid : 1;
>
> uint8_t source_format; /* RS_FORMAT_XXX */
> uint8_t source_tiling; /* ETNA_LAYOUT_XXX */
> @@ -61,6 +62,7 @@ struct rs_state {
>
> /* treat this as opaque structure */
> struct compiled_rs_state {
> + uint8_t source_ts_valid : 1;
> uint32_t RS_CONFIG;
> uint32_t RS_SOURCE_STRIDE;
> uint32_t RS_DEST_STRIDE;
More information about the etnaviv
mailing list