[v3] i965: Fix calculation of layers array length for isl_view

Submitted by Danylo Piliaiev on Sept. 10, 2018, 3:21 p.m.

Details

Message ID 20180910152106.16237-1-danylo.piliaiev@globallogic.com
State New
Headers show
Series "i965: Fix calculation of layers array length for isl_view" ( rev: 3 ) in Mesa

Not browsing as part of any series.

Commit Message

Danylo Piliaiev Sept. 10, 2018, 3:21 p.m.
Handle all cases in calculation of layers count for isl_view
taking into account texture view and image unit.
st_convert_image was taken as a reference.

When u->Layered is true the whole level is taken with respect to
image view. In other case only one layer is taken.

v3: (Józef Kucia and Ilia Mirkin)
    - Rewrote patch by taking st_convert_image as a reference
    - Removed now unused get_image_num_layers function
    - Changed commit message

Fixes: 5a8c8903
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856

Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
---
 .../drivers/dri/i965/brw_wm_surface_state.c   | 32 ++++++++++---------
 1 file changed, 17 insertions(+), 15 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index 944762ec46..9bfe6e2037 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1499,18 +1499,6 @@  update_buffer_image_param(struct brw_context *brw,
    param->stride[0] = _mesa_get_format_bytes(u->_ActualFormat);
 }
 
