[Mesa-dev] [PATCH 15/17] i965/fs: Move some of the prog_data setup into brw_wm_emit

Pohjolainen, Topi topi.pohjolainen at intel.com
Fri Oct 9 00:10:07 PDT 2015


On Thu, Oct 08, 2015 at 05:22:47PM -0700, Jason Ekstrand wrote:
> This commit moves the common/modern stuff.  Some legacy stuff such as
> setting use_alt_mode was left because it needs to know whether or not we're
> an ARB program.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 98 ++++++++++++++++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_wm.c   | 98 ------------------------------------
>  2 files changed, 98 insertions(+), 98 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 146f4b4..0e39b50 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -5114,6 +5114,90 @@ fs_visitor::run_cs()
>     return !failed;
>  }
>  
> +/**
> + * Return a bitfield where bit n is set if barycentric interpolation mode n
> + * (see enum brw_wm_barycentric_interp_mode) is needed by the fragment shader.
> + */
> +static unsigned
> +brw_compute_barycentric_interp_modes(const struct brw_device_info *devinfo,
> +                                     bool shade_model_flat,
> +                                     bool persample_shading,
> +                                     const nir_shader *shader)
> +{
> +   unsigned barycentric_interp_modes = 0;
> +
> +   nir_foreach_variable(var, &shader->inputs) {
> +      enum glsl_interp_qualifier interp_qualifier =
> +         (enum glsl_interp_qualifier)var->data.interpolation;
> +      bool is_centroid = var->data.centroid && !persample_shading;
> +      bool is_sample = var->data.sample || persample_shading;
> +      bool is_gl_Color = (var->data.location == VARYING_SLOT_COL0) ||
> +                         (var->data.location == VARYING_SLOT_COL1);
> +
> +      /* Ignore WPOS and FACE, because they don't require interpolation. */
> +      if (var->data.location == VARYING_SLOT_POS ||
> +          var->data.location == VARYING_SLOT_FACE)
> +         continue;
> +
> +      /* Determine the set (or sets) of barycentric coordinates needed to
> +       * interpolate this variable.  Note that when
> +       * brw->needs_unlit_centroid_workaround is set, centroid interpolation
> +       * uses PIXEL interpolation for unlit pixels and CENTROID interpolation
> +       * for lit pixels, so we need both sets of barycentric coordinates.
> +       */
> +      if (interp_qualifier == INTERP_QUALIFIER_NOPERSPECTIVE) {
> +         if (is_centroid) {
> +            barycentric_interp_modes |=
> +               1 << BRW_WM_NONPERSPECTIVE_CENTROID_BARYCENTRIC;
> +         } else if (is_sample) {
> +            barycentric_interp_modes |=
> +               1 << BRW_WM_NONPERSPECTIVE_SAMPLE_BARYCENTRIC;
> +         }
> +         if ((!is_centroid && !is_sample) ||
> +             devinfo->needs_unlit_centroid_workaround) {
> +            barycentric_interp_modes |=
> +               1 << BRW_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC;
> +         }
> +      } else if (interp_qualifier == INTERP_QUALIFIER_SMOOTH ||
> +                 (!(shade_model_flat && is_gl_Color) &&
> +                  interp_qualifier == INTERP_QUALIFIER_NONE)) {
> +         if (is_centroid) {
> +            barycentric_interp_modes |=
> +               1 << BRW_WM_PERSPECTIVE_CENTROID_BARYCENTRIC;
> +         } else if (is_sample) {
> +            barycentric_interp_modes |=
> +               1 << BRW_WM_PERSPECTIVE_SAMPLE_BARYCENTRIC;
> +         }
> +         if ((!is_centroid && !is_sample) ||
> +             devinfo->needs_unlit_centroid_workaround) {
> +            barycentric_interp_modes |=
> +               1 << BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC;
> +         }
> +      }
> +   }
> +
> +   return barycentric_interp_modes;
> +}
> +
> +static uint8_t
> +computed_depth_mode(const nir_shader *shader)
> +{
> +   if (shader->info.outputs_written & BITFIELD64_BIT(FRAG_RESULT_DEPTH)) {
> +      switch (shader->info.fs.depth_layout) {
> +      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;
> +}
> +
>  const unsigned *
>  brw_wm_fs_emit(const struct brw_compiler *compiler, void *log_data,
>                 void *mem_ctx,
> @@ -5126,6 +5210,20 @@ brw_wm_fs_emit(const struct brw_compiler *compiler, void *log_data,
>                 unsigned *final_assembly_size,
>                 char **error_str)
>  {
> +   /* key->alpha_test_func means simulating alpha testing via discards,
> +    * so the shader definitely kills pixels.
> +    */
> +   prog_data->uses_kill = shader->info.fs.uses_discard || key->alpha_test_func;
> +   prog_data->uses_omask =
> +      shader->info.outputs_written & BITFIELD64_BIT(FRAG_RESULT_SAMPLE_MASK);
> +   prog_data->computed_depth_mode = computed_depth_mode(shader);
> +
> +   prog_data->barycentric_interp_modes =
> +      brw_compute_barycentric_interp_modes(compiler->devinfo,
> +                                           key->flat_shade,
> +                                           key->persample_shading,
> +                                           shader);
> +
>     fs_visitor v(compiler, log_data, mem_ctx, key,
>                  &prog_data->base, prog, shader, 8,
>                  shader_time_index8);
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
> index ab9461a..91bda35 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -39,89 +39,6 @@
>  
>  #include "util/ralloc.h"
>  
> -/**
> - * Return a bitfield where bit n is set if barycentric interpolation mode n
> - * (see enum brw_wm_barycentric_interp_mode) is needed by the fragment shader.
> - */
> -static unsigned
> -brw_compute_barycentric_interp_modes(const struct brw_device_info *devinfo,
> -                                     bool shade_model_flat,
> -                                     bool persample_shading,
> -                                     nir_shader *shader)
> -{
> -   unsigned barycentric_interp_modes = 0;
> -
> -   nir_foreach_variable(var, &shader->inputs) {
> -      enum glsl_interp_qualifier interp_qualifier = var->data.interpolation;
> -      bool is_centroid = var->data.centroid && !persample_shading;
> -      bool is_sample = var->data.sample || persample_shading;
> -      bool is_gl_Color = (var->data.location == VARYING_SLOT_COL0) ||
> -                         (var->data.location == VARYING_SLOT_COL1);
> -
> -      /* Ignore WPOS and FACE, because they don't require interpolation. */
> -      if (var->data.location == VARYING_SLOT_POS ||
> -          var->data.location == VARYING_SLOT_FACE)
> -         continue;
> -
> -      /* Determine the set (or sets) of barycentric coordinates needed to
> -       * interpolate this variable.  Note that when
> -       * brw->needs_unlit_centroid_workaround is set, centroid interpolation
> -       * uses PIXEL interpolation for unlit pixels and CENTROID interpolation
> -       * for lit pixels, so we need both sets of barycentric coordinates.
> -       */
> -      if (interp_qualifier == INTERP_QUALIFIER_NOPERSPECTIVE) {
> -         if (is_centroid) {
> -            barycentric_interp_modes |=
> -               1 << BRW_WM_NONPERSPECTIVE_CENTROID_BARYCENTRIC;
> -         } else if (is_sample) {
> -            barycentric_interp_modes |=
> -               1 << BRW_WM_NONPERSPECTIVE_SAMPLE_BARYCENTRIC;
> -         }
> -         if ((!is_centroid && !is_sample) ||
> -             devinfo->needs_unlit_centroid_workaround) {
> -            barycentric_interp_modes |=
> -               1 << BRW_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC;
> -         }
> -      } else if (interp_qualifier == INTERP_QUALIFIER_SMOOTH ||
> -                 (!(shade_model_flat && is_gl_Color) &&
> -                  interp_qualifier == INTERP_QUALIFIER_NONE)) {
> -         if (is_centroid) {
> -            barycentric_interp_modes |=
> -               1 << BRW_WM_PERSPECTIVE_CENTROID_BARYCENTRIC;
> -         } else if (is_sample) {
> -            barycentric_interp_modes |=
> -               1 << BRW_WM_PERSPECTIVE_SAMPLE_BARYCENTRIC;
> -         }
> -         if ((!is_centroid && !is_sample) ||
> -             devinfo->needs_unlit_centroid_workaround) {
> -            barycentric_interp_modes |=
> -               1 << BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC;
> -         }
> -      }
> -   }
> -
> -   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;
> -}
> -
>  static void
>  assign_fs_binding_table_offsets(const struct brw_device_info *devinfo,
>                                  const struct gl_shader_program *shader_prog,
> @@ -166,15 +83,6 @@ brw_codegen_wm_prog(struct brw_context *brw,
>        fs = (struct brw_shader *)prog->_LinkedShaders[MESA_SHADER_FRAGMENT];
>  
>     memset(&prog_data, 0, sizeof(prog_data));
> -   /* key->alpha_test_func means simulating alpha testing via discards,
> -    * so the shader definitely kills pixels.
> -    */
> -   prog_data.uses_kill = fp->program.UsesKill || key->alpha_test_func;
> -   prog_data.uses_omask =
> -      fp->program.Base.OutputsWritten & BITFIELD64_BIT(FRAG_RESULT_SAMPLE_MASK);
> -   prog_data.computed_depth_mode = computed_depth_mode(&fp->program);
> -
> -   prog_data.early_fragment_tests = fs && fs->base.EarlyFragmentTests;

You store "early_fragment_tests" in patch 6 into shader info but this patch
doesn't add logic to read that and set it for prog_data (I was expecting to
see that in brw_wm_fs_emit()). To me it seems that the corresponding field
in prog_data is now always left to false.

>  
>     /* Use ALT floating point mode for ARB programs so that 0^0 == 1. */
>     if (!prog)
> @@ -209,12 +117,6 @@ brw_codegen_wm_prog(struct brw_context *brw,
>                                   &prog_data.base);
>     }
>  
> -   prog_data.barycentric_interp_modes =
> -      brw_compute_barycentric_interp_modes(brw->intelScreen->devinfo,
> -                                           key->flat_shade,
> -                                           key->persample_shading,
> -                                           fp->program.Base.nir);
> -
>     if (unlikely(brw->perf_debug)) {
>        start_busy = (brw->batch.last_bo &&
>                      drm_intel_bo_busy(brw->batch.last_bo));
> -- 
> 2.5.0.400.gff86faf
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list