[Mesa-dev,06/14] i965: Generalize the update_null_renderbuffer_surface vtbl hook to non-renderbuffers.

Submitted by Francisco Jerez on Feb. 6, 2015, 5:23 p.m.

Details

Message ID 1423243408-24744-6-git-send-email-currojerez@riseup.net
State New
Headers show

Not browsing as part of any series.

Commit Message

Francisco Jerez Feb. 6, 2015, 5:23 p.m.
Null surfaces are going to be useful to have something to point
unbound image units to, as the ARB_shader_image_load_store extension
requires us to behave deterministically in cases where some shader
tries to access an unbound image unit: Invalid stores and atomics are
supposed to be discarded and invalid loads are supposed to return
zero, which is precisely what the null surface does.
---
 src/mesa/drivers/dri/i965/brw_context.h           |  9 ++--
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c  | 53 +++++++++++++----------
 src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 24 +++++-----
 src/mesa/drivers/dri/i965/gen8_surface_state.c    | 25 +++++------
 4 files changed, 55 insertions(+), 56 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index 95ed3c2..e6827e0 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -964,8 +964,6 @@  struct brw_context
 					  struct gl_renderbuffer *rb,
 					  bool layered,
 					  unsigned unit);
-      void (*update_null_renderbuffer_surface)(struct brw_context *brw,
-					       unsigned unit);
 
       void (*emit_texture_surface_state)(struct brw_context *brw,
                                          struct intel_mipmap_tree *mt,
@@ -986,6 +984,11 @@  struct brw_context
                                         unsigned buffer_size,
                                         unsigned pitch,
                                         bool rw);
+      void (*emit_null_surface_state)(struct brw_context *brw,
+                                      unsigned width,
+                                      unsigned height,
+                                      unsigned samples,
+                                      uint32_t *out_offset);
 
       /**
        * Send the appropriate state packets to configure depth, stencil, and
@@ -1335,7 +1338,7 @@  struct brw_context
 
       /**
        * Buffer object used in place of multisampled null render targets on
-       * Gen6.  See brw_update_null_renderbuffer_surface().
+       * Gen6.  See brw_emit_null_surface_state().
        */
       drm_intel_bo *multisampled_null_render_target_bo;
       uint32_t fast_clear_op;
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 8554929..b96d3fb 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -508,7 +508,11 @@  const struct brw_tracked_state brw_wm_pull_constants = {
  * hardware discard the target 0 color output..
  */
 static void
-brw_update_null_renderbuffer_surface(struct brw_context *brw, unsigned int unit)
+brw_emit_null_surface_state(struct brw_context *brw,
+                            unsigned width,
+                            unsigned height,
+                            unsigned samples,
+                            uint32_t *out_offset)
 {
    /* From the Sandy bridge PRM, Vol4 Part1 p71 (Surface Type: Programming
     * Notes):
@@ -528,23 +532,14 @@  brw_update_null_renderbuffer_surface(struct brw_context *brw, unsigned int unit)
     *
     *     - Surface Format must be R8G8B8A8_UNORM.
     */
-   struct gl_context *ctx = &brw->ctx;
-   uint32_t *surf;
    unsigned surface_type = BRW_SURFACE_NULL;
    drm_intel_bo *bo = NULL;
    unsigned pitch_minus_1 = 0;
    uint32_t multisampling_state = 0;
-   /* BRW_NEW_FS_PROG_DATA */
-   uint32_t surf_index =
-      brw->wm.prog_data->binding_table.render_target_start + unit;
-
-   /* _NEW_BUFFERS */
-   const struct gl_framebuffer *fb = ctx->DrawBuffer;
-
-   surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, 6 * 4, 32,
-                          &brw->wm.base.surf_offset[surf_index]);
+   uint32_t *surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, 6 * 4, 32,
+                                    out_offset);
 
-   if (fb->Visual.samples > 1) {
+   if (samples > 1) {
       /* On Gen6, null render targets seem to cause GPU hangs when
        * multisampling.  So work around this problem by rendering into dummy
        * color buffer.
@@ -559,16 +554,15 @@  brw_update_null_renderbuffer_surface(struct brw_context *brw, unsigned int unit)
        * width_in_tiles and height_in_tiles by dividing the width and height
        * by 16 rather than the normal Y-tile size of 32.
        */
-      unsigned width_in_tiles = ALIGN(fb->Width, 16) / 16;
-      unsigned height_in_tiles = ALIGN(fb->Height, 16) / 16;
+      unsigned width_in_tiles = ALIGN(width, 16) / 16;
+      unsigned height_in_tiles = ALIGN(height, 16) / 16;
       unsigned size_needed = (width_in_tiles + height_in_tiles - 1) * 4096;
       brw_get_scratch_bo(brw, &brw->wm.multisampled_null_render_target_bo,
                          size_needed);
       bo = brw->wm.multisampled_null_render_target_bo;
       surface_type = BRW_SURFACE_2D;
       pitch_minus_1 = 127;
-      multisampling_state =
-         brw_get_surface_num_multisamples(fb->Visual.samples);
+      multisampling_state = brw_get_surface_num_multisamples(samples);
    }
 
    surf[0] = (surface_type << BRW_SURFACE_TYPE_SHIFT |
@@ -580,8 +574,8 @@  brw_update_null_renderbuffer_surface(struct brw_context *brw, unsigned int unit)
 		  1 << BRW_SURFACE_WRITEDISABLE_A_SHIFT);
    }
    surf[1] = bo ? bo->offset64 : 0;
