[Mesa-dev] [PATCH] anv/genX: Solve the anv_CmdClearColorImage crash issue

[Xu, Randy]

Thanks, Lionel

See my comments below


Thanks,
Randy

> -----Original Message-----
> From: Landwerlin, Lionel G
> Sent: Monday, March 27, 2017 5:14 PM
> To: Xu, Randy <randy.xu at intel.com>; mesa-dev at lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH] anv/genX: Solve the
> anv_CmdClearColorImage crash issue
> 
> Hi Randy,
> 
> This patch looks good to me.
> 
> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> Cc: "17.0 13.0" <mesa-stable at lists.freedesktop.org>
> 
> Are you planning to send a follow up patch to fix the remaining layoutCount
> accesses too?

I guess you means the layerCount access
I checked the code, there are two kinds of layerCount access, as member of VkImageSubresourceLayers or VkImageSubresourceRange.
Only the layerCount in VkImageSubresourceRange can be VK_REMAINING_ARRAY_LAYERS, and should be accessed through function anv_get_levelCount, two places have been covered in this patch.

typedef struct VkImageSubresourceLayers {
    VkImageAspectFlags    aspectMask;
    uint32_t              mipLevel;
    uint32_t              baseArrayLayer;
    uint32_t              layerCount;			
} VkImageSubresourceLayers;

typedef struct VkImageSubresourceRange {
    VkImageAspectFlags    aspectMask;
    uint32_t              baseMipLevel;
    uint32_t              levelCount;
    uint32_t              baseArrayLayer;
    uint32_t              layerCount;
} VkImageSubresourceRange;


> 
> Cheers,
> 
> -
> Lionel
> 
> On 20/03/17 07:31, Randy Xu wrote:
> > From: Xu Randy <randy.xu at intel.com>
> >
> > We should use anv_get_layerCount() to access layerCount of VkImageSub-
> > resourceRange in anv_CmdClearColorImage and
> anv_CmdClearDepthStencil-
> > Image, which handles the VK_REMAINING_ARRAY_LAYERS (~0) case.
> >
> > Test: Sample multithreadcmdbuf from LunarG can run without crash
> >
> > Signed-off-by: Xu Randy <randy.xu at intel.com>
> > ---
> >   src/intel/vulkan/anv_blorp.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_blorp.c
> > b/src/intel/vulkan/anv_blorp.c index 1f4fec5..9b3910f 100644
> > --- a/src/intel/vulkan/anv_blorp.c
> > +++ b/src/intel/vulkan/anv_blorp.c
> > @@ -830,7 +830,7 @@ void anv_CmdClearColorImage(
> >                           VK_IMAGE_ASPECT_COLOR_BIT, image->tiling);
> >
> >         unsigned base_layer = pRanges[r].baseArrayLayer;
> > -      unsigned layer_count = pRanges[r].layerCount;
> > +      unsigned layer_count = anv_get_layerCount(image, &pRanges[r]);
> >
> >         for (unsigned i = 0; i < anv_get_levelCount(image, &pRanges[r]); i++) {
> >            const unsigned level = pRanges[r].baseMipLevel + i; @@
> > -890,7 +890,7 @@ void anv_CmdClearDepthStencilImage(
> >         bool clear_stencil = pRanges[r].aspectMask &
> > VK_IMAGE_ASPECT_STENCIL_BIT;
> >
> >         unsigned base_layer = pRanges[r].baseArrayLayer;
> > -      unsigned layer_count = pRanges[r].layerCount;
> > +      unsigned layer_count = anv_get_layerCount(image, &pRanges[r]);
> >
> >         for (unsigned i = 0; i < anv_get_levelCount(image, &pRanges[r]); i++) {
> >            const unsigned level = pRanges[r].baseMipLevel + i;
>