[Mesa-dev] [PATCH 3/3] i965/gen4: Add support for single layer in alignment workaround
Jason Ekstrand
jason at jlekstrand.net
Tue Jun 13 23:54:53 UTC 2017
On Tue, Jun 13, 2017 at 4:54 PM, Jason Ekstrand <jason at jlekstrand.net>
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
> 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>
>
That applies to all 3 :-)
> 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(dept
>> h_irb);
>> struct intel_mipmap_tree *stencil_mt = get_stencil_miptree(stencil_ir
>> b);
>> 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
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170613/24c57b5c/attachment-0001.html>
More information about the mesa-dev
mailing list