<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Fri, Aug 24, 2018 at 12:08 AM Tapani Pälli <<a href="mailto:tapani.palli@intel.com">tapani.palli@intel.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On 08/22/2018 05:18 PM, Jason Ekstrand wrote: > On Tue, Aug 21, 2018 at 3:27 AM Tapani Pälli <<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a> > <mailto:<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>>> wrote: > > When adding YUV support, we need to figure out implementation-defined > external format identifier. > > Signed-off-by: Tapani Pälli <<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a> > <mailto:<a href="mailto:tapani.palli@intel.com" target="_blank">tapani.palli@intel.com</a>>> > --- > 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. </blockquote><div> </div><div>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.</div><div> </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > + > + 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 > <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <mailto:<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a>> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > </blockquote></div></div>