On Mon, Jan 29, 2018 at 12:02 PM, Sean Paul <seanpaul@xxxxxxxxxxxx> wrote:
> On Thu, Jan 25, 2018 at 09:23:45PM -0800, John Stultz wrote:
>> On Wed, Jan 24, 2018 at 7:46 AM, Sean Paul <seanpaul@xxxxxxxxxxxx> wrote:
>> > On Tue, Jan 23, 2018 at 03:16:37PM -0800, John Stultz wrote:
>> >> +}
>> >> +#else /* HIKEY960 case*/
>> >> +uint32_t HisiImporter::ConvertHalFormatToDrm(uint32_t hal_format) {
>> >> + switch (hal_format) {
>> >> + case HAL_PIXEL_FORMAT_RGB_888:
>> >> + return DRM_FORMAT_BGR888;
>> >> + case HAL_PIXEL_FORMAT_BGRA_8888:
>> >> + return DRM_FORMAT_XBGR8888;
>> >> + case HAL_PIXEL_FORMAT_RGBX_8888:
>> >> + return DRM_FORMAT_XBGR8888;
>> >> + case HAL_PIXEL_FORMAT_RGBA_8888:
>> >> + return DRM_FORMAT_XBGR8888;
>> >> + case HAL_PIXEL_FORMAT_RGB_565:
>> >> + return DRM_FORMAT_BGR565;
>> >> + case HAL_PIXEL_FORMAT_YV12:
>> >> + return DRM_FORMAT_YVU420;
>> >> + default:
>> >> + ALOGE("Cannot convert hal format to drm format %u", hal_format);
>> >> + return -EINVAL;
>> >> + }
>> >
>> > So it seems like we're smashing a whole bunch of formats into XBGR8888 here, and
>> > I'm not sure how it'll work. If you look above, the primary difference between
>> > the HAL and drm formats is that the subpixels are reversed (ie: RGBA becomes
>> > ABGR). In this snippet, you've got BGRA->XBGR, does this actually work?
>>
>> Honestly the above is some sort of dyslexia nightmare for me, so I may
>> have gotten it slightly wrong. :)
>>
>
> I can certainly sympathize with this. It took me longer than I'd like to admit
> to write the above. This seems like a good reason to change things up!
>
>
>> So my understanding is that for the HiKey960/bifrost the As need to be
>> Xs (and I've verified that we don't get anything if we try to leave
>> the X's), and we have to switch the R/Bs to get the colors right. It
>> may be the R/B switching is due to other quirks in gralloc and the drm
>> driver, I'm not sure.
>
> It'd be nice to figure out. I suspect there's a bug somewhere that's re-
> reversing things.
Yea, I've been digging in and it looks like we can get rid of the
special case conversion by adding DRM_FORMAT_ARGB8888 entries to the
kirin960 drm driver.
So that part seems to be easy to simplify out. I'll continue trying to
sort out the rest of your feedback.
thanks
-john
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
