Discussion:
i965/gen6: Represent stencil surfaces using isl
(too old to reply)
Topi Pohjolainen
2017-06-13 14:49:58 UTC
Permalink
Raw Message
This is part of earlier work in the list. Major difference is that
everything is now rebased on the new "side-by-side" stencil layout.

Topi Pohjolainen (15):
i965/miptree: Refactor mapping table alloc
i965: Add helper for converting isl tiling to bufmgr tiling
i965/miptree: Add isl surface
i965: Prepare up/downsampling for isl based miptrees
i965: Prepare image validation for isl based miptrees
i965: Prepare slice validator for isl based miptrees
i965: Prepare framebuffer validator for isl based miptrees
i965/tex: Prepare image update for isl based miptrees
i965: Prepare slice copy for isl based miptrees
i965/miptree: Add option to resolve offsets using isl_surf
i965: Add isl based miptree creator
i965/blorp: Prepare for isl based miptrees
i965/miptree: Prepare stencil mapping for isl based
i965/miptree: Prepare range getter for isl based
i965/gen6: Use isl for stencil surfaces

src/mesa/drivers/dri/i965/brw_blorp.c | 9 +-
src/mesa/drivers/dri/i965/gen6_depth_state.c | 31 ++--
src/mesa/drivers/dri/i965/intel_blit.h | 13 ++
src/mesa/drivers/dri/i965/intel_fbo.c | 33 +++-
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 258 +++++++++++++++++++++++---
src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 10 +-
src/mesa/drivers/dri/i965/intel_tex_image.c | 19 +-
7 files changed, 324 insertions(+), 49 deletions(-)
--
2.11.0
Topi Pohjolainen
2017-06-13 14:50:02 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 46 ++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 78a223a7f3..061860cdf6 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -2800,27 +2800,57 @@ intel_miptree_updownsample(struct brw_context *brw,
struct intel_mipmap_tree *src,
struct intel_mipmap_tree *dst)
{
+ unsigned src_w, src_h, dst_w, dst_h;
+
+ if (src->surf.size > 0) {
+ src_w = src->surf.logical_level0_px.width;
+ src_h = src->surf.logical_level0_px.height;
+ } else {
+ src_w = src->logical_width0;
+ src_h = src->logical_height0;
+ }
+
+ if (dst->surf.size > 0) {
+ dst_w = dst->surf.logical_level0_px.width;
+ dst_h = dst->surf.logical_level0_px.height;
+ } else {
+ dst_w = dst->logical_width0;
+ dst_h = dst->logical_height0;
+ }
+
brw_blorp_blit_miptrees(brw,
src, 0 /* level */, 0 /* layer */,
src->format, SWIZZLE_XYZW,
dst, 0 /* level */, 0 /* layer */, dst->format,
- 0, 0,
- src->logical_width0, src->logical_height0,
- 0, 0,
- dst->logical_width0, dst->logical_height0,
+ 0, 0, src_w, src_h,
+ 0, 0, dst_w, dst_h,
GL_NEAREST, false, false /*mirror x, y*/,
false, false);

if (src->stencil_mt) {
+ if (src->stencil_mt->surf.size > 0) {
+ src_w = src->stencil_mt->surf.logical_level0_px.width;
+ src_h = src->stencil_mt->surf.logical_level0_px.height;
+ } else {
+ src_w = src->stencil_mt->logical_width0;
+ src_h = src->stencil_mt->logical_height0;
+ }
+
+ if (dst->stencil_mt->surf.size > 0) {
+ dst_w = dst->stencil_mt->surf.logical_level0_px.width;
+ dst_h = dst->stencil_mt->surf.logical_level0_px.height;
+ } else {
+ dst_w = dst->stencil_mt->logical_width0;
+ dst_h = dst->stencil_mt->logical_height0;
+ }
+
brw_blorp_blit_miptrees(brw,
src->stencil_mt, 0 /* level */, 0 /* layer */,
src->stencil_mt->format, SWIZZLE_XYZW,
dst->stencil_mt, 0 /* level */, 0 /* layer */,
dst->stencil_mt->format,
- 0, 0,
- src->logical_width0, src->logical_height0,
- 0, 0,
- dst->logical_width0, dst->logical_height0,
+ 0, 0, src_w, src_h,
+ 0, 0, dst_w, dst_h,
GL_NEAREST, false, false /*mirror x, y*/,
false, false /* decode/encode srgb */);
}
--
2.11.0
Nanley Chery
2017-06-14 23:08:25 UTC
Permalink
Raw Message
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 46 ++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 8 deletions(-)
Patches 2-4 are
Post by Topi Pohjolainen
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 78a223a7f3..061860cdf6 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -2800,27 +2800,57 @@ intel_miptree_updownsample(struct brw_context *brw,
struct intel_mipmap_tree *src,
struct intel_mipmap_tree *dst)
{
+ unsigned src_w, src_h, dst_w, dst_h;
+
+ if (src->surf.size > 0) {
+ src_w = src->surf.logical_level0_px.width;
+ src_h = src->surf.logical_level0_px.height;
+ } else {
+ src_w = src->logical_width0;
+ src_h = src->logical_height0;
+ }
+
+ if (dst->surf.size > 0) {
+ dst_w = dst->surf.logical_level0_px.width;
+ dst_h = dst->surf.logical_level0_px.height;
+ } else {
+ dst_w = dst->logical_width0;
+ dst_h = dst->logical_height0;
+ }
+
brw_blorp_blit_miptrees(brw,
src, 0 /* level */, 0 /* layer */,
src->format, SWIZZLE_XYZW,
dst, 0 /* level */, 0 /* layer */, dst->format,
- 0, 0,
- src->logical_width0, src->logical_height0,
- 0, 0,
- dst->logical_width0, dst->logical_height0,
+ 0, 0, src_w, src_h,
+ 0, 0, dst_w, dst_h,
GL_NEAREST, false, false /*mirror x, y*/,
false, false);
if (src->stencil_mt) {
+ if (src->stencil_mt->surf.size > 0) {
+ src_w = src->stencil_mt->surf.logical_level0_px.width;
+ src_h = src->stencil_mt->surf.logical_level0_px.height;
+ } else {
+ src_w = src->stencil_mt->logical_width0;
+ src_h = src->stencil_mt->logical_height0;
+ }
+
+ if (dst->stencil_mt->surf.size > 0) {
+ dst_w = dst->stencil_mt->surf.logical_level0_px.width;
+ dst_h = dst->stencil_mt->surf.logical_level0_px.height;
+ } else {
+ dst_w = dst->stencil_mt->logical_width0;
+ dst_h = dst->stencil_mt->logical_height0;
+ }
+
brw_blorp_blit_miptrees(brw,
src->stencil_mt, 0 /* level */, 0 /* layer */,
src->stencil_mt->format, SWIZZLE_XYZW,
dst->stencil_mt, 0 /* level */, 0 /* layer */,
dst->stencil_mt->format,
- 0, 0,
- src->logical_width0, src->logical_height0,
- 0, 0,
- dst->logical_width0, dst->logical_height0,
+ 0, 0, src_w, src_h,
+ 0, 0, dst_w, dst_h,
GL_NEAREST, false, false /*mirror x, y*/,
false, false /* decode/encode srgb */);
}
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Jason Ekstrand
2017-06-15 18:15:20 UTC
Permalink
Raw Message
Post by Topi Pohjolainen
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 46
++++++++++++++++++++++-----
Post by Topi Pohjolainen
1 file changed, 38 insertions(+), 8 deletions(-)
Patches 2-4 are
Post by Topi Pohjolainen
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
Post by Topi Pohjolainen
index 78a223a7f3..061860cdf6 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -2800,27 +2800,57 @@ intel_miptree_updownsample(struct brw_context
*brw,
Post by Topi Pohjolainen
struct intel_mipmap_tree *src,
struct intel_mipmap_tree *dst)
{
+ unsigned src_w, src_h, dst_w, dst_h;
+
+ if (src->surf.size > 0) {
+ src_w = src->surf.logical_level0_px.width;
+ src_h = src->surf.logical_level0_px.height;
+ } else {
+ src_w = src->logical_width0;
+ src_h = src->logical_height0;
+ }
+
+ if (dst->surf.size > 0) {
+ dst_w = dst->surf.logical_level0_px.width;
+ dst_h = dst->surf.logical_level0_px.height;
+ } else {
+ dst_w = dst->logical_width0;
+ dst_h = dst->logical_height0;
+ }
+
brw_blorp_blit_miptrees(brw,
src, 0 /* level */, 0 /* layer */,
src->format, SWIZZLE_XYZW,
dst, 0 /* level */, 0 /* layer */,
dst->format,
Post by Topi Pohjolainen
- 0, 0,
- src->logical_width0, src->logical_height0,
- 0, 0,
- dst->logical_width0, dst->logical_height0,
+ 0, 0, src_w, src_h,
+ 0, 0, dst_w, dst_h,
GL_NEAREST, false, false /*mirror x, y*/,
false, false);
if (src->stencil_mt) {
+ if (src->stencil_mt->surf.size > 0) {
+ src_w = src->stencil_mt->surf.logical_level0_px.width;
+ src_h = src->stencil_mt->surf.logical_level0_px.height;
+ } else {
+ src_w = src->stencil_mt->logical_width0;
+ src_h = src->stencil_mt->logical_height0;
+ }
+
+ if (dst->stencil_mt->surf.size > 0) {
+ dst_w = dst->stencil_mt->surf.logical_level0_px.width;
+ dst_h = dst->stencil_mt->surf.logical_level0_px.height;
+ } else {
+ dst_w = dst->stencil_mt->logical_width0;
+ dst_h = dst->stencil_mt->logical_height0;
+ }
This would be way easier if it were just

if (src->stencil_mt)
intel_miptree_updownsample(brw, src->stencil_mt, dst->stencil_mt);

Oh, well. That can be a refactor for another day if you'd like.
Post by Topi Pohjolainen
+
Post by Topi Pohjolainen
brw_blorp_blit_miptrees(brw,
src->stencil_mt, 0 /* level */, 0 /*
layer */,
Post by Topi Pohjolainen
src->stencil_mt->format, SWIZZLE_XYZW,
dst->stencil_mt, 0 /* level */, 0 /*
layer */,
Post by Topi Pohjolainen
dst->stencil_mt->format,
- 0, 0,
- src->logical_width0, src->logical_height0,
- 0, 0,
- dst->logical_width0, dst->logical_height0,
+ 0, 0, src_w, src_h,
+ 0, 0, dst_w, dst_h,
GL_NEAREST, false, false /*mirror x, y*/,
false, false /* decode/encode srgb */);
}
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Topi Pohjolainen
2017-06-13 14:49:59 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 29 +++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 253d833b13..78a223a7f3 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -285,6 +285,26 @@ intel_depth_format_for_depthstencil_format(mesa_format format) {
}
}

+static bool
+create_mapping_table(GLenum target, unsigned first_level, unsigned last_level,
+ unsigned depth0, struct intel_mipmap_level *table)
+{
+ for (unsigned level = first_level; level <= last_level; level++) {
+ const unsigned d = target == GL_TEXTURE_3D ? depth0 >> level : depth0;
+
+ table[level].slice = calloc(d, sizeof(*table[0].slice));
+ if (!table[level].slice)
+ goto unwind;
+ }
+
+ return true;
+
+unwind:
+ for (unsigned level = first_level; level <= last_level; level++)
+ free(table[level].slice);
+
+ return false;
+}

/**
* @param for_bo Indicates that the caller is
@@ -424,6 +444,12 @@ intel_miptree_create_layout(struct brw_context *brw,
}
}

+ if (!create_mapping_table(target, first_level, last_level, depth0,
+ mt->level)) {
+ free(mt);
+ return NULL;
+ }
+
/* Set array_layout to ALL_SLICES_AT_EACH_LOD when array_spacing_lod0 can
* be used. array_spacing_lod0 is only used for non-IMS MSAA surfaces on
* Gen 7 and 8. On Gen 8 and 9 this layout is not available but it is still
@@ -1103,9 +1129,8 @@ intel_miptree_set_level_info(struct intel_mipmap_tree *mt,
DBG("%s level %d, depth %d, offset %d,%d\n", __func__,
level, d, x, y);

- assert(mt->level[level].slice == NULL);
+ assert(mt->level[level].slice);

- mt->level[level].slice = calloc(d, sizeof(*mt->level[0].slice));
mt->level[level].slice[0].x_offset = mt->level[level].level_x;
mt->level[level].slice[0].y_offset = mt->level[level].level_y;
}
--
2.11.0
Nanley Chery
2017-06-13 23:31:26 UTC
Permalink
Raw Message
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 29 +++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 253d833b13..78a223a7f3 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -285,6 +285,26 @@ intel_depth_format_for_depthstencil_format(mesa_format format) {
}
}
+static bool
+create_mapping_table(GLenum target, unsigned first_level, unsigned last_level,
+ unsigned depth0, struct intel_mipmap_level *table)
+{
+ for (unsigned level = first_level; level <= last_level; level++) {
+ const unsigned d = target == GL_TEXTURE_3D ? depth0 >> level : depth0;
There's a bug here. If the target is GL_TEXTURE_3D we should
minify(depth0, level) to avoid setting a depth of 0.

This seems to be more than a refactor. Prior to this patch,
brw_miptree_layout_gen6_hiz_stencil wouldn't shrink the number of slices
per mipmap level as the level increases, but does so now.

-Nanley
Post by Topi Pohjolainen
+
+ table[level].slice = calloc(d, sizeof(*table[0].slice));
+ if (!table[level].slice)
+ goto unwind;
+ }
+
+ return true;
+
+ for (unsigned level = first_level; level <= last_level; level++)
+ free(table[level].slice);
+
+ return false;
+}
/**
@@ -424,6 +444,12 @@ intel_miptree_create_layout(struct brw_context *brw,
}
}
+ if (!create_mapping_table(target, first_level, last_level, depth0,
+ mt->level)) {
+ free(mt);
+ return NULL;
+ }
+
/* Set array_layout to ALL_SLICES_AT_EACH_LOD when array_spacing_lod0 can
* be used. array_spacing_lod0 is only used for non-IMS MSAA surfaces on
* Gen 7 and 8. On Gen 8 and 9 this layout is not available but it is still
@@ -1103,9 +1129,8 @@ intel_miptree_set_level_info(struct intel_mipmap_tree *mt,
DBG("%s level %d, depth %d, offset %d,%d\n", __func__,
level, d, x, y);
- assert(mt->level[level].slice == NULL);
+ assert(mt->level[level].slice);
- mt->level[level].slice = calloc(d, sizeof(*mt->level[0].slice));
mt->level[level].slice[0].x_offset = mt->level[level].level_x;
mt->level[level].slice[0].y_offset = mt->level[level].level_y;
}
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Pohjolainen, Topi
2017-06-14 18:45:46 UTC
Permalink
Raw Message
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 29 +++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 253d833b13..78a223a7f3 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -285,6 +285,26 @@ intel_depth_format_for_depthstencil_format(mesa_format format) {
}
}
+static bool
+create_mapping_table(GLenum target, unsigned first_level, unsigned last_level,
+ unsigned depth0, struct intel_mipmap_level *table)
+{
+ for (unsigned level = first_level; level <= last_level; level++) {
+ const unsigned d = target == GL_TEXTURE_3D ? depth0 >> level : depth0;
There's a bug here. If the target is GL_TEXTURE_3D we should
minify(depth0, level) to avoid setting a depth of 0.
Oops, definitely.
Post by Nanley Chery
This seems to be more than a refactor. Prior to this patch,
brw_miptree_layout_gen6_hiz_stencil wouldn't shrink the number of slices
per mipmap level as the level increases, but does so now.
Right. I actually missed that. How do want to handle that? I could write a
patch against brw_miptree_layout_gen6_hiz_stencil() doing the same thing there
(modifying the argument given to intel_miptree_set_level_info() but keeping
actual allocation size as it was in order to have space for level 0 qpitch).
Post by Nanley Chery
-Nanley
Post by Topi Pohjolainen
+
+ table[level].slice = calloc(d, sizeof(*table[0].slice));
+ if (!table[level].slice)
+ goto unwind;
+ }
+
+ return true;
+
+ for (unsigned level = first_level; level <= last_level; level++)
+ free(table[level].slice);
+
+ return false;
+}
/**
@@ -424,6 +444,12 @@ intel_miptree_create_layout(struct brw_context *brw,
}
}
+ if (!create_mapping_table(target, first_level, last_level, depth0,
+ mt->level)) {
+ free(mt);
+ return NULL;
+ }
+
/* Set array_layout to ALL_SLICES_AT_EACH_LOD when array_spacing_lod0 can
* be used. array_spacing_lod0 is only used for non-IMS MSAA surfaces on
* Gen 7 and 8. On Gen 8 and 9 this layout is not available but it is still
@@ -1103,9 +1129,8 @@ intel_miptree_set_level_info(struct intel_mipmap_tree *mt,
DBG("%s level %d, depth %d, offset %d,%d\n", __func__,
level, d, x, y);
- assert(mt->level[level].slice == NULL);
+ assert(mt->level[level].slice);
- mt->level[level].slice = calloc(d, sizeof(*mt->level[0].slice));
mt->level[level].slice[0].x_offset = mt->level[level].level_x;
mt->level[level].slice[0].y_offset = mt->level[level].level_y;
}
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Nanley Chery
2017-06-14 22:42:16 UTC
Permalink
Raw Message
Post by Pohjolainen, Topi
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 29 +++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 253d833b13..78a223a7f3 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -285,6 +285,26 @@ intel_depth_format_for_depthstencil_format(mesa_format format) {
}
}
+static bool
+create_mapping_table(GLenum target, unsigned first_level, unsigned last_level,
+ unsigned depth0, struct intel_mipmap_level *table)
+{
+ for (unsigned level = first_level; level <= last_level; level++) {
+ const unsigned d = target == GL_TEXTURE_3D ? depth0 >> level : depth0;
There's a bug here. If the target is GL_TEXTURE_3D we should
minify(depth0, level) to avoid setting a depth of 0.
Oops, definitely.
Post by Nanley Chery
This seems to be more than a refactor. Prior to this patch,
brw_miptree_layout_gen6_hiz_stencil wouldn't shrink the number of slices
per mipmap level as the level increases, but does so now.
Right. I actually missed that. How do want to handle that? I could write a
patch against brw_miptree_layout_gen6_hiz_stencil() doing the same thing there
(modifying the argument given to intel_miptree_set_level_info() but keeping
actual allocation size as it was in order to have space for level 0 qpitch).
That should be sufficient.
Post by Pohjolainen, Topi
Post by Nanley Chery
-Nanley
Post by Topi Pohjolainen
+
+ table[level].slice = calloc(d, sizeof(*table[0].slice));
+ if (!table[level].slice)
+ goto unwind;
+ }
+
+ return true;
+
+ for (unsigned level = first_level; level <= last_level; level++)
+ free(table[level].slice);
+
+ return false;
+}
/**
@@ -424,6 +444,12 @@ intel_miptree_create_layout(struct brw_context *brw,
}
}
+ if (!create_mapping_table(target, first_level, last_level, depth0,
+ mt->level)) {
+ free(mt);
+ return NULL;
+ }
+
/* Set array_layout to ALL_SLICES_AT_EACH_LOD when array_spacing_lod0 can
* be used. array_spacing_lod0 is only used for non-IMS MSAA surfaces on
* Gen 7 and 8. On Gen 8 and 9 this layout is not available but it is still
@@ -1103,9 +1129,8 @@ intel_miptree_set_level_info(struct intel_mipmap_tree *mt,
DBG("%s level %d, depth %d, offset %d,%d\n", __func__,
level, d, x, y);
- assert(mt->level[level].slice == NULL);
+ assert(mt->level[level].slice);
- mt->level[level].slice = calloc(d, sizeof(*mt->level[0].slice));
mt->level[level].slice[0].x_offset = mt->level[level].level_x;
mt->level[level].slice[0].y_offset = mt->level[level].level_y;
}
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Topi Pohjolainen
2017-06-13 14:50:01 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index 7acfcb87a4..8479b285cb 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -369,6 +369,8 @@ struct intel_miptree_hiz_buffer

struct intel_mipmap_tree
{
+ struct isl_surf surf;
+
/**
* Buffer object containing the surface.
*
--
2.11.0
Topi Pohjolainen
2017-06-13 14:50:00 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/intel_blit.h | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/intel_blit.h b/src/mesa/drivers/dri/i965/intel_blit.h
index 2604417e2d..5e4d1f5eb4 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.h
+++ b/src/mesa/drivers/dri/i965/intel_blit.h
@@ -28,6 +28,19 @@

#include "brw_context.h"

+static inline unsigned
+isl_tiling_to_bufmgr_tiling(enum isl_tiling tiling)
+{
+ if (tiling == ISL_TILING_X)
+ return I915_TILING_X;
+
+ if (tiling == ISL_TILING_Y0)
+ return I915_TILING_Y;
+
+ /* All other are unknown to buffer allocator. */
+ return I915_TILING_NONE;
+}
+
bool
intelEmitCopyBlit(struct brw_context *brw,
GLuint cpp,
--
2.11.0
Jason Ekstrand
2017-06-15 18:12:29 UTC
Permalink
Raw Message
There are about three different versions of this helper floating around
these days, none of them merged. :-( Let's go ahead and land this and
someone can combine them later.

On Tue, Jun 13, 2017 at 7:50 AM, Topi Pohjolainen <
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_blit.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.h
b/src/mesa/drivers/dri/i965/intel_blit.h
index 2604417e2d..5e4d1f5eb4 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.h
+++ b/src/mesa/drivers/dri/i965/intel_blit.h
@@ -28,6 +28,19 @@
#include "brw_context.h"
+static inline unsigned
+isl_tiling_to_bufmgr_tiling(enum isl_tiling tiling)
+{
+ if (tiling == ISL_TILING_X)
+ return I915_TILING_X;
+
+ if (tiling == ISL_TILING_Y0)
+ return I915_TILING_Y;
+
+ /* All other are unknown to buffer allocator. */
+ return I915_TILING_NONE;
+}
+
bool
intelEmitCopyBlit(struct brw_context *brw,
GLuint cpp,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Nanley Chery
2017-06-16 00:18:22 UTC
Permalink
Raw Message
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_blit.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.h b/src/mesa/drivers/dri/i965/intel_blit.h
index 2604417e2d..5e4d1f5eb4 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.h
+++ b/src/mesa/drivers/dri/i965/intel_blit.h
@@ -28,6 +28,19 @@
#include "brw_context.h"
+static inline unsigned
+isl_tiling_to_bufmgr_tiling(enum isl_tiling tiling)
+{
+ if (tiling == ISL_TILING_X)
+ return I915_TILING_X;
+
+ if (tiling == ISL_TILING_Y0)
I actually just read the patch where this function is used. Don't we
also need to return I915_TILING_Y for ISL_TILING_W? Maybe Jason can
confirm.
Post by Topi Pohjolainen
+ return I915_TILING_Y;
+
+ /* All other are unknown to buffer allocator. */
It would seem that we'd like to assert for unexpected values instead of
failing silently.
Post by Topi Pohjolainen
+ return I915_TILING_NONE;
+}
+
bool
intelEmitCopyBlit(struct brw_context *brw,
GLuint cpp,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Jason Ekstrand
2017-06-16 01:23:24 UTC
Permalink
Raw Message
Post by Topi Pohjolainen
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_blit.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.h
b/src/mesa/drivers/dri/i965/intel_blit.h
Post by Topi Pohjolainen
index 2604417e2d..5e4d1f5eb4 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.h
+++ b/src/mesa/drivers/dri/i965/intel_blit.h
@@ -28,6 +28,19 @@
#include "brw_context.h"
+static inline unsigned
+isl_tiling_to_bufmgr_tiling(enum isl_tiling tiling)
+{
+ if (tiling == ISL_TILING_X)
+ return I915_TILING_X;
+
+ if (tiling == ISL_TILING_Y0)
I actually just read the patch where this function is used. Don't we
also need to return I915_TILING_Y for ISL_TILING_W? Maybe Jason can
confirm.
I'd rather the function not lie... Also, most places in the driver today
that do W-tiling use I915_TILING_NONE. Yes, I915_TILING_NONE is sort of
LINEAR but you could also interpret it as LINEAR || UNKNOWN.
Post by Topi Pohjolainen
Post by Topi Pohjolainen
+ return I915_TILING_Y;
+
+ /* All other are unknown to buffer allocator. */
It would seem that we'd like to assert for unexpected values instead of
failing silently.
I just pulled up my version:

https://cgit.freedesktop.org/~jekstrand/mesa/commit/?h=wip/i965-ccs-mod-v3&id=6133059904aeaa7bd6d4ee364014f491f59083e2

and I went with I915_TILING_NONE there. The reason I did so was so that
you could call

brw_bo_alloc_tiled(brw, name, surf.size,
isl_tiling_to_i915_tiling(surf.tiling), surf.row_pitch, 0);

without having to worry about isl_tiling_to_i915_tiling asserting on you.

The only two things that the tiling is used for are scanout and fenced
maps. Even when we enable things like Yf scan-out, we'll probably do it
through modifiers and not set_tiling. At that point, the only thing that
the I915_TILING parameters are actually needed for is doing fenced GTT maps
(which detile on the fly). Since you can't do a fenced map of anything
other than X and Y-tiled, I don't think we'll ever need to add I915_TILING
parameters for the others because you can't do a fenced (GTT) map of a Yf,
Ys, or W-tiled buffer anyway.

On the other hand, when used in something such as the blit code, asserting
is probably the right thing to do. One option would be to add a little
alloc_bo_for_isl_surf helper which knows to map everything other than X and
Y to LINEAR and make the general function assert.

--Jason
Post by Topi Pohjolainen
Post by Topi Pohjolainen
+ return I915_TILING_NONE;
+}
+
bool
intelEmitCopyBlit(struct brw_context *brw,
GLuint cpp,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Nanley Chery
2017-06-16 16:18:09 UTC
Permalink
Raw Message
Post by Jason Ekstrand
Post by Topi Pohjolainen
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_blit.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.h
b/src/mesa/drivers/dri/i965/intel_blit.h
Post by Topi Pohjolainen
index 2604417e2d..5e4d1f5eb4 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.h
+++ b/src/mesa/drivers/dri/i965/intel_blit.h
@@ -28,6 +28,19 @@
#include "brw_context.h"
+static inline unsigned
+isl_tiling_to_bufmgr_tiling(enum isl_tiling tiling)
+{
+ if (tiling == ISL_TILING_X)
+ return I915_TILING_X;
+
+ if (tiling == ISL_TILING_Y0)
I actually just read the patch where this function is used. Don't we
also need to return I915_TILING_Y for ISL_TILING_W? Maybe Jason can
confirm.
I'd rather the function not lie... Also, most places in the driver today
that do W-tiling use I915_TILING_NONE. Yes, I915_TILING_NONE is sort of
LINEAR but you could also interpret it as LINEAR || UNKNOWN.
Post by Topi Pohjolainen
Post by Topi Pohjolainen
+ return I915_TILING_Y;
+
+ /* All other are unknown to buffer allocator. */
It would seem that we'd like to assert for unexpected values instead of
failing silently.
https://cgit.freedesktop.org/~jekstrand/mesa/commit/?h=wip/i965-ccs-mod-v3&id=6133059904aeaa7bd6d4ee364014f491f59083e2
and I went with I915_TILING_NONE there. The reason I did so was so that
you could call
brw_bo_alloc_tiled(brw, name, surf.size,
isl_tiling_to_i915_tiling(surf.tiling), surf.row_pitch, 0);
without having to worry about isl_tiling_to_i915_tiling asserting on you.
The only two things that the tiling is used for are scanout and fenced
maps. Even when we enable things like Yf scan-out, we'll probably do it
through modifiers and not set_tiling. At that point, the only thing that
the I915_TILING parameters are actually needed for is doing fenced GTT maps
(which detile on the fly). Since you can't do a fenced map of anything
other than X and Y-tiled, I don't think we'll ever need to add I915_TILING
parameters for the others because you can't do a fenced (GTT) map of a Yf,
Ys, or W-tiled buffer anyway.
On the other hand, when used in something such as the blit code, asserting
is probably the right thing to do. One option would be to add a little
alloc_bo_for_isl_surf helper which knows to map everything other than X and
Y to LINEAR and make the general function assert.
--Jason
Thanks for the feedback!
Post by Jason Ekstrand
Post by Topi Pohjolainen
Post by Topi Pohjolainen
+ return I915_TILING_NONE;
+}
+
bool
intelEmitCopyBlit(struct brw_context *brw,
GLuint cpp,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Pohjolainen, Topi
2017-06-16 04:58:02 UTC
Permalink
Raw Message
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_blit.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.h b/src/mesa/drivers/dri/i965/intel_blit.h
index 2604417e2d..5e4d1f5eb4 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.h
+++ b/src/mesa/drivers/dri/i965/intel_blit.h
@@ -28,6 +28,19 @@
#include "brw_context.h"
+static inline unsigned
+isl_tiling_to_bufmgr_tiling(enum isl_tiling tiling)
+{
+ if (tiling == ISL_TILING_X)
+ return I915_TILING_X;
+
+ if (tiling == ISL_TILING_Y0)
I actually just read the patch where this function is used. Don't we
also need to return I915_TILING_Y for ISL_TILING_W? Maybe Jason can
confirm.
These values go to brw_bo_alloc_tiled() -> brw_bo_alloc() ->
bo_alloc_internal() which only understands Y and X. Therefore W is really
treated as NONE.
Post by Nanley Chery
Post by Topi Pohjolainen
+ return I915_TILING_Y;
+
+ /* All other are unknown to buffer allocator. */
It would seem that we'd like to assert for unexpected values instead of
failing silently.
Post by Topi Pohjolainen
+ return I915_TILING_NONE;
+}
+
bool
intelEmitCopyBlit(struct brw_context *brw,
GLuint cpp,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Nanley Chery
2017-06-16 16:18:27 UTC
Permalink
Raw Message
Post by Pohjolainen, Topi
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_blit.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_blit.h b/src/mesa/drivers/dri/i965/intel_blit.h
index 2604417e2d..5e4d1f5eb4 100644
--- a/src/mesa/drivers/dri/i965/intel_blit.h
+++ b/src/mesa/drivers/dri/i965/intel_blit.h
@@ -28,6 +28,19 @@
#include "brw_context.h"
+static inline unsigned
+isl_tiling_to_bufmgr_tiling(enum isl_tiling tiling)
+{
+ if (tiling == ISL_TILING_X)
+ return I915_TILING_X;
+
+ if (tiling == ISL_TILING_Y0)
I actually just read the patch where this function is used. Don't we
also need to return I915_TILING_Y for ISL_TILING_W? Maybe Jason can
confirm.
These values go to brw_bo_alloc_tiled() -> brw_bo_alloc() ->
bo_alloc_internal() which only understands Y and X. Therefore W is really
treated as NONE.
Got it.
Post by Pohjolainen, Topi
Post by Nanley Chery
Post by Topi Pohjolainen
+ return I915_TILING_Y;
+
+ /* All other are unknown to buffer allocator. */
It would seem that we'd like to assert for unexpected values instead of
failing silently.
Post by Topi Pohjolainen
+ return I915_TILING_NONE;
+}
+
bool
intelEmitCopyBlit(struct brw_context *brw,
GLuint cpp,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Topi Pohjolainen
2017-06-13 14:50:03 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 061860cdf6..f44bac988f 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1087,6 +1087,21 @@ intel_miptree_match_image(struct intel_mipmap_tree *mt,
if (mt->target == GL_TEXTURE_CUBE_MAP)
depth = 6;

+ if (mt->surf.size > 0) {
+ if (level >= mt->surf.levels)
+ return false;
+
+ const unsigned level_depth =
+ mt->surf.dim == ISL_SURF_DIM_3D ?
+ minify(mt->surf.logical_level0_px.depth, level) :
+ mt->surf.logical_level0_px.array_len;
+
+ return width == minify(mt->surf.logical_level0_px.width, level) &&
+ height == minify(mt->surf.logical_level0_px.height, level) &&
+ depth == level_depth &&
+ MAX2(image->NumSamples, 1) == mt->surf.samples;
+ }
+
int level_depth = mt->level[level].depth;
if (mt->num_samples > 1) {
switch (mt->msaa_layout) {
--
2.11.0
Nanley Chery
2017-06-16 00:21:42 UTC
Permalink
Raw Message
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 061860cdf6..f44bac988f 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1087,6 +1087,21 @@ intel_miptree_match_image(struct intel_mipmap_tree *mt,
if (mt->target == GL_TEXTURE_CUBE_MAP)
depth = 6;
+ if (mt->surf.size > 0) {
+ if (level >= mt->surf.levels)
+ return false;
+
+ const unsigned level_depth =
+ mt->surf.dim == ISL_SURF_DIM_3D ?
We should be looking up the physical depth shouldn't we? I haven't yet
looked into what width or height should be. I'm done for today, so I
will revisit this series tomorrow.
Post by Topi Pohjolainen
+ mt->surf.logical_level0_px.array_len;
+
+ return width == minify(mt->surf.logical_level0_px.width, level) &&
+ height == minify(mt->surf.logical_level0_px.height, level) &&
+ depth == level_depth &&
+ MAX2(image->NumSamples, 1) == mt->surf.samples;
+ }
+
int level_depth = mt->level[level].depth;
if (mt->num_samples > 1) {
switch (mt->msaa_layout) {
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Pohjolainen, Topi
2017-06-16 04:49:48 UTC
Permalink
Raw Message
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 061860cdf6..f44bac988f 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1087,6 +1087,21 @@ intel_miptree_match_image(struct intel_mipmap_tree *mt,
if (mt->target == GL_TEXTURE_CUBE_MAP)
depth = 6;
+ if (mt->surf.size > 0) {
+ if (level >= mt->surf.levels)
+ return false;
+
+ const unsigned level_depth =
+ mt->surf.dim == ISL_SURF_DIM_3D ?
We should be looking up the physical depth shouldn't we? I haven't yet
looked into what width or height should be. I'm done for today, so I
will revisit this series tomorrow.
Hmm, you are correct when comparing to the existing code - it uses the table
which contains physical values. The given depth, however, comes from
gl_texture_image::Depth which is core concept and deals with logical
values.

The confusing bit here is (which I should have written into the commit
message) is that number of layers can't be altered, only levels. The existing
code checks for depth because it uses that to catch missing levels. The table
is initialised to zeroes and finding depth of zero is taken as meaning that
the desired level doesn't exist.
Post by Nanley Chery
Post by Topi Pohjolainen
+ mt->surf.logical_level0_px.array_len;
+
+ return width == minify(mt->surf.logical_level0_px.width, level) &&
+ height == minify(mt->surf.logical_level0_px.height, level) &&
+ depth == level_depth &&
I should probably assert this instead of failing - as I said I don't see how
number of layers could change. Ian, Ken, Jason, how do you see this?
Post by Nanley Chery
Post by Topi Pohjolainen
+ MAX2(image->NumSamples, 1) == mt->surf.samples;
+ }
+
int level_depth = mt->level[level].depth;
if (mt->num_samples > 1) {
switch (mt->msaa_layout) {
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Jason Ekstrand
2017-06-16 04:57:03 UTC
Permalink
Raw Message
On Thu, Jun 15, 2017 at 9:49 PM, Pohjolainen, Topi <
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
Post by Nanley Chery
Post by Topi Pohjolainen
index 061860cdf6..f44bac988f 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1087,6 +1087,21 @@ intel_miptree_match_image(struct
intel_mipmap_tree *mt,
Post by Nanley Chery
Post by Topi Pohjolainen
if (mt->target == GL_TEXTURE_CUBE_MAP)
depth = 6;
+ if (mt->surf.size > 0) {
+ if (level >= mt->surf.levels)
+ return false;
+
+ const unsigned level_depth =
+ mt->surf.dim == ISL_SURF_DIM_3D ?
We should be looking up the physical depth shouldn't we? I haven't yet
looked into what width or height should be. I'm done for today, so I
will revisit this series tomorrow.
Hmm, you are correct when comparing to the existing code - it uses the table
which contains physical values. The given depth, however, comes from
gl_texture_image::Depth which is core concept and deals with logical
values.
The confusing bit here is (which I should have written into the commit
message) is that number of layers can't be altered, only levels. The existing
code checks for depth because it uses that to catch missing levels. The table
is initialised to zeroes and finding depth of zero is taken as meaning that
the desired level doesn't exist.
It's also worth pointing out that there's no difference between logical and
physical with 3D since 3D can't be multisampled. For 3D, I think logical
is the only one that makes sense.
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
+ mt->surf.logical_level0_px.array_len;
+
+ return width == minify(mt->surf.logical_level0_px.width,
level) &&
Post by Nanley Chery
Post by Topi Pohjolainen
+ height == minify(mt->surf.logical_level0_px.height,
level) &&
Post by Nanley Chery
Post by Topi Pohjolainen
+ depth == level_depth &&
I should probably assert this instead of failing - as I said I don't see how
number of layers could change. Ian, Ken, Jason, how do you see this?
No, you want to return. The whole point of this function is to determine
if this gl_texture_image matches the given level of the miptree. This is
used, for instence, when someone does glTexImage2D and changes the size of
one LOD. In that case, we can't upload to the miptree and we have to split
that LOD out into it's own. Later on, assuming the texture is complete, we
recombine them into a single miptree. It's aweful, but welcome to OpenGL.

--Jason
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
+ MAX2(image->NumSamples, 1) == mt->surf.samples;
+ }
+
int level_depth = mt->level[level].depth;
if (mt->num_samples > 1) {
switch (mt->msaa_layout) {
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Pohjolainen, Topi
2017-06-16 05:00:58 UTC
Permalink
Raw Message
Post by Jason Ekstrand
On Thu, Jun 15, 2017 at 9:49 PM, Pohjolainen, Topi <
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
Post by Nanley Chery
Post by Topi Pohjolainen
index 061860cdf6..f44bac988f 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1087,6 +1087,21 @@ intel_miptree_match_image(struct
intel_mipmap_tree *mt,
Post by Nanley Chery
Post by Topi Pohjolainen
if (mt->target == GL_TEXTURE_CUBE_MAP)
depth = 6;
+ if (mt->surf.size > 0) {
+ if (level >= mt->surf.levels)
+ return false;
+
+ const unsigned level_depth =
+ mt->surf.dim == ISL_SURF_DIM_3D ?
We should be looking up the physical depth shouldn't we? I haven't yet
looked into what width or height should be. I'm done for today, so I
will revisit this series tomorrow.
Hmm, you are correct when comparing to the existing code - it uses the table
which contains physical values. The given depth, however, comes from
gl_texture_image::Depth which is core concept and deals with logical
values.
The confusing bit here is (which I should have written into the commit
message) is that number of layers can't be altered, only levels. The existing
code checks for depth because it uses that to catch missing levels. The table
is initialised to zeroes and finding depth of zero is taken as meaning that
the desired level doesn't exist.
It's also worth pointing out that there's no difference between logical and
physical with 3D since 3D can't be multisampled. For 3D, I think logical
is the only one that makes sense.
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
+ mt->surf.logical_level0_px.array_len;
+
+ return width == minify(mt->surf.logical_level0_px.width,
level) &&
Post by Nanley Chery
Post by Topi Pohjolainen
+ height == minify(mt->surf.logical_level0_px.height,
level) &&
Post by Nanley Chery
Post by Topi Pohjolainen
+ depth == level_depth &&
I should probably assert this instead of failing - as I said I don't see how
number of layers could change. Ian, Ken, Jason, how do you see this?
No, you want to return. The whole point of this function is to determine
if this gl_texture_image matches the given level of the miptree. This is
used, for instence, when someone does glTexImage2D and changes the size of
one LOD. In that case, we can't upload to the miptree and we have to split
that LOD out into it's own. Later on, assuming the texture is complete, we
recombine them into a single miptree. It's aweful, but welcome to OpenGL.
Right, sorry, should have been clearer. I meant:

/* Only number of levels, width and height can be altered. */
assert(depth == level_depth);

return width == minify(mt->surf.logical_level0_px.width, level) &&
height == minify(mt->surf.logical_level0_px.height, level);
Post by Jason Ekstrand
--Jason
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
+ MAX2(image->NumSamples, 1) == mt->surf.samples;
+ }
+
int level_depth = mt->level[level].depth;
if (mt->num_samples > 1) {
switch (mt->msaa_layout) {
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Jason Ekstrand
2017-06-16 05:02:29 UTC
Permalink
Raw Message
On Thu, Jun 15, 2017 at 10:00 PM, Pohjolainen, Topi <
Post by Topi Pohjolainen
Post by Jason Ekstrand
On Thu, Jun 15, 2017 at 9:49 PM, Pohjolainen, Topi <
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 15
+++++++++++++++
Post by Jason Ekstrand
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
1 file changed, 15 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
Post by Nanley Chery
Post by Topi Pohjolainen
index 061860cdf6..f44bac988f 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1087,6 +1087,21 @@ intel_miptree_match_image(struct
intel_mipmap_tree *mt,
Post by Nanley Chery
Post by Topi Pohjolainen
if (mt->target == GL_TEXTURE_CUBE_MAP)
depth = 6;
+ if (mt->surf.size > 0) {
+ if (level >= mt->surf.levels)
+ return false;
+
+ const unsigned level_depth =
+ mt->surf.dim == ISL_SURF_DIM_3D ?
We should be looking up the physical depth shouldn't we? I haven't
yet
Post by Jason Ekstrand
Post by Topi Pohjolainen
Post by Nanley Chery
looked into what width or height should be. I'm done for today, so I
will revisit this series tomorrow.
Hmm, you are correct when comparing to the existing code - it uses the table
which contains physical values. The given depth, however, comes from
gl_texture_image::Depth which is core concept and deals with logical
values.
The confusing bit here is (which I should have written into the commit
message) is that number of layers can't be altered, only levels. The existing
code checks for depth because it uses that to catch missing levels. The table
is initialised to zeroes and finding depth of zero is taken as meaning
that
Post by Jason Ekstrand
Post by Topi Pohjolainen
the desired level doesn't exist.
It's also worth pointing out that there's no difference between logical
and
Post by Jason Ekstrand
physical with 3D since 3D can't be multisampled. For 3D, I think logical
is the only one that makes sense.
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
+ mt->surf.logical_level0_px.array_len;
+
+ return width == minify(mt->surf.logical_level0_px.width,
level) &&
Post by Nanley Chery
Post by Topi Pohjolainen
+ height == minify(mt->surf.logical_level0_px.height,
level) &&
Post by Nanley Chery
Post by Topi Pohjolainen
+ depth == level_depth &&
I should probably assert this instead of failing - as I said I don't
see
Post by Jason Ekstrand
Post by Topi Pohjolainen
how
number of layers could change. Ian, Ken, Jason, how do you see this?
No, you want to return. The whole point of this function is to determine
if this gl_texture_image matches the given level of the miptree. This is
used, for instence, when someone does glTexImage2D and changes the size
of
Post by Jason Ekstrand
one LOD. In that case, we can't upload to the miptree and we have to
split
Post by Jason Ekstrand
that LOD out into it's own. Later on, assuming the texture is complete,
we
Post by Jason Ekstrand
recombine them into a single miptree. It's aweful, but welcome to
OpenGL.
/* Only number of levels, width and height can be altered. */
assert(depth == level_depth);
return width == minify(mt->surf.logical_level0_px.width, level) &&
height == minify(mt->surf.logical_level0_px.height, level);
What I said above still holds, you just now need glTexImage3D to trigger
it. :-)
Post by Topi Pohjolainen
Post by Jason Ekstrand
--Jason
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
+ MAX2(image->NumSamples, 1) == mt->surf.samples;
+ }
+
int level_depth = mt->level[level].depth;
if (mt->num_samples > 1) {
switch (mt->msaa_layout) {
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Pohjolainen, Topi
2017-06-16 05:03:46 UTC
Permalink
Raw Message
Post by Jason Ekstrand
On Thu, Jun 15, 2017 at 10:00 PM, Pohjolainen, Topi <
Post by Topi Pohjolainen
Post by Jason Ekstrand
On Thu, Jun 15, 2017 at 9:49 PM, Pohjolainen, Topi <
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 15
+++++++++++++++
Post by Jason Ekstrand
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
1 file changed, 15 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
Post by Nanley Chery
Post by Topi Pohjolainen
index 061860cdf6..f44bac988f 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1087,6 +1087,21 @@ intel_miptree_match_image(struct
intel_mipmap_tree *mt,
Post by Nanley Chery
Post by Topi Pohjolainen
if (mt->target == GL_TEXTURE_CUBE_MAP)
depth = 6;
+ if (mt->surf.size > 0) {
+ if (level >= mt->surf.levels)
+ return false;
+
+ const unsigned level_depth =
+ mt->surf.dim == ISL_SURF_DIM_3D ?
We should be looking up the physical depth shouldn't we? I haven't
yet
Post by Jason Ekstrand
Post by Topi Pohjolainen
Post by Nanley Chery
looked into what width or height should be. I'm done for today, so I
will revisit this series tomorrow.
Hmm, you are correct when comparing to the existing code - it uses the table
which contains physical values. The given depth, however, comes from
gl_texture_image::Depth which is core concept and deals with logical
values.
The confusing bit here is (which I should have written into the commit
message) is that number of layers can't be altered, only levels. The existing
code checks for depth because it uses that to catch missing levels. The table
is initialised to zeroes and finding depth of zero is taken as meaning
that
Post by Jason Ekstrand
Post by Topi Pohjolainen
the desired level doesn't exist.
It's also worth pointing out that there's no difference between logical
and
Post by Jason Ekstrand
physical with 3D since 3D can't be multisampled. For 3D, I think logical
is the only one that makes sense.
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
+ mt->surf.logical_level0_px.array_len;
+
+ return width == minify(mt->surf.logical_level0_px.width,
level) &&
Post by Nanley Chery
Post by Topi Pohjolainen
+ height == minify(mt->surf.logical_level0_px.height,
level) &&
Post by Nanley Chery
Post by Topi Pohjolainen
+ depth == level_depth &&
I should probably assert this instead of failing - as I said I don't
see
Post by Jason Ekstrand
Post by Topi Pohjolainen
how
number of layers could change. Ian, Ken, Jason, how do you see this?
No, you want to return. The whole point of this function is to determine
if this gl_texture_image matches the given level of the miptree. This is
used, for instence, when someone does glTexImage2D and changes the size
of
Post by Jason Ekstrand
one LOD. In that case, we can't upload to the miptree and we have to
split
Post by Jason Ekstrand
that LOD out into it's own. Later on, assuming the texture is complete,
we
Post by Jason Ekstrand
recombine them into a single miptree. It's aweful, but welcome to
OpenGL.
/* Only number of levels, width and height can be altered. */
assert(depth == level_depth);
return width == minify(mt->surf.logical_level0_px.width, level) &&
height == minify(mt->surf.logical_level0_px.height, level);
What I said above still holds, you just now need glTexImage3D to trigger
it. :-)
Oh, right you are.
Post by Jason Ekstrand
Post by Topi Pohjolainen
Post by Jason Ekstrand
--Jason
Post by Topi Pohjolainen
Post by Nanley Chery
Post by Topi Pohjolainen
+ MAX2(image->NumSamples, 1) == mt->surf.samples;
+ }
+
int level_depth = mt->level[level].depth;
if (mt->num_samples > 1) {
switch (mt->msaa_layout) {
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Topi Pohjolainen
2017-06-13 14:50:05 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/intel_fbo.c | 33 ++++++++++++++++++++++++++++++---
1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
index 478e7b8884..e49f6df408 100644
--- a/src/mesa/drivers/dri/i965/intel_fbo.c
+++ b/src/mesa/drivers/dri/i965/intel_fbo.c
@@ -670,14 +670,41 @@ intel_validate_framebuffer(struct gl_context *ctx, struct gl_framebuffer *fb)

if (depth_mt && stencil_mt) {
if (brw->gen >= 6) {
+ unsigned d_width, d_height, d_depth;
+ unsigned s_width, s_height, s_depth;
+
+ if (depth_mt->surf.size > 0) {
+ d_width = depth_mt->surf.phys_level0_sa.width;
+ d_height = depth_mt->surf.phys_level0_sa.height;
+ d_depth = depth_mt->surf.dim == ISL_SURF_DIM_3D ?
+ depth_mt->surf.phys_level0_sa.depth :
+ depth_mt->surf.phys_level0_sa.array_len;
+ } else {
+ d_width = depth_mt->physical_width0;
+ d_height = depth_mt->physical_height0;
+ d_depth = depth_mt->physical_depth0;
+ }
+
+ if (stencil_mt->surf.size > 0) {
+ s_width = stencil_mt->surf.phys_level0_sa.width;
+ s_height = stencil_mt->surf.phys_level0_sa.height;
+ s_depth = stencil_mt->surf.dim == ISL_SURF_DIM_3D ?
+ stencil_mt->surf.phys_level0_sa.depth :
+ stencil_mt->surf.phys_level0_sa.array_len;
+ } else {
+ s_width = stencil_mt->physical_width0;
+ s_height = stencil_mt->physical_height0;
+ s_depth = stencil_mt->physical_depth0;
+ }
+
/* For gen >= 6, we are using the lod/minimum-array-element fields
* and supporting layered rendering. This means that we must restrict
* the depth & stencil attachments to match in various more retrictive
* ways. (width, height, depth, LOD and layer)
*/
- if (depth_mt->physical_width0 != stencil_mt->physical_width0 ||
- depth_mt->physical_height0 != stencil_mt->physical_height0 ||
- depth_mt->physical_depth0 != stencil_mt->physical_depth0 ||
+ if (d_width != s_width ||
+ d_height != s_height ||
+ d_depth != s_depth ||
depthRb->mt_level != stencilRb->mt_level ||
depthRb->mt_layer != stencilRb->mt_layer) {
fbo_incomplete(fb,
--
2.11.0
Topi Pohjolainen
2017-06-13 14:50:06 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/intel_tex_image.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c b/src/mesa/drivers/dri/i965/intel_tex_image.c
index ea166f019f..b1fe8dd584 100644
--- a/src/mesa/drivers/dri/i965/intel_tex_image.c
+++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
@@ -56,11 +56,24 @@ intel_miptree_create_for_teximage(struct brw_context *brw,
{
GLuint lastLevel;
int width, height, depth;
+ unsigned old_width = 0, old_height = 0, old_depth = 0;
const struct intel_mipmap_tree *old_mt = intelObj->mt;
const unsigned level = intelImage->base.Base.Level;

intel_get_image_dims(&intelImage->base.Base, &width, &height, &depth);

+ if (old_mt && old_mt->surf.size > 0) {
+ old_width = old_mt->surf.logical_level0_px.width;
+ old_height = old_mt->surf.logical_level0_px.height;
+ old_depth = old_mt->surf.dim == ISL_SURF_DIM_3D ?
+ old_mt->surf.logical_level0_px.depth :
+ old_mt->surf.logical_level0_px.array_len;
+ } else if (old_mt) {
+ old_width = old_mt->logical_width0;
+ old_height = old_mt->logical_height0;
+ old_depth = old_mt->logical_depth0;
+ }
+
DBG("%s\n", __func__);

/* Figure out image dimensions at start level. */
@@ -72,19 +85,19 @@ intel_miptree_create_for_teximage(struct brw_context *brw,
assert(level == 0);
break;
case GL_TEXTURE_3D:
- depth = old_mt ? get_base_dim(old_mt->logical_depth0, depth, level) :
+ depth = old_mt ? get_base_dim(old_depth, depth, level) :
depth << level;
/* Fall through */
case GL_TEXTURE_2D:
case GL_TEXTURE_2D_ARRAY:
case GL_TEXTURE_CUBE_MAP:
case GL_TEXTURE_CUBE_MAP_ARRAY:
- height = old_mt ? get_base_dim(old_mt->logical_height0, height, level) :
+ height = old_mt ? get_base_dim(old_height, height, level) :
height << level;
/* Fall through */
case GL_TEXTURE_1D:
case GL_TEXTURE_1D_ARRAY:
- width = old_mt ? get_base_dim(old_mt->logical_width0, width, level) :
+ width = old_mt ? get_base_dim(old_width, width, level) :
width << level;
break;
default:
--
2.11.0
Topi Pohjolainen
2017-06-13 14:50:07 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 28 +++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index f44bac988f..c81d345fbc 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1294,7 +1294,8 @@ intel_miptree_copy_slice_sw(struct brw_context *brw,
{
void *src, *dst;
ptrdiff_t src_stride, dst_stride;
- int cpp = dst_mt->cpp;
+ const unsigned cpp = dst_mt->surf.size > 0 ?
+ (isl_format_get_layout(dst_mt->surf.format)->bpb / 8) : dst_mt->cpp;

intel_miptree_map(brw, src_mt,
src_level, src_layer,
@@ -1355,13 +1356,28 @@ intel_miptree_copy_slice(struct brw_context *brw,
unsigned dst_level, unsigned dst_layer)

{
- uint32_t width = minify(src_mt->physical_width0,
- src_level - src_mt->first_level);
- uint32_t height = minify(src_mt->physical_height0,
- src_level - src_mt->first_level);
mesa_format format = src_mt->format;
+ uint32_t width, height;
+
+ if (src_mt->surf.size > 0) {
+ width = minify(src_mt->surf.phys_level0_sa.width,
+ src_level - src_mt->first_level);
+ height = minify(src_mt->surf.phys_level0_sa.height,
+ src_level - src_mt->first_level);
+
+ const unsigned level_depth = src_mt->surf.dim == ISL_SURF_DIM_3D ?
+ minify(src_mt->surf.phys_level0_sa.depth,
+ src_level - src_mt->first_level) :
+ src_mt->surf.phys_level0_sa.array_len;
+ assert(src_layer < level_depth);
+ } else {
+ width = minify(src_mt->physical_width0,
+ src_level - src_mt->first_level);
+ height = minify(src_mt->physical_height0,
+ src_level - src_mt->first_level);
+ assert(src_layer < src_mt->level[src_level].depth);
+ }

- assert(src_layer < src_mt->level[src_level].depth);
assert(src_mt->format == dst_mt->format);

if (dst_mt->compressed) {
--
2.11.0
Jason Ekstrand
2017-06-15 18:39:44 UTC
Permalink
Raw Message
On Tue, Jun 13, 2017 at 7:50 AM, Topi Pohjolainen <
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 28
+++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index f44bac988f..c81d345fbc 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1294,7 +1294,8 @@ intel_miptree_copy_slice_sw(struct brw_context *brw,
{
void *src, *dst;
ptrdiff_t src_stride, dst_stride;
- int cpp = dst_mt->cpp;
+ const unsigned cpp = dst_mt->surf.size > 0 ?
dst_mt->cpp;
intel_miptree_map(brw, src_mt,
src_level, src_layer,
@@ -1355,13 +1356,28 @@ intel_miptree_copy_slice(struct brw_context *brw,
unsigned dst_level, unsigned dst_layer)
{
- uint32_t width = minify(src_mt->physical_width0,
- src_level - src_mt->first_level);
- uint32_t height = minify(src_mt->physical_height0,
- src_level - src_mt->first_level);
mesa_format format = src_mt->format;
+ uint32_t width, height;
+
+ if (src_mt->surf.size > 0) {
+ width = minify(src_mt->surf.phys_level0_sa.width,
+ src_level - src_mt->first_level);
+ height = minify(src_mt->surf.phys_level0_sa.height,
+ src_level - src_mt->first_level);
+
+ const unsigned level_depth = src_mt->surf.dim == ISL_SURF_DIM_3D ?
This needs a MAYBE_UNUSED or else we'll get warnings in release builds.
Another option would be to do

if (src_mt->surf.dim == ISL_SURF_DIM_3D)
assert(src_layer < minify());
else
assert(src_layer < src_mt->surf.phys_level0_sa.array_len);

I think that would actually be better.
Post by Topi Pohjolainen
+ minify(src_mt->surf.phys_level0_sa.depth,
+ src_mt->surf.phys_level0_sa.array_len;
+ assert(src_layer < level_depth);
+ } else {
+ width = minify(src_mt->physical_width0,
+ src_level - src_mt->first_level);
+ height = minify(src_mt->physical_height0,
+ src_level - src_mt->first_level);
+ assert(src_layer < src_mt->level[src_level].depth);
+ }
- assert(src_layer < src_mt->level[src_level].depth);
assert(src_mt->format == dst_mt->format);
if (dst_mt->compressed) {
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Pohjolainen, Topi
2017-06-15 19:01:53 UTC
Permalink
Raw Message
Post by Jason Ekstrand
On Tue, Jun 13, 2017 at 7:50 AM, Topi Pohjolainen <
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 28
+++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index f44bac988f..c81d345fbc 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1294,7 +1294,8 @@ intel_miptree_copy_slice_sw(struct brw_context *brw,
{
void *src, *dst;
ptrdiff_t src_stride, dst_stride;
- int cpp = dst_mt->cpp;
+ const unsigned cpp = dst_mt->surf.size > 0 ?
+ (isl_format_get_layout(dst_mt->surf.format)->bpb / 8) : dst_mt->cpp;
intel_miptree_map(brw, src_mt,
src_level, src_layer,
@@ -1355,13 +1356,28 @@ intel_miptree_copy_slice(struct brw_context *brw,
unsigned dst_level, unsigned dst_layer)
{
- uint32_t width = minify(src_mt->physical_width0,
- src_level - src_mt->first_level);
- uint32_t height = minify(src_mt->physical_height0,
- src_level - src_mt->first_level);
mesa_format format = src_mt->format;
+ uint32_t width, height;
+
+ if (src_mt->surf.size > 0) {
+ width = minify(src_mt->surf.phys_level0_sa.width,
+ src_level - src_mt->first_level);
+ height = minify(src_mt->surf.phys_level0_sa.height,
+ src_level - src_mt->first_level);
+
+ const unsigned level_depth = src_mt->surf.dim == ISL_SURF_DIM_3D ?
This needs a MAYBE_UNUSED or else we'll get warnings in release builds.
Another option would be to do
if (src_mt->surf.dim == ISL_SURF_DIM_3D)
assert(src_layer < minify());
else
assert(src_layer < src_mt->surf.phys_level0_sa.array_len);
I think that would actually be better.
Thanks, used the latter locally.
Post by Jason Ekstrand
Post by Topi Pohjolainen
+ minify(src_mt->surf.phys_level0_sa.depth,
+ src_mt->surf.phys_level0_sa.array_len;
+ assert(src_layer < level_depth);
+ } else {
+ width = minify(src_mt->physical_width0,
+ src_level - src_mt->first_level);
+ height = minify(src_mt->physical_height0,
+ src_level - src_mt->first_level);
+ assert(src_layer < src_mt->level[src_level].depth);
+ }
- assert(src_layer < src_mt->level[src_level].depth);
assert(src_mt->format == dst_mt->format);
if (dst_mt->compressed) {
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Nanley Chery
2017-06-16 16:36:35 UTC
Permalink
Raw Message
Post by Pohjolainen, Topi
Post by Jason Ekstrand
On Tue, Jun 13, 2017 at 7:50 AM, Topi Pohjolainen <
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 28
+++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index f44bac988f..c81d345fbc 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1294,7 +1294,8 @@ intel_miptree_copy_slice_sw(struct brw_context *brw,
{
void *src, *dst;
ptrdiff_t src_stride, dst_stride;
- int cpp = dst_mt->cpp;
+ const unsigned cpp = dst_mt->surf.size > 0 ?
+ (isl_format_get_layout(dst_mt->surf.format)->bpb / 8) : dst_mt->cpp;
intel_miptree_map(brw, src_mt,
src_level, src_layer,
@@ -1355,13 +1356,28 @@ intel_miptree_copy_slice(struct brw_context *brw,
unsigned dst_level, unsigned dst_layer)
{
- uint32_t width = minify(src_mt->physical_width0,
- src_level - src_mt->first_level);
- uint32_t height = minify(src_mt->physical_height0,
- src_level - src_mt->first_level);
mesa_format format = src_mt->format;
+ uint32_t width, height;
+
+ if (src_mt->surf.size > 0) {
+ width = minify(src_mt->surf.phys_level0_sa.width,
+ src_level - src_mt->first_level);
+ height = minify(src_mt->surf.phys_level0_sa.height,
+ src_level - src_mt->first_level);
+
+ const unsigned level_depth = src_mt->surf.dim == ISL_SURF_DIM_3D ?
This needs a MAYBE_UNUSED or else we'll get warnings in release builds.
Another option would be to do
if (src_mt->surf.dim == ISL_SURF_DIM_3D)
assert(src_layer < minify());
else
assert(src_layer < src_mt->surf.phys_level0_sa.array_len);
I think that would actually be better.
Thanks, used the latter locally.
Patches 7, 8, and revised 9 are
Post by Pohjolainen, Topi
Post by Jason Ekstrand
Post by Topi Pohjolainen
+ minify(src_mt->surf.phys_level0_sa.depth,
+ src_mt->surf.phys_level0_sa.array_len;
+ assert(src_layer < level_depth);
+ } else {
+ width = minify(src_mt->physical_width0,
+ src_level - src_mt->first_level);
+ height = minify(src_mt->physical_height0,
+ src_level - src_mt->first_level);
+ assert(src_layer < src_mt->level[src_level].depth);
+ }
- assert(src_layer < src_mt->level[src_level].depth);
assert(src_mt->format == dst_mt->format);
if (dst_mt->compressed) {
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Topi Pohjolainen
2017-06-13 14:50:12 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index ecb9186715..e2de4df498 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -2501,7 +2501,14 @@ miptree_layer_range_length(const struct intel_mipmap_tree *mt, uint32_t level,
uint32_t start_layer, uint32_t num_layers)
{
assert(level <= mt->last_level);
- uint32_t total_num_layers = mt->level[level].depth;
+ uint32_t total_num_layers;
+
+ if (mt->surf.size > 0)
+ total_num_layers = mt->surf.dim == ISL_SURF_DIM_3D ?
+ minify(mt->surf.phys_level0_sa.depth, level) :
+ mt->surf.phys_level0_sa.array_len;
+ else
+ total_num_layers = mt->level[level].depth;

assert(start_layer < total_num_layers);
if (num_layers == INTEL_REMAINING_LAYERS)
--
2.11.0
Topi Pohjolainen
2017-06-13 14:50:10 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/brw_blorp.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c
index 355f936f06..fee7c43d2c 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.c
+++ b/src/mesa/drivers/dri/i965/brw_blorp.c
@@ -148,8 +148,13 @@ blorp_surf_for_miptree(struct brw_context *brw,
intel_miptree_check_level_layer(mt, *level, start_layer + i);
}

- intel_miptree_get_isl_surf(brw, mt, &tmp_surfs[0]);
- surf->surf = &tmp_surfs[0];
+ if (mt->surf.size > 0) {
+ surf->surf = &mt->surf;
+ } else {
+ intel_miptree_get_isl_surf(brw, mt, &tmp_surfs[0]);
+ surf->surf = &tmp_surfs[0];
+ }
+
surf->addr = (struct blorp_address) {
.buffer = mt->bo,
.offset = mt->offset,
--
2.11.0
Topi Pohjolainen
2017-06-13 14:50:13 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/gen6_depth_state.c | 31 +++++++++++----------------
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 7 ++++++
2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/gen6_depth_state.c b/src/mesa/drivers/dri/i965/gen6_depth_state.c
index dcee1f9b61..0d8785db65 100644
--- a/src/mesa/drivers/dri/i965/gen6_depth_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_depth_state.c
@@ -91,7 +91,8 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
break;
case GL_TEXTURE_3D:
assert(mt);
- depth = MAX2(mt->logical_depth0, 1);
+ depth = mt->surf.size > 0 ? mt->surf.logical_level0_px.depth :
+ MAX2(mt->logical_depth0, 1);
/* fallthrough */
default:
surftype = translate_tex_target(gl_target);
@@ -102,7 +103,10 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,

lod = irb ? irb->mt_level - irb->mt->first_level : 0;

- if (mt) {
+ if (mt && mt->surf.size > 0) {
+ width = mt->surf.logical_level0_px.width;
+ height = mt->surf.logical_level0_px.height;
+ } else if (mt) {
width = mt->logical_width0;
height = mt->logical_height0;
}
@@ -187,27 +191,16 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,

/* Emit stencil buffer. */
if (separate_stencil) {
- uint32_t offset = 0;
+ assert(stencil_mt->format == MESA_FORMAT_S_UINT8);
+ assert(stencil_mt->surf.size > 0);

- if (stencil_mt->array_layout == GEN6_HIZ_STENCIL) {
- assert(stencil_mt->format == MESA_FORMAT_S_UINT8);
-
- /* Note: we can't compute the stencil offset using
- * intel_region_get_aligned_offset(), because stencil_region
- * claims that the region is untiled even though it's W tiled.
- */
- offset = stencil_mt->level[lod].level_y * stencil_mt->pitch +
- stencil_mt->level[lod].level_x * 64;
- }
+ uint32_t offset;
+ isl_surf_get_image_offset_B_tile_sa(&stencil_mt->surf,
+ lod, 0, 0, &offset, NULL, NULL);

BEGIN_BATCH(3);
OUT_BATCH((_3DSTATE_STENCIL_BUFFER << 16) | (3 - 2));
- /* The stencil buffer has quirky pitch requirements. From Vol 2a,
- * 11.5.6.2.1 3DSTATE_STENCIL_BUFFER, field "Surface Pitch":
- * The pitch must be set to 2x the value computed based on width, as
- * the stencil buffer is stored with two rows interleaved.
- */
- OUT_BATCH(2 * stencil_mt->pitch - 1);
+ OUT_BATCH(stencil_mt->surf.row_pitch - 1);
OUT_RELOC(stencil_mt->bo,
I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
offset);
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index e2de4df498..26025311a0 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -730,6 +730,13 @@ miptree_create(struct brw_context *brw,
GLuint num_samples,
uint32_t layout_flags)
{
+ if (brw->gen == 6 && format == MESA_FORMAT_S_UINT8)
+ return make_surface(brw, target, format, first_level, last_level,
+ width0, height0, depth0, num_samples, ISL_TILING_W,
+ ISL_SURF_USAGE_STENCIL_BIT |
+ ISL_SURF_USAGE_TEXTURE_BIT,
+ BO_ALLOC_FOR_RENDER, NULL);
+
struct intel_mipmap_tree *mt;
mesa_format tex_format = format;
mesa_format etc_format = MESA_FORMAT_NONE;
--
2.11.0
Jason Ekstrand
2017-06-15 18:54:48 UTC
Permalink
Raw Message
Series is

Reviewed-by: Jason Ekstrand <***@jlekstrand.net>

Nanley is also reviewing so he may want you to wait for him.

On Tue, Jun 13, 2017 at 7:50 AM, Topi Pohjolainen <
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/gen6_depth_state.c | 31
+++++++++++----------------
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 7 ++++++
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/gen6_depth_state.c
b/src/mesa/drivers/dri/i965/gen6_depth_state.c
index dcee1f9b61..0d8785db65 100644
--- a/src/mesa/drivers/dri/i965/gen6_depth_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_depth_state.c
@@ -91,7 +91,8 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
break;
assert(mt);
- depth = MAX2(mt->logical_depth0, 1);
+ MAX2(mt->logical_depth0, 1);
/* fallthrough */
surftype = translate_tex_target(gl_target);
@@ -102,7 +103,10 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
lod = irb ? irb->mt_level - irb->mt->first_level : 0;
- if (mt) {
+ if (mt && mt->surf.size > 0) {
+ width = mt->surf.logical_level0_px.width;
+ height = mt->surf.logical_level0_px.height;
+ } else if (mt) {
width = mt->logical_width0;
height = mt->logical_height0;
}
@@ -187,27 +191,16 @@ gen6_emit_depth_stencil_hiz(struct brw_context *brw,
/* Emit stencil buffer. */
if (separate_stencil) {
- uint32_t offset = 0;
+ assert(stencil_mt->format == MESA_FORMAT_S_UINT8);
+ assert(stencil_mt->surf.size > 0);
- if (stencil_mt->array_layout == GEN6_HIZ_STENCIL) {
- assert(stencil_mt->format == MESA_FORMAT_S_UINT8);
-
- /* Note: we can't compute the stencil offset using
- * intel_region_get_aligned_offset(), because stencil_region
- * claims that the region is untiled even though it's W tiled.
- */
- offset = stencil_mt->level[lod].level_y * stencil_mt->pitch +
- stencil_mt->level[lod].level_x * 64;
- }
+ uint32_t offset;
+ isl_surf_get_image_offset_B_tile_sa(&stencil_mt->surf,
+ lod, 0, 0, &offset, NULL, NULL);
BEGIN_BATCH(3);
OUT_BATCH((_3DSTATE_STENCIL_BUFFER << 16) | (3 - 2));
- /* The stencil buffer has quirky pitch requirements. From Vol 2a,
- * The pitch must be set to 2x the value computed based on width, as
- * the stencil buffer is stored with two rows interleaved.
- */
- OUT_BATCH(2 * stencil_mt->pitch - 1);
+ OUT_BATCH(stencil_mt->surf.row_pitch - 1);
OUT_RELOC(stencil_mt->bo,
I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
offset);
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index e2de4df498..26025311a0 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -730,6 +730,13 @@ miptree_create(struct brw_context *brw,
GLuint num_samples,
uint32_t layout_flags)
{
+ if (brw->gen == 6 && format == MESA_FORMAT_S_UINT8)
+ return make_surface(brw, target, format, first_level, last_level,
+ width0, height0, depth0, num_samples, ISL_TILING_W,
+ ISL_SURF_USAGE_STENCIL_BIT |
+ ISL_SURF_USAGE_TEXTURE_BIT,
+ BO_ALLOC_FOR_RENDER, NULL);
+
struct intel_mipmap_tree *mt;
mesa_format tex_format = format;
mesa_format etc_format = MESA_FORMAT_NONE;
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Topi Pohjolainen
2017-06-13 14:50:04 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index 8479b285cb..0b85bc12ef 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -748,7 +748,13 @@ intel_miptree_check_level_layer(const struct intel_mipmap_tree *mt,

assert(level >= mt->first_level);
assert(level <= mt->last_level);
- assert(layer < mt->level[level].depth);
+
+ if (mt->surf.size > 0)
+ assert(layer < (mt->surf.dim == ISL_SURF_DIM_3D ?
+ mt->surf.phys_level0_sa.depth :
+ mt->surf.phys_level0_sa.array_len));
+ else
+ assert(layer < mt->level[level].depth);
}

void intel_miptree_reference(struct intel_mipmap_tree **dst,
--
2.11.0
Nanley Chery
2017-06-14 00:26:52 UTC
Permalink
Raw Message
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index 8479b285cb..0b85bc12ef 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -748,7 +748,13 @@ intel_miptree_check_level_layer(const struct intel_mipmap_tree *mt,
assert(level >= mt->first_level);
assert(level <= mt->last_level);
- assert(layer < mt->level[level].depth);
+
+ if (mt->surf.size > 0)
+ assert(layer < (mt->surf.dim == ISL_SURF_DIM_3D ?
+ mt->surf.phys_level0_sa.array_len));
Did you mean to access mt->surf.logical_level0_px here?

-Nanley
Post by Topi Pohjolainen
+ else
+ assert(layer < mt->level[level].depth);
}
void intel_miptree_reference(struct intel_mipmap_tree **dst,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Pohjolainen, Topi
2017-06-14 07:36:16 UTC
Permalink
Raw Message
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index 8479b285cb..0b85bc12ef 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -748,7 +748,13 @@ intel_miptree_check_level_layer(const struct intel_mipmap_tree *mt,
assert(level >= mt->first_level);
assert(level <= mt->last_level);
- assert(layer < mt->level[level].depth);
+
+ if (mt->surf.size > 0)
+ assert(layer < (mt->surf.dim == ISL_SURF_DIM_3D ?
+ mt->surf.phys_level0_sa.array_len));
Did you mean to access mt->surf.logical_level0_px here?
I was just about to say that "Actually no, mt->level[level].depth represents
the number of physical layers." But now reading the current logic I remembered
that Jason just recently changed all that. This is based on the way it was
before. Thanks Nanley!
Post by Nanley Chery
-Nanley
Post by Topi Pohjolainen
+ else
+ assert(layer < mt->level[level].depth);
}
void intel_miptree_reference(struct intel_mipmap_tree **dst,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Nanley Chery
2017-06-14 18:28:39 UTC
Permalink
Raw Message
Post by Pohjolainen, Topi
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index 8479b285cb..0b85bc12ef 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -748,7 +748,13 @@ intel_miptree_check_level_layer(const struct intel_mipmap_tree *mt,
assert(level >= mt->first_level);
assert(level <= mt->last_level);
- assert(layer < mt->level[level].depth);
+
+ if (mt->surf.size > 0)
+ assert(layer < (mt->surf.dim == ISL_SURF_DIM_3D ?
+ mt->surf.phys_level0_sa.array_len));
Did you mean to access mt->surf.logical_level0_px here?
I was just about to say that "Actually no, mt->level[level].depth represents
the number of physical layers."
You may be right. I just expected the other field would be accessed
because in the previous patch you access logical_level0_px instead of
phys_level0_sa. I'm not very experienced with this code so I may have
missed some detail.
Post by Pohjolainen, Topi
But now reading the current logic I remembered
that Jason just recently changed all that. This is based on the way it was
before. Thanks Nanley!
Post by Nanley Chery
-Nanley
Post by Topi Pohjolainen
+ else
+ assert(layer < mt->level[level].depth);
}
void intel_miptree_reference(struct intel_mipmap_tree **dst,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Pohjolainen, Topi
2017-06-14 18:32:46 UTC
Permalink
Raw Message
Post by Nanley Chery
Post by Pohjolainen, Topi
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index 8479b285cb..0b85bc12ef 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -748,7 +748,13 @@ intel_miptree_check_level_layer(const struct intel_mipmap_tree *mt,
assert(level >= mt->first_level);
assert(level <= mt->last_level);
- assert(layer < mt->level[level].depth);
+
+ if (mt->surf.size > 0)
+ assert(layer < (mt->surf.dim == ISL_SURF_DIM_3D ?
+ mt->surf.phys_level0_sa.array_len));
Did you mean to access mt->surf.logical_level0_px here?
I was just about to say that "Actually no, mt->level[level].depth represents
the number of physical layers."
You may be right. I just expected the other field would be accessed
because in the previous patch you access logical_level0_px instead of
phys_level0_sa. I'm not very experienced with this code so I may have
missed some detail.
Funny, I was just double checking this myself :) Things actually are as they
used to be, "mt->level[level].depth" is still based on physical depth. It
will go away altogether once I'm done with color surfaces.
Post by Nanley Chery
Post by Pohjolainen, Topi
But now reading the current logic I remembered
that Jason just recently changed all that. This is based on the way it was
before. Thanks Nanley!
Post by Nanley Chery
-Nanley
Post by Topi Pohjolainen
+ else
+ assert(layer < mt->level[level].depth);
}
void intel_miptree_reference(struct intel_mipmap_tree **dst,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Nanley Chery
2017-06-14 22:45:14 UTC
Permalink
Raw Message
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index 8479b285cb..0b85bc12ef 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -748,7 +748,13 @@ intel_miptree_check_level_layer(const struct intel_mipmap_tree *mt,
assert(level >= mt->first_level);
assert(level <= mt->last_level);
- assert(layer < mt->level[level].depth);
+
+ if (mt->surf.size > 0)
+ assert(layer < (mt->surf.dim == ISL_SURF_DIM_3D ?
Shouldn't we be minifying the depth here?
Post by Topi Pohjolainen
+ mt->surf.phys_level0_sa.array_len));
+ else
+ assert(layer < mt->level[level].depth);
}
void intel_miptree_reference(struct intel_mipmap_tree **dst,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Pohjolainen, Topi
2017-06-15 18:05:01 UTC
Permalink
Raw Message
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
index 8479b285cb..0b85bc12ef 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
@@ -748,7 +748,13 @@ intel_miptree_check_level_layer(const struct intel_mipmap_tree *mt,
assert(level >= mt->first_level);
assert(level <= mt->last_level);
- assert(layer < mt->level[level].depth);
+
+ if (mt->surf.size > 0)
+ assert(layer < (mt->surf.dim == ISL_SURF_DIM_3D ?
Shouldn't we be minifying the depth here?
Yes. Thanks, I'll revise.
Post by Nanley Chery
Post by Topi Pohjolainen
+ mt->surf.phys_level0_sa.array_len));
+ else
+ assert(layer < mt->level[level].depth);
}
void intel_miptree_reference(struct intel_mipmap_tree **dst,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Topi Pohjolainen
2017-06-13 14:50:11 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 32 +++++++++++++++++++++++----
1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 0854b4eb5d..ecb9186715 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -3262,6 +3262,12 @@ intel_miptree_map_s8(struct brw_context *brw,
* temporary buffer back out.
*/
if (!(map->mode & GL_MAP_INVALIDATE_RANGE_BIT)) {
+ /* ISL uses a stencil pitch value that is expected by hardware whereas
+ * traditional miptree uses half of that. Below the value gets supplied
+ * to intel_offset_S8() which expects the legacy interpretation.
+ */
+ const unsigned pitch = mt->surf.size > 0 ?
+ mt->surf.row_pitch / 2 : mt->pitch;
uint8_t *untiled_s8_map = map->ptr;
uint8_t *tiled_s8_map = intel_miptree_map_raw(brw, mt, GL_MAP_READ_BIT);
unsigned int image_x, image_y;
@@ -3270,7 +3276,7 @@ intel_miptree_map_s8(struct brw_context *brw,

for (uint32_t y = 0; y < map->h; y++) {
for (uint32_t x = 0; x < map->w; x++) {
- ptrdiff_t offset = intel_offset_S8(mt->pitch,
+ ptrdiff_t offset = intel_offset_S8(pitch,
x + image_x + map->x,
y + image_y + map->y,
brw->has_swizzling);
@@ -3298,6 +3304,12 @@ intel_miptree_unmap_s8(struct brw_context *brw,
unsigned int slice)
{
if (map->mode & GL_MAP_WRITE_BIT) {
+ /* ISL uses a stencil pitch value that is expected by hardware whereas
+ * traditional miptree uses half of that. Below the value gets supplied
+ * to intel_offset_S8() which expects the legacy interpretation.
+ */
+ const unsigned pitch = mt->surf.size > 0 ?
+ mt->surf.row_pitch / 2: mt->pitch;
unsigned int image_x, image_y;
uint8_t *untiled_s8_map = map->ptr;
uint8_t *tiled_s8_map = intel_miptree_map_raw(brw, mt, GL_MAP_WRITE_BIT);
@@ -3306,7 +3318,7 @@ intel_miptree_unmap_s8(struct brw_context *brw,

for (uint32_t y = 0; y < map->h; y++) {
for (uint32_t x = 0; x < map->w; x++) {
- ptrdiff_t offset = intel_offset_S8(mt->pitch,
+ ptrdiff_t offset = intel_offset_S8(pitch,
image_x + x + map->x,
image_y + y + map->y,
brw->has_swizzling);
@@ -3405,6 +3417,12 @@ intel_miptree_map_depthstencil(struct brw_context *brw,
* temporary buffer back out.
*/
if (!(map->mode & GL_MAP_INVALIDATE_RANGE_BIT)) {
+ /* ISL uses a stencil pitch value that is expected by hardware whereas
+ * traditional miptree uses half of that. Below the value gets supplied
+ * to intel_offset_S8() which expects the legacy interpretation.
+ */
+ const unsigned s_pitch = s_mt->surf.size > 0 ?
+ s_mt->surf.row_pitch / 2 : s_mt->pitch;
uint32_t *packed_map = map->ptr;
uint8_t *s_map = intel_miptree_map_raw(brw, s_mt, GL_MAP_READ_BIT);
uint32_t *z_map = intel_miptree_map_raw(brw, z_mt, GL_MAP_READ_BIT);
@@ -3419,7 +3437,7 @@ intel_miptree_map_depthstencil(struct brw_context *brw,
for (uint32_t y = 0; y < map->h; y++) {
for (uint32_t x = 0; x < map->w; x++) {
int map_x = map->x + x, map_y = map->y + y;
- ptrdiff_t s_offset = intel_offset_S8(s_mt->pitch,
+ ptrdiff_t s_offset = intel_offset_S8(s_pitch,
map_x + s_image_x,
map_y + s_image_y,
brw->has_swizzling);
@@ -3466,6 +3484,12 @@ intel_miptree_unmap_depthstencil(struct brw_context *brw,
bool map_z32f_x24s8 = mt->format == MESA_FORMAT_Z_FLOAT32;

if (map->mode & GL_MAP_WRITE_BIT) {
+ /* ISL uses a stencil pitch value that is expected by hardware whereas
+ * traditional miptree uses half of that. Below the value gets supplied
+ * to intel_offset_S8() which expects the legacy interpretation.
+ */
+ const unsigned s_pitch = s_mt->surf.size > 0 ?
+ s_mt->surf.row_pitch / 2 : s_mt->pitch;
uint32_t *packed_map = map->ptr;
uint8_t *s_map = intel_miptree_map_raw(brw, s_mt, GL_MAP_WRITE_BIT);
uint32_t *z_map = intel_miptree_map_raw(brw, z_mt, GL_MAP_WRITE_BIT);
@@ -3479,7 +3503,7 @@ intel_miptree_unmap_depthstencil(struct brw_context *brw,

for (uint32_t y = 0; y < map->h; y++) {
for (uint32_t x = 0; x < map->w; x++) {
- ptrdiff_t s_offset = intel_offset_S8(s_mt->pitch,
+ ptrdiff_t s_offset = intel_offset_S8(s_pitch,
x + s_image_x + map->x,
y + s_image_y + map->y,
brw->has_swizzling);
--
2.11.0
Jason Ekstrand
2017-06-15 18:48:31 UTC
Permalink
Raw Message
On Tue, Jun 13, 2017 at 7:50 AM, Topi Pohjolainen <
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 32
+++++++++++++++++++++++----
1 file changed, 28 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 0854b4eb5d..ecb9186715 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -3262,6 +3262,12 @@ intel_miptree_map_s8(struct brw_context *brw,
* temporary buffer back out.
*/
if (!(map->mode & GL_MAP_INVALIDATE_RANGE_BIT)) {
+ /* ISL uses a stencil pitch value that is expected by hardware whereas
+ * traditional miptree uses half of that. Below the value gets supplied
+ * to intel_offset_S8() which expects the legacy interpretation.
+ */
+ const unsigned pitch = mt->surf.size > 0 ?
+ mt->surf.row_pitch / 2 : mt->pitch;
uint8_t *untiled_s8_map = map->ptr;
uint8_t *tiled_s8_map = intel_miptree_map_raw(brw, mt,
GL_MAP_READ_BIT);
unsigned int image_x, image_y;
@@ -3270,7 +3276,7 @@ intel_miptree_map_s8(struct brw_context *brw,
for (uint32_t y = 0; y < map->h; y++) {
for (uint32_t x = 0; x < map->w; x++) {
- ptrdiff_t offset = intel_offset_S8(mt->pitch,
+ ptrdiff_t offset = intel_offset_S8(pitch,
Once this is all over, intel_offset_S8 should be updated to just use the
ISL convention.
Post by Topi Pohjolainen
x + image_x + map->x,
y + image_y + map->y,
brw->has_swizzling);
@@ -3298,6 +3304,12 @@ intel_miptree_unmap_s8(struct brw_context *brw,
unsigned int slice)
{
if (map->mode & GL_MAP_WRITE_BIT) {
+ /* ISL uses a stencil pitch value that is expected by hardware whereas
+ * traditional miptree uses half of that. Below the value gets supplied
+ * to intel_offset_S8() which expects the legacy interpretation.
+ */
+ const unsigned pitch = mt->surf.size > 0 ?
+ mt->surf.row_pitch / 2: mt->pitch;
unsigned int image_x, image_y;
uint8_t *untiled_s8_map = map->ptr;
uint8_t *tiled_s8_map = intel_miptree_map_raw(brw, mt,
GL_MAP_WRITE_BIT);
@@ -3306,7 +3318,7 @@ intel_miptree_unmap_s8(struct brw_context *brw,
for (uint32_t y = 0; y < map->h; y++) {
for (uint32_t x = 0; x < map->w; x++) {
- ptrdiff_t offset = intel_offset_S8(mt->pitch,
+ ptrdiff_t offset = intel_offset_S8(pitch,
image_x + x + map->x,
image_y + y + map->y,
brw->has_swizzling);
@@ -3405,6 +3417,12 @@ intel_miptree_map_depthstencil(struct brw_context *brw,
* temporary buffer back out.
*/
if (!(map->mode & GL_MAP_INVALIDATE_RANGE_BIT)) {
+ /* ISL uses a stencil pitch value that is expected by hardware whereas
+ * traditional miptree uses half of that. Below the value gets supplied
+ * to intel_offset_S8() which expects the legacy interpretation.
+ */
+ const unsigned s_pitch = s_mt->surf.size > 0 ?
+ s_mt->surf.row_pitch / 2 : s_mt->pitch;
uint32_t *packed_map = map->ptr;
uint8_t *s_map = intel_miptree_map_raw(brw, s_mt, GL_MAP_READ_BIT);
uint32_t *z_map = intel_miptree_map_raw(brw, z_mt, GL_MAP_READ_BIT);
@@ -3419,7 +3437,7 @@ intel_miptree_map_depthstencil(struct brw_context *brw,
for (uint32_t y = 0; y < map->h; y++) {
for (uint32_t x = 0; x < map->w; x++) {
int map_x = map->x + x, map_y = map->y + y;
- ptrdiff_t s_offset = intel_offset_S8(s_mt->pitch,
+ ptrdiff_t s_offset = intel_offset_S8(s_pitch,
map_x + s_image_x,
map_y + s_image_y,
brw->has_swizzling);
@@ -3466,6 +3484,12 @@ intel_miptree_unmap_depthstencil(struct
brw_context *brw,
bool map_z32f_x24s8 = mt->format == MESA_FORMAT_Z_FLOAT32;
if (map->mode & GL_MAP_WRITE_BIT) {
+ /* ISL uses a stencil pitch value that is expected by hardware whereas
+ * traditional miptree uses half of that. Below the value gets supplied
+ * to intel_offset_S8() which expects the legacy interpretation.
+ */
+ const unsigned s_pitch = s_mt->surf.size > 0 ?
+ s_mt->surf.row_pitch / 2 : s_mt->pitch;
uint32_t *packed_map = map->ptr;
uint8_t *s_map = intel_miptree_map_raw(brw, s_mt, GL_MAP_WRITE_BIT);
uint32_t *z_map = intel_miptree_map_raw(brw, z_mt,
GL_MAP_WRITE_BIT);
@@ -3479,7 +3503,7 @@ intel_miptree_unmap_depthstencil(struct brw_context *brw,
for (uint32_t y = 0; y < map->h; y++) {
for (uint32_t x = 0; x < map->w; x++) {
- ptrdiff_t s_offset = intel_offset_S8(s_mt->pitch,
+ ptrdiff_t s_offset = intel_offset_S8(s_pitch,
x + s_image_x + map->x,
y + s_image_y + map->y,
brw->has_swizzling);
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Pohjolainen, Topi
2017-06-15 19:01:01 UTC
Permalink
Raw Message
Post by Jason Ekstrand
On Tue, Jun 13, 2017 at 7:50 AM, Topi Pohjolainen <
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 32
+++++++++++++++++++++++----
1 file changed, 28 insertions(+), 4 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 0854b4eb5d..ecb9186715 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -3262,6 +3262,12 @@ intel_miptree_map_s8(struct brw_context *brw,
* temporary buffer back out.
*/
if (!(map->mode & GL_MAP_INVALIDATE_RANGE_BIT)) {
+ /* ISL uses a stencil pitch value that is expected by hardware whereas
+ * traditional miptree uses half of that. Below the value gets supplied
+ * to intel_offset_S8() which expects the legacy interpretation.
+ */
+ const unsigned pitch = mt->surf.size > 0 ?
+ mt->surf.row_pitch / 2 : mt->pitch;
uint8_t *untiled_s8_map = map->ptr;
uint8_t *tiled_s8_map = intel_miptree_map_raw(brw, mt, GL_MAP_READ_BIT);
unsigned int image_x, image_y;
@@ -3270,7 +3276,7 @@ intel_miptree_map_s8(struct brw_context *brw,
for (uint32_t y = 0; y < map->h; y++) {
for (uint32_t x = 0; x < map->w; x++) {
- ptrdiff_t offset = intel_offset_S8(mt->pitch,
+ ptrdiff_t offset = intel_offset_S8(pitch,
Once this is all over, intel_offset_S8 should be updated to just use the
ISL convention.
Agreed.
Post by Jason Ekstrand
Post by Topi Pohjolainen
x + image_x + map->x,
y + image_y + map->y,
brw->has_swizzling);
@@ -3298,6 +3304,12 @@ intel_miptree_unmap_s8(struct brw_context *brw,
unsigned int slice)
{
if (map->mode & GL_MAP_WRITE_BIT) {
+ /* ISL uses a stencil pitch value that is expected by hardware whereas
+ * traditional miptree uses half of that. Below the value gets supplied
+ * to intel_offset_S8() which expects the legacy interpretation.
+ */
+ const unsigned pitch = mt->surf.size > 0 ?
+ mt->surf.row_pitch / 2: mt->pitch;
unsigned int image_x, image_y;
uint8_t *untiled_s8_map = map->ptr;
uint8_t *tiled_s8_map = intel_miptree_map_raw(brw, mt, GL_MAP_WRITE_BIT);
@@ -3306,7 +3318,7 @@ intel_miptree_unmap_s8(struct brw_context *brw,
for (uint32_t y = 0; y < map->h; y++) {
for (uint32_t x = 0; x < map->w; x++) {
- ptrdiff_t offset = intel_offset_S8(mt->pitch,
+ ptrdiff_t offset = intel_offset_S8(pitch,
image_x + x + map->x,
image_y + y + map->y,
brw->has_swizzling);
@@ -3405,6 +3417,12 @@ intel_miptree_map_depthstencil(struct brw_context *brw,
* temporary buffer back out.
*/
if (!(map->mode & GL_MAP_INVALIDATE_RANGE_BIT)) {
+ /* ISL uses a stencil pitch value that is expected by hardware whereas
+ * traditional miptree uses half of that. Below the value gets supplied
+ * to intel_offset_S8() which expects the legacy interpretation.
+ */
+ const unsigned s_pitch = s_mt->surf.size > 0 ?
+ s_mt->surf.row_pitch / 2 : s_mt->pitch;
uint32_t *packed_map = map->ptr;
uint8_t *s_map = intel_miptree_map_raw(brw, s_mt, GL_MAP_READ_BIT);
uint32_t *z_map = intel_miptree_map_raw(brw, z_mt, GL_MAP_READ_BIT);
@@ -3419,7 +3437,7 @@ intel_miptree_map_depthstencil(struct brw_context *brw,
for (uint32_t y = 0; y < map->h; y++) {
for (uint32_t x = 0; x < map->w; x++) {
int map_x = map->x + x, map_y = map->y + y;
- ptrdiff_t s_offset = intel_offset_S8(s_mt->pitch,
+ ptrdiff_t s_offset = intel_offset_S8(s_pitch,
map_x + s_image_x,
map_y + s_image_y,
brw->has_swizzling);
@@ -3466,6 +3484,12 @@ intel_miptree_unmap_depthstencil(struct brw_context *brw,
bool map_z32f_x24s8 = mt->format == MESA_FORMAT_Z_FLOAT32;
if (map->mode & GL_MAP_WRITE_BIT) {
+ /* ISL uses a stencil pitch value that is expected by hardware whereas
+ * traditional miptree uses half of that. Below the value gets supplied
+ * to intel_offset_S8() which expects the legacy interpretation.
+ */
+ const unsigned s_pitch = s_mt->surf.size > 0 ?
+ s_mt->surf.row_pitch / 2 : s_mt->pitch;
uint32_t *packed_map = map->ptr;
uint8_t *s_map = intel_miptree_map_raw(brw, s_mt, GL_MAP_WRITE_BIT);
uint32_t *z_map = intel_miptree_map_raw(brw, z_mt,
GL_MAP_WRITE_BIT);
@@ -3479,7 +3503,7 @@ intel_miptree_unmap_depthstencil(struct brw_context *brw,
for (uint32_t y = 0; y < map->h; y++) {
for (uint32_t x = 0; x < map->w; x++) {
- ptrdiff_t s_offset = intel_offset_S8(s_mt->pitch,
+ ptrdiff_t s_offset = intel_offset_S8(s_pitch,
x + s_image_x + map->x,
y + s_image_y + map->y,
brw->has_swizzling);
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Topi Pohjolainen
2017-06-13 14:50:09 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 76 +++++++++++++++++++++++++++
1 file changed, 76 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 212dfa30ec..0854b4eb5d 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -643,6 +643,82 @@ free_aux_state_map(enum isl_aux_state **state)
}

static struct intel_mipmap_tree *
+make_surface(struct brw_context *brw, GLenum target, mesa_format format,
+ unsigned first_level, unsigned last_level,
+ unsigned width0, unsigned height0, unsigned depth0,
+ unsigned num_samples, enum isl_tiling isl_tiling,
+ isl_surf_usage_flags_t isl_usage_flags, uint32_t alloc_flags,
+ struct brw_bo *bo)
+{
+ struct intel_mipmap_tree *mt = calloc(sizeof(*mt), 1);
+ if (!mt)
+ return NULL;
+
+ if (!create_mapping_table(target, first_level, last_level, depth0,
+ mt->level)) {
+ free(mt);
+ return NULL;
+ }
+
+ if (target == GL_TEXTURE_CUBE_MAP ||
+ target == GL_TEXTURE_CUBE_MAP_ARRAY)
+ isl_usage_flags |= ISL_SURF_USAGE_CUBE_BIT;
+
+ DBG("%s: %s %s %ux %u:%u:%u %d..%d <-- %p\n",
+ __func__,
+ _mesa_enum_to_string(target),
+ _mesa_get_format_name(format),
+ num_samples, width0, height0, depth0,
+ first_level, last_level, mt);
+
+ struct isl_surf_init_info init_info = {
+ .dim = get_isl_surf_dim(target),
+ .format = translate_tex_format(brw, format, false),
+ .width = width0,
+ .height = height0,
+ .depth = target == GL_TEXTURE_3D ? depth0 : 1,
+ .levels = last_level - first_level + 1,
+ .array_len = target == GL_TEXTURE_3D ? 1 : depth0,
+ .samples = MAX2(num_samples, 1),
+ .usage = isl_usage_flags,
+ .tiling_flags = 1u << isl_tiling
+ };
+
+ if (!isl_surf_init_s(&brw->isl_dev, &mt->surf, &init_info))
+ goto fail;
+
+ assert(mt->surf.size % mt->surf.row_pitch == 0);
+
+ if (!bo) {
+ unsigned pitch = mt->surf.row_pitch;
+ mt->bo = brw_bo_alloc_tiled(brw->bufmgr, "isl-miptree",
+ mt->surf.row_pitch,
+ mt->surf.size / mt->surf.row_pitch,
+ 1, isl_tiling_to_bufmgr_tiling(isl_tiling),
+ &pitch, alloc_flags);
+ if (!mt->bo)
+ goto fail;
+
+ assert(pitch == mt->surf.row_pitch);
+ } else {
+ mt->bo = bo;
+ }
+
+ mt->first_level = first_level;
+ mt->last_level = last_level;
+ mt->target = target;
+ mt->format = format;
+ mt->refcount = 1;
+ mt->aux_state = NULL;
+
+ return mt;
+
+fail:
+ intel_miptree_release(&mt);
+ return NULL;
+}
+
+static struct intel_mipmap_tree *
miptree_create(struct brw_context *brw,
GLenum target,
mesa_format format,
--
2.11.0
Jason Ekstrand
2017-06-15 23:23:30 UTC
Permalink
Raw Message
On Tue, Jun 13, 2017 at 7:50 AM, Topi Pohjolainen <
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 76
+++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 212dfa30ec..0854b4eb5d 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -643,6 +643,82 @@ free_aux_state_map(enum isl_aux_state **state)
}
static struct intel_mipmap_tree *
+make_surface(struct brw_context *brw, GLenum target, mesa_format format,
+ unsigned first_level, unsigned last_level,
+ unsigned width0, unsigned height0, unsigned depth0,
+ unsigned num_samples, enum isl_tiling isl_tiling,
+ isl_surf_usage_flags_t isl_usage_flags, uint32_t alloc_flags,
+ struct brw_bo *bo)
+{
+ struct intel_mipmap_tree *mt = calloc(sizeof(*mt), 1);
+ if (!mt)
+ return NULL;
+
+ if (!create_mapping_table(target, first_level, last_level, depth0,
+ mt->level)) {
+ free(mt);
+ return NULL;
+ }
You create the mapping table, but I don't see you fill it out anywhere. Am
I missing something? Maybe it would be better to just let it be NULL so we
catch all of the remaining uses of the old table.
Post by Topi Pohjolainen
+
+ if (target == GL_TEXTURE_CUBE_MAP ||
+ target == GL_TEXTURE_CUBE_MAP_ARRAY)
+ isl_usage_flags |= ISL_SURF_USAGE_CUBE_BIT;
+
+ DBG("%s: %s %s %ux %u:%u:%u %d..%d <-- %p\n",
+ __func__,
+ _mesa_enum_to_string(target),
+ _mesa_get_format_name(format),
+ num_samples, width0, height0, depth0,
+ first_level, last_level, mt);
+
+ struct isl_surf_init_info init_info = {
+ .dim = get_isl_surf_dim(target),
+ .format = translate_tex_format(brw, format, false),
+ .width = width0,
+ .height = height0,
+ .depth = target == GL_TEXTURE_3D ? depth0 : 1,
+ .levels = last_level - first_level + 1,
+ .array_len = target == GL_TEXTURE_3D ? 1 : depth0,
+ .samples = MAX2(num_samples, 1),
+ .usage = isl_usage_flags,
+ .tiling_flags = 1u << isl_tiling
+ };
+
+ if (!isl_surf_init_s(&brw->isl_dev, &mt->surf, &init_info))
+ goto fail;
+
+ assert(mt->surf.size % mt->surf.row_pitch == 0);
+
+ if (!bo) {
+ unsigned pitch = mt->surf.row_pitch;
+ mt->bo = brw_bo_alloc_tiled(brw->bufmgr, "isl-miptree",
+ mt->surf.row_pitch,
+ mt->surf.size / mt->surf.row_pitch,
+ 1, isl_tiling_to_bufmgr_tiling(
isl_tiling),
+ &pitch, alloc_flags);
+ if (!mt->bo)
+ goto fail;
+
+ assert(pitch == mt->surf.row_pitch);
+ } else {
+ mt->bo = bo;
+ }
+
+ mt->first_level = first_level;
+ mt->last_level = last_level;
+ mt->target = target;
+ mt->format = format;
+ mt->refcount = 1;
+ mt->aux_state = NULL;
+
+ return mt;
+
+ intel_miptree_release(&mt);
+ return NULL;
+}
+
+static struct intel_mipmap_tree *
miptree_create(struct brw_context *brw,
GLenum target,
mesa_format format,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Pohjolainen, Topi
2017-06-16 04:54:27 UTC
Permalink
Raw Message
Post by Jason Ekstrand
On Tue, Jun 13, 2017 at 7:50 AM, Topi Pohjolainen <
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 76
+++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 212dfa30ec..0854b4eb5d 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -643,6 +643,82 @@ free_aux_state_map(enum isl_aux_state **state)
}
static struct intel_mipmap_tree *
+make_surface(struct brw_context *brw, GLenum target, mesa_format format,
+ unsigned first_level, unsigned last_level,
+ unsigned width0, unsigned height0, unsigned depth0,
+ unsigned num_samples, enum isl_tiling isl_tiling,
+ isl_surf_usage_flags_t isl_usage_flags, uint32_t alloc_flags,
+ struct brw_bo *bo)
+{
+ struct intel_mipmap_tree *mt = calloc(sizeof(*mt), 1);
+ if (!mt)
+ return NULL;
+
+ if (!create_mapping_table(target, first_level, last_level, depth0,
+ mt->level)) {
+ free(mt);
+ return NULL;
+ }
You create the mapping table, but I don't see you fill it out anywhere. Am
I missing something? Maybe it would be better to just let it be NULL so we
catch all of the remaining uses of the old table.
I think you asked this previously also. We still need it to hold
"struct intel_miptree_map *" members (see intel_miptree_attach_map()).
Post by Jason Ekstrand
Post by Topi Pohjolainen
+
+ if (target == GL_TEXTURE_CUBE_MAP ||
+ target == GL_TEXTURE_CUBE_MAP_ARRAY)
+ isl_usage_flags |= ISL_SURF_USAGE_CUBE_BIT;
+
+ DBG("%s: %s %s %ux %u:%u:%u %d..%d <-- %p\n",
+ __func__,
+ _mesa_enum_to_string(target),
+ _mesa_get_format_name(format),
+ num_samples, width0, height0, depth0,
+ first_level, last_level, mt);
+
+ struct isl_surf_init_info init_info = {
+ .dim = get_isl_surf_dim(target),
+ .format = translate_tex_format(brw, format, false),
+ .width = width0,
+ .height = height0,
+ .depth = target == GL_TEXTURE_3D ? depth0 : 1,
+ .levels = last_level - first_level + 1,
+ .array_len = target == GL_TEXTURE_3D ? 1 : depth0,
+ .samples = MAX2(num_samples, 1),
+ .usage = isl_usage_flags,
+ .tiling_flags = 1u << isl_tiling
+ };
+
+ if (!isl_surf_init_s(&brw->isl_dev, &mt->surf, &init_info))
+ goto fail;
+
+ assert(mt->surf.size % mt->surf.row_pitch == 0);
+
+ if (!bo) {
+ unsigned pitch = mt->surf.row_pitch;
+ mt->bo = brw_bo_alloc_tiled(brw->bufmgr, "isl-miptree",
+ mt->surf.row_pitch,
+ mt->surf.size / mt->surf.row_pitch,
+ 1, isl_tiling_to_bufmgr_tiling(
isl_tiling),
+ &pitch, alloc_flags);
+ if (!mt->bo)
+ goto fail;
+
+ assert(pitch == mt->surf.row_pitch);
+ } else {
+ mt->bo = bo;
+ }
+
+ mt->first_level = first_level;
+ mt->last_level = last_level;
+ mt->target = target;
+ mt->format = format;
+ mt->refcount = 1;
+ mt->aux_state = NULL;
+
+ return mt;
+
+ intel_miptree_release(&mt);
+ return NULL;
+}
+
+static struct intel_mipmap_tree *
miptree_create(struct brw_context *brw,
GLenum target,
mesa_format format,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Jason Ekstrand
2017-06-16 04:58:29 UTC
Permalink
Raw Message
On Thu, Jun 15, 2017 at 9:54 PM, Pohjolainen, Topi <
Post by Topi Pohjolainen
Post by Jason Ekstrand
On Tue, Jun 13, 2017 at 7:50 AM, Topi Pohjolainen <
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 76
+++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 212dfa30ec..0854b4eb5d 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -643,6 +643,82 @@ free_aux_state_map(enum isl_aux_state **state)
}
static struct intel_mipmap_tree *
+make_surface(struct brw_context *brw, GLenum target, mesa_format
format,
Post by Jason Ekstrand
Post by Topi Pohjolainen
+ unsigned first_level, unsigned last_level,
+ unsigned width0, unsigned height0, unsigned depth0,
+ unsigned num_samples, enum isl_tiling isl_tiling,
+ isl_surf_usage_flags_t isl_usage_flags, uint32_t
alloc_flags,
Post by Jason Ekstrand
Post by Topi Pohjolainen
+ struct brw_bo *bo)
+{
+ struct intel_mipmap_tree *mt = calloc(sizeof(*mt), 1);
+ if (!mt)
+ return NULL;
+
+ if (!create_mapping_table(target, first_level, last_level, depth0,
+ mt->level)) {
+ free(mt);
+ return NULL;
+ }
You create the mapping table, but I don't see you fill it out anywhere.
Am
Post by Jason Ekstrand
I missing something? Maybe it would be better to just let it be NULL so
we
Post by Jason Ekstrand
catch all of the remaining uses of the old table.
I think you asked this previously also. We still need it to hold
"struct intel_miptree_map *" members (see intel_miptree_attach_map()).
Ugh... That's unfortunate but I understand. Carry on.
Post by Topi Pohjolainen
Post by Jason Ekstrand
Post by Topi Pohjolainen
+
+ if (target == GL_TEXTURE_CUBE_MAP ||
+ target == GL_TEXTURE_CUBE_MAP_ARRAY)
+ isl_usage_flags |= ISL_SURF_USAGE_CUBE_BIT;
+
+ DBG("%s: %s %s %ux %u:%u:%u %d..%d <-- %p\n",
+ __func__,
+ _mesa_enum_to_string(target),
+ _mesa_get_format_name(format),
+ num_samples, width0, height0, depth0,
+ first_level, last_level, mt);
+
+ struct isl_surf_init_info init_info = {
+ .dim = get_isl_surf_dim(target),
+ .format = translate_tex_format(brw, format, false),
+ .width = width0,
+ .height = height0,
+ .depth = target == GL_TEXTURE_3D ? depth0 : 1,
+ .levels = last_level - first_level + 1,
+ .array_len = target == GL_TEXTURE_3D ? 1 : depth0,
+ .samples = MAX2(num_samples, 1),
+ .usage = isl_usage_flags,
+ .tiling_flags = 1u << isl_tiling
+ };
+
+ if (!isl_surf_init_s(&brw->isl_dev, &mt->surf, &init_info))
+ goto fail;
+
+ assert(mt->surf.size % mt->surf.row_pitch == 0);
+
+ if (!bo) {
+ unsigned pitch = mt->surf.row_pitch;
+ mt->bo = brw_bo_alloc_tiled(brw->bufmgr, "isl-miptree",
+ mt->surf.row_pitch,
+ mt->surf.size / mt->surf.row_pitch,
+ 1, isl_tiling_to_bufmgr_tiling(
isl_tiling),
+ &pitch, alloc_flags);
+ if (!mt->bo)
+ goto fail;
+
+ assert(pitch == mt->surf.row_pitch);
+ } else {
+ mt->bo = bo;
+ }
+
+ mt->first_level = first_level;
+ mt->last_level = last_level;
+ mt->target = target;
+ mt->format = format;
+ mt->refcount = 1;
+ mt->aux_state = NULL;
+
+ return mt;
+
+ intel_miptree_release(&mt);
+ return NULL;
+}
+
+static struct intel_mipmap_tree *
miptree_create(struct brw_context *brw,
GLenum target,
mesa_format format,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Nanley Chery
2017-06-16 18:11:31 UTC
Permalink
Raw Message
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 76 +++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 212dfa30ec..0854b4eb5d 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -643,6 +643,82 @@ free_aux_state_map(enum isl_aux_state **state)
}
static struct intel_mipmap_tree *
+make_surface(struct brw_context *brw, GLenum target, mesa_format format,
+ unsigned first_level, unsigned last_level,
+ unsigned width0, unsigned height0, unsigned depth0,
+ unsigned num_samples, enum isl_tiling isl_tiling,
+ isl_surf_usage_flags_t isl_usage_flags, uint32_t alloc_flags,
+ struct brw_bo *bo)
^
const ?
Post by Topi Pohjolainen
+{
+ struct intel_mipmap_tree *mt = calloc(sizeof(*mt), 1);
+ if (!mt)
+ return NULL;
+
+ if (!create_mapping_table(target, first_level, last_level, depth0,
+ mt->level)) {
+ free(mt);
+ return NULL;
+ }
+
+ if (target == GL_TEXTURE_CUBE_MAP ||
+ target == GL_TEXTURE_CUBE_MAP_ARRAY)
+ isl_usage_flags |= ISL_SURF_USAGE_CUBE_BIT;
+
+ DBG("%s: %s %s %ux %u:%u:%u %d..%d <-- %p\n",
+ __func__,
+ _mesa_enum_to_string(target),
+ _mesa_get_format_name(format),
+ num_samples, width0, height0, depth0,
+ first_level, last_level, mt);
+
+ struct isl_surf_init_info init_info = {
+ .dim = get_isl_surf_dim(target),
+ .format = translate_tex_format(brw, format, false),
+ .width = width0,
+ .height = height0,
+ .depth = target == GL_TEXTURE_3D ? depth0 : 1,
+ .levels = last_level - first_level + 1,
+ .array_len = target == GL_TEXTURE_3D ? 1 : depth0,
+ .samples = MAX2(num_samples, 1),
+ .usage = isl_usage_flags,
+ .tiling_flags = 1u << isl_tiling
+ };
+
+ if (!isl_surf_init_s(&brw->isl_dev, &mt->surf, &init_info))
+ goto fail;
+
+ assert(mt->surf.size % mt->surf.row_pitch == 0);
+
+ if (!bo) {
+ unsigned pitch = mt->surf.row_pitch;
^
MAYBE_UNUSED for release builds?

This patch is
Reviewed-by: Nanley Chery <***@intel.com>

I've taken a brief skim at the rest of the patches and it all looks
good. Given that the rest of the series has already been reviewed, I
think I'll stop here.

Cheers,
Nanley
Post by Topi Pohjolainen
+ mt->bo = brw_bo_alloc_tiled(brw->bufmgr, "isl-miptree",
+ mt->surf.row_pitch,
+ mt->surf.size / mt->surf.row_pitch,
+ 1, isl_tiling_to_bufmgr_tiling(isl_tiling),
+ &pitch, alloc_flags);
+ if (!mt->bo)
+ goto fail;
+
+ assert(pitch == mt->surf.row_pitch);
+ } else {
+ mt->bo = bo;
+ }
+
+ mt->first_level = first_level;
+ mt->last_level = last_level;
+ mt->target = target;
+ mt->format = format;
+ mt->refcount = 1;
+ mt->aux_state = NULL;
+
+ return mt;
+
+ intel_miptree_release(&mt);
+ return NULL;
+}
+
+static struct intel_mipmap_tree *
miptree_create(struct brw_context *brw,
GLenum target,
mesa_format format,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Jason Ekstrand
2017-06-16 18:40:39 UTC
Permalink
Raw Message
Post by Topi Pohjolainen
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 76
+++++++++++++++++++++++++++
Post by Topi Pohjolainen
1 file changed, 76 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
Post by Topi Pohjolainen
index 212dfa30ec..0854b4eb5d 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -643,6 +643,82 @@ free_aux_state_map(enum isl_aux_state **state)
}
static struct intel_mipmap_tree *
+make_surface(struct brw_context *brw, GLenum target, mesa_format format,
+ unsigned first_level, unsigned last_level,
+ unsigned width0, unsigned height0, unsigned depth0,
+ unsigned num_samples, enum isl_tiling isl_tiling,
+ isl_surf_usage_flags_t isl_usage_flags, uint32_t
alloc_flags,
Post by Topi Pohjolainen
+ struct brw_bo *bo)
^
const ?
No. For one thing, I'm fairly sure that, at least in the future, we'll
need to be able to modify the bo->is_scanout flag. More importantly,
however, this is creating a mutable miptree which means that what it
returns provides a way of changing the contents of the BO so making the BO
const is a lie.
Post by Topi Pohjolainen
Post by Topi Pohjolainen
+{
+ struct intel_mipmap_tree *mt = calloc(sizeof(*mt), 1);
+ if (!mt)
+ return NULL;
+
+ if (!create_mapping_table(target, first_level, last_level, depth0,
+ mt->level)) {
+ free(mt);
+ return NULL;
+ }
+
+ if (target == GL_TEXTURE_CUBE_MAP ||
+ target == GL_TEXTURE_CUBE_MAP_ARRAY)
+ isl_usage_flags |= ISL_SURF_USAGE_CUBE_BIT;
+
+ DBG("%s: %s %s %ux %u:%u:%u %d..%d <-- %p\n",
+ __func__,
+ _mesa_enum_to_string(target),
+ _mesa_get_format_name(format),
+ num_samples, width0, height0, depth0,
+ first_level, last_level, mt);
+
+ struct isl_surf_init_info init_info = {
+ .dim = get_isl_surf_dim(target),
+ .format = translate_tex_format(brw, format, false),
+ .width = width0,
+ .height = height0,
+ .depth = target == GL_TEXTURE_3D ? depth0 : 1,
+ .levels = last_level - first_level + 1,
+ .array_len = target == GL_TEXTURE_3D ? 1 : depth0,
+ .samples = MAX2(num_samples, 1),
+ .usage = isl_usage_flags,
+ .tiling_flags = 1u << isl_tiling
+ };
+
+ if (!isl_surf_init_s(&brw->isl_dev, &mt->surf, &init_info))
+ goto fail;
+
+ assert(mt->surf.size % mt->surf.row_pitch == 0);
+
+ if (!bo) {
+ unsigned pitch = mt->surf.row_pitch;
^
MAYBE_UNUSED for release builds?
I think it's probably ok since it's passed in to brw_bo_alloc_tiled. Then
again, I'm pretty sure topi has rebased on top of the new
brw_bo_alloc_tiled which gets rid of all this nonsense.
Post by Topi Pohjolainen
This patch is
I've taken a brief skim at the rest of the patches and it all looks
good. Given that the rest of the series has already been reviewed, I
think I'll stop here.
Cheers,
Nanley
Post by Topi Pohjolainen
+ mt->bo = brw_bo_alloc_tiled(brw->bufmgr, "isl-miptree",
+ mt->surf.row_pitch,
+ mt->surf.size / mt->surf.row_pitch,
+ 1, isl_tiling_to_bufmgr_tiling(
isl_tiling),
Post by Topi Pohjolainen
+ &pitch, alloc_flags);
+ if (!mt->bo)
+ goto fail;
+
+ assert(pitch == mt->surf.row_pitch);
+ } else {
+ mt->bo = bo;
+ }
+
+ mt->first_level = first_level;
+ mt->last_level = last_level;
+ mt->target = target;
+ mt->format = format;
+ mt->refcount = 1;
+ mt->aux_state = NULL;
+
+ return mt;
+
+ intel_miptree_release(&mt);
+ return NULL;
+}
+
+static struct intel_mipmap_tree *
miptree_create(struct brw_context *brw,
GLenum target,
mesa_format format,
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Topi Pohjolainen
2017-06-13 14:50:08 UTC
Permalink
Raw Message
Signed-off-by: Topi Pohjolainen <***@intel.com>
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index c81d345fbc..212dfa30ec 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1175,6 +1175,22 @@ intel_miptree_get_image_offset(const struct intel_mipmap_tree *mt,
GLuint level, GLuint slice,
GLuint *x, GLuint *y)
{
+ if (mt->surf.size > 0) {
+ uint32_t x_offset_sa, y_offset_sa;
+
+ assert(level >= mt->first_level);
+ level -= mt->first_level;
+
+ const unsigned z = mt->surf.dim == ISL_SURF_DIM_3D ? slice : 0;
+ slice = mt->surf.dim == ISL_SURF_DIM_3D ? 0 : slice;
+ isl_surf_get_image_offset_sa(&mt->surf, level, slice, z,
+ &x_offset_sa, &y_offset_sa);
+
+ *x = x_offset_sa;
+ *y = y_offset_sa;
+ return;
+ }
+
assert(slice < mt->level[level].depth);

*x = mt->level[level].slice[slice].x_offset;
--
2.11.0
Nanley Chery
2017-06-16 17:17:14 UTC
Permalink
Raw Message
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index c81d345fbc..212dfa30ec 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1175,6 +1175,22 @@ intel_miptree_get_image_offset(const struct intel_mipmap_tree *mt,
GLuint level, GLuint slice,
GLuint *x, GLuint *y)
{
+ if (mt->surf.size > 0) {
+ uint32_t x_offset_sa, y_offset_sa;
+
A comment explaining that we need to perform a subtraction because the
ISL surf begins at first_level may be helpful. It took me some digging
to realize why it was needed.
Post by Topi Pohjolainen
+ assert(level >= mt->first_level);
+ level -= mt->first_level;
+
+ const unsigned z = mt->surf.dim == ISL_SURF_DIM_3D ? slice : 0;
+ slice = mt->surf.dim == ISL_SURF_DIM_3D ? 0 : slice;
+ isl_surf_get_image_offset_sa(&mt->surf, level, slice, z,
+ &x_offset_sa, &y_offset_sa);
+
+ *x = x_offset_sa;
+ *y = y_offset_sa;
I'm assuming that using these intermediate variables avoids casting
warnings?


With or without the code comment, this patch is
Post by Topi Pohjolainen
+ return;
+ }
+
assert(slice < mt->level[level].depth);
*x = mt->level[level].slice[slice].x_offset;
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Pohjolainen, Topi
2017-06-20 07:50:01 UTC
Permalink
Raw Message
Post by Nanley Chery
Post by Topi Pohjolainen
---
src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index c81d345fbc..212dfa30ec 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -1175,6 +1175,22 @@ intel_miptree_get_image_offset(const struct intel_mipmap_tree *mt,
GLuint level, GLuint slice,
GLuint *x, GLuint *y)
{
+ if (mt->surf.size > 0) {
+ uint32_t x_offset_sa, y_offset_sa;
+
A comment explaining that we need to perform a subtraction because the
ISL surf begins at first_level may be helpful. It took me some digging
to realize why it was needed.
I'll add a note.
Post by Nanley Chery
Post by Topi Pohjolainen
+ assert(level >= mt->first_level);
+ level -= mt->first_level;
+
+ const unsigned z = mt->surf.dim == ISL_SURF_DIM_3D ? slice : 0;
+ slice = mt->surf.dim == ISL_SURF_DIM_3D ? 0 : slice;
+ isl_surf_get_image_offset_sa(&mt->surf, level, slice, z,
+ &x_offset_sa, &y_offset_sa);
+
+ *x = x_offset_sa;
+ *y = y_offset_sa;
I'm assuming that using these intermediate variables avoids casting
warnings?
Correct :)
Post by Nanley Chery
With or without the code comment, this patch is
Post by Topi Pohjolainen
+ return;
+ }
+
assert(slice < mt->level[level].depth);
*x = mt->level[level].slice[slice].x_offset;
--
2.11.0
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Loading...