[Mesa-dev] [PATCH v2 1/2] mesa: Add a _mesa_active_fragment_shader_has_side_effects helper

[Tapani Pälli]

On 12/16/2015 10:59 AM, Iago Toral Quiroga wrote:
> 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;
>      }
> 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;

If you make this use '_Shader' it will fix also following test:

ES31-CTS.shader_atomic_counters.advanced-usage-many-draw-calls2

This is filed as bug here:
https://bugs.freedesktop.org/show_bug.cgi?id=93317

(sorry for late notice, took quite some time to figure this out :/)

>   }
>
>   #ifdef __cplusplus
>