[Mesa-dev] [PATCH 3/3] i965/gen4: Add support for single layer in alignment workaround
Pohjolainen, Topi
topi.pohjolainen at gmail.com
Wed Jun 14 07:29:03 UTC 2017
On Tue, Jun 13, 2017 at 04:54:32PM -0700, Jason Ekstrand wrote:
> Looking through your code, I believe it is correct. I'm a bit concerned,
> however, because we're now copying back to the main surface on every draw
> call. If an app ever happens to hit this path, it will be correct, but
> it's performance will tank. Unfortunately, a better implementation would
I was thinking the same. Fortunately in practice it seems that unaligned
levels tend to be rather small (or quite high in the hierarchy) and we should
hit them rarely in the real world.
> be a pain because we would have to track which slice we pulled out and do
> something clever when someone goes to texture from it. The resolve
> framework could handle this but it would be painful. Ken? Thoughts?
>
> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
>
> On Fri, Jun 9, 2017 at 7:04 AM, Topi Pohjolainen <topi.pohjolainen at gmail.com
> > wrote:
>
> > On gen < 6 one doesn't have level or layer specifiers available
> > for render and depth targets. In order to support rendering to
> > specific level/layer, driver needs to manually offset the surface
> > to the desired slice.
> > There are, however, alignment restrictions to respect as well and
> > in come cases the only option is to use temporary single slice
> > surface which driver copies after rendering to the full miptree.
> >
> > Current alignment workaround introduces new texture images which
> > are added to the parent texture object. Texture validation later
> > on copies the additional levels back to the surface that contains
> > the full mipmap.
> > This only works for non-arrayed surfaces and driver currently
> > creates new arrayed images in vain - individual layers within the
> > newly created are still unaligned the same as before.
> >
> > This patch drops this mechanism and instead attaches single
> > temporary slice into the render buffer. This gets immediately
> > copied back to the mipmapped and/or arrayed surface just after
> > the render is done.
> >
> > Sitting on top of earlier series cleaning up the depth buffer
> > state, this patch additionally fixes the following piglit tests:
> >
> > ext_texture_array.copyteximage 2d_array.g45m64
> > ext_texture_array.copyteximage 1d_array.g45m64
> > arb_framebuffer_object.fbo-blit-stretch.g33m64
> > ext_framebuffer_object.fbo-cubemap.g965m64
> > arb_framebuffer_object.fbo-generatemipmap-cubemap.g965m64
> > arb_texture_cube_map.copyteximage cube.g965m64
> > ext_texture_array.copyteximage 1d_array.g965m64
> > ext_texture_array.copyteximage 2d_array.g965m64
> > ext_texture_array.fbo-array.g965m64
> > ext_texture_array.gen-mipmap.g965m64
> > ext_texture_array.fbo-generatemipmap-array.g965m64
> > arb_pixel_buffer_object.texsubimage array pbo.g965m64
> > ext_texture_array.copyteximage 2d_array.ilkm64
> > ext_texture_array.copyteximage 1d_array.ilkm64
> > arb_texture_cube_map.copyteximage cube.ilkm64
> >
> > CC: Kenneth Graunke <kenneth at whitecape.org>
> > CC: Jason Ekstrand <jason at jlekstrand.net>
> > CC: Ian Romanick <ian.d.romanick at intel.com>
> > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > ---
> > src/mesa/drivers/dri/i965/brw_draw.c | 51
> > ++++++++++++++++++++++++
> > src/mesa/drivers/dri/i965/brw_misc_state.c | 4 +-
> > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 3 +-
> > src/mesa/drivers/dri/i965/intel_fbo.c | 19 +++++----
> > src/mesa/drivers/dri/i965/intel_fbo.h | 24 +++++++++++
> > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 2 +-
> > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 7 ++++
> > 7 files changed, 99 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
> > b/src/mesa/drivers/dri/i965/brw_draw.c
> > index 611cb86..cb441c3 100644
> > --- a/src/mesa/drivers/dri/i965/brw_draw.c
> > +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> > @@ -396,6 +396,56 @@ brw_postdraw_set_buffers_need_resolve(struct
> > brw_context *brw)
> > }
> >
> > static void
> > +intel_renderbuffer_move_temp_back(struct brw_context *brw,
> > + struct intel_renderbuffer *irb)
> > +{
> > + if (irb->align_wa_mt == NULL)
> > + return;
> > +
> > + brw_render_cache_set_check_flush(brw, irb->align_wa_mt->bo);
> > +
> > + intel_miptree_copy_slice(brw, irb->align_wa_mt, 0, 0,
> > + irb->mt,
> > + irb->Base.Base.TexImage->Level,
> > irb->mt_layer);
> > +
> > + intel_miptree_reference(&irb->align_wa_mt, NULL);
> > +
> > + /* Finally restore the x,y to correspond to full miptree. */
> > + intel_renderbuffer_set_draw_offset(irb);
> > +
> > + /* Make sure render surface state gets re-emitted with updated
> > miptree. */
> > + brw->NewGLState |= _NEW_BUFFERS;
> > +}
> > +
> > +static void
> > +brw_postdraw_reconcile_align_wa_slices(struct brw_context *brw)
> > +{
> > + struct gl_context *ctx = &brw->ctx;
> > + struct gl_framebuffer *fb = ctx->DrawBuffer;
> > +
> > + struct intel_renderbuffer *depth_irb =
> > + intel_get_renderbuffer(fb, BUFFER_DEPTH);
> > + struct intel_renderbuffer *stencil_irb =
> > + intel_get_renderbuffer(fb, BUFFER_STENCIL);
> > +
> > + if (depth_irb && depth_irb->align_wa_mt)
> > + intel_renderbuffer_move_temp_back(brw, depth_irb);
> > +
> > + if (stencil_irb && stencil_irb->align_wa_mt)
> > + intel_renderbuffer_move_temp_back(brw, stencil_irb);
> > +
> > + for (unsigned i = 0; i < fb->_NumColorDrawBuffers; i++) {
> > + struct intel_renderbuffer *irb =
> > + intel_renderbuffer(fb->_ColorDrawBuffers[i]);
> > +
> > + if (!irb || irb->align_wa_mt == NULL)
> > + continue;
> > +
> > + intel_renderbuffer_move_temp_back(brw, irb);
> > + }
> > +}
> > +
> > +static void
> > brw_predraw_set_aux_buffers(struct brw_context *brw)
> > {
> > if (brw->gen < 9)
> > @@ -626,6 +676,7 @@ retry:
> > intel_batchbuffer_flush(brw);
> >
> > brw_program_cache_check_size(brw);
> > + brw_postdraw_reconcile_align_wa_slices(brw);
> > brw_postdraw_set_buffers_need_resolve(brw);
> >
> > return;
> > diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c
> > b/src/mesa/drivers/dri/i965/brw_misc_state.c
> > index fe021b0..0c25261 100644
> > --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> > @@ -132,7 +132,7 @@ get_stencil_miptree(struct intel_renderbuffer *irb)
> > return NULL;
> > if (irb->mt->stencil_mt)
> > return irb->mt->stencil_mt;
> > - return irb->mt;
> > + return intel_renderbuffer_get_mt(irb);
> > }
> >
> > static bool
> > @@ -268,7 +268,7 @@ brw_emit_depthbuffer(struct brw_context *brw)
> > /* _NEW_BUFFERS */
> > struct intel_renderbuffer *depth_irb = intel_get_renderbuffer(fb,
> > BUFFER_DEPTH);
> > struct intel_renderbuffer *stencil_irb = intel_get_renderbuffer(fb,
> > BUFFER_STENCIL);
> > - struct intel_mipmap_tree *depth_mt = depth_irb ? depth_irb->mt : NULL;
> > + struct intel_mipmap_tree *depth_mt = intel_renderbuffer_get_mt(
> > depth_irb);
> > struct intel_mipmap_tree *stencil_mt = get_stencil_miptree(stencil_
> > irb);
> > uint32_t tile_x = brw->depthstencil.tile_x;
> > uint32_t tile_y = brw->depthstencil.tile_y;
> > 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 3278ef3..8f818ab 100644
> > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> > @@ -1008,7 +1008,8 @@ gen4_update_renderbuffer_surface(struct brw_context
> > *brw,
> > * miptree and render into that.
> > */
> > intel_renderbuffer_move_to_temp(brw, irb, false);
> > - mt = irb->mt;
> > + assert(irb->align_wa_mt);
> > + mt = irb->align_wa_mt;
> > }
> > }
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c
> > b/src/mesa/drivers/dri/i965/intel_fbo.c
> > index a24ddd0..efbaa82 100644
> > --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> > +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> > @@ -993,11 +993,11 @@ intel_renderbuffer_move_to_temp(struct brw_context
> > *brw,
> >
> > intel_get_image_dims(rb->TexImage, &width, &height, &depth);
> >
> > - new_mt = intel_miptree_create(brw, rb->TexImage->TexObject->Target,
> > + assert(irb->align_wa_mt == NULL);
> > + new_mt = intel_miptree_create(brw, GL_TEXTURE_2D,
> > intel_image->base.Base.TexFormat,
> > - intel_image->base.Base.Level,
> > - intel_image->base.Base.Level,
> > - width, height, depth,
> > + 0, 0,
> > + width, height, 1,
> > irb->mt->num_samples,
> > layout_flags);
> >
> > @@ -1005,11 +1005,16 @@ intel_renderbuffer_move_to_temp(struct
> > brw_context *brw,
> > intel_miptree_alloc_hiz(brw, new_mt);
> > }
> >
> > - intel_miptree_copy_teximage(brw, intel_image, new_mt, invalidate);
> > + if (!invalidate)
> > + intel_miptree_copy_slice(brw, intel_image->mt,
> > + intel_image->base.Base.Level,
> > irb->mt_layer,
> > + new_mt, 0, 0);
> >
> > - intel_miptree_reference(&irb->mt, intel_image->mt);
> > - intel_renderbuffer_set_draw_offset(irb);
> > + intel_miptree_reference(&irb->align_wa_mt, new_mt);
> > intel_miptree_release(&new_mt);
> > +
> > + irb->draw_x = 0;
> > + irb->draw_y = 0;
> > }
> >
> > void
> > diff --git a/src/mesa/drivers/dri/i965/intel_fbo.h
> > b/src/mesa/drivers/dri/i965/intel_fbo.h
> > index 2d2ef1e..3c2be82 100644
> > --- a/src/mesa/drivers/dri/i965/intel_fbo.h
> > +++ b/src/mesa/drivers/dri/i965/intel_fbo.h
> > @@ -67,6 +67,16 @@ struct intel_renderbuffer
> > */
> > struct intel_mipmap_tree *singlesample_mt;
> >
> > + /* Gen < 6 doesn't have layer specifier for render targets or depth.
> > Driver
> > + * needs to manually offset surfaces to correct level/layer. There are,
> > + * however, alignment restrictions to respect as well and in come cases
> > + * the only option is to use temporary single slice surface which
> > driver
> > + * copies after rendering to the full miptree.
> > + *
> > + * See intel_renderbuffer_move_to_temp().
> > + */
> > + struct intel_mipmap_tree *align_wa_mt;
> > +
> > /**
> > * \name Miptree view
> > * \{
> > @@ -136,6 +146,14 @@ intel_renderbuffer(struct gl_renderbuffer *rb)
> > return NULL;
> > }
> >
> > +static inline struct intel_mipmap_tree *
> > +intel_renderbuffer_get_mt(struct intel_renderbuffer *irb)
> > +{
> > + if (!irb)
> > + return NULL;
> > +
> > + return (irb->align_wa_mt) ? irb->align_wa_mt : irb->mt;
> > +}
> >
> > /**
> > * \brief Return the framebuffer attachment specified by attIndex.
> > @@ -188,6 +206,12 @@ intel_renderbuffer_get_tile_offsets(struct
> > intel_renderbuffer *irb,
> > uint32_t *tile_x,
> > uint32_t *tile_y)
> > {
> > + if (irb->align_wa_mt) {
> > + *tile_x = 0;
> > + *tile_y = 0;
> > + return 0;
> > + }
> > +
> > return intel_miptree_get_tile_offsets(irb->mt, irb->mt_level,
> > irb->mt_layer,
> > tile_x, tile_y);
> > }
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index a4b2aeb..01b68cd 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -1266,7 +1266,7 @@ intel_miptree_copy_slice_sw(struct brw_context *brw,
> > }
> > }
> >
> > -static void
> > +void
> > intel_miptree_copy_slice(struct brw_context *brw,
> > struct intel_mipmap_tree *src_mt,
> > unsigned src_level, unsigned src_layer,
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > index 41d3d25..7979f3b 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > @@ -853,6 +853,13 @@ void intel_miptree_set_image_offset(struct
> > intel_mipmap_tree *mt,
> > GLuint img, GLuint x, GLuint y);
> >
> > void
> > +intel_miptree_copy_slice(struct brw_context *brw,
> > + struct intel_mipmap_tree *src_mt,
> > + unsigned src_level, unsigned src_layer,
> > + struct intel_mipmap_tree *dst_mt,
> > + unsigned dst_level, unsigned dst_layer);
> > +
> > +void
> > intel_miptree_copy_teximage(struct brw_context *brw,
> > struct intel_texture_image *intelImage,
> > struct intel_mipmap_tree *dst_mt, bool
> > invalidate);
> > --
> > 2.9.3
> >
> >
More information about the mesa-dev
mailing list