Re: [Mesa-dev] [PATCH] i965: Move PSCDEPTH calculations from draw time to compile time.
On Wed, Dec 3, 2014 at 5:58 PM, Kenneth Graunke kenn...@whitecape.org wrote: On Tuesday, December 02, 2014 10:34:49 AM Matt Turner wrote: On Tue, Dec 2, 2014 at 3:50 AM, Kenneth Graunke kenn...@whitecape.org wrote: The Pixel Shader Computed Depth Mode value is entirely based on the shader program, so we can easily do it at compile time. This avoids the if+switch on every 3DSTATE_WM (Gen7)/3DSTATE_PS_EXTRA (Gen8+) upload, and shares a bit more code. This also simplifies the PMA stall code, making it match the formula more closely, and drops a BRW_NEW_FRAGMENT_PROGRAM dependency. (Note that the previous comment was wrong - the code and the documentation have != PSCDEPTH_OFF, not ==.) Signed-off-by: Kenneth Graunke kenn...@whitecape.org --- src/mesa/drivers/dri/i965/brw_context.h | 2 ++ src/mesa/drivers/dri/i965/brw_defines.h | 17 + src/mesa/drivers/dri/i965/brw_wm.c | 21 + src/mesa/drivers/dri/i965/gen7_wm_state.c| 22 +++--- src/mesa/drivers/dri/i965/gen8_depth_state.c | 12 src/mesa/drivers/dri/i965/gen8_ps_state.c| 18 +- 6 files changed, 40 insertions(+), 52 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index ec4b3dd..b4ddc17 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -390,6 +390,8 @@ struct brw_wm_prog_data { /** @} */ } binding_table; + uint8_t computed_depth_mode; Presumably we should use the new enum type here (and below in the function call), and mark the enum definition PACKED. With that, Reviewed-by: Matt Turner matts...@gmail.com Kind of painful - the enum is defined in brw_defines.h; this field is in brw_context.h which doesn't #include that. Minor benefit. I guess we could move the enum declaration to brw_context.h, but...*shrug*? Ah, okay. That'd be annoying. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Move PSCDEPTH calculations from draw time to compile time.
On Tuesday, December 02, 2014 10:34:49 AM Matt Turner wrote: On Tue, Dec 2, 2014 at 3:50 AM, Kenneth Graunke kenn...@whitecape.org wrote: The Pixel Shader Computed Depth Mode value is entirely based on the shader program, so we can easily do it at compile time. This avoids the if+switch on every 3DSTATE_WM (Gen7)/3DSTATE_PS_EXTRA (Gen8+) upload, and shares a bit more code. This also simplifies the PMA stall code, making it match the formula more closely, and drops a BRW_NEW_FRAGMENT_PROGRAM dependency. (Note that the previous comment was wrong - the code and the documentation have != PSCDEPTH_OFF, not ==.) Signed-off-by: Kenneth Graunke kenn...@whitecape.org --- src/mesa/drivers/dri/i965/brw_context.h | 2 ++ src/mesa/drivers/dri/i965/brw_defines.h | 17 + src/mesa/drivers/dri/i965/brw_wm.c | 21 + src/mesa/drivers/dri/i965/gen7_wm_state.c| 22 +++--- src/mesa/drivers/dri/i965/gen8_depth_state.c | 12 src/mesa/drivers/dri/i965/gen8_ps_state.c| 18 +- 6 files changed, 40 insertions(+), 52 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index ec4b3dd..b4ddc17 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -390,6 +390,8 @@ struct brw_wm_prog_data { /** @} */ } binding_table; + uint8_t computed_depth_mode; Presumably we should use the new enum type here (and below in the function call), and mark the enum definition PACKED. With that, Reviewed-by: Matt Turner matts...@gmail.com Kind of painful - the enum is defined in brw_defines.h; this field is in brw_context.h which doesn't #include that. Minor benefit. I guess we could move the enum declaration to brw_context.h, but...*shrug*? signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Move PSCDEPTH calculations from draw time to compile time.
The Pixel Shader Computed Depth Mode value is entirely based on the shader program, so we can easily do it at compile time. This avoids the if+switch on every 3DSTATE_WM (Gen7)/3DSTATE_PS_EXTRA (Gen8+) upload, and shares a bit more code. This also simplifies the PMA stall code, making it match the formula more closely, and drops a BRW_NEW_FRAGMENT_PROGRAM dependency. (Note that the previous comment was wrong - the code and the documentation have != PSCDEPTH_OFF, not ==.) Signed-off-by: Kenneth Graunke kenn...@whitecape.org --- src/mesa/drivers/dri/i965/brw_context.h | 2 ++ src/mesa/drivers/dri/i965/brw_defines.h | 17 + src/mesa/drivers/dri/i965/brw_wm.c | 21 + src/mesa/drivers/dri/i965/gen7_wm_state.c| 22 +++--- src/mesa/drivers/dri/i965/gen8_depth_state.c | 12 src/mesa/drivers/dri/i965/gen8_ps_state.c| 18 +- 6 files changed, 40 insertions(+), 52 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index ec4b3dd..b4ddc17 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -390,6 +390,8 @@ struct brw_wm_prog_data { /** @} */ } binding_table; + uint8_t computed_depth_mode; + bool no_8; bool dual_src_blend; bool uses_pos_offset; diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h index adcf1db..2acd0f8 100644 --- a/src/mesa/drivers/dri/i965/brw_defines.h +++ b/src/mesa/drivers/dri/i965/brw_defines.h @@ -2051,16 +2051,20 @@ enum brw_message_target { # define GEN9_WM_DS_BF_STENCIL_REF_MASK INTEL_MASK(7, 0) # define GEN9_WM_DS_BF_STENCIL_REF_SHIFT0 +enum brw_pixel_shader_computed_depth_mode { + BRW_PSCDEPTH_OFF = 0, /* PS does not compute depth */ + BRW_PSCDEPTH_ON= 1, /* PS computes depth; no guarantee about value */ + BRW_PSCDEPTH_ON_GE = 2, /* PS guarantees output depth = source depth */ + BRW_PSCDEPTH_ON_LE = 3, /* PS guarantees output depth = source depth */ +}; + #define _3DSTATE_PS_EXTRA 0x784F /* GEN8+ */ /* DW1 */ # define GEN8_PSX_PIXEL_SHADER_VALID(1 31) # define GEN8_PSX_PIXEL_SHADER_NO_RT_WRITE (1 30) # define GEN8_PSX_OMASK_TO_RENDER_TARGET(1 29) # define GEN8_PSX_KILL_ENABLE (1 28) -# define GEN8_PSX_PSCDEPTH_OFF (0 26) -# define GEN8_PSX_PSCDEPTH_ON (1 26) -# define GEN8_PSX_PSCDEPTH_ON_GE(2 26) -# define GEN8_PSX_PSCDEPTH_ON_LE(3 26) +# define GEN8_PSX_COMPUTED_DEPTH_MODE_SHIFT 26 # define GEN8_PSX_FORCE_COMPUTED_DEPTH (1 25) # define GEN8_PSX_USES_SOURCE_DEPTH (1 24) # define GEN8_PSX_USES_SOURCE_W (1 23) @@ -2202,10 +2206,7 @@ enum brw_wm_barycentric_interp_mode { # define GEN7_WM_DEPTH_RESOLVE (1 28) # define GEN7_WM_HIERARCHICAL_DEPTH_RESOLVE(1 27) # define GEN7_WM_KILL_ENABLE (1 25) -# define GEN7_WM_PSCDEPTH_OFF (0 23) -# define GEN7_WM_PSCDEPTH_ON (1 23) -# define GEN7_WM_PSCDEPTH_ON_GE(2 23) -# define GEN7_WM_PSCDEPTH_ON_LE(3 23) +# define GEN7_WM_COMPUTED_DEPTH_MODE_SHIFT 23 # define GEN7_WM_USES_SOURCE_DEPTH (1 20) # define GEN7_WM_USES_SOURCE_W (1 19) # define GEN7_WM_POSITION_ZW_PIXEL (0 17) diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c index fe36dd4..7badb23 100644 --- a/src/mesa/drivers/dri/i965/brw_wm.c +++ b/src/mesa/drivers/dri/i965/brw_wm.c @@ -116,6 +116,25 @@ brw_compute_barycentric_interp_modes(struct brw_context *brw, return barycentric_interp_modes; } +static uint8_t +computed_depth_mode(struct gl_fragment_program *fp) +{ + if (fp-Base.OutputsWritten BITFIELD64_BIT(FRAG_RESULT_DEPTH)) { + switch (fp-FragDepthLayout) { + case FRAG_DEPTH_LAYOUT_NONE: + case FRAG_DEPTH_LAYOUT_ANY: + return BRW_PSCDEPTH_ON; + case FRAG_DEPTH_LAYOUT_GREATER: + return BRW_PSCDEPTH_ON_GE; + case FRAG_DEPTH_LAYOUT_LESS: + return BRW_PSCDEPTH_ON_LE; + case FRAG_DEPTH_LAYOUT_UNCHANGED: + return BRW_PSCDEPTH_OFF; + } + } + return BRW_PSCDEPTH_OFF; +} + bool brw_wm_prog_data_compare(const void *in_a, const void *in_b) { @@ -161,6 +180,8 @@ bool do_wm_prog(struct brw_context *brw, */ prog_data.uses_kill = fp-program.UsesKill || key-alpha_test_func; + prog_data.computed_depth_mode = computed_depth_mode(fp-program); + /* Allocate the references to the uniforms that will end
Re: [Mesa-dev] [PATCH] i965: Move PSCDEPTH calculations from draw time to compile time.
On Tue, Dec 2, 2014 at 3:50 AM, Kenneth Graunke kenn...@whitecape.org wrote: The Pixel Shader Computed Depth Mode value is entirely based on the shader program, so we can easily do it at compile time. This avoids the if+switch on every 3DSTATE_WM (Gen7)/3DSTATE_PS_EXTRA (Gen8+) upload, and shares a bit more code. This also simplifies the PMA stall code, making it match the formula more closely, and drops a BRW_NEW_FRAGMENT_PROGRAM dependency. (Note that the previous comment was wrong - the code and the documentation have != PSCDEPTH_OFF, not ==.) Signed-off-by: Kenneth Graunke kenn...@whitecape.org --- src/mesa/drivers/dri/i965/brw_context.h | 2 ++ src/mesa/drivers/dri/i965/brw_defines.h | 17 + src/mesa/drivers/dri/i965/brw_wm.c | 21 + src/mesa/drivers/dri/i965/gen7_wm_state.c| 22 +++--- src/mesa/drivers/dri/i965/gen8_depth_state.c | 12 src/mesa/drivers/dri/i965/gen8_ps_state.c| 18 +- 6 files changed, 40 insertions(+), 52 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index ec4b3dd..b4ddc17 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -390,6 +390,8 @@ struct brw_wm_prog_data { /** @} */ } binding_table; + uint8_t computed_depth_mode; Presumably we should use the new enum type here (and below in the function call), and mark the enum definition PACKED. With that, Reviewed-by: Matt Turner matts...@gmail.com ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev