[Mesa-dev] [PATCH 3/8] anv/android: add GetAndroidHardwareBufferPropertiesANDROID

[Tapani Pälli]

On 08/22/2018 05:18 PM, Jason Ekstrand wrote:
> On Tue, Aug 21, 2018 at 3:27 AM Tapani Pälli <tapani.palli at intel.com 
> <mailto:tapani.palli at intel.com>> wrote:
> 
>     When adding YUV support, we need to figure out implementation-defined
>     external format identifier.
> 
>     Signed-off-by: Tapani Pälli <tapani.palli at intel.com
>     <mailto:tapani.palli at intel.com>>
>     ---
>       src/intel/vulkan/anv_android.c | 99
>     ++++++++++++++++++++++++++++++++++++++++++
>       1 file changed, 99 insertions(+)
> 
>     diff --git a/src/intel/vulkan/anv_android.c
>     b/src/intel/vulkan/anv_android.c
>     index 46c41d57861..7d0eb588e2b 100644
>     --- a/src/intel/vulkan/anv_android.c
>     +++ b/src/intel/vulkan/anv_android.c
>     @@ -29,6 +29,8 @@
>       #include <sync/sync.h>
> 
>       #include "anv_private.h"
>     +#include "vk_format_info.h"
>     +#include "vk_util.h"
> 
>       static int anv_hal_open(const struct hw_module_t* mod, const char*
>     id, struct hw_device_t** dev);
>       static int anv_hal_close(struct hw_device_t *dev);
>     @@ -96,6 +98,103 @@ anv_hal_close(struct hw_device_t *dev)
>          return -1;
>       }
> 
>     +static VkResult
>     +get_ahw_buffer_format_properties(
>     +   VkDevice device_h,
>     +   const struct AHardwareBuffer *buffer,
>     +   VkAndroidHardwareBufferFormatPropertiesANDROID *pProperties)
>     +{
>     +   ANV_FROM_HANDLE(anv_device, device, device_h);
>     +
>     +   /* Get a description of buffer contents . */
>     +   AHardwareBuffer_Desc desc;
>     +   AHardwareBuffer_describe(buffer, &desc);
>     +
>     +   /* Verify description. */
>     +   uint64_t gpu_usage =
>     +      AHARDWAREBUFFER_USAGE_GPU_SAMPLED_IMAGE |
>     +      AHARDWAREBUFFER_USAGE_GPU_COLOR_OUTPUT |
>     +      AHARDWAREBUFFER_USAGE_GPU_DATA_BUFFER;
>     +
>     +   /* "Buffer must be a valid Android hardware buffer object with
>     at least
>     +    * one of the AHARDWAREBUFFER_USAGE_GPU_* usage flags."
>     +    */
>     +   if (!(desc.usage & (gpu_usage)))
>     +      return VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR;
>     +
>     +   /* Fill properties fields based on description. */
>     +   VkAndroidHardwareBufferFormatPropertiesANDROID *p = pProperties;
>     +
>     +   p->pNext = NULL;
> 
> 
> You shouldn't be overwriting pNext.  That's used by the client to let 
> them chain in multiple structs to fill out in case Google ever extends 
> this extension.  Also, while we're here, it'd be good to throw in an 
> assert that p->sType is the right thing.

Yes of course, will remove.

>     +   p->format = vk_format_from_android(desc.format);
>     +   p->externalFormat = 1; /* XXX */
>     +
>     +   const struct anv_format *anv_format = anv_get_format(p->format);
>     +   struct anv_physical_device *physical_device =
>     +      &device->instance->physicalDevice;
>     +   const struct gen_device_info *devinfo = &physical_device->info;
> 
> 
> If all you need is devinfo, that's avilable in the device; you don't 
> need to get the physical device for it.  Should be device->info.

OK

>     +
>     +   p->formatFeatures =
>     +      anv_get_image_format_features(devinfo, p->format,
>     +                                    anv_format,
>     VK_IMAGE_TILING_OPTIMAL);
>     +
>     +   /* "The formatFeatures member *must* include
>     +    *  VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT and at least one of
>     +    *  VK_FORMAT_FEATURE_MIDPOINT_CHROMA_SAMPLES_BIT or
>     +    *  VK_FORMAT_FEATURE_COSITED_CHROMA_SAMPLES_BIT"
>     +    */
>     +   p->formatFeatures |=
>     +      VK_FORMAT_FEATURE_MIDPOINT_CHROMA_SAMPLES_BIT;
> 
> 
> Uh... Why not just throw in SAMPLED_BIT?  For that matter, all of the 
> formats you have in your conversion helpers support sampling.  Maybe 
> just replace that with an assert for now.

Yeah, VK_FORMAT_FEATURE_SAMPLED_IMAGE_BIT is there. Well thing is that 
dEQP checks explicitly that either 
VK_FORMAT_FEATURE_MIDPOINT_CHROMA_SAMPLES_BIT or 
VK_FORMAT_FEATURE_COSITED_CHROMA_SAMPLES_BIT exists (independent of 
surface format). TBH I'm not sure what to do about that.


>     +
>     +   p->samplerYcbcrConversionComponents.r =
>     VK_COMPONENT_SWIZZLE_IDENTITY;
>     +   p->samplerYcbcrConversionComponents.g =
>     VK_COMPONENT_SWIZZLE_IDENTITY;
>     +   p->samplerYcbcrConversionComponents.b =
>     VK_COMPONENT_SWIZZLE_IDENTITY;
>     +   p->samplerYcbcrConversionComponents.a =
>     VK_COMPONENT_SWIZZLE_IDENTITY;
>     +
>     +   p->suggestedYcbcrModel =
>     VK_SAMPLER_YCBCR_MODEL_CONVERSION_RGB_IDENTITY;
>     +   p->suggestedYcbcrRange = VK_SAMPLER_YCBCR_RANGE_ITU_FULL;
>     +   p->suggestedXChromaOffset = VK_CHROMA_LOCATION_COSITED_EVEN;
>     +   p->suggestedYChromaOffset = VK_CHROMA_LOCATION_COSITED_EVEN;
>     +
>     +   return VK_SUCCESS;
>     +}
>     +
>     +VkResult
>     +anv_GetAndroidHardwareBufferPropertiesANDROID(
>     +   VkDevice device_h,
>     +   const struct AHardwareBuffer *buffer,
>     +   VkAndroidHardwareBufferPropertiesANDROID *pProperties)
>     +{
>     +   ANV_FROM_HANDLE(anv_device, dev, device_h);
>     +   struct anv_physical_device *pdevice =
>     &dev->instance->physicalDevice;
>     +
>     +   VkAndroidHardwareBufferFormatPropertiesANDROID *format_prop =
>     +      vk_find_struct(pProperties->pNext,
>     +                   
>       ANDROID_HARDWARE_BUFFER_FORMAT_PROPERTIES_ANDROID);
>     +
>     +   /* Fill format properties of an Android hardware buffer. */
>     +   if (format_prop)
>     +      get_ahw_buffer_format_properties(device_h, buffer, format_prop);
>     +
>     +   /* Get a description of buffer contents . */
>     +   AHardwareBuffer_Desc desc;
>     +   AHardwareBuffer_describe(buffer, &desc);
> 
> 
> I don't see you using this anywyere.  get_ahw_bufer_format_properties 
> calls it itself.  An artefact of rebsing perhaps?  Should probably drop 
> it if it's not needed.

Yes this was only used for debugging, will remove.

>     +
>     +   const native_handle_t *handle =
>     +      AHardwareBuffer_getNativeHandle(buffer);
>     +   int dma_buf = (handle && handle->numFds) ? handle->data[0] : -1;
>     +   if (dma_buf < 0)
>     +      return VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR;
>     +
>     +   /* All memory types. */
>     +   uint32_t memory_types = (1ull << pdevice->memory.type_count) - 1;
>     +
>     +   pProperties->allocationSize = lseek(dma_buf, 0, SEEK_END);
>     +   pProperties->memoryTypeBits = memory_types;
>     +
>     +   return VK_SUCCESS;
>     +}
>     +
>       VkResult
>       anv_image_from_gralloc(VkDevice device_h,
>                              const VkImageCreateInfo *base_info,
>     -- 
>     2.14.4
> 
>     _______________________________________________
>     mesa-dev mailing list
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>