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

Jason Ekstrand jason at jlekstrand.net
Fri Aug 24 14:04:27 UTC 2018


On Fri, Aug 24, 2018 at 12:08 AM Tapani Pälli <tapani.palli at intel.com>
wrote:

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

That's annoying...  Is that in a Khronos CTS test or a Google test?  Either
way, we should try to get it corrected.  If you don't know how to do that,
I can send some e-mails.

--Jason


> >     +
> >     +   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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180824/7b44f134/attachment.html>


More information about the mesa-dev mailing list