-   surf[2] = ((fb->Width - 1) << BRW_SURFACE_WIDTH_SHIFT |
-              (fb->Height - 1) << BRW_SURFACE_HEIGHT_SHIFT);
+   surf[2] = ((width - 1) << BRW_SURFACE_WIDTH_SHIFT |
+              (height - 1) << BRW_SURFACE_HEIGHT_SHIFT);
 
    /* From Sandy bridge PRM, Vol4 Part1 p82 (Tiled Surface: Programming
     * Notes):
@@ -595,7 +589,7 @@  brw_update_null_renderbuffer_surface(struct brw_context *brw, unsigned int unit)
 
    if (bo) {
       drm_intel_bo_emit_reloc(brw->batch.bo,
-                              brw->wm.base.surf_offset[surf_index] + 4,
+                              *out_offset + 4,
                               bo, 0,
                               I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER);
    }
@@ -715,6 +709,8 @@  static void
 brw_update_renderbuffer_surfaces(struct brw_context *brw)
 {
    struct gl_context *ctx = &brw->ctx;
+   /* _NEW_BUFFERS */
+   const struct gl_framebuffer *fb = ctx->DrawBuffer;
    GLuint i;
 
    /* _NEW_BUFFERS | _NEW_COLOR */
@@ -725,11 +721,21 @@  brw_update_renderbuffer_surfaces(struct brw_context *brw)
 	    brw->vtbl.update_renderbuffer_surface(brw, ctx->DrawBuffer->_ColorDrawBuffers[i],
                                                   ctx->DrawBuffer->MaxNumLayers > 0, i);
 	 } else {
-	    brw->vtbl.update_null_renderbuffer_surface(brw, i);
+            const uint32_t surf_index =
+               brw->wm.prog_data->binding_table.render_target_start + i;
+
+            brw->vtbl.emit_null_surface_state(
+               brw, fb->Width, fb->Height, fb->Visual.samples,
+               &brw->wm.base.surf_offset[surf_index]);
 	 }
       }
    } else {
-      brw->vtbl.update_null_renderbuffer_surface(brw, 0);
+      const uint32_t surf_index =
+         brw->wm.prog_data->binding_table.render_target_start;
+
+      brw->vtbl.emit_null_surface_state(
+         brw, fb->Width, fb->Height, fb->Visual.samples,
+         &brw->wm.base.surf_offset[surf_index]);
    }
    brw->state.dirty.brw |= BRW_NEW_SURFACES;
 }
@@ -959,7 +965,6 @@  gen4_init_vtable_surface_functions(struct brw_context *brw)
 {
    brw->vtbl.update_texture_surface = brw_update_texture_surface;
    brw->vtbl.update_renderbuffer_surface = brw_update_renderbuffer_surface;
-   brw->vtbl.update_null_renderbuffer_surface =
-      brw_update_null_renderbuffer_surface;
+   brw->vtbl.emit_null_surface_state = brw_emit_null_surface_state;
    brw->vtbl.emit_buffer_surface_state = gen4_emit_buffer_surface_state;
 }
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 3413f88..c5fc5da 100644
--- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
@@ -389,7 +389,7 @@  gen7_update_texture_surface(struct gl_context *ctx,
 }
 
 /**
- * Creates a null renderbuffer surface.
+ * Creates a null surface.
  *
  * This is used when the shader doesn't write to any color output.  An FB
  * write to target 0 will still be emitted, because that's how the thread is
@@ -397,7 +397,11 @@  gen7_update_texture_surface(struct gl_context *ctx,
  * hardware discard the target 0 color output..
  */
 static void
-gen7_update_null_renderbuffer_surface(struct brw_context *brw, unsigned unit)
+gen7_emit_null_surface_state(struct brw_context *brw,
+                             unsigned width,
+                             unsigned height,
+                             unsigned samples,
+                             uint32_t *out_offset)
 {
    /* From the Ivy bridge PRM, Vol4 Part1 p62 (Surface Type: Programming
     * Notes):
@@ -414,15 +418,8 @@  gen7_update_null_renderbuffer_surface(struct brw_context *brw, unsigned unit)
     *     depth buffer’s corresponding state for all render target surfaces,
     *     including null.
     */
-   struct gl_context *ctx = &brw->ctx;
-
-   /* _NEW_BUFFERS */
-   const struct gl_framebuffer *fb = ctx->DrawBuffer;
-   uint32_t surf_index =
-      brw->wm.prog_data->binding_table.render_target_start + unit;
-
    uint32_t *surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, 8 * 4, 32,
-                                    &brw->wm.base.surf_offset[surf_index]);
+                                    out_offset);
    memset(surf, 0, 8 * 4);
 
    /* From the Ivybridge PRM, Volume 4, Part 1, page 65,
@@ -433,8 +430,8 @@  gen7_update_null_renderbuffer_surface(struct brw_context *brw, unsigned unit)
              BRW_SURFACEFORMAT_B8G8R8A8_UNORM << BRW_SURFACE_FORMAT_SHIFT |
              GEN7_SURFACE_TILING_Y;
 
-   surf[2] = SET_FIELD(fb->Width - 1, GEN7_SURFACE_WIDTH) |
-             SET_FIELD(fb->Height - 1, GEN7_SURFACE_HEIGHT);
+   surf[2] = SET_FIELD(width - 1, GEN7_SURFACE_WIDTH) |
+             SET_FIELD(height - 1, GEN7_SURFACE_HEIGHT);
 
    gen7_check_surface_setup(surf, true /* is_render_target */);
 }
