[hwc] drm_hwcomposer: remove NVIDIA importer

There's no opensource implementation for the NVIDIA gralloc implementation,
so remove it as it is not testable.

As all of the gralloc perform() operations are specific to it, they can be
removed, too.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 Android.mk      |   5 -
 drmhwcgralloc.h |  33 -----
 platformnv.cpp  | 374 --------------------------------------------------------
 3 files changed, 412 deletions(-)
 delete mode 100644 platformnv.cpp

Hey Rob,

+Thierry

This looks good to me, does anyone have a strong opinion the other way?
I'll let this one soak for a day or two more.


Rob.

On Fri, 2017-10-06 at 17:02 -0500, Rob Herring wrote:
> There's no opensource implementation for the NVIDIA gralloc
> implementation,
> so remove it as it is not testable.
> 
> As all of the gralloc perform() operations are specific to it, they
> can be
> removed, too.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  Android.mk      |   5 -
>  drmhwcgralloc.h |  33 -----
>  platformnv.cpp  | 374 ----------------------------------------------
> ----------
>  3 files changed, 412 deletions(-)
>  delete mode 100644 platformnv.cpp
> 
> diff --git a/Android.mk b/Android.mk
> index 99bbcac3e984..d7dc6782e1e0 100644
> --- a/Android.mk
> +++ b/Android.mk
> @@ -71,7 +71,6 @@ LOCAL_SRC_FILES := \
>  	hwcutils.cpp \
>  	platform.cpp \
>  	platformdrmgeneric.cpp \
> -	platformnv.cpp \
>  	separate_rects.cpp \
>  	virtualcompositorworker.cpp \
>  	vsyncworker.cpp
> @@ -80,11 +79,7 @@ LOCAL_CPPFLAGS += \
>  	-DHWC2_USE_CPP11 \
>  	-DHWC2_INCLUDE_STRINGIFICATION
>  
> -ifeq ($(strip $(BOARD_DRM_HWCOMPOSER_BUFFER_IMPORTER)),nvidia-
> gralloc)
> -LOCAL_CPPFLAGS += -DUSE_NVIDIA_IMPORTER
> -else
>  LOCAL_CPPFLAGS += -DUSE_DRM_GENERIC_IMPORTER
> -endif
>  
>  LOCAL_MODULE := hwcomposer.drm
>  LOCAL_MODULE_TAGS := optional
> diff --git a/drmhwcgralloc.h b/drmhwcgralloc.h
> index c4a42eaf1c54..759746a67949 100644
> --- a/drmhwcgralloc.h
> +++ b/drmhwcgralloc.h
> @@ -19,39 +19,6 @@
>  
>  #include <stdint.h>
>  
> -enum {
> -  /* perform(const struct gralloc_module_t *mod,
> -   *	   int op,
> -   *	   int drm_fd,
> -   *	   buffer_handle_t buffer,
> -   *	   struct hwc_drm_bo *bo);
> -   */
> -  GRALLOC_MODULE_PERFORM_DRM_IMPORT = 0xffeeff00,
> -
> -  /* perform(const struct gralloc_module_t *mod,
> -   *	   int op,
> -   *	   buffer_handle_t buffer,
> -   *	   void (*free_callback)(void *),
> -   *	   void *priv);
> -   */
> -  GRALLOC_MODULE_PERFORM_SET_IMPORTER_PRIVATE = 0xffeeff01,
> -
> -  /* perform(const struct gralloc_module_t *mod,
> -   *	   int op,
> -   *	   buffer_handle_t buffer,
> -   *	   void (*free_callback)(void *),
> -   *	   void **priv);
> -   */
> -  GRALLOC_MODULE_PERFORM_GET_IMPORTER_PRIVATE = 0xffeeff02,
> -
> -  /* perform(const struct gralloc_module_t *mod,
> -   *     int op,
> -   *     buffer_handle_t buffer,
> -   *     int *usage);
> -   */
> -  GRALLOC_MODULE_PERFORM_GET_USAGE = 0xffeeff03,
> -};
> -
>  typedef struct hwc_drm_bo {
>    uint32_t width;
>    uint32_t height;
> diff --git a/platformnv.cpp b/platformnv.cpp
> deleted file mode 100644
> index e7b6be3430a8..000000000000
> --- a/platformnv.cpp
> +++ /dev/null
> @@ -1,374 +0,0 @@
> -/*
> - * 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-nv"
> -
> -#include "drmresources.h"
> -#include "platform.h"
> -#include "platformnv.h"
> -
> -#include <cinttypes>
> -#include <stdatomic.h>
> -#include <drm/drm_fourcc.h>
> -#include <xf86drm.h>
> -#include <xf86drmMode.h>
> -
> -#include <cutils/log.h>
> -#include <hardware/gralloc.h>
> -
> -#ifndef EGL_NATIVE_HANDLE_ANDROID_NVX
> -#define EGL_NATIVE_HANDLE_ANDROID_NVX 0x322A
> -#endif
> -
> -namespace android {
> -
> -#ifdef USE_NVIDIA_IMPORTER
> -// static
> -Importer *Importer::CreateInstance(DrmResources *drm) {
> -  NvImporter *importer = new NvImporter(drm);
> -  if (!importer)
> -    return NULL;
> -
> -  int ret = importer->Init();
> -  if (ret) {
> -    ALOGE("Failed to initialize the nv importer %d", ret);
> -    delete importer;
> -    return NULL;
> -  }
> -  return importer;
> -}
> -#endif
> -
> -NvImporter::NvImporter(DrmResources *drm) : drm_(drm) {
> -}
> -
> -NvImporter::~NvImporter() {
> -}
> -
> -int NvImporter::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;
> -}
> -
> -
> -EGLImageKHR NvImporter::ImportImage(EGLDisplay egl_display,
> buffer_handle_t handle) {
> -  return eglCreateImageKHR(
> -      egl_display, EGL_NO_CONTEXT, EGL_NATIVE_HANDLE_ANDROID_NVX,
> -      (EGLClientBuffer)handle, NULL /* no attribs */);
> -}
> -
> -int NvImporter::ImportBuffer(buffer_handle_t handle, hwc_drm_bo_t
> *bo) {
> -  memset(bo, 0, sizeof(hwc_drm_bo_t));
> -  NvBuffer_t *buf = GrallocGetNvBuffer(handle);
> -  if (buf) {
> -    atomic_fetch_add(&buf->ref, 1);
> -    *bo = buf->bo;
> -    return 0;
> -  }
> -
> -  buf = new NvBuffer_t();
> -  if (!buf) {
> -    ALOGE("Failed to allocate new NvBuffer_t");
> -    return -ENOMEM;
> -  }
> -  buf->bo.priv = buf;
> -  buf->importer = this;
> -
> -  // We initialize the reference count to 2 since NvGralloc is still
> using this
> -  // buffer (will be cleared in the NvGrallocRelease), and the other
> -  // reference is for HWC (this ImportBuffer call).
> -  atomic_init(&buf->ref, 2);
> -
> -  int ret = gralloc_->perform(gralloc_,
> GRALLOC_MODULE_PERFORM_DRM_IMPORT,
> -                              drm_->fd(), handle, &buf->bo);
> -  if (ret) {
> -    ALOGE("GRALLOC_MODULE_PERFORM_DRM_IMPORT failed %d", ret);
> -    delete buf;
> -    return ret;
> -  }
> -
> -  ret = drmModeAddFB2(drm_->fd(), buf->bo.width, buf->bo.height,
> buf->bo.format,
> -                      buf->bo.gem_handles, buf->bo.pitches, buf-
> >bo.offsets,
> -                      &buf->bo.fb_id, 0);
> -  if (ret) {
> -    ALOGE("Failed to add fb %d", ret);
> -    ReleaseBufferImpl(&buf->bo);
> -    delete buf;
> -    return ret;
> -  }
> -
> -  ret = GrallocSetNvBuffer(handle, buf);
> -  if (ret) {
> -    /* This will happen is persist.tegra.gpu_mapping_cache is 0/off,
> -     * or if NV gralloc runs out of "priv slots" (currently 3 per
> buffer,
> -     * only one of which should be used by drm_hwcomposer). */
> -    ALOGE("Failed to register free callback for imported buffer %d",
> ret);
> -    ReleaseBufferImpl(&buf->bo);
> -    delete buf;
> -    return ret;
> -  }
> -  *bo = buf->bo;
> -  return 0;
> -}
> -
> -int NvImporter::ReleaseBuffer(hwc_drm_bo_t *bo) {
> -  NvBuffer_t *buf = (NvBuffer_t *)bo->priv;
> -  if (!buf) {
> -    ALOGE("Freeing bo %" PRIu32 ", buf is NULL!", bo->fb_id);
> -    return 0;
> -  }
> -  if (atomic_fetch_sub(&buf->ref, 1) > 1)
> -    return 0;
> -
> -  ReleaseBufferImpl(bo);
> -  delete buf;
> -  return 0;
> -}
> -
> -// static
> -void NvImporter::NvGrallocRelease(void *nv_buffer) {
> -  NvBuffer_t *buf = (NvBuffer *)nv_buffer;
> -  buf->importer->ReleaseBuffer(&buf->bo);
> -}
> -
> -void NvImporter::ReleaseBufferImpl(hwc_drm_bo_t *bo) {
> -  if (bo->fb_id) {
> -    int ret = drmModeRmFB(drm_->fd(), bo->fb_id);
> -    if (ret)
> -      ALOGE("Failed to rm fb %d", ret);
> -  }
> -
> -  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 {
> -      /* Clear any duplicate gem handle as well but don't close
> again */
> -      for (int j = i + 1; j < num_gem_handles; j++)
> -        if (bo->gem_handles[j] == bo->gem_handles[i])
> -          bo->gem_handles[j] = 0;
> -      bo->gem_handles[i] = 0;
> -    }
> -  }
> -}
> -
> -NvImporter::NvBuffer_t
> *NvImporter::GrallocGetNvBuffer(buffer_handle_t handle) {
> -  void *priv = NULL;
> -  int ret =
> -      gralloc_->perform(gralloc_,
> GRALLOC_MODULE_PERFORM_GET_IMPORTER_PRIVATE,
> -                        handle, NvGrallocRelease, &priv);
> -  return ret ? NULL : (NvBuffer_t *)priv;
> -}
> -
> -int NvImporter::GrallocSetNvBuffer(buffer_handle_t handle,
> NvBuffer_t *buf) {
> -  return gralloc_->perform(gralloc_,
> -                           GRALLOC_MODULE_PERFORM_SET_IMPORTER_PRIVA
> TE, handle,
> -                           NvGrallocRelease, buf);
> -}
> -
> -#ifdef USE_NVIDIA_IMPORTER
> -// static
> -std::unique_ptr<Planner> Planner::CreateInstance(DrmResources *) {
> -  std::unique_ptr<Planner> planner(new Planner);
> -  planner->AddStage<PlanStageNvLimits>();
> -  planner->AddStage<PlanStageProtectedRotated>();
> -  planner->AddStage<PlanStageProtected>();
> -  planner->AddStage<PlanStagePrecomp>();
> -  planner->AddStage<PlanStageGreedy>();
> -  return planner;
> -}
> -#endif
> -
> -static DrmPlane *GetCrtcPrimaryPlane(DrmCrtc *crtc,
> -                                     std::vector<DrmPlane *>
> *planes) {
> -  for (auto i = planes->begin(); i != planes->end(); ++i) {
> -    if ((*i)->GetCrtcSupported(*crtc)) {
> -      DrmPlane *plane = *i;
> -      planes->erase(i);
> -      return plane;
> -    }
> -  }
> -  return NULL;
> -}
> -
> -int PlanStageProtectedRotated::ProvisionPlanes(
> -    std::vector<DrmCompositionPlane> *composition,
> -    std::map<size_t, DrmHwcLayer *> &layers, DrmCrtc *crtc,
> -    std::vector<DrmPlane *> *planes) {
> -  int ret;
> -  int protected_zorder = -1;
> -  for (auto i = layers.begin(); i != layers.end();) {
> -    if (!i->second->protected_usage() || !i->second->transform) {
> -      ++i;
> -      continue;
> -    }
> -
> -    auto primary_iter = planes->begin();
> -    for (; primary_iter != planes->end(); ++primary_iter) {
> -      if ((*primary_iter)->type() == DRM_PLANE_TYPE_PRIMARY)
> -        break;
> -    }
> -
> -    // We cheat a little here. Since there can only be one primary
> plane per
> -    // crtc, we know we'll only hit this case once. So we blindly
> insert the
> -    // protected content at the beginning of the composition,
> knowing this path
> -    // won't be taken a second time during the loop.
> -    if (primary_iter != planes->end()) {
> -      composition->emplace(composition->begin(),
> -                           DrmCompositionPlane::Type::kLayer,
> *primary_iter,
> -                           crtc, i->first);
> -      planes->erase(primary_iter);
> -      protected_zorder = i->first;
> -    } else {
> -      ALOGE("Could not provision primary plane for protected/rotated
> layer");
> -    }
> -    i = layers.erase(i);
> -  }
> -
> -  if (protected_zorder == -1)
> -    return 0;
> -
> -  // Add any layers below the protected content to the
> precomposition since we
> -  // need to punch a hole through them.
> -  for (auto i = layers.begin(); i != layers.end();) {
> -    // Skip layers above the z-order of the protected content
> -    if (i->first > static_cast<size_t>(protected_zorder)) {
> -      ++i;
> -      continue;
> -    }
> -
> -    // If there's no precomp layer already queued, queue one now.
> -    DrmCompositionPlane *precomp = GetPrecomp(composition);
> -    if (precomp) {
> -      precomp->source_layers().emplace_back(i->first);
> -    } else {
> -      if (planes->size()) {
> -        DrmPlane *precomp_plane = planes->back();
> -        planes->pop_back();
> -        composition-
> >emplace_back(DrmCompositionPlane::Type::kPrecomp,
> -                                  precomp_plane, crtc, i->first);
> -      } else {
> -        ALOGE("Not enough planes to reserve for precomp fb");
> -      }
> -    }
> -    i = layers.erase(i);
> -  }
> -  return 0;
> -}
> -
> -bool PlanStageNvLimits::CheckLayer(size_t zorder, DrmHwcLayer
> *layer) {
> -    auto src_w = layer->source_crop.width();
> -    auto src_h = layer->source_crop.height();
> -    auto dst_w = layer->display_frame.width();
> -    auto dst_h = layer->display_frame.height();
> -    int h_limit = 4;
> -    int v_limit;
> -
> -    switch (layer->buffer->format) {
> -      case DRM_FORMAT_ARGB8888:
> -      case DRM_FORMAT_ABGR8888:
> -      case DRM_FORMAT_XBGR8888:
> -      case DRM_FORMAT_XRGB8888:
> -        // tegra driver assumes any layer with alpha channel has
> premult
> -        // blending, avoid handling it this is not the case. This is
> not an
> -        // issue for bottom-most layer since there's nothing to
> blend with
> -        if (zorder > 0 && layer->blending !=
> DrmHwcBlending::kPreMult)
> -          return false;
> -
> -        v_limit = 2;
> -        break;
> -      case DRM_FORMAT_YVU420:
> -      case DRM_FORMAT_YUV420:
> -      case DRM_FORMAT_YUV422:
> -      case DRM_FORMAT_UYVY:
> -      case DRM_FORMAT_YUYV:
> -      case DRM_FORMAT_NV12:
> -      case DRM_FORMAT_NV21:
> -      case DRM_FORMAT_RGB565:
> -      case DRM_FORMAT_BGR565:
> -        v_limit = 4;
> -        break;
> -      default:
> -        v_limit = 2;
> -        break;
> -    }
> -
> -    if (layer->transform &
> -        (DrmHwcTransform::kRotate90 | DrmHwcTransform::kRotate270))
> -      std::swap(dst_w, dst_h);
> -
> -    // check for max supported down scaling
> -    if (((src_w / dst_w) > h_limit) || ((src_h / dst_h) > v_limit))
> -      return false;
> -
> -    return true;
> -}
> -
> -int PlanStageNvLimits::ProvisionPlanes(
> -    std::vector<DrmCompositionPlane> *composition,
> -    std::map<size_t, DrmHwcLayer *> &layers, DrmCrtc *crtc,
> -    std::vector<DrmPlane *> *planes) {
> -  int ret;
> -
> -  for (auto i = layers.begin(); i != layers.end();) {
> -    // Skip layer if supported
> -    if (CheckLayer(i->first, i->second)) {
> -      i++;
> -      continue;
> -    }
> -
> -    if (i->second->protected_usage()) {
> -      // Drop the layer if unsupported and protected, this will just
> display
> -      // black in the area of this layer but it's better than
> failing miserably
> -      i = layers.erase(i);
> -      continue;
> -    }
> -
> -    // If there's no precomp layer already queued, queue one now.
> -    DrmCompositionPlane *precomp = GetPrecomp(composition);
> -    if (precomp) {
> -      precomp->source_layers().emplace_back(i->first);
> -    } else if (!planes->empty()) {
> -      DrmPlane *precomp_plane = planes->back();
> -      planes->pop_back();
> -      composition->emplace_back(DrmCompositionPlane::Type::kPrecomp,
> -                                precomp_plane, crtc, i->first);
> -    } else {
> -      ALOGE("Not enough planes to reserve for precomp fb");
> -    }
> -    i = layers.erase(i);
> -  }
> -
> -  return 0;
> -}
> -}

