[Mesa-dev] [PATCH] egl/android: Remove handling of RGB_888 pixel format

[Rob Clark]

On Thu, Nov 10, 2016 at 10:29 AM, Tomasz Figa <tfiga at chromium.org> wrote:
> On Thu, Nov 10, 2016 at 11:42 PM, Rob Clark <robdclark at gmail.com> wrote:
>> On Wed, Nov 9, 2016 at 11:23 PM, Tomasz Figa <tfiga at chromium.org> wrote:
>>> On Thu, Nov 10, 2016 at 5:21 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>> On 9 November 2016 at 08:32, Tomasz Figa <tfiga at chromium.org> wrote:
>>>>> It is currently completely broken, as it ends up using RGBX_8888 on
>>>>> hardware side, due to no way of distinguishing between these two in the
>>>>> DRI API, while HAL_PIXEL_FORMAT_RGB_888 is clearly defined to be the
>>>>> 3-byte per pixel RGB format.
>>>>>
>>>>> Signed-off-by: Tomasz Figa <tfiga at chromium.org>
>>>> Nicely spotted ! I wonder if there will be any real benefit in using
>>>> the RGB_888.
>>>
>>> Hard to say. Personally I haven't worked with any platform using it,
>>> but AFAIK some Qualcomm SoCs support it. I guess a question to Rob
>>> Clark. ;)
>>
>> adreno does support rendering to 888 (and as well scanning it out).
>> I'm not really sure what, if anything, uses 888.  I suppose there
>> might be some small bandwidth advantage.  If android uses 888 then
>> maybe we are better off adding the missing DRI format.
>
> Android itself has RGBA_8888 hardcoded in many places and RGB_888
> looks optional (which would make sense given that there are many
> platforms without hardware support for this format). Some apps might
> end up using it if they don't request specific native format, just 8
> bits per each component with no alpha (since that matches both RGB_888
> and RGBX_8888).
>
>>
>> (although the current code looks like it would be somewhat broken if
>> android tried..  I have heard reports that a couple apps just render
>> black, which could I suppose be related.)
>
> Hmm, I'd probably expect some garbage instead of black, but who knows...

yeah, could well be unrelated.. and if many other SoC's don't support
888 then it is more likely that apps wouldn't try to use it, I guess.

Anyways, when someone gets around to debugging those, we can always
add back 888 if that turned out to be the problem.

BR,
-R

>>
>> BR,
>> -R
>>
>>>> If so we can extend the DRI loader/driver interface for that (and then
>>>> go and fix the drivers).
>>>>
>>>> Shouldn't platform_surfaceless.c need a similar trick ?
>>>
>>> Looking at the code there, I don't think it expects any formats in
>>> particular, especially since there are no window surfaces involved.
>>>
>>>>
>>>> Regardless:
>>>> Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
>>>
>>> Thanks.
>>>
>>> Best regards,
>>> Tomasz