[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 20:03:29 UTC 2017
On Wed, Jun 14, 2017 at 10:48:09AM -0700, Ian Romanick wrote:
> On 06/09/2017 07:04 AM, Topi Pohjolainen 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
>
> I wish I had noticed this before I had Mark open a bug for it:
>
> https://bugs.freedesktop.org/show_bug.cgi?id=101414
Sorry about this Ian, it is a wrong call. I blindly took it from the
change list after I saw it there every time (I have been churning my isl work
a lot in jenkins). I don't even touch i915 driver here.
>
> > 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);
> >
>
More information about the mesa-dev
mailing list