[RFC][PATCH 1/2] drm_hwcomposer: Error out on YUV layer as it would fail for single planes
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; default: ALOGE("Cannot convert hal format to drm format %u", hal_format); return -EINVAL; @@ -88,10 +86,15 @@ EGLImageKHR DrmGenericImporter::ImportImage(EGLDisplay egl_display, buffer_handl gralloc_drm_handle_t *gr_handle = gralloc_drm_handle(handle); if (!gr_handle) return NULL; + + EGLint fmt = ConvertHalFormatToDrm(gr_handle->format); + if (fmt < 0) + return NULL; + EGLint attr[] = { EGL_WIDTH, gr_handle->width, EGL_HEIGHT, gr_handle->height, - EGL_LINUX_DRM_FOURCC_EXT, (EGLint)ConvertHalFormatToDrm(gr_handle->format), + EGL_LINUX_DRM_FOURCC_EXT, fmt, EGL_DMA_BUF_PLANE0_FD_EXT, gr_handle->prime_fd, EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0, EGL_DMA_BUF_PLANE0_PITCH_EXT, gr_handle->stride, @@ -112,10 +115,14 @@ int DrmGenericImporter::ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) { return ret; }
+ uint32_t fmt = ConvertHalFormatToDrm(gr_handle->format); + if (fmt < 0) + return fmt; + memset(bo, 0, sizeof(hwc_drm_bo_t)); bo->width = gr_handle->width; bo->height = gr_handle->height; - bo->format = ConvertHalFormatToDrm(gr_handle->format); + bo->format = fmt; bo->usage = gr_handle->usage; bo->pitches[0] = gr_handle->stride; bo->gem_handles[0] = gem_handle;
This allows for importing buffers allocated from the hikey and hikey960 gralloc implelementations.
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 --- v2: * Make platformhisi and the generic importer exclusive in the build * Fixup vendor check v3: * Unify format conversions * Subclass the platformdrmgeneric importer implementation to reduce code duplication * Rework to avoid board specific logic (tweak gralloc to be consistent between the two) v4: * Minor cleanups as suggested by Alexandru-Cosmin Gheorghe --- Android.mk | 13 +++++ platformdrmgeneric.h | 2 +- platformhisi.cpp | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++ platformhisi.h | 48 ++++++++++++++++++ 4 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 platformhisi.cpp create mode 100644 platformhisi.h
diff --git a/Android.mk b/Android.mk index ee5b8bf..f37e4c3 100644 --- a/Android.mk +++ b/Android.mk @@ -74,7 +74,20 @@ LOCAL_CPPFLAGS += \ -DHWC2_USE_CPP11 \ -DHWC2_INCLUDE_STRINGIFICATION
+ +ifeq ($(TARGET_PRODUCT),hikey960) +LOCAL_CPPFLAGS += -DUSE_HISI_IMPORTER +LOCAL_SRC_FILES += platformhisi.cpp +LOCAL_C_INCLUDES += device/linaro/hikey/gralloc960/ +else +ifeq ($(TARGET_PRODUCT),hikey) +LOCAL_CPPFLAGS += -DUSE_HISI_IMPORTER +LOCAL_SRC_FILES += platformhisi.cpp +LOCAL_C_INCLUDES += device/linaro/hikey/gralloc/ +else LOCAL_CPPFLAGS += -DUSE_DRM_GENERIC_IMPORTER +endif +endif
LOCAL_MODULE := hwcomposer.drm LOCAL_MODULE_TAGS := optional diff --git a/platformdrmgeneric.h b/platformdrmgeneric.h index 8376580..fbe059b 100644 --- a/platformdrmgeneric.h +++ b/platformdrmgeneric.h @@ -35,8 +35,8 @@ class DrmGenericImporter : public Importer { int ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) override; int ReleaseBuffer(hwc_drm_bo_t *bo) override;
- private: uint32_t ConvertHalFormatToDrm(uint32_t hal_format); + private:
DrmResources *drm_;
diff --git a/platformhisi.cpp b/platformhisi.cpp new file mode 100644 index 0000000..16c5e6f --- /dev/null +++ b/platformhisi.cpp @@ -0,0 +1,135 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "hwc-platform-hisi" + +#include "drmresources.h" +#include "platform.h" +#include "platformhisi.h" + + +#include <drm/drm_fourcc.h> +#include <cinttypes> +#include <stdatomic.h> +#include <xf86drm.h> +#include <xf86drmMode.h> + +#include <cutils/log.h> +#include <hardware/gralloc.h> +#include "gralloc_priv.h" + + +namespace android { + +Importer *Importer::CreateInstance(DrmResources *drm) { + HisiImporter *importer = new HisiImporter(drm); + if (!importer) + return NULL; + + int ret = importer->Init(); + if (ret) { + ALOGE("Failed to initialize the hisi importer %d", ret); + delete importer; + return NULL; + } + return importer; +} + +HisiImporter::HisiImporter(DrmResources *drm) : DrmGenericImporter(drm), drm_(drm) { +} + +HisiImporter::~HisiImporter() { +} + +int HisiImporter::Init() { + int ret = hw_get_module(GRALLOC_HARDWARE_MODULE_ID, + (const hw_module_t **)&gralloc_); + if (ret) { + ALOGE("Failed to open gralloc module %d", ret); + return ret; + } + + if (strcasecmp(gralloc_->common.author, "ARM Ltd.")) + ALOGW("Using non-ARM gralloc module: %s/%s\n", gralloc_->common.name, + gralloc_->common.author); + + return 0; +} + +EGLImageKHR HisiImporter::ImportImage(EGLDisplay egl_display, buffer_handle_t handle) { + private_handle_t const *hnd = reinterpret_cast < private_handle_t const *>(handle); + if (!hnd) + return NULL; + + EGLint fmt = ConvertHalFormatToDrm(hnd->req_format); + if (fmt < 0) + return NULL; + + EGLint attr[] = { + EGL_WIDTH, hnd->width, + EGL_HEIGHT, hnd->height, + EGL_LINUX_DRM_FOURCC_EXT, fmt, + EGL_DMA_BUF_PLANE0_FD_EXT, hnd->share_fd, + EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0, + EGL_DMA_BUF_PLANE0_PITCH_EXT, hnd->byte_stride, + EGL_NONE, + }; + return eglCreateImageKHR(egl_display, EGL_NO_CONTEXT, EGL_LINUX_DMA_BUF_EXT, NULL, attr); +} + +int HisiImporter::ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) { + private_handle_t const *hnd = reinterpret_cast < private_handle_t const *>(handle); + if (!hnd) + return -EINVAL; + + uint32_t gem_handle; + int ret = drmPrimeFDToHandle(drm_->fd(), hnd->share_fd, &gem_handle); + if (ret) { + ALOGE("failed to import prime fd %d ret=%d", hnd->share_fd, ret); + return ret; + } + + EGLint fmt = ConvertHalFormatToDrm(hnd->req_format); + if (fmt < 0) + return fmt; + + memset(bo, 0, sizeof(hwc_drm_bo_t)); + bo->width = hnd->width; + bo->height = hnd->height; + bo->format = fmt; + bo->usage = hnd->usage; + bo->pitches[0] = hnd->byte_stride; + bo->gem_handles[0] = gem_handle; + bo->offsets[0] = 0; + + ret = drmModeAddFB2(drm_->fd(), bo->width, bo->height, bo->format, + bo->gem_handles, bo->pitches, bo->offsets, &bo->fb_id, 0); + if (ret) { + ALOGE("could not create drm fb %d", ret); + return ret; + } + + return ret; +} + +std::unique_ptr<Planner> Planner::CreateInstance(DrmResources *) { + std::unique_ptr<Planner> planner(new Planner); + planner->AddStage<PlanStageGreedy>(); + return planner; +} +} + + diff --git a/platformhisi.h b/platformhisi.h new file mode 100644 index 0000000..46f4595 --- /dev/null +++ b/platformhisi.h @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2015 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_PLATFORM_HISI_H_ +#define ANDROID_PLATFORM_HISI_H_ + +#include "drmresources.h" +#include "platform.h" +#include "platformdrmgeneric.h" + +#include <stdatomic.h> + +#include <hardware/gralloc.h> + +namespace android { + +class HisiImporter : public DrmGenericImporter { + public: + HisiImporter(DrmResources *drm); + ~HisiImporter() override; + + int Init(); + + EGLImageKHR ImportImage(EGLDisplay egl_display, buffer_handle_t handle) override; + int ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) override; + + private: + + DrmResources *drm_; + + const gralloc_module_t *gralloc_; +}; +} + +#endif
Hey John,
This patch looks good to me.
I have yet to build it, and I haven't brought my HiKey960 up for testing quite yet.
On 03/07/2018 12:19 AM, John Stultz wrote:
This allows for importing buffers allocated from the hikey and hikey960 gralloc implelementations.
"implelementations" -> "implementations"
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
v2:
- Make platformhisi and the generic importer exclusive in the build
- Fixup vendor check
v3:
- Unify format conversions
- Subclass the platformdrmgeneric importer implementation to reduce code duplication
- Rework to avoid board specific logic (tweak gralloc to be consistent between the two)
v4:
- Minor cleanups as suggested by Alexandru-Cosmin Gheorghe
Android.mk | 13 +++++ platformdrmgeneric.h | 2 +- platformhisi.cpp | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++ platformhisi.h | 48 ++++++++++++++++++ 4 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 platformhisi.cpp create mode 100644 platformhisi.h
diff --git a/Android.mk b/Android.mk index ee5b8bf..f37e4c3 100644 --- a/Android.mk +++ b/Android.mk @@ -74,7 +74,20 @@ LOCAL_CPPFLAGS += \ -DHWC2_USE_CPP11 \ -DHWC2_INCLUDE_STRINGIFICATION
+ifeq ($(TARGET_PRODUCT),hikey960) +LOCAL_CPPFLAGS += -DUSE_HISI_IMPORTER +LOCAL_SRC_FILES += platformhisi.cpp +LOCAL_C_INCLUDES += device/linaro/hikey/gralloc960/ +else +ifeq ($(TARGET_PRODUCT),hikey) +LOCAL_CPPFLAGS += -DUSE_HISI_IMPORTER +LOCAL_SRC_FILES += platformhisi.cpp +LOCAL_C_INCLUDES += device/linaro/hikey/gralloc/ +else LOCAL_CPPFLAGS += -DUSE_DRM_GENERIC_IMPORTER +endif +endif
LOCAL_MODULE := hwcomposer.drm LOCAL_MODULE_TAGS := optional diff --git a/platformdrmgeneric.h b/platformdrmgeneric.h index 8376580..fbe059b 100644 --- a/platformdrmgeneric.h +++ b/platformdrmgeneric.h @@ -35,8 +35,8 @@ class DrmGenericImporter : public Importer { int ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) override; int ReleaseBuffer(hwc_drm_bo_t *bo) override;
- private: uint32_t ConvertHalFormatToDrm(uint32_t hal_format);
private:
DrmResources *drm_;
diff --git a/platformhisi.cpp b/platformhisi.cpp new file mode 100644 index 0000000..16c5e6f --- /dev/null +++ b/platformhisi.cpp @@ -0,0 +1,135 @@ +/*
- Copyright (C) 2015 The Android Open Source Project
- Licensed under the Apache License, Version 2.0 (the "License");
- you may not use this file except in compliance with the License.
- You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
- Unless required by applicable law or agreed to in writing, software
- distributed under the License is distributed on an "AS IS" BASIS,
- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- See the License for the specific language governing permissions and
- limitations under the License.
- */
+#define LOG_TAG "hwc-platform-hisi"
+#include "drmresources.h" +#include "platform.h" +#include "platformhisi.h"
+#include <drm/drm_fourcc.h> +#include <cinttypes> +#include <stdatomic.h> +#include <xf86drm.h> +#include <xf86drmMode.h>
+#include <cutils/log.h> +#include <hardware/gralloc.h> +#include "gralloc_priv.h"
+namespace android {
+Importer *Importer::CreateInstance(DrmResources *drm) {
- HisiImporter *importer = new HisiImporter(drm);
- if (!importer)
- return NULL;
- int ret = importer->Init();
- if (ret) {
- ALOGE("Failed to initialize the hisi importer %d", ret);
- delete importer;
- return NULL;
- }
- return importer;
+}
+HisiImporter::HisiImporter(DrmResources *drm) : DrmGenericImporter(drm), drm_(drm) { +}
+HisiImporter::~HisiImporter() { +}
+int HisiImporter::Init() {
- int ret = hw_get_module(GRALLOC_HARDWARE_MODULE_ID,
(const hw_module_t **)&gralloc_);- if (ret) {
- ALOGE("Failed to open gralloc module %d", ret);
- return ret;
- }
- if (strcasecmp(gralloc_->common.author, "ARM Ltd."))
- ALOGW("Using non-ARM gralloc module: %s/%s\n", gralloc_->common.name,
gralloc_->common.author);- return 0;
+}
+EGLImageKHR HisiImporter::ImportImage(EGLDisplay egl_display, buffer_handle_t handle) {
- private_handle_t const *hnd = reinterpret_cast < private_handle_t const *>(handle);
- if (!hnd)
- return NULL;
- EGLint fmt = ConvertHalFormatToDrm(hnd->req_format);
- if (fmt < 0)
- return NULL;
- EGLint attr[] = {
- EGL_WIDTH, hnd->width,
- EGL_HEIGHT, hnd->height,
- EGL_LINUX_DRM_FOURCC_EXT, fmt,
- EGL_DMA_BUF_PLANE0_FD_EXT, hnd->share_fd,
- EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0,
- EGL_DMA_BUF_PLANE0_PITCH_EXT, hnd->byte_stride,
- EGL_NONE,
- };
- return eglCreateImageKHR(egl_display, EGL_NO_CONTEXT, EGL_LINUX_DMA_BUF_EXT, NULL, attr);
+}
+int HisiImporter::ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) {
- private_handle_t const *hnd = reinterpret_cast < private_handle_t const *>(handle);
- if (!hnd)
- return -EINVAL;
- uint32_t gem_handle;
- int ret = drmPrimeFDToHandle(drm_->fd(), hnd->share_fd, &gem_handle);
- if (ret) {
- ALOGE("failed to import prime fd %d ret=%d", hnd->share_fd, ret);
- return ret;
- }
- EGLint fmt = ConvertHalFormatToDrm(hnd->req_format);
- if (fmt < 0)
- return fmt;
- memset(bo, 0, sizeof(hwc_drm_bo_t));
- bo->width = hnd->width;
- bo->height = hnd->height;
- bo->format = fmt;
- bo->usage = hnd->usage;
- bo->pitches[0] = hnd->byte_stride;
- bo->gem_handles[0] = gem_handle;
- bo->offsets[0] = 0;
- ret = drmModeAddFB2(drm_->fd(), bo->width, bo->height, bo->format,
bo->gem_handles, bo->pitches, bo->offsets, &bo->fb_id, 0);- if (ret) {
- ALOGE("could not create drm fb %d", ret);
- return ret;
- }
- return ret;
+}
+std::unique_ptr<Planner> Planner::CreateInstance(DrmResources *) {
- std::unique_ptr<Planner> planner(new Planner);
- planner->AddStage<PlanStageGreedy>();
- return planner;
+} +}
diff --git a/platformhisi.h b/platformhisi.h new file mode 100644 index 0000000..46f4595 --- /dev/null +++ b/platformhisi.h @@ -0,0 +1,48 @@ +/*
- Copyright (C) 2015 The Android Open Source Project
- Licensed under the Apache License, Version 2.0 (the "License");
- you may not use this file except in compliance with the License.
- You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
- Unless required by applicable law or agreed to in writing, software
- distributed under the License is distributed on an "AS IS" BASIS,
- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- See the License for the specific language governing permissions and
- limitations under the License.
- */
+#ifndef ANDROID_PLATFORM_HISI_H_ +#define ANDROID_PLATFORM_HISI_H_
+#include "drmresources.h" +#include "platform.h" +#include "platformdrmgeneric.h"
+#include <stdatomic.h>
+#include <hardware/gralloc.h>
+namespace android {
+class HisiImporter : public DrmGenericImporter {
- public:
- HisiImporter(DrmResources *drm);
- ~HisiImporter() override;
- int Init();
- EGLImageKHR ImportImage(EGLDisplay egl_display, buffer_handle_t handle) override;
- int ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) override;
- private:
- DrmResources *drm_;
- const gralloc_module_t *gralloc_;
+}; +}
+#endif
On Thu, Mar 8, 2018 at 3:16 AM, Robert Foss robert.foss@collabora.com wrote:
Hey John,
This patch looks good to me.
I have yet to build it, and I haven't brought my HiKey960 up for testing quite yet.
On 03/07/2018 12:19 AM, John Stultz wrote:
This allows for importing buffers allocated from the hikey and hikey960 gralloc implelementations.
"implelementations" -> "implementations"
Fixed in my tree. Let me know if you have any further feedback after testing and I'll re-submit.
thanks -john
Hey John,
Feel free to add my ACK.
Rob.
On 03/08/2018 11:08 PM, John Stultz wrote:
On Thu, Mar 8, 2018 at 3:16 AM, Robert Foss robert.foss@collabora.com wrote:
Hey John,
This patch looks good to me.
I have yet to build it, and I haven't brought my HiKey960 up for testing quite yet.
On 03/07/2018 12:19 AM, John Stultz wrote:
This allows for importing buffers allocated from the hikey and hikey960 gralloc implelementations.
"implelementations" -> "implementations"
Fixed in my tree. Let me know if you have any further feedback after testing and I'll re-submit.
thanks -john
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.
default: ALOGE("Cannot convert hal format to drm format %u", hal_format); return -EINVAL;@@ -88,10 +86,15 @@ EGLImageKHR DrmGenericImporter::ImportImage(EGLDisplay egl_display, buffer_handl gralloc_drm_handle_t *gr_handle = gralloc_drm_handle(handle); if (!gr_handle) return NULL;
- EGLint fmt = ConvertHalFormatToDrm(gr_handle->format);
- if (fmt < 0)
- return NULL;
- EGLint attr[] = { EGL_WIDTH, gr_handle->width, EGL_HEIGHT, gr_handle->height,
- EGL_LINUX_DRM_FOURCC_EXT, (EGLint)ConvertHalFormatToDrm(gr_handle->format),
- EGL_LINUX_DRM_FOURCC_EXT, fmt, EGL_DMA_BUF_PLANE0_FD_EXT, gr_handle->prime_fd, EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0, EGL_DMA_BUF_PLANE0_PITCH_EXT, gr_handle->stride,
@@ -112,10 +115,14 @@ int DrmGenericImporter::ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t *bo) { return ret; }
- uint32_t fmt = ConvertHalFormatToDrm(gr_handle->format);
- if (fmt < 0)
return fmt;- memset(bo, 0, sizeof(hwc_drm_bo_t)); bo->width = gr_handle->width; bo->height = gr_handle->height;
- bo->format = ConvertHalFormatToDrm(gr_handle->format);
- bo->format = fmt; bo->usage = gr_handle->usage; bo->pitches[0] = gr_handle->stride; bo->gem_handles[0] = gem_handle;
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.
thanks -john
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
Hi John,
On 8 March 2018 at 19:43, John Stultz john.stultz@linaro.org wrote:
On Thu, Mar 8, 2018 at 3:10 AM, Robert Foss robert.foss@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.
Cheers, Daniel
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@linaro.org wrote:
On Thu, Mar 8, 2018 at 3:10 AM, Robert Foss robert.foss@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.
Cheers, Daniel
Regards, Alex Gheorghe
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@linaro.org wrote:
On Thu, Mar 8, 2018 at 3:10 AM, Robert Foss robert.foss@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.
Cheers, Daniel
Regards, Alex 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@linaro.org wrote:
On Thu, Mar 8, 2018 at 3:10 AM, Robert Foss robert.foss@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
dri-devel@lists.freedesktop.org
-
Alexandru-Cosmin Gheorghe -
Daniel Stone -
John Stultz -
Robert Foss