This patchset is an inital RFC I wanted to send out to get some early review and feedback.
I've been working to enable the drm_hwcomposer for HiKey and HiKey960 boards in AOSP, and this patchset contains the required changes to the drm_hwcomposer code to get things working.
I'm really quite naive when it comes to graphics, and I'm sure I don't fully understand the drm_hwcomposer code, so forgive me if I've done anything terribly stupid here.
There are a few terrible hacks involved, which I'd really love to get any guidance on how things should be properly done here.
The first is I'm using Rob's patch to force use of single plane, without which I don't get anything on the screen.
The second is that after getting the drm_hwcomposer running, I was seeing tons of tearing, which was exactly the behavior I'm making this effort to avoid. So I have another hack patch that removes some checks that fail which keep us from doing fence handling properly. Again, any tips for what deeper issues need to be addressed would be helpful!
Finally, the third hack isn't in this patchset, but in the kernel, where with the drm_hwcomposer, on initialization, we would hit the case where it was trying to enable vsync while the crtc is off. So I worked around that here: https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hike...
For that last issue, I'm not sure if this is due to some quirk w/ the drm_hwcomposer initialization, or some other quirk of the kirin drm driver, but in development of this, I've also worked with an older drm based hwc implementations which didn't trigger this issue.
Anyway, outside of those hacks I'm sure there can be further improvement as well, so I wanted to get it out for some initial review.
If anyone wants to try the code out on either a HiKey or HiKey960, you will need this patchset on top of the freedesktop/master branch of drm_hwcomposer. You'll also need the freedesktop/master branch of libdrm. You'll need the following patches to the device/linaro/hikey project: https://github.com/johnstultz-work/android_device_linaro_hikey/commits/drm_h...
And you'll need to replace the kernel Image-dtb with one built from the following tree: https://git.linaro.org/people/john.stultz/android-dev.git/log/?h=dev/hikey-h...
Any thoughts or feedback will be very much appreciated!
Many thanks to Rob Herring and Matt Szczesiak for help to get this working so far!
Thanks so much for your time! -john
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
John Stultz (3): drm_hwcomposer: glworker: Add build time options for certain shader feature names drm_hwcomposer: Add platformhisi buffer importer for hikey and hikey960 drm_hwcomposer: HACK: Fix tearing on hikey/hikey960
Rob Herring (2): drm_hwcomposer: provide a common gralloc handle definition drm_hwcomposer: HACK: force single plane
Android.mk | 13 ++- drmdisplaycompositor.cpp | 9 ++- drmhwctwo.cpp | 4 +- glworker.cpp | 15 +++- gralloc_drm_handle.h | 87 +++++++++++++++++++++ platformhisi.cpp | 200 +++++++++++++++++++++++++++++++++++++++++++++++ platformhisi.h | 50 ++++++++++++ 7 files changed, 368 insertions(+), 10 deletions(-) create mode 100644 gralloc_drm_handle.h create mode 100644 platformhisi.cpp create mode 100644 platformhisi.h
From: Rob Herring robh@kernel.org
EGL, gralloc, and HWC must all have a common definition of fd's and int's in native_handle_t to share the fd and width, height, format, etc. of a dmabuf.
Move the definition into HWC so we aren't dependent on a specific gralloc implementation and so we don't have to create an importer just for different native_handle_t layouts. This will allow supporting multiple gralloc implementations that conform to this layout.
For now, this is aligned with gbm_gralloc's struct. Once we change gbm_gralloc and mesa to point to this copy, we can make modifications to the struct.
Change-Id: I0e0e9994c7a13e6c47f00a70d13cd2ef9b1543d3 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 Signed-off-by: Rob Herring robh@kernel.org [jstultz: This patch is important to be able to build AOSP without having to include the gbm_gralloc project.] Signed-off-by: John Stultz john.stultz@linaro.org --- Android.mk | 1 - gralloc_drm_handle.h | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 gralloc_drm_handle.h
diff --git a/Android.mk b/Android.mk index 8b11e37..ee5b8bf 100644 --- a/Android.mk +++ b/Android.mk @@ -47,7 +47,6 @@ LOCAL_SHARED_LIBRARIES := \ LOCAL_STATIC_LIBRARIES := libdrmhwc_utils
LOCAL_C_INCLUDES := \ - external/drm_gralloc \ system/core/libsync
LOCAL_SRC_FILES := \ diff --git a/gralloc_drm_handle.h b/gralloc_drm_handle.h new file mode 100644 index 0000000..e2f35dd --- /dev/null +++ b/gralloc_drm_handle.h @@ -0,0 +1,87 @@ +/* + * Copyright (C) 2010-2011 Chia-I Wu olvaffe@gmail.com + * Copyright (C) 2010-2011 LunarG Inc. + * Copyright (C) 2016-2017 Linaro, Ltd., Rob Herring robh@kernel.org + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +#ifndef _GRALLOC_DRM_HANDLE_H_ +#define _GRALLOC_DRM_HANDLE_H_ + +#include <cutils/native_handle.h> + +#ifdef __cplusplus +extern "C" { +#endif + +struct gralloc_drm_handle_t { + native_handle_t base; + + /* file descriptors */ + int prime_fd; + + /* integers */ + int magic; + + int width; + int height; + int format; + int usage; + + int name; /* the name of the bo */ + int stride; /* the stride in bytes */ + int data_owner; /* owner of data (for validation) */ + + uint64_t modifier __attribute__((aligned(8))); /* buffer modifiers */ + union { + void *data; /* private pointer for gralloc */ + uint64_t reserved; + } __attribute__((aligned(8))); +}; +#define GRALLOC_HANDLE_MAGIC 0x5f47424d +#define GRALLOC_HANDLE_NUM_FDS 1 +#define GRALLOC_HANDLE_NUM_INTS ( \ + ((sizeof(struct gralloc_drm_handle_t) - sizeof(native_handle_t))/sizeof(int)) \ + - GRALLOC_HANDLE_NUM_FDS) + +static inline struct gralloc_drm_handle_t *gralloc_drm_handle(buffer_handle_t _handle) +{ + struct gralloc_drm_handle_t *handle = + (struct gralloc_drm_handle_t *) _handle; + + if (handle && (handle->base.version != sizeof(handle->base) || + handle->base.numInts != GRALLOC_HANDLE_NUM_INTS || + handle->base.numFds != GRALLOC_HANDLE_NUM_FDS || + handle->magic != GRALLOC_HANDLE_MAGIC)) + return NULL; + + return handle; +} + +static inline int gralloc_drm_get_prime_fd(buffer_handle_t _handle) +{ + struct gralloc_drm_handle_t *handle = gralloc_drm_handle(_handle); + return (handle) ? handle->prime_fd : -1; +} + +#ifdef __cplusplus +} +#endif +#endif /* _GRALLOC_DRM_HANDLE_H_ */
On Wed, Jan 10, 2018 at 12:05 AM, John Stultz john.stultz@linaro.org wrote:
From: Rob Herring robh@kernel.org
EGL, gralloc, and HWC must all have a common definition of fd's and int's in native_handle_t to share the fd and width, height, format, etc. of a dmabuf.
Move the definition into HWC so we aren't dependent on a specific gralloc implementation and so we don't have to create an importer just for different native_handle_t layouts. This will allow supporting multiple gralloc implementations that conform to this layout.
For now, this is aligned with gbm_gralloc's struct. Once we change gbm_gralloc and mesa to point to this copy, we can make modifications to the struct.
Change-Id: I0e0e9994c7a13e6c47f00a70d13cd2ef9b1543d3 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 Signed-off-by: Rob Herring robh@kernel.org [jstultz: This patch is important to be able to build AOSP without having to include the gbm_gralloc project.] Signed-off-by: John Stultz john.stultz@linaro.org
Android.mk | 1 - gralloc_drm_handle.h | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 gralloc_drm_handle.h
We have since decided that libdrm is a better place for this. Robert Foss is working on that.
But you shouldn't need this at all with your custom importer.
Rob
On Wed, Jan 10, 2018 at 12:39 PM, Rob Herring robh@kernel.org wrote:
On Wed, Jan 10, 2018 at 12:05 AM, John Stultz john.stultz@linaro.org wrote:
From: Rob Herring robh@kernel.org
EGL, gralloc, and HWC must all have a common definition of fd's and int's in native_handle_t to share the fd and width, height, format, etc. of a dmabuf.
Move the definition into HWC so we aren't dependent on a specific gralloc implementation and so we don't have to create an importer just for different native_handle_t layouts. This will allow supporting multiple gralloc implementations that conform to this layout.
For now, this is aligned with gbm_gralloc's struct. Once we change gbm_gralloc and mesa to point to this copy, we can make modifications to the struct.
Change-Id: I0e0e9994c7a13e6c47f00a70d13cd2ef9b1543d3 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 Signed-off-by: Rob Herring robh@kernel.org [jstultz: This patch is important to be able to build AOSP without having to include the gbm_gralloc project.] Signed-off-by: John Stultz john.stultz@linaro.org
Android.mk | 1 - gralloc_drm_handle.h | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 gralloc_drm_handle.h
We have since decided that libdrm is a better place for this. Robert Foss is working on that.
But you shouldn't need this at all with your custom importer.
Unfortunately the platformdrmgeneric.cpp is part of the build either way, and tries to #include the gralloc_drm_handle.h file (the USE_DRM_GENERIC_IMPORTER conditional is under the includes), which causes build failures.
But I'll rework the Andorid.mk logic to conditionally add the platformhisi.cpp or platformdrmgeneric.cpp so this patch can be skipped.
thanks -john
In order to get the hikey960, which uses the mali bifrost driver working with drm_hwcomposer, its needed to tweak some extension and funciton names used in the shaders.
Specifically: * GL_OES_EGL_image_external_essl3 instead of GL_OES_EGL_image_external * texture() instead of texture2D()
Which is configured using a build time definition.
Credit to Matt Szczesiak for suggesting these changes to get hikey960 working!
I'm a bit new to all this, and I expect there may be a better way to do this, so I'd love any feedback or comments!
Change-Id: I2c8f08341ad086479b66241b903c79b00f2a0feb 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 Signed-off-by: John Stutlz john.stultz@linaro.org --- glworker.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/glworker.cpp b/glworker.cpp index ca726bf..c35d1b6 100644 --- a/glworker.cpp +++ b/glworker.cpp @@ -41,6 +41,17 @@
#define MAX_OVERLAPPING_LAYERS 64
+#ifdef USE_TEXTURE_FN + #define TEXTURE_STR "texture" +#else + #define TEXTURE_STR "texture2D" +#endif + +#ifdef USE_IMAGE_EXTERNAL_ESSL3 + #define IMAGE_EXTERNAL_STR "GL_OES_EGL_image_external_essl3" +#else + #define IMAGE_EXTERNAL_STR "GL_OES_EGL_image_external" +#endif namespace android {
// clang-format off @@ -237,7 +248,7 @@ static std::string GenerateFragmentShader(int layer_count) { std::ostringstream fragment_shader_stream; fragment_shader_stream << "#version 300 es\n" << "#define LAYER_COUNT " << layer_count << "\n" - << "#extension GL_OES_EGL_image_external : require\n" + << "#extension " << IMAGE_EXTERNAL_STR << " : require\n" << "precision mediump float;\n"; for (int i = 0; i < layer_count; ++i) { fragment_shader_stream << "uniform samplerExternalOES uLayerTexture" << i @@ -257,7 +268,7 @@ static std::string GenerateFragmentShader(int layer_count) { fragment_shader_stream << " if (alphaCover > 0.5/255.0) {\n"; // clang-format off fragment_shader_stream - << " texSample = texture2D(uLayerTexture" << i << ",\n" + << " texSample = " << TEXTURE_STR << "(uLayerTexture" << i << ",\n" << " fTexCoords[" << i << "]);\n" << " multRgb = texSample.rgb *\n" << " max(texSample.a, uLayerPremult[" << i << "]);\n"
On Tue, Jan 09, 2018 at 10:05:42PM -0800, John Stultz wrote:
In order to get the hikey960, which uses the mali bifrost driver working with drm_hwcomposer, its needed to tweak some extension and funciton names used in the shaders.
Specifically:
- GL_OES_EGL_image_external_essl3 instead of GL_OES_EGL_image_external
- texture() instead of texture2D()
Which is configured using a build time definition.
Build time is kinda uncool, at least in the spirit of multiarch kernels :-) Can't we try a few alternatives until the glsl compiler takes it? For extensions you could/should even query them upfront and then pick the right one. -Daniel
Credit to Matt Szczesiak for suggesting these changes to get hikey960 working!
I'm a bit new to all this, and I expect there may be a better way to do this, so I'd love any feedback or comments!
Change-Id: I2c8f08341ad086479b66241b903c79b00f2a0feb 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 Signed-off-by: John Stutlz john.stultz@linaro.org
glworker.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/glworker.cpp b/glworker.cpp index ca726bf..c35d1b6 100644 --- a/glworker.cpp +++ b/glworker.cpp @@ -41,6 +41,17 @@
#define MAX_OVERLAPPING_LAYERS 64
+#ifdef USE_TEXTURE_FN
- #define TEXTURE_STR "texture"
+#else
- #define TEXTURE_STR "texture2D"
+#endif
+#ifdef USE_IMAGE_EXTERNAL_ESSL3
- #define IMAGE_EXTERNAL_STR "GL_OES_EGL_image_external_essl3"
+#else
- #define IMAGE_EXTERNAL_STR "GL_OES_EGL_image_external"
+#endif namespace android {
// clang-format off @@ -237,7 +248,7 @@ static std::string GenerateFragmentShader(int layer_count) { std::ostringstream fragment_shader_stream; fragment_shader_stream << "#version 300 es\n" << "#define LAYER_COUNT " << layer_count << "\n"
<< "#extension GL_OES_EGL_image_external : require\n"
<< "#extension " << IMAGE_EXTERNAL_STR << " : require\n" << "precision mediump float;\n";@@ -257,7 +268,7 @@ static std::string GenerateFragmentShader(int layer_count) { fragment_shader_stream << " if (alphaCover > 0.5/255.0) {\n"; // clang-format off fragment_shader_stream
<< " texSample = texture2D(uLayerTexture" << i << ",\n"
<< " texSample = " << TEXTURE_STR << "(uLayerTexture" << i << ",\n" << " fTexCoords[" << i << "]);\n" << " multRgb = texSample.rgb *\n" << " max(texSample.a, uLayerPremult[" << i << "]);\n"-- 2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
This allows for importing buffers allocated from the hikey and hikey960 gralloc implelementations.
Change-Id: I81abdd4d1dc7d9f2ef31078c91679b532d3262fd 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 Signed-off-by: John Stultz john.stultz@linaro.org --- Android.mk | 12 ++++ platformhisi.cpp | 200 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ platformhisi.h | 50 ++++++++++++++ 3 files changed, 262 insertions(+) create mode 100644 platformhisi.cpp create mode 100644 platformhisi.h
diff --git a/Android.mk b/Android.mk index ee5b8bf..caafce1 100644 --- a/Android.mk +++ b/Android.mk @@ -66,6 +66,7 @@ LOCAL_SRC_FILES := \ hwcutils.cpp \ platform.cpp \ platformdrmgeneric.cpp \ + platformhisi.cpp \ separate_rects.cpp \ virtualcompositorworker.cpp \ vsyncworker.cpp @@ -74,7 +75,18 @@ LOCAL_CPPFLAGS += \ -DHWC2_USE_CPP11 \ -DHWC2_INCLUDE_STRINGIFICATION
+ +ifeq ($(TARGET_PRODUCT),hikey960) +LOCAL_CPPFLAGS += -DUSE_HISI_IMPORTER -DUSE_IMAGE_EXTERNAL_ESSL3 -DUSE_TEXTURE_FN +LOCAL_C_INCLUDES += device/linaro/hikey/gralloc960/ +else +ifeq ($(TARGET_PRODUCT),hikey) +LOCAL_CPPFLAGS += -DUSE_HISI_IMPORTER -DHIKEY +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/platformhisi.cpp b/platformhisi.cpp new file mode 100644 index 0000000..123c8f2 --- /dev/null +++ b/platformhisi.cpp @@ -0,0 +1,200 @@ +/* + * 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 { + +#ifdef USE_HISI_IMPORTER +// static +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; +} +#endif + +HisiImporter::HisiImporter(DrmResources *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, "NVIDIA")) + ALOGW("Using non-NVIDIA gralloc module: %s/%s\n", gralloc_->common.name, + gralloc_->common.author); + + return 0; +} + +#ifdef HIKEY +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_ARGB8888; + case HAL_PIXEL_FORMAT_RGBX_8888: + return DRM_FORMAT_XBGR8888; + case HAL_PIXEL_FORMAT_RGBA_8888: + 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; + } +} +#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; + } +} +#endif /* HIKEY */ + +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 attr[] = { + EGL_WIDTH, hnd->width, + EGL_HEIGHT, hnd->height, + EGL_LINUX_DRM_FOURCC_EXT, (EGLint)ConvertHalFormatToDrm(hnd->req_format), + 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; + } + + memset(bo, 0, sizeof(hwc_drm_bo_t)); + bo->width = hnd->width; + bo->height = hnd->height; + bo->format = ConvertHalFormatToDrm(hnd->req_format); + bo->usage = hnd->usage; +#ifdef HIKEY + bo->pitches[0] = hnd->width * 4; +#else + bo->pitches[0] = hnd->byte_stride; +#endif + 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; +} + +int HisiImporter::ReleaseBuffer(hwc_drm_bo_t *bo) { + if (bo->fb_id) + if (drmModeRmFB(drm_->fd(), bo->fb_id)) + ALOGE("Failed to rm fb"); + + struct drm_gem_close gem_close; + memset(&gem_close, 0, sizeof(gem_close)); + int num_gem_handles = sizeof(bo->gem_handles) / sizeof(bo->gem_handles[0]); + for (int i = 0; i < num_gem_handles; i++) { + if (!bo->gem_handles[i]) + continue; + + gem_close.handle = bo->gem_handles[i]; + int ret = drmIoctl(drm_->fd(), DRM_IOCTL_GEM_CLOSE, &gem_close); + if (ret) + ALOGE("Failed to close gem handle %d %d", i, ret); + else + bo->gem_handles[i] = 0; + } + return 0; +} + +#ifdef USE_HISI_IMPORTER +std::unique_ptr<Planner> Planner::CreateInstance(DrmResources *) { + std::unique_ptr<Planner> planner(new Planner); + planner->AddStage<PlanStageGreedy>(); + return planner; +} +#endif + +} + + diff --git a/platformhisi.h b/platformhisi.h new file mode 100644 index 0000000..f7a7d8c --- /dev/null +++ b/platformhisi.h @@ -0,0 +1,50 @@ +/* + * 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 Importer { + 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; + int ReleaseBuffer(hwc_drm_bo_t *bo) override; + + private: + uint32_t ConvertHalFormatToDrm(uint32_t hal_format); + + DrmResources *drm_; + + const gralloc_module_t *gralloc_; +}; +} + +#endif
From: Rob Herring robh@kernel.org
Change from Rob to force use of single plane.
Without this we don't see anything on the screen.
Suggestions for alternative solutions here would be greatly appreciated!
Change-Id: Ic963c356ab1ef241d92fde6c890936b560f52051 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 Signed-off-by: John Stultz john.stultz@linaro.org --- drmhwctwo.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drmhwctwo.cpp b/drmhwctwo.cpp index dfca1a6..5ab4595 100644 --- a/drmhwctwo.cpp +++ b/drmhwctwo.cpp @@ -692,12 +692,10 @@ HWC2::Error DrmHwcTwo::HwcDisplay::ValidateDisplay(uint32_t *num_types, case HWC2::Composition::SolidColor: case HWC2::Composition::Cursor: case HWC2::Composition::Sideband: + default: layer.set_validated_type(HWC2::Composition::Client); ++*num_types; break; - default: - layer.set_validated_type(layer.sf_type()); - break; } } return *num_types ? HWC2::Error::HasChanges : HWC2::Error::None;
When using drm_hwcomposer with the hikey/hikey960 boards, the resulting display shows lots of tearing.
I'm not much of an expert in how this code should work, but it seems that we never call sync_wait(), and thus don't seem to be handling the fences properly? I'm not sure.
Anyway, in a daze, I started cutting out code trying to make sure we call the CreateNextTimelineFence() and fb.set_release_fence_fd().
After doing so the tearing went away. I'm really not sure what is wrong that requires these hacks. It may be the hikey/hikey960 drm driver is incorrectly reporting or handling something?
We do only have a single plane and no hardware compositing on the boards, so we are having to force the gpu to do all the compositing.
Any ideas for what a proper fix here would be? Or even just hints on why this might make things work?
Change-Id: Ifba58f6f1cb00e5271892c0241e4891abe211f22 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 Signed-off-by: John Stultz john.stultz@linaro.org --- drmdisplaycompositor.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp index acd13b8..6c391d6 100644 --- a/drmdisplaycompositor.cpp +++ b/drmdisplaycompositor.cpp @@ -299,10 +299,11 @@ int DrmDisplayCompositor::ApplySquash(DrmDisplayComposition *display_comp) { display_comp->importer()); pre_compositor_->Finish();
- if (ret) { +/* if (ret) { ALOGE("Failed to squash layers"); return ret; } +*/
ret = display_comp->CreateNextTimelineFence(); if (ret <= 0) { @@ -390,8 +391,8 @@ int DrmDisplayCompositor::PrepareFrame(DrmDisplayComposition *display_comp) { std::vector<DrmHwcLayer> &layers = display_comp->layers(); std::vector<DrmCompositionPlane> &comp_planes = display_comp->composition_planes(); - std::vector<DrmCompositionRegion> &squash_regions = - display_comp->squash_regions(); +// std::vector<DrmCompositionRegion> &squash_regions = +// display_comp->squash_regions(); std::vector<DrmCompositionRegion> &pre_comp_regions = display_comp->pre_comp_regions();
@@ -405,7 +406,7 @@ int DrmDisplayCompositor::PrepareFrame(DrmDisplayComposition *display_comp) { }
int squash_layer_index = -1; - if (squash_regions.size() > 0) { + if (1) { //squash_regions.size() > 0) { squash_framebuffer_index_ = (squash_framebuffer_index_ + 1) % 2; ret = ApplySquash(display_comp); if (ret)
On Tue, Jan 9, 2018 at 10:05 PM, John Stultz john.stultz@linaro.org wrote:
When using drm_hwcomposer with the hikey/hikey960 boards, the resulting display shows lots of tearing.
I'm not much of an expert in how this code should work, but it seems that we never call sync_wait(), and thus don't seem to be handling the fences properly? I'm not sure.
Anyway, in a daze, I started cutting out code trying to make sure we call the CreateNextTimelineFence() and fb.set_release_fence_fd().
After doing so the tearing went away. I'm really not sure what is wrong that requires these hacks. It may be the hikey/hikey960 drm driver is incorrectly reporting or handling something?
We do only have a single plane and no hardware compositing on the boards, so we are having to force the gpu to do all the compositing.
Any ideas for what a proper fix here would be? Or even just hints on why this might make things work?
So I've maybe started to understand things a bit (though probably not much) more here. Again, my apologies for being very new to all this.
This patch actually isn't necessary on the HiKey960, as in the mix of testing both boards, I didn't get it working until after the HiKey board so I incorrectly assumed this would be needed.
This patch is really only useful to avoid the tearing I see with the HiKey board.
The core issue on HiKey is that in DrmDisplayCompositor::PrepareFrame(), when we initialize the precompositor_ to a new GLWorkerCompositor, the eglCreateContext() call fails because we're asking for version 3, which the HiKey's utgard mali implementation doesn't support (thus why the pre_compositor_->Composite() call always fails in ApplySquash() - though still need to figure out why squash_regions are empty).
Asking for version 2 gets us along a bit further, but then I hit other trouble with the eglMakeCurrent() call in BeginContext, as passing EGL_NO_SURFACE, EGL_NO_SURFACE without EGL_NO_CONTEXT seems to trigger an error in the mali GL implementation. However, using EGL_NO_CONTEXT causes surfaceflinger to crash, so I just hacked it up to skip the eglMakeCurrent call to keep things going.
The next issue that I run into is maybe the more problematic one: The vertex and fragment shaders in the glworker.c are marked as "#version 300 es", which utgard can't handle (apparently it maxes out at "#version 100", but can't seemingly handle other syntax like "in").
So the bigger question I guess I have is: Is the drm_hwcomposer as a project targeting a certain minimal GLES version, or is it open to supporting older/limited hardware like the utgard level mali? I can look into trying to figure out if what needs to be done can be done in a simpler shader language, or alternatively we can add fallback code to still allow for fence handling for tear-free functionality w/o the GL compositor (which is basically what this hack patch in-effect allows).
Are either of these preferred? Or none?
Thoughts?
thanks -john
On Thu, Jan 11, 2018 at 11:20 PM, John Stultz john.stultz@linaro.org wrote:
On Tue, Jan 9, 2018 at 10:05 PM, John Stultz john.stultz@linaro.org wrote:
When using drm_hwcomposer with the hikey/hikey960 boards, the resulting display shows lots of tearing.
I'm not much of an expert in how this code should work, but it seems that we never call sync_wait(), and thus don't seem to be handling the fences properly? I'm not sure.
Anyway, in a daze, I started cutting out code trying to make sure we call the CreateNextTimelineFence() and fb.set_release_fence_fd().
After doing so the tearing went away. I'm really not sure what is wrong that requires these hacks. It may be the hikey/hikey960 drm driver is incorrectly reporting or handling something?
We do only have a single plane and no hardware compositing on the boards, so we are having to force the gpu to do all the compositing.
Any ideas for what a proper fix here would be? Or even just hints on why this might make things work?
So I've maybe started to understand things a bit (though probably not much) more here. Again, my apologies for being very new to all this.
This patch actually isn't necessary on the HiKey960, as in the mix of testing both boards, I didn't get it working until after the HiKey board so I incorrectly assumed this would be needed.
This patch is really only useful to avoid the tearing I see with the HiKey board.
The core issue on HiKey is that in DrmDisplayCompositor::PrepareFrame(), when we initialize the precompositor_ to a new GLWorkerCompositor, the eglCreateContext() call fails because we're asking for version 3, which the HiKey's utgard mali implementation doesn't support (thus why the pre_compositor_->Composite() call always fails in ApplySquash() - though still need to figure out why squash_regions are empty).
Asking for version 2 gets us along a bit further, but then I hit other trouble with the eglMakeCurrent() call in BeginContext, as passing EGL_NO_SURFACE, EGL_NO_SURFACE without EGL_NO_CONTEXT seems to trigger an error in the mali GL implementation. However, using EGL_NO_CONTEXT causes surfaceflinger to crash, so I just hacked it up to skip the eglMakeCurrent call to keep things going.
The next issue that I run into is maybe the more problematic one: The vertex and fragment shaders in the glworker.c are marked as "#version 300 es", which utgard can't handle (apparently it maxes out at "#version 100", but can't seemingly handle other syntax like "in").
So the bigger question I guess I have is: Is the drm_hwcomposer as a project targeting a certain minimal GLES version, or is it open to supporting older/limited hardware like the utgard level mali? I can look into trying to figure out if what needs to be done can be done in a simpler shader language, or alternatively we can add fallback code to still allow for fence handling for tear-free functionality w/o the GL compositor (which is basically what this hack patch in-effect allows).
It requires ES 3.0 'cause no one has implemented anything else... Doesn't passing CTS since N at least require ES 3.0?
I think having a fallback path that requires no GL is needed. The fence handling there should be simple pass thru to/from the kernel display driver.
Rob
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
John Stultz -
Rob Herring