[RFC][PATCH v3] drm_hwcomposer: Add platformhisi buffer importer for hikey and hikey960

[RFC][PATCH v3] drm_hwcomposer: Add platformhisi buffer importer for hikey and hikey960

[John Stultz]

This allows for importing buffers allocated from the
hikey and hikey960 gralloc implelementations.

Feedback or comments would be greatly appreciated!

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>
Change-Id: I81abdd4d1dc7d9f2ef31078c91679b532d3262fd
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
The full patchset I'm testing with can be found here:
https://github.com/johnstultz-work/drm_hwcomposer/commits/drm_hwc

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)
---
 Android.mk           |  13 +++++
 platformdrmgeneric.h |   2 +-
 platformhisi.cpp     | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++
 platformhisi.h       |  48 +++++++++++++++++++
 4 files changed, 194 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..5f17c20
--- /dev/null
+++ b/platformhisi.cpp
@@ -0,0 +1,132 @@
+/*
+ * 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) : 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 attr[] = {
+    EGL_WIDTH, hnd->width,
+    EGL_HEIGHT, hnd->height,
+    EGL_LINUX_DRM_FOURCC_EXT, (EGLint)DrmGenericImporter::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 = DrmGenericImporter::ConvertHalFormatToDrm(hnd->req_format);
+  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;
+}
+
+#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..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
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH v3] drm_hwcomposer: Add platformhisi buffer importer for hikey and hikey960

[Alexandru-Cosmin Gheorghe]

Hi John,

Some comments bellow,

On Thu, Feb 08, 2018 at 04:40:05PM -0800, John Stultz wrote:
> This allows for importing buffers allocated from the
> hikey and hikey960 gralloc implelementations.
> 
> Feedback or comments would be greatly appreciated!
> 
> 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>
> Change-Id: I81abdd4d1dc7d9f2ef31078c91679b532d3262fd
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> The full patchset I'm testing with can be found here:
> https://github.com/johnstultz-work/drm_hwcomposer/commits/drm_hwc
> 
> 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)
> ---
>  Android.mk           |  13 +++++
>  platformdrmgeneric.h |   2 +-
>  platformhisi.cpp     | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  platformhisi.h       |  48 +++++++++++++++++++
>  4 files changed, 194 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..5f17c20
> --- /dev/null
> +++ b/platformhisi.cpp
> @@ -0,0 +1,132 @@
> +/*
> + * 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

Isn't this pointless this file seems to be added only of if
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) : 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) {
If the scope is reusing, I think you could go further and create an
overload/helper ImportImage(EGLDisplay, width, height, fd, bystride),
that's called for both HisiImport and DrmGenericImporter.
> +  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)DrmGenericImporter::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 = DrmGenericImporter::ConvertHalFormatToDrm(hnd->req_format);
> +  bo->usage = hnd->usage;
> +  bo->pitches[0] = hnd->byte_stride;
This would work only for 1 plane formats, 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.
> +  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;
> +}
> +
> +#ifdef USE_HISI_IMPORTER
> +std::unique_ptr<Planner> Planner::CreateInstance(DrmResources *) {
> +  std::unique_ptr<Planner> planner(new Planner);
> +  planner->AddStage<PlanStageGreedy>();
> +  return planner;
> +}
> +#endif
> +
> +}

I suppose this is a reminiscence, since you where planning to remove
platformdrmgeneric.
> +
> +
> 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
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC][PATCH v3] drm_hwcomposer: Add platformhisi buffer importer for hikey and hikey960

[John Stultz]

On Tue, Feb 13, 2018 at 8:34 AM, Alexandru-Cosmin Gheorghe
<Alexandru-Cosmin.Gheorghe@arm.com> wrote:
> Hi John,
>
> Some comments bellow,
>
> On Thu, Feb 08, 2018 at 04:40:05PM -0800, John Stultz wrote:
>> This allows for importing buffers allocated from the
>> hikey and hikey960 gralloc implelementations.
>>
>> Feedback or comments would be greatly appreciated!
>>
>> 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>
>> Change-Id: I81abdd4d1dc7d9f2ef31078c91679b532d3262fd
>> Signed-off-by: John Stultz <john.stultz@linaro.org>
>> ---
>> The full patchset I'm testing with can be found here:
>> https://github.com/johnstultz-work/drm_hwcomposer/commits/drm_hwc
>>
>> 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)
>> ---
>>  Android.mk           |  13 +++++
>>  platformdrmgeneric.h |   2 +-
>>  platformhisi.cpp     | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  platformhisi.h       |  48 +++++++++++++++++++
>>  4 files changed, 194 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..5f17c20
>> --- /dev/null
>> +++ b/platformhisi.cpp
>> @@ -0,0 +1,132 @@
>> +/*
>> + * 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
>
> Isn't this pointless this file seems to be added only of if
> USE_HISI_IMPORTER.

Fair point. I'll clean that up.

>> +EGLImageKHR HisiImporter::ImportImage(EGLDisplay egl_display, buffer_handle_t handle) {
> If the scope is reusing, I think you could go further and create an
> overload/helper ImportImage(EGLDisplay, width, height, fd, bystride),
> that's called for both HisiImport and DrmGenericImporter.

I'll look into this. I'm not sure how minimal folks want to be, but
I'll take a pass and see if it seems cleaner.


>> +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 = DrmGenericImporter::ConvertHalFormatToDrm(hnd->req_format);
>> +  bo->usage = hnd->usage;
>> +  bo->pitches[0] = hnd->byte_stride;
> This would work only for 1 plane formats, 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.

Will take a shot at this too.


>> +#ifdef USE_HISI_IMPORTER
>> +std::unique_ptr<Planner> Planner::CreateInstance(DrmResources *) {
>> +  std::unique_ptr<Planner> planner(new Planner);
>> +  planner->AddStage<PlanStageGreedy>();
>> +  return planner;
>> +}
>> +#endif
>> +
>> +}
>
> I suppose this is a reminiscence, since you where planning to remove
> platformdrmgeneric.

Likely, I'll try to clean it up.

Thanks so much again for the review and feedback! I really appreciate it!
-john
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel