[Mesa-dev] [PATCH] anv/android: handle storage images in vkGetSwapchainGrallocUsageANDROID
Tapani Pälli
tapani.palli at intel.com
Wed Dec 5 11:51:27 UTC 2018
On 12/5/18 1:44 PM, Bas Nieuwenhuizen wrote:
> On Wed, Dec 5, 2018 at 12:37 PM Tapani Pälli <tapani.palli at intel.com> wrote:
>>
>>
>>
>> On 12/5/18 1:22 PM, Bas Nieuwenhuizen wrote:
>>> On Wed, Dec 5, 2018 at 12:15 PM Tapani Pälli <tapani.palli at intel.com> wrote:
>>>>
>>>>
>>>>
>>>> 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.
>>>
>>> Actually swapchain creation has the following valid usage rule:
>>>
>>> "The implied image creation parameters of the swapchain must be
>>> supported as reported by vkGetPhysicalDeviceImageFormatProperties"
>>>
>>> So since those formats don't support the STORAGE usage bit, that test
>>> fails and you are not allowed to create a swapchain with those formats
>>> and storage, even if the surface capabiliities expose the STORAGE
>>> usage bit in general.
>>
>> Right ... this stuff was done because comment in the swapchain setting
>> the bits seems like maybe it's not thought through:
>>
>> // TODO(jessehall): I think these are right, but haven't thought hard about
>> // it. Do we need to query the driver for support of any of these?
>
> That was from before the spec was changed to add that rule.
OK if I understand correctly, so should we rather then try to fix those
tests to skip instead of fail?
>>
>>>>
>>>>> (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
>>>>>
More information about the mesa-dev
mailing list