[Mesa-dev] [PATCH] anv/android: handle storage images in vkGetSwapchainGrallocUsageANDROID

[Tapani Pälli]

On 12/5/18 1:01 PM, Bas Nieuwenhuizen wrote:
> On Fri, Sep 7, 2018 at 12:54 AM Kevin Strasser <kevin.strasser at intel.com> wrote:
>>
>> Android P and earlier expect that the surface supports storage images, and
>> so many of the tests fail when the framework checks for that support. The
>> framework also includes various image format and usage combinations that are
>> invalid for the hardware.
>>
>> Drop the STORAGE restriction from the HAL and whitelist a pair of
>> formats so that existing versions of Android can pass these tests.
>>
>> Fixes:
>>     dEQP-VK.wsi.android.*
>>
>> Signed-off-by: Kevin Strasser <kevin.strasser at intel.com>
>> ---
>>   src/intel/vulkan/anv_android.c | 23 ++++++++++++++---------
>>   1 file changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/intel/vulkan/anv_android.c b/src/intel/vulkan/anv_android.c
>> index 46c41d5..e2640b8 100644
>> --- a/src/intel/vulkan/anv_android.c
>> +++ b/src/intel/vulkan/anv_android.c
>> @@ -234,7 +234,7 @@ VkResult anv_GetSwapchainGrallocUsageANDROID(
>>      *grallocUsage = 0;
>>      intel_logd("%s: format=%d, usage=0x%x", __func__, format, imageUsage);
>>
>> -   /* WARNING: Android Nougat's libvulkan.so hardcodes the VkImageUsageFlags
>> +   /* WARNING: Android's libvulkan.so hardcodes the VkImageUsageFlags
>>       * returned to applications via VkSurfaceCapabilitiesKHR::supportedUsageFlags.
>>       * The relevant code in libvulkan/swapchain.cpp contains this fun comment:
>>       *
>> @@ -247,7 +247,7 @@ VkResult anv_GetSwapchainGrallocUsageANDROID(
>>       * dEQP-VK.wsi.android.swapchain.*.image_usage to fail.
>>       */
>>
>> -   const VkPhysicalDeviceImageFormatInfo2KHR image_format_info = {
>> +   VkPhysicalDeviceImageFormatInfo2KHR image_format_info = {
> 
> Why remove the const here?
> 
>>         .sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_IMAGE_FORMAT_INFO_2_KHR,
>>         .format = format,
>>         .type = VK_IMAGE_TYPE_2D,
>> @@ -255,6 +255,17 @@ VkResult anv_GetSwapchainGrallocUsageANDROID(
>>         .usage = imageUsage,
>>      };
>>
>> +   /* Android P and earlier doesn't check if the physical device supports a
>> +    * given format and usage combination before calling this function. Omit the
>> +    * storage requirement to make the tests pass.
>> +    */
>> +#if ANDROID_API_LEVEL <= 28
>> +   if (format == VK_FORMAT_R8G8B8A8_SRGB ||
>> +       format == VK_FORMAT_R5G6B5_UNORM_PACK16) {
>> +      image_format_info.usage &= ~VK_IMAGE_USAGE_STORAGE_BIT;
>> +   }
>> +#endif
> 
> I don't think you need this. Per the vulkan spec you can only use an
> format + usage combination for a swapchain if it is supported per
> ImageFormatProperties, using essentially the same check happening
> above. I know CTs has been bad at this, but Vulkan CTS should have
> been fixed for a bit now. (I don't think all the fixes are in Android
> CTS 9.0_r4 yet, maybe the next release?)

AFAIK the problem here is not about CTS. It's the swapchain 
implementation that always requires storage support.

> (Also silently removing the usage bit is bad, because the app could
> try actually using images stores with the image ...)

True, it is not nice ..


>> +
>>      VkImageFormatProperties2KHR image_format_props = {
>>         .sType = VK_STRUCTURE_TYPE_IMAGE_FORMAT_PROPERTIES_2_KHR,
>>      };
>> @@ -268,19 +279,13 @@ VkResult anv_GetSwapchainGrallocUsageANDROID(
>>                          "inside %s", __func__);
>>      }
>>
>> -   /* Reject STORAGE here to avoid complexity elsewhere. */
>> -   if (imageUsage & VK_IMAGE_USAGE_STORAGE_BIT) {
>> -      return vk_errorf(device->instance, device, VK_ERROR_FORMAT_NOT_SUPPORTED,
>> -                       "VK_IMAGE_USAGE_STORAGE_BIT unsupported for gralloc "
>> -                       "swapchain");
>> -   }
>> -
>>      if (unmask32(&imageUsage, VK_IMAGE_USAGE_TRANSFER_DST_BIT |
>>                                VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT))
>>         *grallocUsage |= GRALLOC_USAGE_HW_RENDER;
>>
>>      if (unmask32(&imageUsage, VK_IMAGE_USAGE_TRANSFER_SRC_BIT |
>>                                VK_IMAGE_USAGE_SAMPLED_BIT |
>> +                             VK_IMAGE_USAGE_STORAGE_BIT |
>>                                VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT))
>>         *grallocUsage |= GRALLOC_USAGE_HW_TEXTURE;
>>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>