On Fri, Oct 06, 2017 at 05:02:03PM -0500, Rob Herring wrote:
> There's no opensource implementation for the NVIDIA gralloc implementation,

> so remove it as it is not testable.

> 

> As all of the gralloc perform() operations are specific to it, they can be

> removed, too.

> 

> Signed-off-by: Rob Herring <robh@kernel.org>

> ---

>  Android.mk      |   5 -

>  drmhwcgralloc.h |  33 -----

>  platformnv.cpp  | 374 --------------------------------------------------------

>  3 files changed, 412 deletions(-)

>  delete mode 100644 platformnv.cpp


This is in agreement with other parts of the open source graphics stack,
so:

Acked-by: Thierry Reding <treding@nvidia.com>


If anyone wants to revive this, they'd have to provide an open source
implementation of gralloc first, and then this commit could always be
reverted.

Thierry

Hey Thierry,

On Mon, 2017-10-09 at 15:07 +0200, Thierry Reding wrote:
> On Fri, Oct 06, 2017 at 05:02:03PM -0500, Rob Herring wrote:
> > There's no opensource implementation for the NVIDIA gralloc
> > implementation,
> > so remove it as it is not testable.
> > 
> > As all of the gralloc perform() operations are specific to it, they
> > can be
> > removed, too.
> > 
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> >  Android.mk      |   5 -
> >  drmhwcgralloc.h |  33 -----
> >  platformnv.cpp  | 374 --------------------------------------------
> > ------------
> >  3 files changed, 412 deletions(-)
> >  delete mode 100644 platformnv.cpp
> 
> This is in agreement with other parts of the open source graphics
> stack,
> so:
> 
> Acked-by: Thierry Reding <treding@nvidia.com>
> 
> If anyone wants to revive this, they'd have to provide an open source
> implementation of gralloc first, and then this commit could always be
> reverted.
> 
> Thierry

