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

[Lionel Landwerlin]

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...@intel.com>; mesa-dev@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.landwer...@intel.com>
Cc: "17.0 13.0" <mesa-sta...@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).


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...@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...@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;


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev