Iago Toral Quiroga <ito...@igalia.com> writes:
> Some drivers can disable the FS unit if there is nothing in the shader code
> that writes to an output (i.e. color, depth, etc). Right now, mesa has
> a function to check for atomic buffers and the i965 driver also checks for
> images. Refactor this logic into a generic function that we can use for
> any source of side effects in a fragment shader. Sugested by Jason.
> ---
> src/mesa/drivers/dri/i965/gen7_wm_state.c | 6 +-----
> src/mesa/drivers/dri/i965/gen8_ps_state.c | 3 +--
> src/mesa/main/mtypes.h | 15 ++++++++++++---
> 3 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_state.c
> b/src/mesa/drivers/dri/i965/gen7_wm_state.c
> index 06d5e65..a6d1028 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_state.c
> @@ -77,13 +77,9 @@ upload_wm_state(struct brw_context *brw)
> dw1 |= GEN7_WM_KILL_ENABLE;
> }
>
> - if (_mesa_active_fragment_shader_has_atomic_ops(&brw->ctx)) {
> - dw1 |= GEN7_WM_DISPATCH_ENABLE;
> - }
> -
> /* _NEW_BUFFERS | _NEW_COLOR */
> if (brw_color_buffer_write_enabled(brw) || writes_depth ||
> - prog_data->base.nr_image_params ||
> + _mesa_active_fragment_shader_has_side_effects(&brw->ctx) ||
> dw1 & GEN7_WM_KILL_ENABLE) {
> dw1 |= GEN7_WM_DISPATCH_ENABLE;
> }
Hey, it looks like SSBOs are still missing a couple of things that could
make their side effects rather non-deterministic on i965 hardware: On
HSW you should probably set the UAV_ONLY WM state bit when there are no
colour or depth buffer writes as is done for images below in this same
function, and on all hardware you should set the early depth/stencil
control field to PSEXEC unless early fragment tests are enabled to make
sure that the fragment shader is executed regardless of whether
per-fragment tests pass or not as the spec requires.
> diff --git a/src/mesa/drivers/dri/i965/gen8_ps_state.c
> b/src/mesa/drivers/dri/i965/gen8_ps_state.c
> index 945f710..3cc8c68 100644
> --- a/src/mesa/drivers/dri/i965/gen8_ps_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_ps_state.c
> @@ -90,8 +90,7 @@ gen8_upload_ps_extra(struct brw_context *brw,
> *
> * BRW_NEW_FS_PROG_DATA | BRW_NEW_FRAGMENT_PROGRAM | _NEW_BUFFERS |
> _NEW_COLOR
> */
> - if ((_mesa_active_fragment_shader_has_atomic_ops(&brw->ctx) ||
> - prog_data->base.nr_image_params) &&
> + if (_mesa_active_fragment_shader_has_side_effects(&brw->ctx) &&
> !brw_color_buffer_write_enabled(brw))
> dw1 |= GEN8_PSX_SHADER_HAS_UAV;
>
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 191a9ea..834ba59 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -4538,11 +4538,20 @@ enum _debug
> DEBUG_INCOMPLETE_FBO = (1 << 3)
> };
>
> +/**
> + * Checks if the active fragment shader program can have side effects due
> + * to use of things like atomic buffers or images
> + */
> static inline bool
> -_mesa_active_fragment_shader_has_atomic_ops(const struct gl_context *ctx)
> +_mesa_active_fragment_shader_has_side_effects(const struct gl_context *ctx)
> {
> - return ctx->Shader._CurrentFragmentProgram != NULL &&
> -
> ctx->Shader._CurrentFragmentProgram->_LinkedShaders[MESA_SHADER_FRAGMENT]->NumAtomicBuffers
> > 0;
> + const struct gl_shader *sh;
> +
> + if (!ctx->Shader._CurrentFragmentProgram)
> + return false;
> +
> + sh =
> ctx->Shader._CurrentFragmentProgram->_LinkedShaders[MESA_SHADER_FRAGMENT];
> + return sh->NumAtomicBuffers > 0 || sh->NumImages > 0;
> }
>
> #ifdef __cplusplus
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