Excellent, thanks for the quick reply.


Rob.

Hey again,

Just pushed this patch.


Rob.

On Mon, 2017-10-09 at 15:58 +0200, Robert Foss wrote:
> Hey Thierry,
> 
> On Mon, 2017-10-09 at 15:07 +0200, Thierry Reding wrote:
> > On Fri, Oct 06, 2017 at 05:02:03PM -0500, Rob Herring wrote:
> > > There's no opensource implementation for the NVIDIA gralloc
> > > implementation,
> > > so remove it as it is not testable.
> > > 
> > > As all of the gralloc perform() operations are specific to it,
> > > they
> > > can be
> > > removed, too.
> > > 
> > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > ---
> > >  Android.mk      |   5 -
> > >  drmhwcgralloc.h |  33 -----
> > >  platformnv.cpp  | 374 ------------------------------------------
> > > --
> > > ------------
> > >  3 files changed, 412 deletions(-)
> > >  delete mode 100644 platformnv.cpp
> > 
> > This is in agreement with other parts of the open source graphics
> > stack,
> > so:
> > 
> > Acked-by: Thierry Reding <treding@nvidia.com>
> > 
> > If anyone wants to revive this, they'd have to provide an open
> > source
> > implementation of gralloc first, and then this commit could always
> > be
> > reverted.
> > 
> > Thierry
> 
> Excellent, thanks for the quick reply.
> 
> 
> Rob.