[Mesa-dev] [PATCH 12/12] i965: Simplify handling of VUE map changes.

Chris Forbes chrisf at ijw.co.nz
Thu Sep 3 15:57:09 PDT 2015


This had got pretty tangled.

For the series:

Reviewed-by: Chris Forbes <chrisf at ijw.co.nz>

On Sat, Aug 29, 2015 at 9:24 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> The old code was disasterously complex - spread across multiple atoms
> which may not even run, inspecting the dirty bits to try and decide
> whether it was necessary to do checks...storing VS information in
> brw_context...extra flagging...
>
> This code tripped me and Carl up very badly when working on the
> shader cache code.  It's very fragile and hard to maintain.
>
> Now that geometry shaders only depend on their inputs and don't have
> to worry about the VS VUE map, we can dramatically simplify this:
> just compute the VUE map coming out of the geometry shader stage
> in brw_upload_programs.  If it changes, flag it.  Done.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_context.h      | 12 +-----------
>  src/mesa/drivers/dri/i965/brw_gs.c           | 14 +-------------
>  src/mesa/drivers/dri/i965/brw_state_upload.c | 14 +++++++++++++-
>  src/mesa/drivers/dri/i965/brw_vs.c           | 13 -------------
>  4 files changed, 15 insertions(+), 38 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index c9c47dd..91258be 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -194,7 +194,6 @@ enum brw_state_id {
>     BRW_STATE_GS_CONSTBUF,
>     BRW_STATE_PROGRAM_CACHE,
>     BRW_STATE_STATE_BASE_ADDRESS,
> -   BRW_STATE_VUE_MAP_VS,
>     BRW_STATE_VUE_MAP_GEOM_OUT,
>     BRW_STATE_TRANSFORM_FEEDBACK,
>     BRW_STATE_RASTERIZER_DISCARD,
> @@ -276,7 +275,6 @@ enum brw_state_id {
>  #define BRW_NEW_GS_CONSTBUF             (1ull << BRW_STATE_GS_CONSTBUF)
>  #define BRW_NEW_PROGRAM_CACHE           (1ull << BRW_STATE_PROGRAM_CACHE)
>  #define BRW_NEW_STATE_BASE_ADDRESS      (1ull << BRW_STATE_STATE_BASE_ADDRESS)
> -#define BRW_NEW_VUE_MAP_VS              (1ull << BRW_STATE_VUE_MAP_VS)
>  #define BRW_NEW_VUE_MAP_GEOM_OUT        (1ull << BRW_STATE_VUE_MAP_GEOM_OUT)
>  #define BRW_NEW_TRANSFORM_FEEDBACK      (1ull << BRW_STATE_TRANSFORM_FEEDBACK)
>  #define BRW_NEW_RASTERIZER_DISCARD      (1ull << BRW_STATE_RASTERIZER_DISCARD)
> @@ -1362,16 +1360,8 @@ struct brw_context
>     } curbe;
>
>     /**
> -    * Layout of vertex data exiting the vertex shader.
> -    *
> -    * BRW_NEW_VUE_MAP_VS is flagged when this VUE map changes.
> -    */
> -   struct brw_vue_map vue_map_vs;
> -
> -   /**
>      * Layout of vertex data exiting the geometry portion of the pipleine.
> -    * This comes from the geometry shader if one exists, otherwise from the
> -    * vertex shader.
> +    * This comes from the last enabled shader stage (GS, DS, or VS).
>      *
>      * BRW_NEW_VUE_MAP_GEOM_OUT is flagged when the VUE map changes.
>      */
> diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c
> index da8af88..6b25150 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs.c
> @@ -292,8 +292,7 @@ brw_gs_state_dirty(struct brw_context *brw)
>     return brw_state_dirty(brw,
>                            _NEW_TEXTURE,
>                            BRW_NEW_GEOMETRY_PROGRAM |
> -                          BRW_NEW_TRANSFORM_FEEDBACK |
> -                          BRW_NEW_VUE_MAP_VS);
> +                          BRW_NEW_TRANSFORM_FEEDBACK);
>  }
>
>  static void
> @@ -331,11 +330,6 @@ brw_upload_gs_prog(struct brw_context *brw)
>
>     if (gp == NULL) {
>        /* No geometry shader.  Vertex data just passes straight through. */
> -      if (brw->ctx.NewDriverState & BRW_NEW_VUE_MAP_VS) {
> -         brw->vue_map_geom_out = brw->vue_map_vs;
> -         brw->ctx.NewDriverState |= BRW_NEW_VUE_MAP_GEOM_OUT;
> -      }
> -
>        if (brw->gen == 6 &&
>            (brw->ctx.NewDriverState & BRW_NEW_TRANSFORM_FEEDBACK)) {
>           gen6_brw_upload_ff_gs_prog(brw);
> @@ -362,12 +356,6 @@ brw_upload_gs_prog(struct brw_context *brw)
>        (void)success;
>     }
>     brw->gs.base.prog_data = &brw->gs.prog_data->base.base;
> -
> -   if (brw->gs.prog_data->base.vue_map.slots_valid !=
> -       brw->vue_map_geom_out.slots_valid) {
> -      brw->vue_map_geom_out = brw->gs.prog_data->base.vue_map;
> -      brw->ctx.NewDriverState |= BRW_NEW_VUE_MAP_GEOM_OUT;
> -   }
>  }
>
>  bool
> diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c
> index 9de42ce..22161fc 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
> @@ -589,7 +589,6 @@ static struct dirty_bit_map brw_bits[] = {
>     DEFINE_BIT(BRW_NEW_GS_CONSTBUF),
>     DEFINE_BIT(BRW_NEW_PROGRAM_CACHE),
>     DEFINE_BIT(BRW_NEW_STATE_BASE_ADDRESS),
> -   DEFINE_BIT(BRW_NEW_VUE_MAP_VS),
>     DEFINE_BIT(BRW_NEW_VUE_MAP_GEOM_OUT),
>     DEFINE_BIT(BRW_NEW_TRANSFORM_FEEDBACK),
>     DEFINE_BIT(BRW_NEW_RASTERIZER_DISCARD),
> @@ -644,6 +643,19 @@ brw_upload_programs(struct brw_context *brw,
>        else
>           brw_upload_gs_prog(brw);
>
> +      /* Update the VUE map for data exiting the GS stage of the pipeline.
> +       * This comes from the last enabled shader stage.
> +       */
> +      GLbitfield64 old_slots = brw->vue_map_geom_out.slots_valid;
> +      if (brw->geometry_program)
> +         brw->vue_map_geom_out = brw->gs.prog_data->base.vue_map;
> +      else
> +         brw->vue_map_geom_out = brw->vs.prog_data->base.vue_map;
> +
> +      /* If the layout has changed, signal BRW_NEW_VUE_MAP_GEOM_OUT. */
> +      if (old_slots != brw->vue_map_geom_out.slots_valid)
> +         brw->ctx.NewDriverState |= BRW_NEW_VUE_MAP_GEOM_OUT;
> +
>        brw_upload_wm_prog(brw);
>     } else if (pipeline == BRW_COMPUTE_PIPELINE) {
>        brw_upload_cs_prog(brw);
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
> index 4e0d34f..3aa6d4f 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -367,19 +367,6 @@ brw_upload_vs_prog(struct brw_context *brw)
>        assert(success);
>     }
>     brw->vs.base.prog_data = &brw->vs.prog_data->base.base;
> -
> -   if (brw->vs.prog_data->base.vue_map.slots_valid !=
> -       brw->vue_map_geom_out.slots_valid) {
> -      brw->vue_map_vs = brw->vs.prog_data->base.vue_map;
> -      brw->ctx.NewDriverState |= BRW_NEW_VUE_MAP_VS;
> -      if (brw->gen < 6) {
> -         /* No geometry shader support, so the VS VUE map is the VUE map for
> -          * the output of the "geometry" portion of the pipeline.
> -          */
> -         brw->vue_map_geom_out = brw->vue_map_vs;
> -         brw->ctx.NewDriverState |= BRW_NEW_VUE_MAP_GEOM_OUT;
> -      }
> -   }
>  }
>
>  bool
> --
> 2.5.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list