[Mesa-dev] [PATCH 2/3] i965: Specialize SURFACE_STATE creation for shader time.

Paul Berry stereotype441 at gmail.com
Thu Feb 7 10:24:31 PST 2013


On 6 February 2013 23:26, Kenneth Graunke <kenneth at whitecape.org> wrote:

> This is basically a copy and paste of gen7_create_constant_surface, but
> with the parameters filled in to offer a simpler interface.
>
> It will diverge shortly.
>
> I didn't bother adding it to the vtable for now since shader time is
> only exposed on Gen7+.
>
> NOTE: This is a candidate for the 9.1 branch.
> Cc: Eric Anholt <eric at anholt.net>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_state.h             |  2 ++
>  src/mesa/drivers/dri/i965/brw_vs_surface_state.c  |  4 +--
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  |  4 +--
>  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 35
> +++++++++++++++++++++++
>  4 files changed, 39 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_state.h
> b/src/mesa/drivers/dri/i965/brw_state.h
> index adc64e3..7e4fecc 100644
> --- a/src/mesa/drivers/dri/i965/brw_state.h
> +++ b/src/mesa/drivers/dri/i965/brw_state.h
> @@ -215,6 +215,8 @@ void gen7_set_surface_mcs_info(struct brw_context *brw,
>                                 bool is_render_target);
>  void gen7_check_surface_setup(uint32_t *surf, bool is_render_target);
>  void gen7_init_vtable_surface_functions(struct brw_context *brw);
> +void gen7_create_shader_time_surface(struct brw_context *brw,
> +                                     uint32_t *out_offset);
>
>  /* brw_wm_sampler_state.c */
>  uint32_t translate_wrap_mode(GLenum wrap, bool using_nearest);
> diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> index 4da7eaa..9aa775f 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> @@ -142,9 +142,7 @@ brw_vs_upload_binding_table(struct brw_context *brw)
>     int i;
>
>     if (INTEL_DEBUG & DEBUG_SHADER_TIME) {
> -      intel->vtbl.create_constant_surface(brw, brw->shader_time.bo, 0,
> -                                          brw->shader_time.bo->size,
> -
>  &brw->vs.surf_offset[SURF_INDEX_VS_SHADER_TIME]);
> +      gen7_create_shader_time_surface(brw,
> &brw->vs.surf_offset[SURF_INDEX_VS_SHADER_TIME]);
>
>        assert(brw->vs.prog_data->num_surfaces <=
> SURF_INDEX_VS_SHADER_TIME);
>        brw->vs.prog_data->num_surfaces = SURF_INDEX_VS_SHADER_TIME;
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 77b7ab3..f0cc9b6 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -1470,9 +1470,7 @@ brw_upload_wm_binding_table(struct brw_context *brw)
>     int i;
>
>     if (INTEL_DEBUG & DEBUG_SHADER_TIME) {
> -      intel->vtbl.create_constant_surface(brw, brw->shader_time.bo, 0,
> -                                          brw->shader_time.bo->size,
> -
>  &brw->wm.surf_offset[SURF_INDEX_WM_SHADER_TIME]);
> +      gen7_create_shader_time_surface(brw,
> &brw->wm.surf_offset[SURF_INDEX_WM_SHADER_TIME]);
>     }
>
>     /* Might want to calculate nr_surfaces first, to avoid taking up so
> much
> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> index 179024a..6a4c009 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> @@ -422,6 +422,41 @@ gen7_create_constant_surface(struct brw_context *brw,
>     gen7_check_surface_setup(surf, false /* is_render_target */);
>  }
>
> +/**
> + * Create a surface for shader time.
> + */
> +void
> +gen7_create_shader_time_surface(struct brw_context *brw, uint32_t
> *out_offset)
> +{
> +   struct intel_context *intel = &brw->intel;
> +   const int w = brw->shader_time.bo->size - 1;
> +
> +   uint32_t *surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
> +                                    8 * 4, 32, out_offset);
>

gen7_create_constant_surface has a "memset(surf, 0, 8 * 4);" here.  I think
we need to do that in this function too, or we risk setting some MBZ fields
to garbage.


> +
> +   surf[0] = BRW_SURFACE_BUFFER << BRW_SURFACE_TYPE_SHIFT |
> +             BRW_SURFACEFORMAT_R32G32B32A32_FLOAT <<
> BRW_SURFACE_FORMAT_SHIFT |
> +             BRW_SURFACE_RC_READ_WRITE;
> +
> +   surf[1] = brw->shader_time.bo->offset; /* reloc */
> +
> +   surf[2] = SET_FIELD(w & 0x7f, GEN7_SURFACE_WIDTH) |
> +             SET_FIELD((w >> 7) & 0x1fff, GEN7_SURFACE_HEIGHT);
> +   surf[3] = SET_FIELD((w >> 20) & 0x7f, BRW_SURFACE_DEPTH) |
> +             (16 - 1); /* stride between samples */
>

You might want to include a brief comment here explaining why you're not
setting surf[7] here on Haswell.  (We talked about it in person and I think
you're right that it's not necessary, but since the HW docs aren't
spectacularly clear on this point it would be nice to have some explanatory
text).


> +
> +   /* Emit relocation to surface contents.  Section 5.1.1 of the gen4
> +    * bspec ("Data Cache") says that the data cache does not exist as
> +    * a separate cache and is just the sampler cache.
> +    */
> +   drm_intel_bo_emit_reloc(intel->batch.bo,
> +                          *out_offset + 4,
> +                          brw->shader_time.bo, 0,
> +                          I915_GEM_DOMAIN_SAMPLER, 0);
> +
> +   gen7_check_surface_setup(surf, false /* is_render_target */);
> +}
> +
>  static void
>  gen7_update_null_renderbuffer_surface(struct brw_context *brw, unsigned
> unit)
>  {
> --
> 1.8.1.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>


With that fixed, this patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130207/d320c4c8/attachment-0001.html>


More information about the mesa-dev mailing list