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

[Tapani Pälli]

On 29.11.2017 20:43, Robert Foss wrote:
> Hey,
> 
> On Tue, 2017-11-28 at 11:49 +0000, Emil Velikov 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/deb323eafa321c725805a
>>> 702ed19cb4983346b60
>>>
>>> https://github.com/intel/external-mesa/commit/7cc01beaf540e29862853
>>> 561ef93c6c4e86c4c1a
>>>
>>> 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.
> 
> I had a look at the chromiumos/minigbm implementation, and it does not
> contain a gralloc1 implementation as far as I can see. I assume that it
> is available somewhere, but maybe not on a public branch.
> 
> Would it be possible to make the minigbm gralloc1 impl. public? That
> way I could submit a patch mirroring what intel/minigbm does.

Android-IA has one here:

https://github.com/intel/minigbm/tree/master/cros_gralloc

> If you fine folks as at Google prefer to roll it yourselves, just give
> me a poke.
> 
> Those are the two options I'm seeing.
> 
> As for gralloc0 support, would it be needed?
> 
>>
>> 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 ;-)
> 
> Maybe doing both is reasonable.
>