[Mesa-dev] [PATCH 5/5] i965 gen7: add support for layered color renderbuffers
Jordan Justen
jljusten at gmail.com
Tue May 21 13:51:59 PDT 2013
On Tue, May 21, 2013 at 1:33 PM, Eric Anholt <eric at anholt.net> wrote:
> Paul Berry <stereotype441 at gmail.com> writes:
>
>> On 17 May 2013 21:44, Chia-I Wu <olvaffe at gmail.com> wrote:
>>
>>> On Sat, May 18, 2013 at 10:11 AM, Jordan Justen
>>> <jordan.l.justen at intel.com> wrote:
>>> > Rather than pointing the surface_state directly at a single
>>> > sub-image of the texture for rendering, we now point the
>>> > surface_state at the top level of the texture, and configure
>>> > the surface_state as needed based on this.
>>> >
>>> > We now also need to stop setting the FORCE_ZERO_RTAINDEX bit
>>> > in the clip date so render target array values other than zero
>>> > will be used.
>>> >
>>> > Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
>>> > ---
>>> > src/mesa/drivers/dri/i965/brw_defines.h | 2 +
>>> > src/mesa/drivers/dri/i965/gen7_clip_state.c | 3 +-
>>> > src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 63
>>> +++++++++++++++------
>>> > 3 files changed, 48 insertions(+), 20 deletions(-)
>>> >
>>> > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
>>> b/src/mesa/drivers/dri/i965/brw_defines.h
>>> > index fedd78c..d61151f 100644
>>> > --- a/src/mesa/drivers/dri/i965/brw_defines.h
>>> > +++ b/src/mesa/drivers/dri/i965/brw_defines.h
>>> > @@ -539,6 +539,8 @@
>>> > #define GEN7_SURFACE_MULTISAMPLECOUNT_8 (3 << 3)
>>> > #define GEN7_SURFACE_MSFMT_MSS (0 << 6)
>>> > #define GEN7_SURFACE_MSFMT_DEPTH_STENCIL (1 << 6)
>>> > +#define GEN7_SURFACE_MIN_ARRAY_ELEMENT_SHIFT 18
>>> > +#define GEN7_SURFACE_RENDER_TARGET_VIEW_EXTENT_SHIFT 7
>>> >
>>> > /* Surface state DW5 */
>>> > #define BRW_SURFACE_X_OFFSET_SHIFT 25
>>> > diff --git a/src/mesa/drivers/dri/i965/gen7_clip_state.c
>>> b/src/mesa/drivers/dri/i965/gen7_clip_state.c
>>> > index 29a5ed5..1256f32 100644
>>> > --- a/src/mesa/drivers/dri/i965/gen7_clip_state.c
>>> > +++ b/src/mesa/drivers/dri/i965/gen7_clip_state.c
>>> > @@ -107,8 +107,7 @@ upload_clip_state(struct brw_context *brw)
>>> > GEN6_CLIP_XY_TEST |
>>> > dw2);
>>> > OUT_BATCH(U_FIXED(0.125, 3) << GEN6_CLIP_MIN_POINT_WIDTH_SHIFT |
>>> > - U_FIXED(255.875, 3) << GEN6_CLIP_MAX_POINT_WIDTH_SHIFT |
>>> > - GEN6_CLIP_FORCE_ZERO_RTAINDEX);
>>> > + U_FIXED(255.875, 3) << GEN6_CLIP_MAX_POINT_WIDTH_SHIFT);
>>> > ADVANCE_BATCH();
>>> > }
>>> >
>>> > 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 6c01545..5f15eff 100644
>>> > --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
>>> > +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
>>> > @@ -23,6 +23,7 @@
>>> > #include "main/mtypes.h"
>>> > #include "main/blend.h"
>>> > #include "main/samplerobj.h"
>>> > +#include "main/texformat.h"
>>> > #include "program/prog_parameter.h"
>>> >
>>> > #include "intel_mipmap_tree.h"
>>> > @@ -529,12 +530,13 @@ gen7_update_renderbuffer_surface(struct
>>> brw_context *brw,
>>> > struct gl_context *ctx = &intel->ctx;
>>> > struct intel_renderbuffer *irb = intel_renderbuffer(rb);
>>> > struct intel_region *region = irb->mt->region;
>>> > - uint32_t tile_x, tile_y;
>>> > uint32_t format;
>>> > /* _NEW_BUFFERS */
>>> > gl_format rb_format = _mesa_get_render_format(ctx,
>>> intel_rb_format(irb));
>>> > -
>>> > - assert(!layered);
>>> > + uint32_t surftype;
>>> > + bool is_array = false;
>>> > + int depth = rb->Depth > 0 ? rb->Depth - 1 : 0;
>>> > + int min_array_element = 0;
>>> >
>>> > uint32_t *surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
>>> > 8 * 4, 32,
>>> &brw->wm.surf_offset[unit]);
>>> > @@ -550,7 +552,23 @@ gen7_update_renderbuffer_surface(struct brw_context
>>> *brw,
>>> > __FUNCTION__, _mesa_get_format_name(rb_format));
>>> > }
>>> >
>>> > - surf[0] = BRW_SURFACE_2D << BRW_SURFACE_TYPE_SHIFT |
>>> > + if (rb->TexImage) {
>>> > + surftype = translate_tex_target(rb->TexImage->TexObject->Target);
>>> > + is_array =
>>> _mesa_tex_target_is_array(rb->TexImage->TexObject->Target);
>>> > + if (rb->TexImage->TexObject->Target == GL_TEXTURE_CUBE_MAP_ARRAY)
>>> {
>>> > + assert(rb->Depth > 0);
>>> > + surftype = BRW_SURFACE_2D;
>>> > + depth = (6 * (depth + 1)) - 1;
>>> > + } else if (rb->TexImage->TexObject->Target ==
>>> GL_TEXTURE_CUBE_MAP) {
>>> > + surftype = BRW_SURFACE_2D;
>>> > + depth = 5;
>>> > + is_array = true;
>>> > + }
>>> > + } else {
>>> > + surftype = BRW_SURFACE_2D;
>>> > + }
>>> > +
>>> > + surf[0] = surftype << BRW_SURFACE_TYPE_SHIFT |
>>> > format << BRW_SURFACE_FORMAT_SHIFT |
>>> > (irb->mt->array_spacing_lod0 ? GEN7_SURFACE_ARYSPC_LOD0
>>> > : GEN7_SURFACE_ARYSPC_FULL) |
>>> > @@ -561,24 +579,33 @@ gen7_update_renderbuffer_surface(struct
>>> brw_context *brw,
>>> > if (irb->mt->align_w == 8)
>>> > surf[0] |= GEN7_SURFACE_HALIGN_8;
>>> >
>>> > - /* reloc */
>>> > - surf[1] = intel_renderbuffer_tile_offsets(irb, &tile_x, &tile_y) +
>>> > - region->bo->offset; /* reloc */
>>> > + if (is_array) {
>>> > + surf[0] |= GEN7_SURFACE_IS_ARRAY;
>>> > + }
>>> > +
>>> > + if (!layered) {
>>> > + if (irb->mt->num_samples > 1) {
>>> > + min_array_element = irb->mt_layer / irb->mt->num_samples;
>>> > + } else {
>>> > + min_array_element = irb->mt_layer;
>>> > + }
>>> > + }
>>> > +
>>> > + surf[1] = region->bo->offset;
>>> >
>>> > assert(brw->has_surface_tile_offset);
>>> > - /* Note that the low bits of these fields are missing, so
>>> > - * there's the possibility of getting in trouble.
>>> > - */
>>> > - assert(tile_x % 4 == 0);
>>> > - assert(tile_y % 2 == 0);
>>> > - surf[5] = SET_FIELD(tile_x / 4, BRW_SURFACE_X_OFFSET) |
>>> > - SET_FIELD(tile_y / 2, BRW_SURFACE_Y_OFFSET);
>>> >
>>> > - surf[2] = SET_FIELD(rb->Width - 1, GEN7_SURFACE_WIDTH) |
>>> > - SET_FIELD(rb->Height - 1, GEN7_SURFACE_HEIGHT);
>>> > - surf[3] = region->pitch - 1;
>>> > + surf[5] = irb->mt_level;
>>> The PRM says that the LOD field of all render targets and the depth
>>> buffer must be the same. Is this going to violate that?
>>>
>>
>> Based on some discussions with the hardware architects, and the fact that
>> this patch series doesn't cause any piglit regressions, we believe that is
>> not actually a hard requirement.
>
> Do we have a single piglit test that would hit this case, though? I
> don't think we do, which I think needs to change before we land this.
I'm working on a piglit test to specifically target this.
But, with this changeset in place, any test or application that uses
depth along with a color lod != 0 will be testing depth lod != color
lod. (Since depth is still pegged at lod 0.)
While this is also not a specific test for this, I also tried TF2 and
Portal with this change, and did not notice any issues.
-Jordan
More information about the mesa-dev
mailing list