On Thu, Mar 08, 2018 at 11:43:25AM -0800, John Stultz wrote:
On Thu, Mar 8, 2018 at 3:10 AM, Robert Foss robert.foss@collabora.com wrote:
Hey John,
On 03/07/2018 12:19 AM, John Stultz wrote:
As suggested by Alexandru-Cosmin Gheorghe:
ConvertHALFormatToDrm logic would work only for 1 plane formats, and probably gets rejected by drmModeAddFb2, but to save debugging time maybe it worth removing DRM_FORMAT_YVU420 from ConvertHALFormatToDrm and checking it's return code.
So this patch tries to do this.
Cc: Marissa Wall marissaw@google.com Cc: Sean Paul seanpaul@google.com Cc: Dmitry Shmidt dimitrysh@google.com Cc: Robert Foss robert.foss@collabora.com Cc: Matt Szczesiak matt.szczesiak@arm.com Cc: Liviu Dudau Liviu.Dudau@arm.com Cc: David Hanna david.hanna11@gmail.com Cc: Rob Herring rob.herring@linaro.org Cc: Alexandru-Cosmin Gheorghe Alexandru-Cosmin.Gheorghe@arm.com Signed-off-by: John Stultz john.stultz@linaro.org
platformdrmgeneric.cpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/platformdrmgeneric.cpp b/platformdrmgeneric.cpp index 741d42b..33f1ea0 100644 --- a/platformdrmgeneric.cpp +++ b/platformdrmgeneric.cpp @@ -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.
My line of thought was both DrmGenericImporter::ImportBuffer and HisiImporter::ConvertHalFormatToDrm don't work for planar formats(YVU420), so ConvertHalFormatToDrm should convert only the formats supported by the Importer. I realize now, that I might be wrong since this would also affect ImportImage, however at first sight DrmGenericImporter::ImportImage doesn't seem to support multi-planar as well, isn't it?
thanks -john