On 06/26/2012 07:28 AM, Daniel Vetter wrote:
> Similar treatment to the depth count pipe_control writes
> - Add the CS_STALL workaround, timestamp writes are non-zero post-sync
> ops, too.
> - Also ensure that we write the full 64bits by using the 5 dword long
> variant of pipe_control.
> ---
> src/mesa/drivers/dri/i965/brw_queryobj.c | 32 ++++++++++++++++++++++++++---
> 1 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_queryobj.c
> b/src/mesa/drivers/dri/i965/brw_queryobj.c
> index dae7af0..36de43e 100644
> --- a/src/mesa/drivers/dri/i965/brw_queryobj.c
> +++ b/src/mesa/drivers/dri/i965/brw_queryobj.c
> @@ -131,14 +131,26 @@ brw_begin_query(struct gl_context *ctx, struct
> gl_query_object *q)
> 4096, 4096);
>
> if (intel->gen >= 6) {
> - BEGIN_BATCH(4);
> - OUT_BATCH(_3DSTATE_PIPE_CONTROL);
> + BEGIN_BATCH(9);
> +
> + /* Workaround: A non-zero post-sync op (i.e. the DEPTH_COUNT write
> below
> + * needs a pipe control with CS_STALL set beforehand.
> + * Workaround: CS_STALL can't be set alone, we pick
> STALL_AT_SCOREBOARD
> + * like the kernel. */
It's a timestamp write here, not depth count. :) I would actually
prefer a comment along the lines of:
/* Workaround: A non-zero post-sync op (i.e. the timestamp write below)
* must be preceded by a CS stall. See rationale in the comments for
* intel_emit_post_sync_nonzero_flush().
*/
The workaround chain is pretty well explained in the comments on that
function, and so may be more helpful than "like the kernel."
As for the substance of the patch (actually adding the CS stall), that
seems reasonable to me. Odd though. Normally failing to do this
results in GPU hangs. Haven't seen any of those. Still.
> + OUT_BATCH(_3DSTATE_PIPE_CONTROL);
> + OUT_BATCH(PIPE_CONTROL_CS_STALL |
> + PIPE_CONTROL_STALL_AT_SCOREBOARD);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> +
> + OUT_BATCH(_3DSTATE_PIPE_CONTROL_5);
OUT_BATCH(_3DSTATE_PIPE_CONTROL | (5 - 2))
> OUT_BATCH(PIPE_CONTROL_WRITE_TIMESTAMP);
> OUT_RELOC(query->bo,
> I915_GEM_DOMAIN_INSTRUCTION, I915_GEM_DOMAIN_INSTRUCTION,
> PIPE_CONTROL_GLOBAL_GTT_WRITE |
> 0);
> OUT_BATCH(0);
> + OUT_BATCH(0);
> ADVANCE_BATCH();
>
> } else {
> @@ -199,14 +211,26 @@ brw_end_query(struct gl_context *ctx, struct
> gl_query_object *q)
> switch (query->Base.Target) {
> case GL_TIME_ELAPSED_EXT:
> if (intel->gen >= 6) {
> - BEGIN_BATCH(4);
> - OUT_BATCH(_3DSTATE_PIPE_CONTROL);
> + BEGIN_BATCH(9);
> +
> + /* Workaround: A non-zero post-sync op (i.e. the DEPTH_COUNT write
> below
> + * needs a pipe control with CS_STALL set beforehand.
> + * Workaround: CS_STALL can't be set alone, we pick
> STALL_AT_SCOREBOARD
> + * like the kernel. */
ditto (comment)
> + OUT_BATCH(_3DSTATE_PIPE_CONTROL);
> + OUT_BATCH(PIPE_CONTROL_CS_STALL |
> + PIPE_CONTROL_STALL_AT_SCOREBOARD);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> +
> + OUT_BATCH(_3DSTATE_PIPE_CONTROL_5);
ditto (5 - 2)
> OUT_BATCH(PIPE_CONTROL_WRITE_TIMESTAMP);
> OUT_RELOC(query->bo,
> I915_GEM_DOMAIN_INSTRUCTION, I915_GEM_DOMAIN_INSTRUCTION,
> PIPE_CONTROL_GLOBAL_GTT_WRITE |
> 8);
> OUT_BATCH(0);
> + OUT_BATCH(0);
> ADVANCE_BATCH();
>
> } else {
With those changes,
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
