[Mesa-dev] [PATCH 3/4] i965/state: Create separate dirty state bits for each pipeline

[Kristian Høgsberg]

On Tue, Mar 10, 2015 at 10:49 AM, Jordan Justen
<jordan.l.justen at intel.com> wrote:
> When uploading state for a pipeline, we will save changed state for
> the other pipelines.
>
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_context.h      |  1 +
>  src/mesa/drivers/dri/i965/brw_state_upload.c | 41 ++++++++++++++++++++++------
>  2 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index fd43b07..902de18 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1101,6 +1101,7 @@ struct brw_context
>     GLuint NewGLState;
>     struct {
>        struct brw_state_flags dirty;
> +      struct brw_state_flags pipelines[BRW_NUM_PIPELINES];
>     } state;
>
>     struct brw_cache cache;
> diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c
> index dbfdc92..6966f06 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
> @@ -580,15 +580,16 @@ brw_upload_programs(struct brw_context *brw)
>     brw_upload_wm_prog(brw);
>  }
>
> -/***********************************************************************
> - * Emit all state:
> - */
> -void brw_upload_render_state(struct brw_context *brw)
> +static inline void
> +brw_upload_pipeline_state(struct brw_context *brw,
> +                          enum brw_pipeline pipeline)
>  {
>     struct gl_context *ctx = &brw->ctx;
>     struct brw_state_flags *state = &brw->state.dirty;
>     int i;
>     static int dirty_count = 0;
> +   struct brw_state_flags *pipeline_state =
> +      &brw->state.pipelines[pipeline];
>
>     state->mesa |= brw->NewGLState;
>     brw->NewGLState = 0;
> @@ -627,6 +628,12 @@ void brw_upload_render_state(struct brw_context *brw)
>        brw->state.dirty.brw |= BRW_NEW_NUM_SAMPLES;
>     }
>
> +   if ((pipeline_state->mesa | pipeline_state->brw) != 0) {
> +      state->mesa |= pipeline_state->mesa;
> +      state->brw |= pipeline_state->brw;
> +      memset(pipeline_state, 0, sizeof(struct brw_state_flags));
> +   }
> +
>     if ((state->mesa | state->brw) == 0)
>        return;
>
> @@ -636,6 +643,9 @@ void brw_upload_render_state(struct brw_context *brw)
>
>     brw_upload_programs(brw);
>
> +   const struct brw_tracked_state *atoms =
> +      brw_pipeline_first_atom(brw, pipeline);

Instead of looping over num_atoms[] to determine the start of the
compute pipeline atoms, I think it'd be simpler to just have the
compute atoms be a separate array in struct brw_context.

>     if (unlikely(INTEL_DEBUG)) {
>        /* Debug version which enforces various sanity checks on the
>         * state flags which are generated and checked to help ensure
> @@ -645,8 +655,8 @@ void brw_upload_render_state(struct brw_context *brw)
>        memset(&examined, 0, sizeof(examined));
>        prev = *state;
>
> -      for (i = 0; i < brw->num_atoms[BRW_RENDER_PIPELINE]; i++) {
> -        const struct brw_tracked_state *atom = &brw->atoms[i];
> +      for (i = 0; i < brw->num_atoms[pipeline]; i++) {

This makes the loop condition a more complex expression.  I don't
think gcc computes and caches brw->num_atoms[pipeline] outside the
loop so this adds a bit of code to this very hot loop.  I'd try
something like

  int num_atoms = brw->num_atoms[pipeline]

and use num_atoms in the loop condition instead.  It may be the source
of the slight performance regression on OglBatch7.

> +        const struct brw_tracked_state *atom = &atoms[i];
>          struct brw_state_flags generated;
>
>          if (check_state(state, &atom->dirty)) {
> @@ -665,8 +675,8 @@ void brw_upload_render_state(struct brw_context *brw)
>        }
>     }
>     else {
> -      for (i = 0; i < brw->num_atoms[BRW_RENDER_PIPELINE]; i++) {
> -        const struct brw_tracked_state *atom = &brw->atoms[i];
> +      for (i = 0; i < brw->num_atoms[pipeline]; i++) {
> +        const struct brw_tracked_state *atom = &atoms[i];
>
>          if (check_state(state, &atom->dirty)) {
>             atom->emit(brw);
> @@ -685,8 +695,23 @@ void brw_upload_render_state(struct brw_context *brw)
>          fprintf(stderr, "\n");
>        }
>     }
> +
> +   /* Save all dirty state into the other pipelines */
> +   for (int i = 0; i < BRW_NUM_PIPELINES; i++) {
> +      if (i != pipeline) {
> +         brw->state.pipelines[i].mesa |= state->mesa;
> +         brw->state.pipelines[i].brw |= state->brw;
> +      }
> +   }
>  }
>
> +/***********************************************************************
> + * Emit all state:
> + */
> +void brw_upload_render_state(struct brw_context *brw)
> +{
> +   brw_upload_pipeline_state(brw, BRW_RENDER_PIPELINE);
> +}
>
>  /**
>   * Clear dirty bits to account for the fact that the state emitted by
> --
> 2.1.4
>