[RFC][PATCH 1/2] drm_hwcomposer: Error out on YUV layer as it would fail for single planes

[Alexandru-Cosmin Gheorghe]

On Fri, Mar 09, 2018 at 12:06:09PM +0100, Robert Foss wrote:
> Hey,
> 
> 
> On 03/09/2018 10:55 AM, Alexandru-Cosmin Gheorghe wrote:
> >Hi Daniel,
> >
> >On Fri, Mar 09, 2018 at 09:29:24AM +0000, Daniel Stone wrote:
> >>Hi John,
> >>
> >>On 8 March 2018 at 19:43, John Stultz <john.stultz at linaro.org> wrote:
> >>>On Thu, Mar 8, 2018 at 3:10 AM, Robert Foss <robert.foss at collabora.com> wrote:
> >>>>>@@ -76,8 +76,6 @@ uint32_t
> >>>>>DrmGenericImporter::ConvertHalFormatToDrm(uint32_t hal_format) {
> >>>>>         return DRM_FORMAT_ABGR8888;
> >>>>>       case HAL_PIXEL_FORMAT_RGB_565:
> >>>>>         return DRM_FORMAT_BGR565;
> >>>>>-    case HAL_PIXEL_FORMAT_YV12:
> >>>>>-      return DRM_FORMAT_YVU420;
> >>>>
> >>>>I'm not sure I understand the rationale for removing YVU420.
> >>>
> >>>Mostly its on Alexandru's suggestion, as I don't have any experience
> >>>w/ using YVU420.  Per his suggestion, my sense was that since its a
> >>>multi-plane format it was expected to fail with drmModeAddFB2().
> >>>
> >>>If that's incorrect, I'm fine with dropping this change.
> >>
> >>drmModeAddFB2 absolutely works with multi-planar formats. I have no
> >>idea about the internals of HWC or why multi-planar formats aren't
> >>supported there though.
> >>
> >drmModeAddFb2 is fine, it definitely works if you pass the right data to it.
> >Which I don't think ImportBuffer does for DRM_FORMAT_YVU420, it
> >populates just one buffer handle.
> >
> 
> So maybe the proper solution is to split ConvertHalFormatToDrm() into two parts,
> one for doing format conversion, and one for doing verifying that a format
> is supported.
> 
> Additionally, maybe the better place for a format conversion function to
> live is in libdrm/android/gralloc_handle.h, since it will come in handy in
> other projects too.
>
I agree, it would make much more sense to have the format conversion
function in gralloc_handle.h, since it doesn't/shouldn't depend at all
of the importer type.
 
> >>Cheers,
> >>Daniel
> >
> >Regards,
> >Alex Gheorghe
> >

-- 
Cheers,
Alex G