[Mesa-dev] [PATCH 4/4] r600g: only emit gfx cmd when there is actual work in it

[Marek Olšák]

You forgot about fences and queries other than timestamp. All queries
must be emitted even if there is no rendering between them (the GL
spec says that if a query is busy, any later query must be busy too,
and empty queries are allowed - we have piglit tests for all that).

Anyway, I think this is not needed and it's also prone to errors as your
patch shows. The current mechanism that prevents an empty CS from
being emitted is sufficient. The CS flushing is skipped if:
- cs->cdw == ctx->start_cs_cmd.num_dw in r600_context_flush, or
- cs->cdw == 0 in radeon_drm_cs_flush

Marek

On Fri, Jan 25, 2013 at 6:50 PM,  <j.glisse at gmail.com> wrote:
> From: Jerome Glisse <jglisse at redhat.com>
>
> Signed-off-by: Jerome Glisse <jglisse at redhat.com>
> ---
>  src/gallium/drivers/r600/evergreen_compute.c | 2 ++
>  src/gallium/drivers/r600/r600_hw_context.c   | 1 +
>  src/gallium/drivers/r600/r600_pipe.c         | 6 ++++++
>  src/gallium/drivers/r600/r600_pipe.h         | 1 +
>  src/gallium/drivers/r600/r600_query.c        | 2 ++
>  src/gallium/drivers/r600/r600_state_common.c | 1 +
>  6 files changed, 13 insertions(+)
>
> diff --git a/src/gallium/drivers/r600/evergreen_compute.c b/src/gallium/drivers/r600/evergreen_compute.c
> index f4a7905..977595e 100644
> --- a/src/gallium/drivers/r600/evergreen_compute.c
> +++ b/src/gallium/drivers/r600/evergreen_compute.c
> @@ -308,6 +308,8 @@ static void evergreen_emit_direct_dispatch(
>         r600_write_value(cs, grid_layout[2]);
>         /* VGT_DISPATCH_INITIATOR = COMPUTE_SHADER_EN */
>         r600_write_value(cs, 1);
> +
> +       rctx->rings.gfx.cdraw++;
>  }
>
>  static void compute_emit_cs(struct r600_context *ctx, const uint *block_layout,
> diff --git a/src/gallium/drivers/r600/r600_hw_context.c b/src/gallium/drivers/r600/r600_hw_context.c
> index d7518a5..511a276 100644
> --- a/src/gallium/drivers/r600/r600_hw_context.c
> +++ b/src/gallium/drivers/r600/r600_hw_context.c
> @@ -1122,6 +1122,7 @@ void r600_cp_dma_copy_buffer(struct r600_context *rctx,
>                 size -= byte_count;
>                 src_offset += byte_count;
>                 dst_offset += byte_count;
> +               rctx->rings.gfx.cdraw++;
>         }
>  }
>
> diff --git a/src/gallium/drivers/r600/r600_pipe.c b/src/gallium/drivers/r600/r600_pipe.c
> index 6767412..af08cff 100644
> --- a/src/gallium/drivers/r600/r600_pipe.c
> +++ b/src/gallium/drivers/r600/r600_pipe.c
> @@ -120,6 +120,10 @@ static void r600_flush(struct pipe_context *ctx, unsigned flags)
>         struct pipe_query *render_cond = NULL;
>         unsigned render_cond_mode = 0;
>
> +       if (!rctx->rings.gfx.cdraw) {
> +               return;
> +       }
> +
>         rctx->rings.gfx.flushing = true;
>         /* Disable render condition. */
>         if (rctx->current_render_cond) {
> @@ -130,6 +134,7 @@ static void r600_flush(struct pipe_context *ctx, unsigned flags)
>
>         r600_context_flush(rctx, flags);
>         rctx->rings.gfx.flushing = false;
> +       rctx->rings.gfx.cdraw = 0;
>         r600_begin_new_cs(rctx);
>
>         /* Re-enable render condition. */
> @@ -387,6 +392,7 @@ static struct pipe_context *r600_create_context(struct pipe_screen *screen, void
>                 goto fail;
>         }
>
> +       rctx->rings.gfx.cdraw = 0;
>         rctx->rings.gfx.cs = rctx->ws->cs_create(rctx->ws, RING_GFX);
>         rctx->rings.gfx.flush = r600_flush_gfx_ring;
>         rctx->ws->cs_set_flush_callback(rctx->rings.gfx.cs, r600_flush_from_winsys, rctx);
> diff --git a/src/gallium/drivers/r600/r600_pipe.h b/src/gallium/drivers/r600/r600_pipe.h
> index 31dcd05..5c72756 100644
> --- a/src/gallium/drivers/r600/r600_pipe.h
> +++ b/src/gallium/drivers/r600/r600_pipe.h
> @@ -418,6 +418,7 @@ struct r600_fetch_shader {
>  struct r600_ring {
>         struct radeon_winsys_cs         *cs;
>         bool                            flushing;
> +       unsigned                        cdraw;
>         void (*flush)(void *ctx, unsigned flags);
>  };
>
> diff --git a/src/gallium/drivers/r600/r600_query.c b/src/gallium/drivers/r600/r600_query.c
> index 0335189..7916f2d 100644
> --- a/src/gallium/drivers/r600/r600_query.c
> +++ b/src/gallium/drivers/r600/r600_query.c
> @@ -149,6 +149,7 @@ static void r600_emit_query_begin(struct r600_context *ctx, struct r600_query *q
>                 cs->buf[cs->cdw++] = (3 << 29) | ((va >> 32UL) & 0xFF);
>                 cs->buf[cs->cdw++] = 0;
>                 cs->buf[cs->cdw++] = 0;
> +               ctx->rings.gfx.cdraw++;
>                 break;
>         default:
>                 assert(0);
> @@ -201,6 +202,7 @@ static void r600_emit_query_end(struct r600_context *ctx, struct r600_query *que
>                 cs->buf[cs->cdw++] = (3 << 29) | ((va >> 32UL) & 0xFF);
>                 cs->buf[cs->cdw++] = 0;
>                 cs->buf[cs->cdw++] = 0;
> +               ctx->rings.gfx.cdraw++;
>                 break;
>         default:
>                 assert(0);
> diff --git a/src/gallium/drivers/r600/r600_state_common.c b/src/gallium/drivers/r600/r600_state_common.c
> index b547d64..d4616ce 100644
> --- a/src/gallium/drivers/r600/r600_state_common.c
> +++ b/src/gallium/drivers/r600/r600_state_common.c
> @@ -1439,6 +1439,7 @@ static void r600_draw_vbo(struct pipe_context *ctx, const struct pipe_draw_info
>                 r600_trace_emit(rctx);
>         }
>  #endif
> +       rctx->rings.gfx.cdraw++;
>
>         /* Set the depth buffer as dirty. */
>         if (rctx->framebuffer.state.zsbuf) {
> --
> 1.7.11.7
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev