[Mesa-dev] [PATCH 2/3] i965/fs: Pass usage of depth, W, and sample mask through prog_data

Kenneth Graunke kenneth at whitecape.org
Thu Feb 11 20:20:18 UTC 2016


On Thursday, February 11, 2016 10:04:27 AM PST Jason Ekstrand wrote:
> We really need to stop pulling information directly out of shaders for
> state setup.  For one thing, if we want any sort of an on-disk shader
> cache, having all of this metadata in one place is going to be crucial.
> Also, passing it all through prog_data cleans up the compiler <-> state
> setup API substantially.
> ---
>  src/mesa/drivers/dri/i965/brw_compiler.h  |  3 +++
>  src/mesa/drivers/dri/i965/brw_fs.cpp      | 15 ++++++++++-----
>  src/mesa/drivers/dri/i965/brw_wm_iz.cpp   |  7 ++++---
>  src/mesa/drivers/dri/i965/brw_wm_state.c  |  3 +--
>  src/mesa/drivers/dri/i965/gen7_wm_state.c |  8 +++-----
>  src/mesa/drivers/dri/i965/gen8_ps_state.c |  5 +++--
>  6 files changed, 24 insertions(+), 17 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_compiler.h b/src/mesa/drivers/
dri/i965/brw_compiler.h
> index 5e88310..b8423ec 100644
> --- a/src/mesa/drivers/dri/i965/brw_compiler.h
> +++ b/src/mesa/drivers/dri/i965/brw_compiler.h
> @@ -384,6 +384,9 @@ struct brw_wm_prog_data {
>     bool uses_pos_offset;
>     bool uses_omask;
>     bool uses_kill;
> +   bool uses_src_depth;
> +   bool uses_src_w;
> +   bool uses_sample_mask;
>     bool pulls_bary;
>     uint32_t prog_offset_16;
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/
i965/brw_fs.cpp
> index c0af749..75761cb 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -4854,8 +4854,6 @@ fs_visitor::setup_fs_payload_gen6()
>     brw_wm_prog_data *prog_data = (brw_wm_prog_data*) this->prog_data;
>     brw_wm_prog_key *key = (brw_wm_prog_key*) this->key;
>  
> -   bool uses_depth =
> -      (nir->info.inputs_read & (1 << VARYING_SLOT_POS)) != 0;
>     unsigned barycentric_interp_modes =
>        (stage == MESA_SHADER_FRAGMENT) ?
>        ((brw_wm_prog_data*) this->prog_data)->barycentric_interp_modes : 0;
> @@ -4884,7 +4882,9 @@ fs_visitor::setup_fs_payload_gen6()
>     }
>  
>     /* R27: interpolated depth if uses source depth */
> -   if (uses_depth) {
> +   prog_data->uses_src_depth =
> +      (nir->info.inputs_read & (1 << VARYING_SLOT_POS)) != 0;
> +   if (prog_data->uses_src_depth) {
>        payload.source_depth_reg = payload.num_regs;
>        payload.num_regs++;
>        if (dispatch_width == 16) {
> @@ -4892,8 +4892,11 @@ fs_visitor::setup_fs_payload_gen6()
>           payload.num_regs++;
>        }
>     }
> +
>     /* R29: interpolated W set if GEN6_WM_USES_SOURCE_W. */
> -   if (uses_depth) {
> +   prog_data->uses_src_w =
> +      (nir->info.inputs_read & (1 << VARYING_SLOT_POS)) != 0;
> +   if (prog_data->uses_src_w) {
>        payload.source_w_reg = payload.num_regs;
>        payload.num_regs++;
>        if (dispatch_width == 16) {
> @@ -4910,7 +4913,9 @@ fs_visitor::setup_fs_payload_gen6()
>     }
>  
>     /* R32: MSAA input coverage mask */
> -   if (nir->info.system_values_read & SYSTEM_BIT_SAMPLE_MASK_IN) {
> +   prog_data->uses_sample_mask =
> +      (nir->info.system_values_read & SYSTEM_BIT_SAMPLE_MASK_IN) != 0;
> +   if (prog_data->uses_sample_mask) {
>        assert(devinfo->gen >= 7);
>        payload.sample_mask_in_reg = payload.num_regs;
>        payload.num_regs++;
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_iz.cpp b/src/mesa/drivers/dri/
i965/brw_wm_iz.cpp
> index 9d9f4e4..bfd14f2 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_iz.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_wm_iz.cpp
> @@ -123,12 +123,11 @@ static const struct {
>  void fs_visitor::setup_fs_payload_gen4()
>  {
>     assert(stage == MESA_SHADER_FRAGMENT);
> +   brw_wm_prog_data *prog_data = (brw_wm_prog_data*) this->prog_data;
>     brw_wm_prog_key *key = (brw_wm_prog_key*) this->key;
>     GLuint reg = 2;
>     bool kill_stats_promoted_workaround = false;
>     int lookup = key->iz_lookup;
> -   bool uses_depth =
> -      (nir->info.inputs_read & (1 << VARYING_SLOT_POS)) != 0;
>  
>     assert(lookup < IZ_BIT_MAX);
>  
> @@ -143,7 +142,9 @@ void fs_visitor::setup_fs_payload_gen4()
>        kill_stats_promoted_workaround = true;
>     }
>  
> -   if (wm_iz_table[lookup].sd_present || uses_depth ||
> +   prog_data->uses_src_depth =
> +      (nir->info.inputs_read & (1 << VARYING_SLOT_POS)) != 0;
> +   if (wm_iz_table[lookup].sd_present || prog_data->uses_src_depth ||
>         kill_stats_promoted_workaround) {
>        payload.source_depth_reg = reg;
>        reg += 2;
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_state.c b/src/mesa/drivers/
dri/i965/brw_wm_state.c
> index ec54ef2..6bf0a55 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_state.c
> @@ -175,8 +175,7 @@ brw_upload_wm_unit(struct brw_context *brw)
>     }
>  
>     /* BRW_NEW_FRAGMENT_PROGRAM */
> -   wm->wm5.program_uses_depth = (fp->Base.InputsRead &
> -				 (1 << VARYING_SLOT_POS)) != 0;
> +   wm->wm5.program_uses_depth = prog_data->uses_src_depth;
>     wm->wm5.program_computes_depth = (fp->Base.OutputsWritten &
>  				     BITFIELD64_BIT(FRAG_RESULT_DEPTH)) != 0;
>     /* _NEW_BUFFERS
> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_state.c b/src/mesa/drivers/
dri/i965/gen7_wm_state.c
> index 7def5f5..9b01f55 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_state.c
> @@ -37,9 +37,6 @@ static void
>  upload_wm_state(struct brw_context *brw)
>  {
>     struct gl_context *ctx = &brw->ctx;
> -   /* BRW_NEW_FRAGMENT_PROGRAM */
> -   const struct brw_fragment_program *fp =
> -      brw_fragment_program_const(brw->fragment_program);
>     /* BRW_NEW_FS_PROG_DATA */
>     const struct brw_wm_prog_data *prog_data = brw->wm.prog_data;
>     bool writes_depth = prog_data->computed_depth_mode != BRW_PSCDEPTH_OFF;
> @@ -61,7 +58,8 @@ upload_wm_state(struct brw_context *brw)
>     if (ctx->Polygon.StippleFlag)
>        dw1 |= GEN7_WM_POLYGON_STIPPLE_ENABLE;
>  
> -   if (fp->program.Base.InputsRead & VARYING_BIT_POS)
> +   assert(prog_data->uses_src_depth == prog_data->uses_src_w);
> +   if (prog_data->uses_src_depth)
>        dw1 |= GEN7_WM_USES_SOURCE_DEPTH | GEN7_WM_USES_SOURCE_W;

I don't see any hardware restrictions that require these bits to be set
identically...why don't we just make this:

   if (prog_data->uses_src_depth)
      dw1 |= GEN7_WM_USES_SOURCE_DEPTH;

   if (prog_data->uses_src_w)
      dw1 |= GEN7_WM_USES_SOURCE_W;

>     dw1 |= prog_data->computed_depth_mode << 
GEN7_WM_COMPUTED_DEPTH_MODE_SHIFT;
> @@ -100,7 +98,7 @@ upload_wm_state(struct brw_context *brw)
>        dw2 |= GEN7_WM_MSDISPMODE_PERSAMPLE;
>     }
>  
> -   if (fp->program.Base.SystemValuesRead & SYSTEM_BIT_SAMPLE_MASK_IN) {
> +   if (prog_data->uses_sample_mask) {
>        dw1 |= GEN7_WM_USES_INPUT_COVERAGE_MASK;
>     }
>  
> diff --git a/src/mesa/drivers/dri/i965/gen8_ps_state.c b/src/mesa/drivers/
dri/i965/gen8_ps_state.c
> index 74cdcef..3fc26c0 100644
> --- a/src/mesa/drivers/dri/i965/gen8_ps_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_ps_state.c
> @@ -46,14 +46,15 @@ gen8_upload_ps_extra(struct brw_context *brw,
>     if (prog_data->num_varying_inputs != 0)
>        dw1 |= GEN8_PSX_ATTRIBUTE_ENABLE;
>  
> -   if (fp->Base.InputsRead & VARYING_BIT_POS)
> +   assert(prog_data->uses_src_depth == prog_data->uses_src_w);
> +   if (prog_data->uses_src_depth)
>        dw1 |= GEN8_PSX_USES_SOURCE_DEPTH | GEN8_PSX_USES_SOURCE_W;

Ditto.

Otherwise, the series is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

>  
>     if (multisampled_fbo &&
>         _mesa_get_min_invocations_per_fragment(ctx, fp, false) > 1)
>        dw1 |= GEN8_PSX_SHADER_IS_PER_SAMPLE;
>  
> -   if (fp->Base.SystemValuesRead & SYSTEM_BIT_SAMPLE_MASK_IN) {
> +   if (prog_data->uses_sample_mask) {
>        if (brw->gen >= 9)
>           dw1 |= BRW_PSICMS_INNER << 
GEN9_PSX_SHADER_NORMAL_COVERAGE_MASK_SHIFT;
>        else
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160211/fb3f6fcb/attachment.sig>


More information about the mesa-dev mailing list