[Mesa-dev] i965: Fix 1D Array Shadow miptree layout issue (leading to assert or hang)

Submitted by Jordan Justen on July 31, 2014, 7:42 a.m.

Details

Message ID 1406792531-17918-1-git-send-email-jordan.l.justen@intel.com
State Accepted
Commit c860a379d2fa09da1711591b6aef4a885d224ea0
Headers show

Not browsing as part of any series.

Commit Message

Jordan Justen July 31, 2014, 7:42 a.m.
Fixes assertion failure in piglit (gen6, gen8):
spec/glsl-1.30/execution/tex-miplevel-selection textureOffset 1DArrayShadow

In release builds of Mesa, this was observed to cause a GPU hang on
gen8.

Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ben Widawsky <ben@bwidawsk.net>
Cc: Jason Ekstrand <jason@jlekstrand.net>
Cc: "10.2" <mesa-stable@lists.freedesktop.org>
---
 On a gen6 piglit quick run:
 15: crash => pass
 1: crash => fail

 src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 22c5ab8..f504e06 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -244,6 +244,26 @@  intel_miptree_create_layout(struct brw_context *brw,
        _mesa_get_format_name(format),
        first_level, last_level, depth0, mt);
 
+   if (target == GL_TEXTURE_1D_ARRAY) {
+      /* For a 1D Array texture the OpenGL API will treat the height0
+       * parameter as the number of array slices. For Intel hardware, we treat
+       * the 1D array as a 2D Array with a height of 1.
+       *
+       * So, when we first come through this path to create a 1D Array
+       * texture, height0 stores the number of slices, and depth0 is 1. In
+       * this case, we want to swap height0 and depth0.
+       *
+       * Since some miptrees will be created based on the base miptree, we may
+       * come through this path and see height0 as 1 and depth0 being the
+       * number of slices. In this case we don't need to do the swap.
+       */
+      assert(height0 == 1 || depth0 == 1);
+      if (height0 > 1) {
+         depth0 = height0;
+         height0 = 1;
+      }
+   }
+
    mt->target = target;
    mt->format = format;
    mt->first_level = first_level;

Comments

On Thu, Jul 31, 2014 at 12:42:11AM -0700, Jordan Justen wrote:
> Fixes assertion failure in piglit (gen6, gen8):
> spec/glsl-1.30/execution/tex-miplevel-selection textureOffset 1DArrayShadow
> 
> In release builds of Mesa, this was observed to cause a GPU hang on
> gen8.
> 
> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ben Widawsky <ben@bwidawsk.net>
> Cc: Jason Ekstrand <jason@jlekstrand.net>
> Cc: "10.2" <mesa-stable@lists.freedesktop.org>

I was hoping I would have a chance to fix this one :-(

Tested-by: Ben Widawsky <benjamin.widawsky@intel.com>

> ---
>  On a gen6 piglit quick run:
>  15: crash => pass
>  1: crash => fail
> 
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 22c5ab8..f504e06 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -244,6 +244,26 @@ intel_miptree_create_layout(struct brw_context *brw,
>         _mesa_get_format_name(format),
>         first_level, last_level, depth0, mt);
>  
> +   if (target == GL_TEXTURE_1D_ARRAY) {
> +      /* For a 1D Array texture the OpenGL API will treat the height0
> +       * parameter as the number of array slices. For Intel hardware, we treat
> +       * the 1D array as a 2D Array with a height of 1.
> +       *
> +       * So, when we first come through this path to create a 1D Array
> +       * texture, height0 stores the number of slices, and depth0 is 1. In
> +       * this case, we want to swap height0 and depth0.
> +       *
> +       * Since some miptrees will be created based on the base miptree, we may
> +       * come through this path and see height0 as 1 and depth0 being the
> +       * number of slices. In this case we don't need to do the swap.
> +       */
> +      assert(height0 == 1 || depth0 == 1);
> +      if (height0 > 1) {
> +         depth0 = height0;
> +         height0 = 1;
> +      }
> +   }
> +
>     mt->target = target;
>     mt->format = format;
>     mt->first_level = first_level;
> -- 
> 2.0.1
>