@@ -563,8 +560,7 @@  gen7_init_vtable_surface_functions(struct brw_context *brw)
 {
    brw->vtbl.update_texture_surface = gen7_update_texture_surface;
    brw->vtbl.update_renderbuffer_surface = gen7_update_renderbuffer_surface;
-   brw->vtbl.update_null_renderbuffer_surface =
-      gen7_update_null_renderbuffer_surface;
+   brw->vtbl.emit_null_surface_state = gen7_emit_null_surface_state;
    brw->vtbl.emit_texture_surface_state = gen7_emit_texture_surface_state;
    brw->vtbl.emit_buffer_surface_state = gen7_emit_buffer_surface_state;
 }
diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c
index 0a1f33a..632084f 100644
--- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
+++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
@@ -292,7 +292,7 @@  gen8_update_texture_surface(struct gl_context *ctx,
 }
 
 /**
- * Creates a null renderbuffer surface.
+ * Creates a null surface.
  *
  * This is used when the shader doesn't write to any color output.  An FB
  * write to target 0 will still be emitted, because that's how the thread is
@@ -300,23 +300,19 @@  gen8_update_texture_surface(struct gl_context *ctx,
  * hardware discard the target 0 color output..
  */
 static void
-gen8_update_null_renderbuffer_surface(struct brw_context *brw, unsigned unit)
+gen8_emit_null_surface_state(struct brw_context *brw,
+                             unsigned width,
+                             unsigned height,
+                             unsigned samples,
+                             uint32_t *out_offset)
 {
-   struct gl_context *ctx = &brw->ctx;
-
-   /* _NEW_BUFFERS */
-   const struct gl_framebuffer *fb = ctx->DrawBuffer;
-   uint32_t surf_index =
-      brw->wm.prog_data->binding_table.render_target_start + unit;
-
-   uint32_t *surf =
-      allocate_surface_state(brw, &brw->wm.base.surf_offset[surf_index]);
+   uint32_t *surf = allocate_surface_state(brw, out_offset);
 
    surf[0] = BRW_SURFACE_NULL << BRW_SURFACE_TYPE_SHIFT |
              BRW_SURFACEFORMAT_B8G8R8A8_UNORM << BRW_SURFACE_FORMAT_SHIFT |
              GEN8_SURFACE_TILING_Y;
-   surf[2] = SET_FIELD(fb->Width - 1, GEN7_SURFACE_WIDTH) |
-             SET_FIELD(fb->Height - 1, GEN7_SURFACE_HEIGHT);
+   surf[2] = SET_FIELD(width - 1, GEN7_SURFACE_WIDTH) |
+             SET_FIELD(height - 1, GEN7_SURFACE_HEIGHT);
 }
 
 /**
@@ -461,8 +457,7 @@  gen8_init_vtable_surface_functions(struct brw_context *brw)
 {
    brw->vtbl.update_texture_surface = gen8_update_texture_surface;
    brw->vtbl.update_renderbuffer_surface = gen8_update_renderbuffer_surface;
-   brw->vtbl.update_null_renderbuffer_surface =
-      gen8_update_null_renderbuffer_surface;
+   brw->vtbl.emit_null_surface_state = gen8_emit_null_surface_state;
    brw->vtbl.emit_texture_surface_state = gen8_emit_texture_surface_state;
    brw->vtbl.emit_buffer_surface_state = gen8_emit_buffer_surface_state;
 }

Comments

On Friday, February 06, 2015 07:23:20 PM Francisco Jerez wrote:
> Null surfaces are going to be useful to have something to point
> unbound image units to, as the ARB_shader_image_load_store extension
> requires us to behave deterministically in cases where some shader
> tries to access an unbound image unit: Invalid stores and atomics are
> supposed to be discarded and invalid loads are supposed to return
> zero, which is precisely what the null surface does.
> ---
>  src/mesa/drivers/dri/i965/brw_context.h           |  9 ++--
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  | 53 +++++++++++++----------
>  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 24 +++++-----
>  src/mesa/drivers/dri/i965/gen8_surface_state.c    | 25 +++++------
>  4 files changed, 55 insertions(+), 56 deletions(-)

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>