[Mesa-dev] [PATCH] egl/android: Partially handle HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED

[Emil Velikov]

On 28 November 2017 at 10:45, Tapani Pälli <tapani.palli at intel.com> wrote:
> Hi;
>
>
> On 11/27/2017 04:14 PM, Robert Foss wrote:
>>
>> From: Tomasz Figa <tfiga at chromium.org>
>>
>> There is no API available to properly query the IMPLEMENTATION_DEFINED
>> format. As a workaround we rely here on gralloc allocating either
>> an arbitrary YCbCr 4:2:0 or RGBX_8888, with the latter being recognized
>> by lock_ycbcr failing.
>>
>> Reviewed-on: https://chromium-review.googlesource.com/566793
>>
>> Signed-off-by: Tomasz Figa <tfiga at chromium.org>
>> Reviewed-by: Chad Versace <chadversary at chromium.org>
>> Signed-off-by: Robert Foss <robert.foss at collabora.com>
>> ---
>>   src/egl/drivers/dri2/platform_android.c | 39
>> +++++++++++++++++++++++++++++++--
>>   1 file changed, 37 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/egl/drivers/dri2/platform_android.c
>> b/src/egl/drivers/dri2/platform_android.c
>> index 63223e9a69..ae914d79c1 100644
>> --- a/src/egl/drivers/dri2/platform_android.c
>> +++ b/src/egl/drivers/dri2/platform_android.c
>> @@ -59,6 +59,10 @@ static const struct droid_yuv_format
>> droid_yuv_formats[] = {
>>      { HAL_PIXEL_FORMAT_YCbCr_420_888,   0, 1, __DRI_IMAGE_FOURCC_YUV420
>> },
>>      { HAL_PIXEL_FORMAT_YCbCr_420_888,   1, 1, __DRI_IMAGE_FOURCC_YVU420
>> },
>>      { HAL_PIXEL_FORMAT_YV12,            1, 1, __DRI_IMAGE_FOURCC_YVU420
>> },
>> +   /* HACK: See droid_create_image_from_prime_fd() and b/32077885. */
>> +   { HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED,   0, 2,
>> __DRI_IMAGE_FOURCC_NV12 },
>> +   { HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED,   0, 1,
>> __DRI_IMAGE_FOURCC_YUV420 },
>> +   { HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED,   1, 1,
>> __DRI_IMAGE_FOURCC_YVU420 },
>
>
> One alternative way would be to ask gralloc about these formats. On gralloc0
> this would need a perform() hook and gralloc1 has getFormat(). This is how
> it is done currently on Android-IA, see following commits:
>
> https://github.com/intel/external-mesa/commit/deb323eafa321c725805a702ed19cb4983346b60
>
> https://github.com/intel/external-mesa/commit/7cc01beaf540e29862853561ef93c6c4e86c4c1a
>
> Do you think this approach would work with Chromium as well?
>
i think the Android-IA approach looks good, although it depends on
local gralloc0 changes. With gralloc1 on the horizon, I don't know how
much sense it makes to extend the predecessor.
AFAICT the patch should not cause any issues and it's nicely documented.

Perhaps someone from the Google/CrOS team can assist in making the bug
public, although even then it might be better to focus on a 'perfect'
gralloc1?

IMHO the patch looks perfectly reasonable and we could merge it even,
if we were to switch to gralloc1 in the not too distant future ;-)

-Emil