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

[Tomasz Figa]

On Tue, Nov 28, 2017 at 8:49 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> 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 ;-)

Looks very similar to my earlier internal proposal and should work for
us to, but need to test to make sure.

Best regards,
Tomasz