drm_hwcomposer: Cache FB and gem in gralloc buffer
Move drm fb object creation to the importer.
In NV importer, use GRALLOC_MODULE_PERFORM_SET_IMPORTER_PRIVATE
and GRALLOC_MODULE_PERFORM_GET_IMPORTER_PRIVATE to cache the hwc_drm_bo
in the gralloc buffer. This avoids the need to recreate fb objects
again each frame, and also greatly simplifies managing the lifetime of
the gem handles.
The drm_gralloc importer does not support caching at this point. In an
attempt to keep it somewhat working, add hwc_import_bo_release function
that hwc calls when it's ok to rm fb. If hwc_import_bo_release returns
true, then hwc will clean up gem handles as it did before.
We should consider doing a follow-up patch that adds fb/gem caching to
drm_gralloc importer also. Then some of the code that is kept in this
patch for backwards compatibility can be removed.
Change-Id: I92857dcaddf8cea219ebc041e16ef76087a1b696
Reviewed-on: https://chrome-internal-review.googlesource.com/200895
Reviewed-by: Stéphane Marchesin <marcheu@google.com>
Commit-Queue: Stéphane Marchesin <marcheu@google.com>
Tested-by: Stéphane Marchesin <marcheu@google.com>
diff --git a/drm_hwcomposer.h b/drm_hwcomposer.h
index 35b3aa9..b94e5dd 100644
--- a/drm_hwcomposer.h
+++ b/drm_hwcomposer.h
@@ -17,7 +17,29 @@
struct hwc_import_context;
enum {
- GRALLOC_MODULE_PERFORM_DRM_IMPORT = 0xffeeff00
+ /* 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,
};
struct hwc_drm_bo {
@@ -29,13 +51,12 @@
uint32_t gem_handles[4];
uint32_t fb_id;
int acquire_fence_fd;
-
- /* TODO: This is bad voodoo, remove it once drm_gralloc uses dma_buf */
- int importer_fd;
};
int hwc_import_init(struct hwc_import_context **ctx);
int hwc_import_destroy(struct hwc_import_context *ctx);
-int hwc_create_bo_from_import(int fd, struct hwc_import_context *ctx,
+int hwc_import_bo_create(int fd, struct hwc_import_context *ctx,
buffer_handle_t buf, struct hwc_drm_bo *bo);
+bool hwc_import_bo_release(int fd, hwc_import_context *ctx,
+ struct hwc_drm_bo *bo);
diff --git a/hwcomposer.cpp b/hwcomposer.cpp
index 3f20e53..7a4b302 100644
--- a/hwcomposer.cpp
+++ b/hwcomposer.cpp
@@ -296,45 +296,39 @@
return ret;
}
- memset(&args, 0, sizeof(args));
- for (i = 0; i < ARRAY_SIZE(hd->front.gem_handles); i++) {
- if (!hd->front.gem_handles[i])
- continue;
+ if (hwc_import_bo_release(hd->ctx->fd, hd->ctx->import_ctx, &hd->front)) {
+ memset(&args, 0, sizeof(args));
+ for (i = 0; i < ARRAY_SIZE(hd->front.gem_handles); i++) {
+ if (!hd->front.gem_handles[i])
+ continue;
- /* check for duplicate handle in buf_queue */
- bool found;
+ /* check for duplicate handle in buf_queue */
+ bool found;
- ret = pthread_mutex_lock(&hd->set_worker.lock);
- if (ret) {
- ALOGE("Failed to lock set lock in wait_and_set() %d", ret);
- continue;
- }
+ ret = pthread_mutex_lock(&hd->set_worker.lock);
+ if (ret) {
+ ALOGE("Failed to lock set lock in wait_and_set() %d", ret);
+ continue;
+ }
- found = false;
- for (std::list<struct hwc_drm_bo>::iterator bi = hd->buf_queue.begin();
- bi != hd->buf_queue.end();
- ++bi)
- for (int j = 0; j < ARRAY_SIZE(bi->gem_handles); j++)
- if (hd->front.gem_handles[i] == bi->gem_handles[j] )
+ found = false;
+ for (std::list<struct hwc_drm_bo>::iterator bi = hd->buf_queue.begin();
+ bi != hd->buf_queue.end();
+ ++bi)
+ for (int j = 0; j < ARRAY_SIZE(bi->gem_handles); j++)
+ if (hd->front.gem_handles[i] == bi->gem_handles[j] )
+ found = true;
+
+ for (int j = 0; j < ARRAY_SIZE(buf->gem_handles); j++)
+ if (hd->front.gem_handles[i] == buf->gem_handles[j])
found = true;
- for (int j = 0; j < ARRAY_SIZE(buf->gem_handles); j++)
- if (hd->front.gem_handles[i] == buf->gem_handles[j])
- found = true;
-
- if (!found) {
- args.handle = hd->front.gem_handles[i];
- drmIoctl(hd->ctx->fd, DRM_IOCTL_GEM_CLOSE, &args);
- }
- if (pthread_mutex_unlock(&hd->set_worker.lock))
- ALOGE("Failed to unlock set lock in wait_and_set() %d", ret);
- }
-
- if (hd->front.fb_id) {
- ret = drmModeRmFB(hd->ctx->fd, hd->front.fb_id);
- if (ret) {
- ALOGE("Failed to rm fb from front %d", ret);
- return ret;
+ if (!found) {
+ args.handle = hd->front.gem_handles[i];
+ drmIoctl(hd->ctx->fd, DRM_IOCTL_GEM_CLOSE, &args);
+ }
+ if (pthread_mutex_unlock(&hd->set_worker.lock))
+ ALOGE("Failed to unlock set lock in wait_and_set() %d", ret);
}
}
@@ -445,7 +439,7 @@
goto out;
}
- ret = hwc_create_bo_from_import(ctx->fd, ctx->import_ctx, layer->handle,
+ ret = hwc_import_bo_create(ctx->fd, ctx->import_ctx, layer->handle,
&buf);
if (ret) {
ALOGE("Failed to import handle to drm bo %d", ret);
@@ -454,14 +448,6 @@
buf.acquire_fence_fd = layer->acquireFenceFd;
layer->acquireFenceFd = -1;
- ret = drmModeAddFB2(hd->ctx->fd, buf.width,
- buf.height, buf.format, buf.gem_handles, buf.pitches,
- buf.offsets, &buf.fb_id, 0);
- if (ret) {
- ALOGE("could not create drm fb %d", ret);
- goto out;
- }
-
/*
* TODO: Retire and release can use the same sync point here b/c hwc is
* restricted to one layer. Once that is no longer true, this will need
diff --git a/hwcomposer_import_drm_gralloc.cpp b/hwcomposer_import_drm_gralloc.cpp
index e2e2e94..b724c83 100644
--- a/hwcomposer_import_drm_gralloc.cpp
+++ b/hwcomposer_import_drm_gralloc.cpp
@@ -85,7 +85,7 @@
}
}
-int hwc_create_bo_from_import(int fd, hwc_import_context *ctx,
+int hwc_import_bo_create(int fd, hwc_import_context *ctx,
buffer_handle_t handle, struct hwc_drm_bo *bo)
{
gralloc_drm_handle_t *gr_handle = gralloc_drm_handle(handle);
@@ -116,5 +116,26 @@
bo->gem_handles[0] = gem_handle;
bo->offsets[0] = 0;
- return 0;
+ ret = drmModeAddFB2(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;
+}
+
+bool hwc_import_bo_release(int fd, hwc_import_context *ctx,
+ struct hwc_drm_bo *bo)
+{
+ (void)ctx;
+
+ if (bo->fb_id)
+ if (drmModeRmFB(fd, bo->fb_id))
+ ALOGE("Failed to rm fb");
+
+ /* hwc may close the gem handles now. */
+ return true;
}
diff --git a/hwcomposer_import_nv_gralloc.cpp b/hwcomposer_import_nv_gralloc.cpp
index 4b9ad0f..f012aab 100644
--- a/hwcomposer_import_nv_gralloc.cpp
+++ b/hwcomposer_import_nv_gralloc.cpp
@@ -19,8 +19,12 @@
#include <cutils/log.h>
#include <hardware/gralloc.h>
+#include <xf86drm.h>
+#include <xf86drmMode.h>
#include "drm_hwcomposer.h"
+#define ARRAY_SIZE(arr) (int)(sizeof(arr) / sizeof((arr)[0]))
+
struct hwc_import_context {
const gralloc_module_t *gralloc_module;
};
@@ -63,11 +67,105 @@
return 0;
}
-int hwc_create_bo_from_import(int fd, hwc_import_context *ctx,
- buffer_handle_t handle, struct hwc_drm_bo *bo)
+struct importer_priv
{
+ int drm_fd;
+ struct hwc_drm_bo bo;
+};
+
+static void free_priv(void *p)
+{
+ struct importer_priv *priv = (struct importer_priv *)p;
+ struct drm_gem_close gem_close;
+ int i, ret;
+
+ if (priv->bo.fb_id) {
+ ret = drmModeRmFB(priv->drm_fd, priv->bo.fb_id);
+ if (ret)
+ ALOGE("Failed to rm fb %d", ret);
+ }
+
+ memset(&gem_close, 0, sizeof(gem_close));
+ for (i = 0; i < ARRAY_SIZE(priv->bo.gem_handles); i++) {
+ if (!priv->bo.gem_handles[i])
+ continue;
+ gem_close.handle = priv->bo.gem_handles[i];
+ ret = drmIoctl(priv->drm_fd, DRM_IOCTL_GEM_CLOSE, &gem_close);
+ if (ret)
+ ALOGE("Failed to close gem handle %d", ret);
+ }
+
+ free(priv);
+}
+
+static int
+hwc_import_set_priv(hwc_import_context *ctx, buffer_handle_t handle, struct importer_priv *priv)
+{
+ int ret;
const gralloc_module_t *g = ctx->gralloc_module;
- bo->importer_fd = -1;
- return g->perform(g, GRALLOC_MODULE_PERFORM_DRM_IMPORT, fd, handle, bo);
+ return g->perform(g, GRALLOC_MODULE_PERFORM_SET_IMPORTER_PRIVATE, handle, free_priv, priv);
+}
+
+static struct importer_priv *
+hwc_import_get_priv(hwc_import_context *ctx, buffer_handle_t handle)
+{
+ int ret;
+ void *priv = NULL;
+ const gralloc_module_t *g = ctx->gralloc_module;
+
+ ret = g->perform(g, GRALLOC_MODULE_PERFORM_GET_IMPORTER_PRIVATE, handle, free_priv, &priv);
+ return ret ? NULL : (struct importer_priv *)priv;
+}
+
+int hwc_import_bo_create(int fd, hwc_import_context *ctx,
+ buffer_handle_t handle, struct hwc_drm_bo *bo)
+{
+ int ret = 0;
+ const gralloc_module_t *g = ctx->gralloc_module;
+
+ /* Get imported bo that is cached in gralloc buffer, or create a new one. */
+ struct importer_priv *priv = hwc_import_get_priv(ctx, handle);
+ if (!priv) {
+ priv = (struct importer_priv *)calloc(1, sizeof(*priv));
+ if (!priv)
+ return -ENOMEM;
+ priv->drm_fd = fd;
+
+ ret = g->perform(g, GRALLOC_MODULE_PERFORM_DRM_IMPORT, fd, handle, &priv->bo);
+ if (ret) {
+ ALOGE("GRALLOC_MODULE_PERFORM_DRM_IMPORT failed %d", ret);
+ free_priv(priv);
+ return ret;
+ }
+
+ ret = drmModeAddFB2(fd, priv->bo.width, priv->bo.height,
+ priv->bo.format, priv->bo.gem_handles,
+ priv->bo.pitches, priv->bo.offsets,
+ &priv->bo.fb_id, 0);
+ if (ret) {
+ ALOGE("Failed to add fb %d", ret);
+ free_priv(priv);
+ return ret;
+ }
+
+ ret = hwc_import_set_priv(ctx, handle, priv);
+ 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);
+ free_priv(priv);
+ return ret;
+ }
+ }
+ *bo = priv->bo;
+ return ret;
+}
+
+bool hwc_import_bo_release(int fd, hwc_import_context *ctx,
+ struct hwc_drm_bo *bo)
+{
+ /* hwc may not close the gem handles, we own them */
+ return false;
}