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

Chris Forbes chrisf at ijw.co.nz
Sat Sep 26 02:41:30 PDT 2015


For the v2 series:

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

On Sat, Sep 12, 2015 at 6:58 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.
>
> v2: Also check vue_map.separable.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> Reviewed-by: Chris Forbes <chrisf at ijw.co.nz> [v1]
> ---
>  src/mesa/drivers/dri/i965/brw_context.h      | 12 +-----------
>  src/mesa/drivers/dri/i965/brw_gs.c           | 16 +---------------
>  src/mesa/drivers/dri/i965/brw_state_upload.c | 16 +++++++++++++++-
>  src/mesa/drivers/dri/i965/brw_vs.c           | 15 ---------------
>  4 files changed, 17 insertions(+), 42 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 772a9fd..4cac30c 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)
> @@ -1375,16 +1373,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 77be9d9..1f219c0 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs.c
> @@ -297,8 +297,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
> @@ -336,11 +335,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);
> @@ -367,14 +361,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->gs.prog_data->base.vue_map.separate !=
> -       brw->vue_map_geom_out.separate) {
> -      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 14627d5..89fde52 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
> @@ -593,7 +593,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),
> @@ -648,6 +647,21 @@ 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;
> +      bool old_separate = brw->vue_map_geom_out.separate;
> +      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 ||
> +          old_separate != brw->vue_map_geom_out.separate)
> +         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 adc1810..6f4c110 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -368,21 +368,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->vs.prog_data->base.vue_map.separate !=
> -       brw->vue_map_geom_out.separate) {
> -      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.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150926/d2f0e01f/attachment.html>


More information about the mesa-dev mailing list