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>

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 @@ struct hwc_drm_bo {
 	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);

hwcomposer.cpp

@@ -296,45 +296,39 @@ static int hwc_wait_and_set(struct hwc_drm_display *hd,
 		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 @@ static int hwc_set_display(hwc_context_t *ctx, int display,
 		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 @@ static int hwc_set_display(hwc_context_t *ctx, int display,
 	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

hwcomposer_import_drm_gralloc.cpp

@@ -85,7 +85,7 @@ static uint32_t hwc_convert_hal_format_to_drm_format(uint32_t hal_format)
 	}
 }
 
-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 @@ int hwc_create_bo_from_import(int fd, hwc_import_context *ctx,
 	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;
 }

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 @@ int hwc_import_destroy(struct hwc_import_context *ctx)
 	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;
 }