[Mesa-stable] [Mesa-dev] [PATCH] anv/genX: Solve the anv_CmdClearColorImage crash issue
Xu, Randy
randy.xu at intel.com
Mon Mar 27 14:31:46 UTC 2017
Thanks,
Randy
> -----Original Message-----
> From: Landwerlin, Lionel G
> Sent: Monday, March 27, 2017 10:06 PM
> To: Xu, Randy <randy.xu at intel.com>; mesa-dev at lists.freedesktop.org
> Cc: mesa-stable at lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH] anv/genX: Solve the
> anv_CmdClearColorImage crash issue
>
> On 27/03/17 12:51, Xu, Randy wrote:
> > 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.
>
> Thanks, the spec wasn't clear to me since it's not explicitly said that
> REMAINING is an invalid value (unless I missed something).
IMHO, the REMAINING (~0) is valid but has special meaning " it can set levelCount and layerCount to the special values VK_REMAINING_MIP_LEVELS and VK_REMAINING_ARRAY_LAYERS without knowing the exact number of mip levels or layers.", so we need use anv_get_levelCount() function to decode the VkImageSubresourceRange::layerCount variable.
>
> >
> > 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;
>
More information about the mesa-stable
mailing list