[Mesa-dev] [PATCH v2] i965: Fix mipmap offsets for HiZ and separate stencil buffers.

[Kenneth Graunke]

On 05/02/2012 02:48 PM, Paul Berry wrote:
> When rendering to a miplevel other than 0 within a color, depth,
> stencil, or HiZ buffer, we need to tell the GPU to render to an offset
> within the buffer, so that the data is written into the correct
> miplevel.  We do this using a coarse offset (in pages), and a fine
> adjustment (the so-called "tile_x" and "tile_y" values, which are
> measured in pixels).
>
> We have always computed the coarse offset and fine adjustment using
> intel_renderbuffer_tile_offsets() function.  This worked fine for
> color and combined depth/stencil buffers, but failed to work properly
> when HiZ and separate stencil were in use.  It failed to work because
> there is only one set of fine adjustment controls shared by the HiZ,
> depth, and stencil buffers, so we need to choose tile_x and tile_y
> values that are compatible with the tiling of all three buffers, and
> then compute separate coarse offsets for each buffer.
>
> This patch fixes the HiZ and separate stencil case by replacing the
> call to intel_renderbuffer_tile_offsets() with calls to two functions:
> intel_region_get_tile_masks(), which determines how much of the
> adjustment can be performed using offsets and how much can be
> performed using tile_x and tile_y, and
> intel_region_get_aligned_offset(), which computes the coarse offset.
>
> intel_region_get_tile_offsets() is still used for color renderbuffers,
> so to avoid code duplication, I've re-worked it to use
> intel_region_get_tile_masks() and intel_region_get_aligned_offset().
>
> On i965 Gen6, fixes piglit tests
> "texturing/depthstencil-render-miplevels 1024 X" where X is one of
> (depth, depth_and_stencil, depth_stencil_single_binding, depth_x,
> depth_x_and_stencil, stencil, stencil_and_depth, stencil_and_depth_x).
>
> On i965 Gen7, the variants of
> "texturing/depthstencil-render-miplevels" that contain a stencil
> buffer still fail, due to another problem: Gen7 seems to ignore the 3
> LSB's of the tile_y adjustment (and possibly also tile_x).
>
> v2: Removed spurious comments.  Added assertions to check
> preconditions of intel_region_get_aligned_offset().

I think this is right.  I see what you're doing and it seems like a good 
idea.  Since it fixes things and allows you to merge your other patch 
which fixes GPU hangs, I'm going to give it an:

Acked-by: Kenneth Graunke <kenneth at whitecape.org>

That said, I'm displeased with the fact that a lot of complicated code 
has been cut & pasted between brw_misc_state, gen7_misc_state, gen6_hiz, 
and gen7_hiz.  I'd rather coalesce it.  Plus, the existing 
emit_depthbuffer code was already quite hairy and in need of refactoring...

I'll look into that I guess.