[Mesa-dev] [PATCH] i965: Rename MIPTREE_LAYOUT_ALLOC_* -> MIPTREE_LAYOUT_TILING_*.
Ben Widawsky
ben at bwidawsk.net
Thu Aug 6 11:14:17 PDT 2015
On Thu, Aug 06, 2015 at 11:03:55AM -0700, Matt Turner wrote:
> Ben suggested that I rename MIPTREE_LAYOUT_ALLOC_ANY_TILED since it
> needed to include no tiling at all, but the name
> MIPTREE_LAYOUT_ALLOC_ANY is pretty nondescriptive. We can avoid
> confusion by replacing "ALLOC" with "TILING" in the identifiers.
The only nit I have, and the primary reason I chose "ALLOC" was because the flag
is only relevant when you're creating a miptree with a new BO. There are
functions to import a BO into a miptree, and those should never have such flags
set. I was just trying to make it very explicit that this flag is only valid
when you are creating the BO.
If you can think of any better way to describe it, I'd be appreciative, but since
you fixed the bug, I don't mind you doing it as you like.
> ---
> Once reviewed, I plan to land the patches in the following order
>
> i965: Request a miptree with no tiling intel_miptree_map_blit().
> i965: Correct a mistake that always forced texture tiling.
> i965: Rename MIPTREE_LAYOUT_ALLOC_* -> MIPTREE_LAYOUT_TILING_*.
>
> The ordering of the first two is particularly important, since the
> bug fixed in the first was masked by the bug in the second, landing
> them in the other order would expose the bug fixed in the first.
>
> src/mesa/drivers/dri/i965/brw_tex_layout.c | 8 ++++----
> src/mesa/drivers/dri/i965/intel_fbo.c | 2 +-
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 16 ++++++++--------
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 8 ++++----
> src/mesa/drivers/dri/i965/intel_tex.c | 2 +-
> src/mesa/drivers/dri/i965/intel_tex_image.c | 2 +-
> src/mesa/drivers/dri/i965/intel_tex_validate.c | 2 +-
> 7 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> index fb78b08..c9ee526 100644
> --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
> +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
> @@ -630,12 +630,12 @@ brw_miptree_choose_tiling(struct brw_context *brw,
> /* Some usages may want only one type of tiling, like depth miptrees (Y
> * tiled), or temporary BOs for uploading data once (linear).
> */
> - switch (layout_flags & MIPTREE_LAYOUT_ALLOC_ANY_TILED) {
> - case MIPTREE_LAYOUT_ALLOC_ANY_TILED:
> + switch (layout_flags & MIPTREE_LAYOUT_TILING_ANY) {
> + case MIPTREE_LAYOUT_TILING_ANY:
> break;
> - case MIPTREE_LAYOUT_ALLOC_YTILED:
> + case MIPTREE_LAYOUT_TILING_Y:
> return I915_TILING_Y;
> - case MIPTREE_LAYOUT_ALLOC_LINEAR:
> + case MIPTREE_LAYOUT_TILING_NONE:
> return I915_TILING_NONE;
> }
Just sanity checking myself, but the r-b at the bottom is assuming this is the
only functional change (it was all I could spot).
>
> diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
> index 87ba624..72648b0 100644
> --- a/src/mesa/drivers/dri/i965/intel_fbo.c
> +++ b/src/mesa/drivers/dri/i965/intel_fbo.c
> @@ -1023,7 +1023,7 @@ intel_renderbuffer_move_to_temp(struct brw_context *brw,
> int width, height, depth;
>
> uint32_t layout_flags = MIPTREE_LAYOUT_ACCELERATED_UPLOAD |
> - MIPTREE_LAYOUT_ALLOC_ANY_TILED;
> + MIPTREE_LAYOUT_TILING_ANY;
>
> intel_miptree_get_dimensions_for_image(rb->TexImage, &width, &height, &depth);
>
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index cb2791d..e85c3f0 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -455,7 +455,7 @@ intel_miptree_create_layout(struct brw_context *brw,
> uint32_t stencil_flags = MIPTREE_LAYOUT_ACCELERATED_UPLOAD;
> if (brw->gen == 6) {
> stencil_flags |= MIPTREE_LAYOUT_FORCE_ALL_SLICE_AT_LOD |
> - MIPTREE_LAYOUT_ALLOC_ANY_TILED;
> + MIPTREE_LAYOUT_TILING_ANY;
> }
>
> mt->stencil_mt = intel_miptree_create(brw,
> @@ -759,8 +759,8 @@ intel_miptree_create_for_bo(struct brw_context *brw,
> /* The BO already has a tiling format and we shouldn't confuse the lower
> * layers by making it try to find a tiling format again.
> */
> - assert((layout_flags & MIPTREE_LAYOUT_ALLOC_ANY_TILED) == 0);
> - assert((layout_flags & MIPTREE_LAYOUT_ALLOC_LINEAR) == 0);
> + assert((layout_flags & MIPTREE_LAYOUT_TILING_ANY) == 0);
> + assert((layout_flags & MIPTREE_LAYOUT_TILING_NONE) == 0);
>
> layout_flags |= MIPTREE_LAYOUT_FOR_BO;
> mt = intel_miptree_create_layout(brw, target, format,
> @@ -874,7 +874,7 @@ intel_miptree_create_for_renderbuffer(struct brw_context *brw,
> bool ok;
> GLenum target = num_samples > 1 ? GL_TEXTURE_2D_MULTISAMPLE : GL_TEXTURE_2D;
> const uint32_t layout_flags = MIPTREE_LAYOUT_ACCELERATED_UPLOAD |
> - MIPTREE_LAYOUT_ALLOC_ANY_TILED;
> + MIPTREE_LAYOUT_TILING_ANY;
>
>
> mt = intel_miptree_create(brw, target, format, 0, 0,
> @@ -1385,7 +1385,7 @@ intel_miptree_alloc_mcs(struct brw_context *brw,
> * "The MCS surface must be stored as Tile Y."
> */
> const uint32_t mcs_flags = MIPTREE_LAYOUT_ACCELERATED_UPLOAD |
> - MIPTREE_LAYOUT_ALLOC_YTILED;
> + MIPTREE_LAYOUT_TILING_Y;
> mt->mcs_mt = intel_miptree_create(brw,
> mt->target,
> format,
> @@ -1444,7 +1444,7 @@ intel_miptree_alloc_non_msrt_mcs(struct brw_context *brw,
> ALIGN(mt->logical_height0, height_divisor) / height_divisor;
> assert(mt->logical_depth0 == 1);
> uint32_t layout_flags = MIPTREE_LAYOUT_ACCELERATED_UPLOAD |
> - MIPTREE_LAYOUT_ALLOC_YTILED;
> + MIPTREE_LAYOUT_TILING_Y;
> if (brw->gen >= 8) {
> layout_flags |= MIPTREE_LAYOUT_FORCE_HALIGN16;
> }
> @@ -1709,7 +1709,7 @@ intel_hiz_miptree_buf_create(struct brw_context *brw,
> if (!buf)
> return NULL;
>
> - layout_flags |= MIPTREE_LAYOUT_ALLOC_ANY_TILED;
> + layout_flags |= MIPTREE_LAYOUT_TILING_ANY;
> buf->mt = intel_miptree_create(brw,
> mt->target,
> mt->format,
> @@ -2149,7 +2149,7 @@ intel_miptree_map_blit(struct brw_context *brw,
> map->mt = intel_miptree_create(brw, GL_TEXTURE_2D, mt->format,
> 0, 0,
> map->w, map->h, 1,
> - 0, MIPTREE_LAYOUT_ALLOC_LINEAR);
> + 0, MIPTREE_LAYOUT_TILING_NONE);
>
> if (!map->mt) {
> fprintf(stderr, "Failed to allocate blit temporary\n");
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 506c258..790d312 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -536,10 +536,10 @@ enum {
> MIPTREE_LAYOUT_DISABLE_AUX = 1 << 3,
> MIPTREE_LAYOUT_FORCE_HALIGN16 = 1 << 4,
>
> - MIPTREE_LAYOUT_ALLOC_YTILED = 1 << 5,
> - MIPTREE_LAYOUT_ALLOC_LINEAR = 1 << 6,
> - MIPTREE_LAYOUT_ALLOC_ANY_TILED = MIPTREE_LAYOUT_ALLOC_YTILED |
> - MIPTREE_LAYOUT_ALLOC_LINEAR,
> + MIPTREE_LAYOUT_TILING_Y = 1 << 5,
> + MIPTREE_LAYOUT_TILING_NONE = 1 << 6,
> + MIPTREE_LAYOUT_TILING_ANY = MIPTREE_LAYOUT_TILING_Y |
> + MIPTREE_LAYOUT_TILING_NONE,
Can you add a comment about why "ANY" doesn't really mean any? (X-tiling is
useless garbage)
> };
>
> struct intel_mipmap_tree *intel_miptree_create(struct brw_context *brw,
> diff --git a/src/mesa/drivers/dri/i965/intel_tex.c b/src/mesa/drivers/dri/i965/intel_tex.c
> index 8fa5e3c..e16b0de 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex.c
> @@ -145,7 +145,7 @@ intel_alloc_texture_storage(struct gl_context *ctx,
> 0, levels - 1,
> width, height, depth,
> num_samples,
> - MIPTREE_LAYOUT_ALLOC_ANY_TILED);
> + MIPTREE_LAYOUT_TILING_ANY);
>
> if (intel_texobj->mt == NULL) {
> return false;
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c b/src/mesa/drivers/dri/i965/intel_tex_image.c
> index 3611280..93a8cde 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> @@ -80,7 +80,7 @@ intel_miptree_create_for_teximage(struct brw_context *brw,
> height,
> depth,
> intelImage->base.Base.NumSamples,
> - layout_flags | MIPTREE_LAYOUT_ALLOC_ANY_TILED);
> + layout_flags | MIPTREE_LAYOUT_TILING_ANY);
> }
>
> static void
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_validate.c b/src/mesa/drivers/dri/i965/intel_tex_validate.c
> index 6ebf381..d3fb252 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_validate.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_validate.c
> @@ -137,7 +137,7 @@ intel_finalize_mipmap_tree(struct brw_context *brw, GLuint unit)
> width, height, depth, validate_last_level + 1);
>
> const uint32_t layout_flags = MIPTREE_LAYOUT_ACCELERATED_UPLOAD |
> - MIPTREE_LAYOUT_ALLOC_ANY_TILED;
> + MIPTREE_LAYOUT_TILING_ANY;
> intelObj->mt = intel_miptree_create(brw,
> intelObj->base.Target,
> firstImage->base.Base.TexFormat,
Reviewed-by: Ben Widawsky <ben at bwidawsk.net>
More information about the mesa-dev
mailing list