[Mesa-dev] [PATCH 6/9] i965: Don't relayout a texture just for baselevel changes.
Chad Versace
chad.versace at linux.intel.com
Fri Sep 27 15:50:27 PDT 2013
On 09/18/2013 12:59 PM, Eric Anholt wrote:
> As long as the baselevel, maxlevel still sit inside the range we had
> previously validated, there's no need to reallocate the texture.
>
> I also hope this makes our texture validation logic much more obvious.
> It's taken me enough tries to write this change, that's for sure. Reduces
> miptree copy count on a piglit run by 1.3%, though the change in amount of
> data moved is much smaller.
> ---
> src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 1 -
> src/mesa/drivers/dri/i965/intel_tex_obj.h | 3 ++
> src/mesa/drivers/dri/i965/intel_tex_validate.c | 60 ++++++++++++++---------
> 3 files changed, 39 insertions(+), 25 deletions(-)
>
> 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 115b296..60d26ab 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> @@ -282,7 +282,6 @@ gen7_update_texture_surface(struct gl_context *ctx,
> struct intel_texture_object *intelObj = intel_texture_object(tObj);
> struct intel_mipmap_tree *mt = intelObj->mt;
> struct gl_texture_image *firstImage = tObj->Image[0][tObj->BaseLevel];
> - struct intel_texture_image *intel_image = intel_texture_image(firstImage);
> struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
>
> if (tObj->Target == GL_TEXTURE_BUFFER) {
This hunks belongs to patch 1.
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_obj.h b/src/mesa/drivers/dri/i965/intel_tex_obj.h
> index e30dd8a..2d783c3 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_obj.h
> +++ b/src/mesa/drivers/dri/i965/intel_tex_obj.h
> @@ -41,6 +41,9 @@ struct intel_texture_object
> */
> unsigned int _MaxLevel;
>
> + unsigned int validated_first_level;
> + unsigned int validated_last_level;
> +
> /* On validation any active images held in main memory or in other
> * regions will be copied to this region and the old storage freed.
> */
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_validate.c b/src/mesa/drivers/dri/i965/intel_tex_validate.c
> index c34c144..42533bb 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_validate.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_validate.c
> @@ -11,31 +11,32 @@
> #define FILE_DEBUG_FLAG DEBUG_TEXTURE
>
> /**
> - * When validating, we only care about the texture images that could
> - * be seen, so for non-mipmapped modes we want to ignore everything
> - * but BaseLevel.
> + * Sets our driver-specific variant of tObj->_MaxLevel for later surface state
> + * upload.
> + *
> + * If we're only ensuring that there is storage for the first miplevel of a
> + * texture, then in texture setup we're going to have to make sure we don't
> + * allow sampling beyond level 0.
> */
> static void
> intel_update_max_level(struct intel_texture_object *intelObj,
> struct gl_sampler_object *sampler)
> {
> struct gl_texture_object *tObj = &intelObj->base;
> - int maxlevel;
>
> if (sampler->MinFilter == GL_NEAREST ||
> sampler->MinFilter == GL_LINEAR) {
> - maxlevel = tObj->BaseLevel;
> + intelObj->_MaxLevel = tObj->BaseLevel;
> } else {
> - maxlevel = tObj->_MaxLevel;
> - }
> -
> - if (intelObj->_MaxLevel != maxlevel) {
> - intelObj->_MaxLevel = maxlevel;
> - intelObj->needs_validate = true;
> + intelObj->_MaxLevel = tObj->_MaxLevel;
> }
> }
>
> -/*
> +/**
> + * At rendering-from-a-texture time, make sure that the texture object has a
> + * miptree that can hold the entire texture based on
> + * BaseLevel/MaxLevel/filtering, and copy in any texture images that are
> + * stored in other miptrees.
> */
> GLuint
> intel_finalize_mipmap_tree(struct brw_context *brw, GLuint unit)
> @@ -53,18 +54,26 @@ intel_finalize_mipmap_tree(struct brw_context *brw, GLuint unit)
> if (tObj->Target == GL_TEXTURE_BUFFER)
> return true;
>
> - /* We know/require this is true by now:
> + /* We know that this is true by now, and if it wasn't, we might have
> + * mismatched level sizes and the copies would fail.
> */
> assert(intelObj->base._BaseComplete);
>
> - /* What levels must the tree include at a minimum?
> - */
> intel_update_max_level(intelObj, sampler);
> - if (intelObj->mt && intelObj->mt->first_level != tObj->BaseLevel)
> - intelObj->needs_validate = true;
>
> - if (!intelObj->needs_validate)
> + /* What levels does this validated texture image require? */
> + int validate_first_level = tObj->BaseLevel;
> + int validate_last_level = intelObj->_MaxLevel;
> +
> + /* Skip the loop over images in the common case of no images having
> + * changed. But if the GL_BASE_LEVEL / GL_MAX_LEVEL change to something we
> + * haven't looked at, then we do need to look at those new images.
> + */
I parsed this phrase badly: "if the GL_BASE_LEVEL / GL_MAX_LEVEL change". Please rephrase
it to "if GL_BASE_LEVEL or GL_MAX_LEVEL change".
> + if (!intelObj->needs_validate &&
> + validate_first_level >= intelObj->validated_first_level &&
> + validate_last_level <= intelObj->validated_last_level) {
> return true;
> + }
>
> firstImage = intel_texture_image(tObj->Image[0][tObj->BaseLevel]);
>
> @@ -78,8 +87,8 @@ intel_finalize_mipmap_tree(struct brw_context *brw, GLuint unit)
> */
> if (intelObj->mt &&
> (!intel_miptree_match_image(intelObj->mt, &firstImage->base.Base) ||
> - intelObj->mt->first_level != tObj->BaseLevel ||
> - intelObj->mt->last_level < intelObj->_MaxLevel)) {
> + validate_first_level < intelObj->mt->first_level ||
> + validate_last_level > intelObj->mt->last_level)) {
> intel_miptree_release(&intelObj->mt);
> }
>
> @@ -93,13 +102,14 @@ intel_finalize_mipmap_tree(struct brw_context *brw, GLuint unit)
> perf_debug("Creating new %s %dx%dx%d %d..%d miptree to handle finalized "
> "texture miptree.\n",
> _mesa_get_format_name(firstImage->base.Base.TexFormat),
> - width, height, depth, tObj->BaseLevel, intelObj->_MaxLevel);
> + width, height, depth,
> + validate_first_level, validate_last_level);
>
> intelObj->mt = intel_miptree_create(brw,
> intelObj->base.Target,
> firstImage->base.Base.TexFormat,
> - tObj->BaseLevel,
> - intelObj->_MaxLevel,
> + validate_first_level,
> + validate_last_level,
> width,
> height,
> depth,
> @@ -114,7 +124,7 @@ intel_finalize_mipmap_tree(struct brw_context *brw, GLuint unit)
> */
> nr_faces = _mesa_num_tex_faces(intelObj->base.Target);
> for (face = 0; face < nr_faces; face++) {
> - for (i = tObj->BaseLevel; i <= intelObj->_MaxLevel; i++) {
> + for (i = validate_first_level; i <= validate_last_level; i++) {
> struct intel_texture_image *intelImage =
> intel_texture_image(intelObj->base.Image[face][i]);
> /* skip too small size mipmap */
> @@ -134,6 +144,8 @@ intel_finalize_mipmap_tree(struct brw_context *brw, GLuint unit)
> }
> }
>
> + intelObj->validated_first_level = validate_first_level;
> + intelObj->validated_last_level = validate_last_level;
> intelObj->needs_validate = false;
>
> return true;
>
This is fragile stuff. I did my best to dig into this code and verify the patch,
so... FWIW, with the two fixes it has my r-b.
More information about the mesa-dev
mailing list