Re: [Mesa-dev] [PATCH 1/3] i965: Don't forget the cube map padding on gen5+.
On 10/08/2013 10:36 AM, Eric Anholt wrote:
We had a fixup for gen4's 3d-layout cubemaps (which, iirc, we'd
experimentally found to be necessary!), but while the spec still requires
it on gen5, we'd been missing it in the array-layout cubemaps.
I think we didn't bother with that patch because we only support
GL_ARB_texture_cube_map_array on GEN6+. While I think this is a
reasonable change (keep the paths similar), I don't think it needs to be
picked back to stable... unless there's a way I'm not seeing that could
cause it to get hit...
For master,
Reviewed-by: Ian Romanick ian.d.roman...@intel.com
Cc: 9.1 9.2 mesa-sta...@lists.freedesktop.org
---
src/mesa/drivers/dri/i965/brw_tex_layout.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
b/src/mesa/drivers/dri/i965/brw_tex_layout.c
index e4e66b4..e9128a3 100644
--- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
+++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
@@ -197,6 +197,18 @@ brw_miptree_layout_2d(struct intel_mipmap_tree *mt)
}
static void
+align_cube(struct intel_mipmap_tree *mt)
+{
+ /* The 965's sampler lays cachelines out according to how accesses
+* in the texture surfaces run, so they may be vertical through
+* memory. As a result, the docs say in Surface Padding Requirements:
+* Sampling Engine Surfaces that two extra rows of padding are required.
+*/
+ if (mt-target == GL_TEXTURE_CUBE_MAP)
+ mt-total_height += 2;
+}
+
+static void
brw_miptree_layout_texture_array(struct brw_context *brw,
struct intel_mipmap_tree *mt)
{
@@ -220,6 +232,8 @@ brw_miptree_layout_texture_array(struct brw_context *brw,
}
}
mt-total_height = qpitch * mt-physical_depth0;
+
+ align_cube(mt);
}
static void
@@ -291,13 +305,7 @@ brw_miptree_layout_texture_3d(struct brw_context *brw,
}
}
- /* The 965's sampler lays cachelines out according to how accesses
-* in the texture surfaces run, so they may be vertical through
-* memory. As a result, the docs say in Surface Padding Requirements:
-* Sampling Engine Surfaces that two extra rows of padding are required.
-*/
- if (mt-target == GL_TEXTURE_CUBE_MAP)
- mt-total_height += 2;
+ align_cube(mt);
}
void
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] i965: Don't forget the cube map padding on gen5+.
Ian Romanick i...@freedesktop.org writes: On 10/08/2013 10:36 AM, Eric Anholt wrote: We had a fixup for gen4's 3d-layout cubemaps (which, iirc, we'd experimentally found to be necessary!), but while the spec still requires it on gen5, we'd been missing it in the array-layout cubemaps. I think we didn't bother with that patch because we only support GL_ARB_texture_cube_map_array on GEN6+. While I think this is a reasonable change (keep the paths similar), I don't think it needs to be picked back to stable... unless there's a way I'm not seeing that could cause it to get hit... As of gen5, cube maps on all hardware are always laid out as arrays, not as weird 3d textures -- that's what I was referring to. pgpjj4w9AklTp.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] i965: Don't forget the cube map padding on gen5+.
On 10/08/2013 11:22 AM, Ian Romanick wrote: On 10/08/2013 10:36 AM, Eric Anholt wrote: We had a fixup for gen4's 3d-layout cubemaps (which, iirc, we'd experimentally found to be necessary!), but while the spec still requires it on gen5, we'd been missing it in the array-layout cubemaps. I think we didn't bother with that patch because we only support GL_ARB_texture_cube_map_array on GEN6+. While I think this is a reasonable change (keep the paths similar), I don't think it needs to be picked back to stable... unless there's a way I'm not seeing that could cause it to get hit... For master, Reviewed-by: Ian Romanick ian.d.roman...@intel.com Ian, I believe this patch is about ordinary cube maps, not cube map arrays. On Gen4, cube maps are stored similarly to 3D textures. On Gen5+, cube maps are stored as 2D array textures, indexed by the cube face. It sounds like the fix Eric discovered for Gen4 cubemaps also applies to Gen5+ cubemaps (even though they are layed out differently) --Ken ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] i965: Don't forget the cube map padding on gen5+.
On 10/08/2013 10:36 AM, Eric Anholt wrote:
We had a fixup for gen4's 3d-layout cubemaps (which, iirc, we'd
experimentally found to be necessary!), but while the spec still requires
it on gen5, we'd been missing it in the array-layout cubemaps.
Ah, I see it now:
From the Sandybridge PRM, Volume 1, Part 1, Section 7.19...OR...the
Ivybridge PRM, Volume 1, Part 1, Section 6.19 (Surface Padding
Requirements):
For cube surfaces, an additional two rows of padding are required at
the bottom of the surface. This must be ensured regardless of whether
the surface is stored tiled or linear. This is due to the potential
rotation of cache line orientation from memory to cache.
So it sounds like this is indeed necessary on all platforms. Unless
they just forgot to remove this text from the docs. Have you observed
it to fix anything?
Even if you haven't, we should do it anyway - it won't hurt anything and
may avoid page faults which manifest as hangs.
Reviewed-by: Kenneth Graunke kenn...@whitecape.org
Cc: 9.1 9.2 mesa-sta...@lists.freedesktop.org
---
src/mesa/drivers/dri/i965/brw_tex_layout.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c
b/src/mesa/drivers/dri/i965/brw_tex_layout.c
index e4e66b4..e9128a3 100644
--- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
+++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
@@ -197,6 +197,18 @@ brw_miptree_layout_2d(struct intel_mipmap_tree *mt)
}
static void
+align_cube(struct intel_mipmap_tree *mt)
+{
+ /* The 965's sampler lays cachelines out according to how accesses
+* in the texture surfaces run, so they may be vertical through
+* memory. As a result, the docs say in Surface Padding Requirements:
+* Sampling Engine Surfaces that two extra rows of padding are required.
+*/
+ if (mt-target == GL_TEXTURE_CUBE_MAP)
+ mt-total_height += 2;
+}
+
+static void
brw_miptree_layout_texture_array(struct brw_context *brw,
struct intel_mipmap_tree *mt)
{
@@ -220,6 +232,8 @@ brw_miptree_layout_texture_array(struct brw_context *brw,
}
}
mt-total_height = qpitch * mt-physical_depth0;
+
+ align_cube(mt);
}
static void
@@ -291,13 +305,7 @@ brw_miptree_layout_texture_3d(struct brw_context *brw,
}
}
- /* The 965's sampler lays cachelines out according to how accesses
-* in the texture surfaces run, so they may be vertical through
-* memory. As a result, the docs say in Surface Padding Requirements:
-* Sampling Engine Surfaces that two extra rows of padding are required.
-*/
- if (mt-target == GL_TEXTURE_CUBE_MAP)
- mt-total_height += 2;
+ align_cube(mt);
}
void
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] i965: Don't forget the cube map padding on gen5+.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 10/08/2013 01:28 PM, Eric Anholt wrote: Ian Romanick i...@freedesktop.org writes: On 10/08/2013 10:36 AM, Eric Anholt wrote: We had a fixup for gen4's 3d-layout cubemaps (which, iirc, we'd experimentally found to be necessary!), but while the spec still requires it on gen5, we'd been missing it in the array-layout cubemaps. I think we didn't bother with that patch because we only support GL_ARB_texture_cube_map_array on GEN6+. While I think this is a reasonable change (keep the paths similar), I don't think it needs to be picked back to stable... unless there's a way I'm not seeing that could cause it to get hit... As of gen5, cube maps on all hardware are always laid out as arrays, not as weird 3d textures -- that's what I was referring to. Oh, that's right. Carry on. :) -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.13 (GNU/Linux) iEYEARECAAYFAlJUfRwACgkQX1gOwKyEAw/xWgCeMoP1cO//JufiKBs37xJ9vPsm aNAAn2FaP58e/R4C28Vq13lMlf4IK77V =bHTO -END PGP SIGNATURE- ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] i965: Don't forget the cube map padding on gen5+.
On 10/08/2013 10:36 AM, Eric Anholt wrote: We had a fixup for gen4's 3d-layout cubemaps (which, iirc, we'd experimentally found to be necessary!), but while the spec still requires it on gen5, we'd been missing it in the array-layout cubemaps. Cc: 9.1 9.2 mesa-sta...@lists.freedesktop.org --- src/mesa/drivers/dri/i965/brw_tex_layout.c | 22 +++--- 1 file changed, 15 insertions(+), 7 deletions(-) Patches 1 and 2 are Reviewed-by: Chad Versace chad.vers...@linux.intel.com ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] i965: Don't forget the cube map padding on gen5+.
Kenneth Graunke kenn...@whitecape.org writes: On 10/08/2013 10:36 AM, Eric Anholt wrote: We had a fixup for gen4's 3d-layout cubemaps (which, iirc, we'd experimentally found to be necessary!), but while the spec still requires it on gen5, we'd been missing it in the array-layout cubemaps. Ah, I see it now: From the Sandybridge PRM, Volume 1, Part 1, Section 7.19...OR...the Ivybridge PRM, Volume 1, Part 1, Section 6.19 (Surface Padding Requirements): For cube surfaces, an additional two rows of padding are required at the bottom of the surface. This must be ensured regardless of whether the surface is stored tiled or linear. This is due to the potential rotation of cache line orientation from memory to cache. So it sounds like this is indeed necessary on all platforms. Unless they just forgot to remove this text from the docs. Have you observed it to fix anything? No, but it's hard to trigger -- you need your cube map to be large, unaligned in the y direction, and to land at the very end of the address space. pgpUsgn7q0JHt.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
