[Mesa-dev] [PATCH] i965: Fix calculation of layers array length for isl_view

[Ilia Mirkin]

On Fri, Sep 7, 2018 at 11:09 AM, Danylo Piliaiev
<danylo.piliaiev at gmail.com> wrote:
>
> On 9/7/18 5:48 PM, Ilia Mirkin wrote:
>>
>> On Fri, Sep 7, 2018 at 10:41 AM, Danylo Piliaiev
>> <danylo.piliaiev at gmail.com> wrote:
>>>
>>> Comment for array_len field states:
>>>   "Indicates the number of array elements starting at
>>>     Base Array Layer."
>>>
>>> And most usages of array_len expect it to be equal or less than
>>>   total layers - base layer
>>>
>>> Fixes: 5a8c8903
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107856
>>>
>>> Signed-off-by: Danylo Piliaiev <danylo.piliaiev at globallogic.com>
>>> ---
>>>   src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 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 42af41aca3..6adf4a5836 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>>> @@ -1539,6 +1539,8 @@ 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 base_layer = obj->MinLayer + u->_Layer;
>>>            const unsigned num_layers = u->Layered ?
>>>               get_image_num_layers(mt, obj->Target, u->Level) : 1;
>>>
>>> @@ -1546,8 +1548,8 @@ update_image_surface(struct brw_context *brw,
>>>               .format = format,
>>>               .base_level = obj->MinLevel + u->Level,
>>>               .levels = 1,
>>> -            .base_array_layer = obj->MinLayer + u->_Layer,
>>> -            .array_len = num_layers,
>>> +            .base_array_layer = base_layer,
>>> +            .array_len = num_layers - base_layer,
>>
>> See above - num_layers can be 1 if the image isn't bound as a layered
>> image. But base layer can be whatever -- so this will end up as
>> negative. I think the adjustment needs to be done only for the
>> u->Layered case.
>
> Oh, I see it now, thanks! Unless Lionel's patch for this issue is better
> I'll send v2 of my patch.

I believe yours is much closer to right. Lionel's was conceptually
wrong. (Or I'm the one who's confused - reminder - I'm not an Intel
driver developer, and ultimately won't approve or reject your patches.
But I will point out things that I think are off.)

  -ilia