-static unsigned
-get_image_num_layers(const struct intel_mipmap_tree *mt, GLenum target,
-                     unsigned level)
-{
-   if (target == GL_TEXTURE_CUBE_MAP)
-      return 6;
-
-   return target == GL_TEXTURE_3D ?
-      minify(mt->surf.logical_level0_px.depth, level) :
-      mt->surf.logical_level0_px.array_len;
-}
-
 static void
 update_image_surface(struct brw_context *brw,
                      struct gl_image_unit *u,
@@ -1541,14 +1529,28 @@  update_image_surface(struct brw_context *brw,
       } else {
          struct intel_texture_object *intel_obj = intel_texture_object(obj);
          struct intel_mipmap_tree *mt = intel_obj->mt;
-         const unsigned num_layers = u->Layered ?
-            get_image_num_layers(mt, obj->Target, u->Level) : 1;
+
+         unsigned base_layer, num_layers;
+         if (u->Layered) {
+            if (obj->Target == GL_TEXTURE_3D) {
+               base_layer = 0;
+               num_layers = minify(mt->surf.logical_level0_px.depth, u->Level);
+            } else {
+               base_layer = obj->MinLayer;
+               num_layers = obj->Immutable ?
+                                obj->NumLayers :
+                                mt->surf.logical_level0_px.array_len;
+            }
+         } else {
+            base_layer = obj->MinLayer + u->_Layer;
+            num_layers = 1;
+         }
 
          struct isl_view view = {
             .format = format,
             .base_level = obj->MinLevel + u->Level,
             .levels = 1,
-            .base_array_layer = obj->MinLayer + u->_Layer,
+            .base_array_layer = base_layer,
             .array_len = num_layers,
             .swizzle = ISL_SWIZZLE_IDENTITY,
             .usage = ISL_SURF_USAGE_STORAGE_BIT,

Comments

I have made a Piglit test that exercises the issue:

https://patchwork.freedesktop.org/patch/258180/

- Danil

On 9/10/18 6:21 PM, Danylo Piliaiev wrote:
> Handle all cases in calculation of layers count for isl_view
> taking into account texture view and image unit.
> st_convert_image was taken as a reference.
>
> When u->Layered is true the whole level is taken with respect to
> image view. In other case only one layer is taken.
>
> v3: (Józef Kucia and Ilia Mirkin)
>      - Rewrote patch by taking st_convert_image as a reference
>      - Removed now unused get_image_num_layers function
>      - Changed commit message
>
> Fixes: 5a8c8903
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
>
> Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
> ---
>   .../drivers/dri/i965/brw_wm_surface_state.c   | 32 ++++++++++---------
>   1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 944762ec46..9bfe6e2037 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -1499,18 +1499,6 @@ update_buffer_image_param(struct brw_context *brw,
>      param->stride[0] = _mesa_get_format_bytes(u->_ActualFormat);
>   }
>   
> -static unsigned
> -get_image_num_layers(const struct intel_mipmap_tree *mt, GLenum target,
> -                     unsigned level)
> -{
> -   if (target == GL_TEXTURE_CUBE_MAP)
> -      return 6;
> -
> -   return target == GL_TEXTURE_3D ?
> -      minify(mt->surf.logical_level0_px.depth, level) :
> -      mt->surf.logical_level0_px.array_len;
> -}
> -
>   static void
>   update_image_surface(struct brw_context *brw,
>                        struct gl_image_unit *u,
> @@ -1541,14 +1529,28 @@ update_image_surface(struct brw_context *brw,
>         } else {
>            struct intel_texture_object *intel_obj = intel_texture_object(obj);
>            struct intel_mipmap_tree *mt = intel_obj->mt;
> -         const unsigned num_layers = u->Layered ?
> -            get_image_num_layers(mt, obj->Target, u->Level) : 1;
> +
> +         unsigned base_layer, num_layers;
> +         if (u->Layered) {
> +            if (obj->Target == GL_TEXTURE_3D) {
> +               base_layer = 0;
> +               num_layers = minify(mt->surf.logical_level0_px.depth, u->Level);
> +            } else {
> +               base_layer = obj->MinLayer;
> +               num_layers = obj->Immutable ?
> +                                obj->NumLayers :
> +                                mt->surf.logical_level0_px.array_len;
> +            }
> +         } else {
> +            base_layer = obj->MinLayer + u->_Layer;
> +            num_layers = 1;
> +         }
>   
>            struct isl_view view = {
>               .format = format,
>               .base_level = obj->MinLevel + u->Level,
>               .levels = 1,
> -            .base_array_layer = obj->MinLayer + u->_Layer,
> +            .base_array_layer = base_layer,
>               .array_len = num_layers,
>               .swizzle = ISL_SWIZZLE_IDENTITY,
>               .usage = ISL_SURF_USAGE_STORAGE_BIT,
Hello,

Could anyone look at the patch?
Thanks!

On 10/24/18 2:22 PM, Danylo Piliaiev wrote:
> I have made a Piglit test that exercises the issue:
>
> https://patchwork.freedesktop.org/patch/258180/
>
> - Danil
>
> On 9/10/18 6:21 PM, Danylo Piliaiev wrote:
>> Handle all cases in calculation of layers count for isl_view
>> taking into account texture view and image unit.
>> st_convert_image was taken as a reference.
>>
>> When u->Layered is true the whole level is taken with respect to
>> image view. In other case only one layer is taken.
>>
>> v3: (Józef Kucia and Ilia Mirkin)
>>      - Rewrote patch by taking st_convert_image as a reference
>>      - Removed now unused get_image_num_layers function
>>      - Changed commit message
>>
>> Fixes: 5a8c8903
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
>>
>> Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
>> ---
>>   .../drivers/dri/i965/brw_wm_surface_state.c   | 32 ++++++++++---------
>>   1 file changed, 17 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c 
>> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> index 944762ec46..9bfe6e2037 100644
>> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> @@ -1499,18 +1499,6 @@ update_buffer_image_param(struct brw_context 
>> *brw,
>>      param->stride[0] = _mesa_get_format_bytes(u->_ActualFormat);
>>   }
>>   -static unsigned
>> -get_image_num_layers(const struct intel_mipmap_tree *mt, GLenum target,
>> -                     unsigned level)
>> -{
>> -   if (target == GL_TEXTURE_CUBE_MAP)
>> -      return 6;
>> -
>> -   return target == GL_TEXTURE_3D ?
>> -      minify(mt->surf.logical_level0_px.depth, level) :
>> -      mt->surf.logical_level0_px.array_len;
>> -}
>> -
>>   static void
>>   update_image_surface(struct brw_context *brw,
>>                        struct gl_image_unit *u,
>> @@ -1541,14 +1529,28 @@ update_image_surface(struct brw_context *brw,
>>         } else {
>>            struct intel_texture_object *intel_obj = 
>> intel_texture_object(obj);
>>            struct intel_mipmap_tree *mt = intel_obj->mt;
>> -         const unsigned num_layers = u->Layered ?
>> -            get_image_num_layers(mt, obj->Target, u->Level) : 1;
>> +
>> +         unsigned base_layer, num_layers;
>> +         if (u->Layered) {
>> +            if (obj->Target == GL_TEXTURE_3D) {
>> +               base_layer = 0;
>> +               num_layers = minify(mt->surf.logical_level0_px.depth, 
>> u->Level);
>> +            } else {
>> +               base_layer = obj->MinLayer;
>> +               num_layers = obj->Immutable ?
>> +                                obj->NumLayers :
>> + mt->surf.logical_level0_px.array_len;
>> +            }
>> +         } else {
>> +            base_layer = obj->MinLayer + u->_Layer;
>> +            num_layers = 1;
>> +         }
>>              struct isl_view view = {
>>               .format = format,
>>               .base_level = obj->MinLevel + u->Level,
>>               .levels = 1,
>> -            .base_array_layer = obj->MinLayer + u->_Layer,
>> +            .base_array_layer = base_layer,
>>               .array_len = num_layers,
>>               .swizzle = ISL_SWIZZLE_IDENTITY,
>>               .usage = ISL_SURF_USAGE_STORAGE_BIT,
>
On Mon, Sep 10, 2018 at 10:21 AM Danylo Piliaiev <danylo.piliaiev@gmail.com>
wrote:

> Handle all cases in calculation of layers count for isl_view
> taking into account texture view and image unit.
> st_convert_image was taken as a reference.
>
> When u->Layered is true the whole level is taken with respect to
> image view. In other case only one layer is taken.
>
> v3: (Józef Kucia and Ilia Mirkin)
>     - Rewrote patch by taking st_convert_image as a reference
>     - Removed now unused get_image_num_layers function
>     - Changed commit message
>
> Fixes: 5a8c8903
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
>
> Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
> ---
>  .../drivers/dri/i965/brw_wm_surface_state.c   | 32 ++++++++++---------
>  1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 944762ec46..9bfe6e2037 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -1499,18 +1499,6 @@ update_buffer_image_param(struct brw_context *brw,
>     param->stride[0] = _mesa_get_format_bytes(u->_ActualFormat);
>  }
>
> -static unsigned
> -get_image_num_layers(const struct intel_mipmap_tree *mt, GLenum target,
> -                     unsigned level)
> -{
> -   if (target == GL_TEXTURE_CUBE_MAP)
> -      return 6;
> -
> -   return target == GL_TEXTURE_3D ?
> -      minify(mt->surf.logical_level0_px.depth, level) :
> -      mt->surf.logical_level0_px.array_len;
> -}
> -
>  static void
>  update_image_surface(struct brw_context *brw,
>                       struct gl_image_unit *u,
> @@ -1541,14 +1529,28 @@ update_image_surface(struct brw_context *brw,
>        } else {
>           struct intel_texture_object *intel_obj =
> intel_texture_object(obj);
>           struct intel_mipmap_tree *mt = intel_obj->mt;
> -         const unsigned num_layers = u->Layered ?
> -            get_image_num_layers(mt, obj->Target, u->Level) : 1;
> +
> +         unsigned base_layer, num_layers;
> +         if (u->Layered) {
> +            if (obj->Target == GL_TEXTURE_3D) {
> +               base_layer = 0;
> +               num_layers = minify(mt->surf.logical_level0_px.depth,
> u->Level);
> +            } else {
> +               base_layer = obj->MinLayer;
> +               num_layers = obj->Immutable ?
> +                                obj->NumLayers :
> +                                mt->surf.logical_level0_px.array_len;
>

Doesn't this need to be array_len - base_layer?  I'm not sure on the others
without digging.


> +            }
> +         } else {
> +            base_layer = obj->MinLayer + u->_Layer;
> +            num_layers = 1;
> +         }
>
>           struct isl_view view = {
>              .format = format,
>              .base_level = obj->MinLevel + u->Level,
>              .levels = 1,
> -            .base_array_layer = obj->MinLayer + u->_Layer,
> +            .base_array_layer = base_layer,
>              .array_len = num_layers,
>              .swizzle = ISL_SWIZZLE_IDENTITY,
>              .usage = ISL_SURF_USAGE_STORAGE_BIT,
> --
> 2.18.0
>
>
On Tue, Nov 13, 2018 at 4:53 PM Jason Ekstrand <jason@jlekstrand.net> wrote:
>
> On Mon, Sep 10, 2018 at 10:21 AM Danylo Piliaiev <danylo.piliaiev@gmail.com> wrote:
>>
>> Handle all cases in calculation of layers count for isl_view
>> taking into account texture view and image unit.
>> st_convert_image was taken as a reference.
>>
>> When u->Layered is true the whole level is taken with respect to
>> image view. In other case only one layer is taken.
>>
>> v3: (Józef Kucia and Ilia Mirkin)
>>     - Rewrote patch by taking st_convert_image as a reference
>>     - Removed now unused get_image_num_layers function
>>     - Changed commit message
>>
>> Fixes: 5a8c8903
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
>>
>> Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
>> ---
>>  .../drivers/dri/i965/brw_wm_surface_state.c   | 32 ++++++++++---------
>>  1 file changed, 17 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> index 944762ec46..9bfe6e2037 100644
>> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> @@ -1499,18 +1499,6 @@ update_buffer_image_param(struct brw_context *brw,
>>     param->stride[0] = _mesa_get_format_bytes(u->_ActualFormat);
>>  }
>>
>> -static unsigned
>> -get_image_num_layers(const struct intel_mipmap_tree *mt, GLenum target,
>> -                     unsigned level)
>> -{
>> -   if (target == GL_TEXTURE_CUBE_MAP)
>> -      return 6;
>> -
>> -   return target == GL_TEXTURE_3D ?
>> -      minify(mt->surf.logical_level0_px.depth, level) :
>> -      mt->surf.logical_level0_px.array_len;
>> -}
>> -
>>  static void
>>  update_image_surface(struct brw_context *brw,
>>                       struct gl_image_unit *u,
>> @@ -1541,14 +1529,28 @@ update_image_surface(struct brw_context *brw,
>>        } else {
>>           struct intel_texture_object *intel_obj = intel_texture_object(obj);
>>           struct intel_mipmap_tree *mt = intel_obj->mt;
>> -         const unsigned num_layers = u->Layered ?
>> -            get_image_num_layers(mt, obj->Target, u->Level) : 1;
>> +
>> +         unsigned base_layer, num_layers;
>> +         if (u->Layered) {
>> +            if (obj->Target == GL_TEXTURE_3D) {
>> +               base_layer = 0;
>> +               num_layers = minify(mt->surf.logical_level0_px.depth, u->Level);
>> +            } else {
>> +               base_layer = obj->MinLayer;
>> +               num_layers = obj->Immutable ?
>> +                                obj->NumLayers :
>> +                                mt->surf.logical_level0_px.array_len;
>
>
> Doesn't this need to be array_len - base_layer?  I'm not sure on the others without digging.

Probably not intuitively obvious, but MinLayer/NumLayers are only set
for Immutable textures.

  -ilia
Hi Ilia, Jason,

All these cases are indeed could induce confusion,
I'll try to explain all of them:

Texture is not layered:
     Layers: 1
     Base Layer: %layer of image unit% + %layer of texture view%

Layered 3D texture:
     Layers: Entire level is bound, texture view does not affect layers 
of it
     Base Layer: 0

Layered non 3D texture (Array):
     Immutable (it's a texture view, see _mesa_set_texture_view_state):
         Layers: correct values of MinLayer and NumLayers is baked into 
texObject in texture_view and in _mesa_set_texture_view_state
         Base Layer: MinLayer

     Mutable (MinLayer is always 0 since it can be set only via texture 
view which makes texture immutable):
         Layers: array_len
         Base Layer: 0

So while the code from my pov should work it could be presented better.
However I'm not sure what will be better:

     } else if (obj->Immutable) {
         base_layer = obj->MinLayer;
         num_layers = obj->NumLayers;
     } else {
         base_layer = 0;
         num_layers = mt->surf.logical_level0_px.array_len;
     }

Or always calculate base_layer as "obj->MinLayer + u->_Layer" before the 
conditions,
     which will look slightly misleading and induce questions of why we 
don't subtract them from array_len.

What do you think?

Thanks!

On 11/14/18 12:01 AM, Ilia Mirkin wrote:
> On Tue, Nov 13, 2018 at 4:53 PM Jason Ekstrand <jason@jlekstrand.net> wrote:
>> On Mon, Sep 10, 2018 at 10:21 AM Danylo Piliaiev <danylo.piliaiev@gmail.com> wrote:
>>> Handle all cases in calculation of layers count for isl_view
>>> taking into account texture view and image unit.
>>> st_convert_image was taken as a reference.
>>>
>>> When u->Layered is true the whole level is taken with respect to
>>> image view. In other case only one layer is taken.
>>>
>>> v3: (Józef Kucia and Ilia Mirkin)
>>>      - Rewrote patch by taking st_convert_image as a reference
>>>      - Removed now unused get_image_num_layers function
>>>      - Changed commit message
>>>
>>> Fixes: 5a8c8903
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
>>>
>>> Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
>>> ---
>>>   .../drivers/dri/i965/brw_wm_surface_state.c   | 32 ++++++++++---------
>>>   1 file changed, 17 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>>> index 944762ec46..9bfe6e2037 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>>> @@ -1499,18 +1499,6 @@ update_buffer_image_param(struct brw_context *brw,
>>>      param->stride[0] = _mesa_get_format_bytes(u->_ActualFormat);
>>>   }
>>>
>>> -static unsigned
>>> -get_image_num_layers(const struct intel_mipmap_tree *mt, GLenum target,
>>> -                     unsigned level)
>>> -{
>>> -   if (target == GL_TEXTURE_CUBE_MAP)
>>> -      return 6;
>>> -
>>> -   return target == GL_TEXTURE_3D ?
>>> -      minify(mt->surf.logical_level0_px.depth, level) :
>>> -      mt->surf.logical_level0_px.array_len;
>>> -}
>>> -
>>>   static void
>>>   update_image_surface(struct brw_context *brw,
>>>                        struct gl_image_unit *u,
>>> @@ -1541,14 +1529,28 @@ update_image_surface(struct brw_context *brw,
>>>         } else {
>>>            struct intel_texture_object *intel_obj = intel_texture_object(obj);
>>>            struct intel_mipmap_tree *mt = intel_obj->mt;
>>> -         const unsigned num_layers = u->Layered ?
>>> -            get_image_num_layers(mt, obj->Target, u->Level) : 1;
>>> +
>>> +         unsigned base_layer, num_layers;
>>> +         if (u->Layered) {
>>> +            if (obj->Target == GL_TEXTURE_3D) {
>>> +               base_layer = 0;
>>> +               num_layers = minify(mt->surf.logical_level0_px.depth, u->Level);
>>> +            } else {
>>> +               base_layer = obj->MinLayer;
>>> +               num_layers = obj->Immutable ?
>>> +                                obj->NumLayers :
>>> +                                mt->surf.logical_level0_px.array_len;
>>
>> Doesn't this need to be array_len - base_layer?  I'm not sure on the others without digging.
> Probably not intuitively obvious, but MinLayer/NumLayers are only set
> for Immutable textures.
>
>    -ilia
On Wed, Nov 14, 2018 at 6:02 AM Danylo Piliaiev <danylo.piliaiev@gmail.com>
wrote:

> Hi Ilia, Jason,
>
> All these cases are indeed could induce confusion,
> I'll try to explain all of them:
>
> Texture is not layered:
>      Layers: 1
>      Base Layer: %layer of image unit% + %layer of texture view%
>
> Layered 3D texture:
>      Layers: Entire level is bound, texture view does not affect layers
> of it
>      Base Layer: 0
>
> Layered non 3D texture (Array):
>      Immutable (it's a texture view, see _mesa_set_texture_view_state):
>          Layers: correct values of MinLayer and NumLayers is baked into
> texObject in texture_view and in _mesa_set_texture_view_state
>          Base Layer: MinLayer
>
>      Mutable (MinLayer is always 0 since it can be set only via texture
> view which makes texture immutable):
>          Layers: array_len
>          Base Layer: 0
>

Right.  Would you mind adding

assert(obj->Immutable || obj->MinLayer == 0);

With that,

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


> So while the code from my pov should work it could be presented better.
> However I'm not sure what will be better:
>
>      } else if (obj->Immutable) {
>          base_layer = obj->MinLayer;
>          num_layers = obj->NumLayers;
>      } else {
>          base_layer = 0;
>          num_layers = mt->surf.logical_level0_px.array_len;
>      }
>
> Or always calculate base_layer as "obj->MinLayer + u->_Layer" before the
> conditions,
>      which will look slightly misleading and induce questions of why we
> don't subtract them from array_len.
>
> What do you think?
>
> Thanks!
>
> On 11/14/18 12:01 AM, Ilia Mirkin wrote:
> > On Tue, Nov 13, 2018 at 4:53 PM Jason Ekstrand <jason@jlekstrand.net>
> wrote:
> >> On Mon, Sep 10, 2018 at 10:21 AM Danylo Piliaiev <
> danylo.piliaiev@gmail.com> wrote:
> >>> Handle all cases in calculation of layers count for isl_view
> >>> taking into account texture view and image unit.
> >>> st_convert_image was taken as a reference.
> >>>
> >>> When u->Layered is true the whole level is taken with respect to
> >>> image view. In other case only one layer is taken.
> >>>
> >>> v3: (Józef Kucia and Ilia Mirkin)
> >>>      - Rewrote patch by taking st_convert_image as a reference
> >>>      - Removed now unused get_image_num_layers function
> >>>      - Changed commit message
> >>>
> >>> Fixes: 5a8c8903
> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
> >>>
> >>> Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
> >>> ---
> >>>   .../drivers/dri/i965/brw_wm_surface_state.c   | 32
> ++++++++++---------
> >>>   1 file changed, 17 insertions(+), 15 deletions(-)
> >>>
> >>> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> >>> index 944762ec46..9bfe6e2037 100644
> >>> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> >>> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> >>> @@ -1499,18 +1499,6 @@ update_buffer_image_param(struct brw_context
> *brw,
> >>>      param->stride[0] = _mesa_get_format_bytes(u->_ActualFormat);
> >>>   }
> >>>
> >>> -static unsigned
> >>> -get_image_num_layers(const struct intel_mipmap_tree *mt, GLenum
> target,
> >>> -                     unsigned level)
> >>> -{
> >>> -   if (target == GL_TEXTURE_CUBE_MAP)
> >>> -      return 6;
> >>> -
> >>> -   return target == GL_TEXTURE_3D ?
> >>> -      minify(mt->surf.logical_level0_px.depth, level) :
> >>> -      mt->surf.logical_level0_px.array_len;
> >>> -}
> >>> -
> >>>   static void
> >>>   update_image_surface(struct brw_context *brw,
> >>>                        struct gl_image_unit *u,
> >>> @@ -1541,14 +1529,28 @@ update_image_surface(struct brw_context *brw,
> >>>         } else {
> >>>            struct intel_texture_object *intel_obj =
> intel_texture_object(obj);
> >>>            struct intel_mipmap_tree *mt = intel_obj->mt;
> >>> -         const unsigned num_layers = u->Layered ?
> >>> -            get_image_num_layers(mt, obj->Target, u->Level) : 1;
> >>> +
> >>> +         unsigned base_layer, num_layers;
> >>> +         if (u->Layered) {
> >>> +            if (obj->Target == GL_TEXTURE_3D) {
> >>> +               base_layer = 0;
> >>> +               num_layers = minify(mt->surf.logical_level0_px.depth,
> u->Level);
> >>> +            } else {
> >>> +               base_layer = obj->MinLayer;
> >>> +               num_layers = obj->Immutable ?
> >>> +                                obj->NumLayers :
> >>> +                                mt->surf.logical_level0_px.array_len;
> >>
> >> Doesn't this need to be array_len - base_layer?  I'm not sure on the
> others without digging.
> > Probably not intuitively obvious, but MinLayer/NumLayers are only set
> > for Immutable textures.
> >
> >    -ilia
>
>