On Wednesday, April 29, 2015 08:58:18 AM Pohjolainen, Topi wrote: > On Tue, Apr 28, 2015 at 02:45:27PM -0700, Kenneth Graunke wrote: > > On Wednesday, April 22, 2015 11:47:33 PM Topi Pohjolainen wrote: > > > Also changed a couple of direct shifts into SET_FIELD(). > > > > > > Signed-off-by: Topi Pohjolainen <topi.pohjolai...@intel.com> > > > --- > > > src/mesa/drivers/dri/i965/brw_context.h | 3 ++- > > > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 30 > > > +++++++++++++---------- > > > src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 14 +++++------ > > > src/mesa/drivers/dri/i965/gen8_surface_state.c | 10 +++----- > > > 4 files changed, 29 insertions(+), 28 deletions(-) > > > > > > diff --git a/src/mesa/drivers/dri/i965/brw_context.h > > > b/src/mesa/drivers/dri/i965/brw_context.h > > > index b90d329..ae28955 100644 > > > --- a/src/mesa/drivers/dri/i965/brw_context.h > > > +++ b/src/mesa/drivers/dri/i965/brw_context.h > > > @@ -964,10 +964,11 @@ struct brw_context > > > { > > > void (*update_texture_surface)(struct brw_context *brw, > > > const struct intel_mipmap_tree *mt, > > > - struct gl_texture_object *tObj, > > > uint32_t tex_format, > > > bool is_integer_format, > > > GLenum target, uint32_t > > > effective_depth, > > > + uint32_t min_layer, > > > + uint32_t min_lod, uint32_t > > > mip_count, > > > int swizzle, uint32_t *surf_offset, > > > bool for_gather); > > > uint32_t (*update_renderbuffer_surface)(struct brw_context *brw, > > > 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 f7acad4..ad5ddb5 100644 > > > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > > > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > > > @@ -310,16 +310,16 @@ update_buffer_texture_surface(struct gl_context > > > *ctx, > > > static void > > > brw_update_texture_surface(struct brw_context *brw, > > > const struct intel_mipmap_tree *mt, > > > - struct gl_texture_object *tObj, > > > uint32_t tex_format, > > > bool is_integer_format /* unused */, > > > GLenum target, > > > uint32_t effective_depth /* unused */, > > > + uint32_t min_layer /* unused */, > > > + uint32_t min_lod, uint32_t mip_count, > > > int swizzle /* unused */, > > > uint32_t *surf_offset, > > > bool for_gather) > > > { > > > - struct intel_texture_object *intelObj = intel_texture_object(tObj); > > > uint32_t *surf; > > > > > > surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE, > > > @@ -361,16 +361,16 @@ brw_update_texture_surface(struct brw_context *brw, > > > > > > surf[1] = mt->bo->offset64 + mt->offset; /* reloc */ > > > > > > - surf[2] = ((intelObj->_MaxLevel - tObj->BaseLevel) << > > > BRW_SURFACE_LOD_SHIFT | > > > - (mt->logical_width0 - 1) << BRW_SURFACE_WIDTH_SHIFT | > > > - (mt->logical_height0 - 1) << BRW_SURFACE_HEIGHT_SHIFT); > > > + surf[2] = SET_FIELD(mip_count, BRW_SURFACE_LOD) | > > > + SET_FIELD(mt->logical_width0 - 1, BRW_SURFACE_WIDTH) | > > > + SET_FIELD(mt->logical_height0 - 1, BRW_SURFACE_HEIGHT); > > > > > > - surf[3] = (brw_get_surface_tiling_bits(mt->tiling) | > > > - (mt->logical_depth0 - 1) << BRW_SURFACE_DEPTH_SHIFT | > > > - (mt->pitch - 1) << BRW_SURFACE_PITCH_SHIFT); > > > + surf[3] = brw_get_surface_tiling_bits(mt->tiling) | > > > + SET_FIELD(mt->logical_depth0 - 1, BRW_SURFACE_DEPTH) | > > > + SET_FIELD(mt->pitch - 1, BRW_SURFACE_PITCH); > > > > > > - surf[4] = (brw_get_surface_num_multisamples(mt->num_samples) | > > > - SET_FIELD(tObj->BaseLevel - mt->first_level, > > > BRW_SURFACE_MIN_LOD)); > > > + surf[4] = brw_get_surface_num_multisamples(mt->num_samples) | > > > + SET_FIELD(min_lod, BRW_SURFACE_MIN_LOD); > > > > This is not equivalent...Min Lod used to be: > > > > tObj->BaseLevel - mt->first_level > > > > and now it is: > > > > tObj->MinLevel + tObj->BaseLevel - mt->first_level > > > > I would really appreciate it if you could make this a separate patch > > from the refactoring, for easier bisectability. (First add tObj->MinLevel > > to the Gen4-6 code, then do this refactor.) > > > > It seems like a fine change, but is certainly worth noting in the commit > > message. Perhaps this is what fixed some tests? > > Mark helped me to bisect that with Jenkins, it wasn't this patch. It was > > i965: Pass slice details as parameters for surface setup > > > Anyway, as the series grew I started forgetting things I meant to say or > fix. This was something I meant to address, my apologies for you to need to > figure it out manually. I actually thought adding assert and comment here: > > /* Mininum level setting is only used for ARB_texture_view which isn't > * enabled before gen7. > */ > assert(ctx->Extensions.ARB_texture_view || tObj->MinLevel == 0); > > How would you feel about that?
Ahh, right, that is only used for texture view. So they are equivalent. Cool. Seems like a reasonable assertion to add.
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev