[Mesa-dev] [PATCH 25/30] i965/gs: make the state atom for compiling Gen7 geometry shaders.
Kenneth Graunke
kenneth at whitecape.org
Thu Aug 22 14:22:53 PDT 2013
On 08/20/2013 11:30 AM, Paul Berry wrote:
> ---
> src/mesa/drivers/dri/i965/Makefile.sources | 1 +
> src/mesa/drivers/dri/i965/brw_context.h | 2 +
> src/mesa/drivers/dri/i965/brw_defines.h | 10 +
> src/mesa/drivers/dri/i965/brw_state.h | 1 +
> src/mesa/drivers/dri/i965/brw_state_cache.c | 3 +
> src/mesa/drivers/dri/i965/brw_state_dump.c | 5 +-
> src/mesa/drivers/dri/i965/brw_state_upload.c | 1 +
> src/mesa/drivers/dri/i965/brw_vec4_gs.c | 286 ++++++++++++++++++++++
> src/mesa/drivers/dri/i965/brw_vec4_gs.h | 41 ++++
> src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp | 32 +++
> src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h | 13 +
> src/mesa/drivers/dri/i965/brw_vs.c | 13 +-
> 12 files changed, 401 insertions(+), 7 deletions(-)
> create mode 100644 src/mesa/drivers/dri/i965/brw_vec4_gs.c
> create mode 100644 src/mesa/drivers/dri/i965/brw_vec4_gs.h
>
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources b/src/mesa/drivers/dri/i965/Makefile.sources
> index 81a16ff..1f3abac 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -88,6 +88,7 @@ i965_FILES = \
> brw_vec4.cpp \
> brw_vec4_copy_propagation.cpp \
> brw_vec4_emit.cpp \
> + brw_vec4_gs.c \
> brw_vec4_gs_visitor.cpp \
> brw_vec4_live_variables.cpp \
> brw_vec4_reg_allocate.cpp \
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index a6d0786..2addf99 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -693,6 +693,7 @@ enum brw_cache_id {
> BRW_VS_PROG,
> BRW_GS_UNIT,
> BRW_GS_PROG,
> + BRW_VEC4_GS_PROG,
> BRW_CLIP_VP,
> BRW_CLIP_UNIT,
> BRW_CLIP_PROG,
> @@ -783,6 +784,7 @@ enum shader_time_shader_type {
> #define CACHE_NEW_VS_PROG (1<<BRW_VS_PROG)
> #define CACHE_NEW_GS_UNIT (1<<BRW_GS_UNIT)
> #define CACHE_NEW_GS_PROG (1<<BRW_GS_PROG)
> +#define CACHE_NEW_VEC4_GS_PROG (1<<BRW_VEC4_GS_PROG)
I would love to see the old ones renamed to
BRW_FF_GS_PROG/CACHE_NEW_FF_GS_PROG and new ones added as
BRW_GS_PROG/CACHE_NEW_GS_PROG.
I think that anyone working on the geometry shader code is going to
forget the "VEC4" and start using the old names.
> #define CACHE_NEW_CLIP_VP (1<<BRW_CLIP_VP)
> #define CACHE_NEW_CLIP_UNIT (1<<BRW_CLIP_UNIT)
> #define CACHE_NEW_CLIP_PROG (1<<BRW_CLIP_PROG)
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
> index 832ff55..58bdd69 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -1283,6 +1283,11 @@ enum brw_message_target {
> # define GEN7_URB_ENTRY_SIZE_SHIFT 16
> # define GEN7_URB_STARTING_ADDRESS_SHIFT 25
>
> +/* "GS URB Entry Allocation Size" is a U9-1 field, so the maximum gs_size
> + * is 2^9, or 512. It's counted in multiples of 64 bytes.
> + */
> +#define GEN7_MAX_GS_URB_ENTRY_SIZE_BYTES (512*64)
> +
> #define _3DSTATE_PUSH_CONSTANT_ALLOC_VS 0x7912 /* GEN7+ */
> #define _3DSTATE_PUSH_CONSTANT_ALLOC_PS 0x7916 /* GEN7+ */
> # define GEN7_PUSH_CONSTANT_BUFFER_OFFSET_SHIFT 16
> @@ -1347,6 +1352,11 @@ enum brw_message_target {
> # define BRW_GS_EDGE_INDICATOR_0 (1 << 8)
> # define BRW_GS_EDGE_INDICATOR_1 (1 << 9)
>
> +/* 3DSTATE_GS "Output Vertex Size" has an effective maximum of 62. It's
> + * counted in multiples of 16 bytes.
> + */
> +#define GEN7_MAX_GS_OUTPUT_VERTEX_SIZE_BYTES (62*16)
> +
> #define _3DSTATE_HS 0x781B /* GEN7+ */
> #define _3DSTATE_TE 0x781C /* GEN7+ */
> #define _3DSTATE_DS 0x781D /* GEN7+ */
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
> index 8597687..8a2b77e 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -72,6 +72,7 @@ extern const struct brw_tracked_state brw_vs_samplers;
> extern const struct brw_tracked_state brw_vs_ubo_surfaces;
> extern const struct brw_tracked_state brw_gs_ubo_surfaces;
> extern const struct brw_tracked_state brw_vs_unit;
> +extern const struct brw_tracked_state brw_vec4_gs_prog;
> extern const struct brw_tracked_state brw_wm_prog;
> extern const struct brw_tracked_state brw_renderbuffer_surfaces;
> extern const struct brw_tracked_state brw_texture_surfaces;
> diff --git a/src/mesa/drivers/dri/i965/brw_state_cache.c b/src/mesa/drivers/dri/i965/brw_state_cache.c
> index ddb275f..f07fb4a 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_cache.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_cache.c
> @@ -50,6 +50,7 @@
> #include "brw_vs.h"
> #include "brw_wm.h"
> #include "brw_vs.h"
> +#include "brw_vec4_gs.h"
>
> #define FILE_DEBUG_FLAG DEBUG_STATE
>
> @@ -341,8 +342,10 @@ brw_init_caches(struct brw_context *brw)
> 4096, 64);
>
> cache->aux_compare[BRW_VS_PROG] = brw_vs_prog_data_compare;
> + cache->aux_compare[BRW_VEC4_GS_PROG] = brw_vec4_gs_prog_data_compare;
> cache->aux_compare[BRW_WM_PROG] = brw_wm_prog_data_compare;
> cache->aux_free[BRW_VS_PROG] = brw_vs_prog_data_free;
> + cache->aux_free[BRW_VEC4_GS_PROG] = brw_vec4_gs_prog_data_free;
> cache->aux_free[BRW_WM_PROG] = brw_wm_prog_data_free;
> }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_state_dump.c b/src/mesa/drivers/dri/i965/brw_state_dump.c
> index a42a049..b7160ff 100644
> --- a/src/mesa/drivers/dri/i965/brw_state_dump.c
> +++ b/src/mesa/drivers/dri/i965/brw_state_dump.c
> @@ -499,8 +499,11 @@ dump_prog_cache(struct brw_context *brw)
> name = "VS kernel";
> break;
> case BRW_GS_PROG:
> - name = "GS kernel";
> + name = "GS kernel (fixed function)";
> break;
> + case BRW_VEC4_GS_PROG:
> + name = "GS kernel";
> + break;
^^^ I think this is a pretty good rationale for renaming the caches.
I read through the rest of this patch and it looks great.
Modulo silly renaming,
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
More information about the mesa-dev
mailing list