On Thu, May 29, 2014 at 01:53:41PM -0700, Jordan Justen 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. > > Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com> > --- > src/mesa/drivers/dri/i965/brw_defines.h | 2 + > src/mesa/drivers/dri/i965/gen6_surface_state.c | 71 > +++++++++++++------------- > 2 files changed, 38 insertions(+), 35 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h > b/src/mesa/drivers/dri/i965/brw_defines.h > index c38e447..d010d61 100644 > --- a/src/mesa/drivers/dri/i965/brw_defines.h > +++ b/src/mesa/drivers/dri/i965/brw_defines.h > @@ -548,6 +548,8 @@ > /* Surface state DW4 */ > #define BRW_SURFACE_MIN_LOD_SHIFT 28 > #define BRW_SURFACE_MIN_LOD_MASK INTEL_MASK(31, 28) > +#define BRW_SURFACE_MIN_ARRAY_ELEMENT_SHIFT 17 > +#define BRW_SURFACE_RENDER_TARGET_VIEW_EXTENT_SHIFT 8
If you introdeced the corresponding masks you could use SET_FIELD() later on. > #define BRW_SURFACE_MULTISAMPLECOUNT_1 (0 << 4) > #define BRW_SURFACE_MULTISAMPLECOUNT_4 (2 << 4) > #define GEN7_SURFACE_MULTISAMPLECOUNT_1 (0 << 3) > diff --git a/src/mesa/drivers/dri/i965/gen6_surface_state.c > b/src/mesa/drivers/dri/i965/gen6_surface_state.c > index 9fec372..728488a 100644 > --- a/src/mesa/drivers/dri/i965/gen6_surface_state.c > +++ b/src/mesa/drivers/dri/i965/gen6_surface_state.c > @@ -26,6 +26,7 @@ > #include "main/blend.h" > #include "main/mtypes.h" > #include "main/samplerobj.h" > +#include "main/texformat.h" > #include "program/prog_parameter.h" > > #include "intel_mipmap_tree.h" > @@ -54,30 +55,18 @@ gen6_update_renderbuffer_surface(struct brw_context *brw, > struct intel_renderbuffer *irb = intel_renderbuffer(rb); > struct intel_mipmap_tree *mt = irb->mt; > uint32_t *surf; > - uint32_t tile_x, tile_y; > uint32_t format = 0; > /* _NEW_BUFFERS */ > mesa_format rb_format = _mesa_get_render_format(ctx, > intel_rb_format(irb)); > + uint32_t surftype; > + int depth = MAX2(rb->Depth, 1); > + int min_array_element; > + } > + GLenum gl_target = rb->TexImage ? > + rb->TexImage->TexObject->Target : GL_TEXTURE_2D; > + > uint32_t surf_index = > brw->wm.prog_data->binding_table.render_target_start + unit; > > - assert(!layered); > - > - if (rb->TexImage && !brw->has_surface_tile_offset) { > - intel_renderbuffer_get_tile_offsets(irb, &tile_x, &tile_y); > - > - if (tile_x != 0 || tile_y != 0) { > - /* Original gen4 hardware couldn't draw to a non-tile-aligned > - * destination in a miptree unless you actually setup your renderbuffer > - * as a miptree and used the fragile lod/array_index/etc. controls to > - * select the image. So, instead, we just make a new single-level > - * miptree and render into that. > - */ > - intel_renderbuffer_move_to_temp(brw, irb, false); > - mt = irb->mt; > - } > - } > - > intel_miptree_used_for_rendering(irb->mt); > > surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, 6 * 4, 32, > @@ -89,30 +78,42 @@ gen6_update_renderbuffer_surface(struct brw_context *brw, > __FUNCTION__, _mesa_get_format_name(rb_format)); > } > > - surf[0] = (BRW_SURFACE_2D << BRW_SURFACE_TYPE_SHIFT | > - format << BRW_SURFACE_FORMAT_SHIFT); > + switch (gl_target) { > + case GL_TEXTURE_CUBE_MAP_ARRAY: > + case GL_TEXTURE_CUBE_MAP: > + surftype = BRW_SURFACE_2D; > + depth *= 6; > + break; > + default: > + surftype = translate_tex_target(gl_target); > + break; > + } Now we have this switch statement three times (gen6, gen7, gen8). I wouldn't mind having an inline for it in brw_state.h - though it maybe just me. There is quite a bit a duplication in the surface_state files anyway, and this wouldn't make a great difference. > + > + if (layered) { > + min_array_element = 0; > + } else { > + min_array_element = irb->mt_layer; > + } This could be simply in the declaration: const int min_array_element = layered ? 0 : irb->mt_layer; In gen7 equivalent the 'layered' argument is ignored and we simply always honor the renderbuffer settings (irb->mt_layer). This is now different, and in fact I wonder the purpose of the layered-argument in the first place. > + > + surf[0] = (surftype << BRW_SURFACE_TYPE_SHIFT | > + format << BRW_SURFACE_FORMAT_SHIFT); I think it would be clearer to start using the SET_FIELD() here and in the rest of the file. surf[0] = SET_FIELD(surftype, BRW_SURFACE_TYPE) | SET_FIELD(format, BRW_SURFACE_FORMAT); > > /* reloc */ > - surf[1] = (intel_renderbuffer_get_tile_offsets(irb, &tile_x, &tile_y) + > - mt->bo->offset64); > + surf[1] = mt->bo->offset64; > > - surf[2] = ((rb->Width - 1) << BRW_SURFACE_WIDTH_SHIFT | > - (rb->Height - 1) << BRW_SURFACE_HEIGHT_SHIFT); > + surf[2] = ((irb->mt->logical_width0 - 1) << BRW_SURFACE_WIDTH_SHIFT | > + (irb->mt->logical_height0 - 1) << BRW_SURFACE_HEIGHT_SHIFT | > + ((irb->mt_level - irb->mt->first_level)) << > BRW_SURFACE_LOD_SHIFT); > > surf[3] = (brw_get_surface_tiling_bits(mt->tiling) | > + depth << BRW_SURFACE_DEPTH_SHIFT | > (mt->pitch - 1) << BRW_SURFACE_PITCH_SHIFT); > > - surf[4] = brw_get_surface_num_multisamples(mt->num_samples); > - > - assert(brw->has_surface_tile_offset || (tile_x == 0 && tile_y == 0)); > - /* 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] = ((tile_x / 4) << BRW_SURFACE_X_OFFSET_SHIFT | > - (tile_y / 2) << BRW_SURFACE_Y_OFFSET_SHIFT | > - (mt->align_h == 4 ? BRW_SURFACE_VERTICAL_ALIGN_ENABLE : 0)); > + surf[4] = brw_get_surface_num_multisamples(mt->num_samples) | > + min_array_element << BRW_SURFACE_MIN_ARRAY_ELEMENT_SHIFT | > + depth << BRW_SURFACE_RENDER_TARGET_VIEW_EXTENT_SHIFT; > + > + surf[5] = (mt->align_h == 4 ? BRW_SURFACE_VERTICAL_ALIGN_ENABLE : 0); You can drop the parentheses; > > if (brw->gen < 6) { > /* _NEW_COLOR */ > -- > 2.0.0.rc4 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev