[Mesa-dev] [PATCH 3/8] anv/android: add GetAndroidHardwareBufferPropertiesANDROID
Tapani Pälli
tapani.palli at intel.com
Mon Aug 27 04:54:49 UTC 2018
On 08/24/2018 05:04 PM, Jason Ekstrand wrote:
> On Fri, Aug 24, 2018 at 12:08 AM Tapani Pälli <tapani.palli at intel.com
> <mailto: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>
> > <mailto: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>
> > <mailto: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.
But per the Vulkan spec quote above it's not a bug, it says that
"formatFeatures member must include sampled bit and at least one of
MIDPOINT_CHROMA_SAMPLES or COSITED_CHROMA_SAMPLES. So .. I guess I
should limit the supported formats to only those that have
anv_format->can_ycbcr() set?
> --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>
> <mailto:mesa-dev at lists.freedesktop.org
> <mailto:mesa-dev at lists.freedesktop.org>>
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
More information about the mesa-dev
mailing list