[PATCH v2 00/22] drm: Add GEM backed framebuffer library

[PATCH v2 00/22] drm: Add GEM backed framebuffer library

[Noralf Trønnes]

This adds helpers for drivers that don't subclass drm_framebuffer and
are backed by drm_gem_object(s). drm_fb_cma_helper is converted to use
the helpers.

In this second version I've pushed the new functions out to the cma
drivers, so I could clean up the cma library.

This patchset is part of a process to add a shmem gem library like the
cma library. The common parts between the two goes into core or helpers.

Noralf.

Noralf Trønnes (22):
  drm: Add GEM backed framebuffer library
  drm/fb-cma-helper: Use drm_gem_framebuffer_helper
  drm/tinydrm: Use drm_gem_framebuffer_helper
  drm/arc: Use drm_gem_fb_create()
  drm/arm/hdlcd: Use drm_gem_fb_create()
  drm/arm/mali: Use drm_gem_fb_create()
  drm/atmel-hlcdc: Use drm_gem_fb_create()
  drm/fsl-dcu: Use drm_gem_fb_create()
  drm/hisilicon/kirin: Use drm_gem_fb_create()
  drm/imx: Use drm_gem_fb_create() and drm_gem_fb_prepare_fb()
  drm/meson: Use drm_gem_fb_create()
  drm/mxsfb: Use drm_gem_fb_create() and drm_gem_fb_prepare_fb()
  drm/pl111: Use drm_gem_fb_create() and drm_gem_fb_prepare_fb()
  drm/rcar-du: Use drm_gem_fb_create()
  drm/shmobile: Use drm_gem_fb_create()
  drm/sti: Use drm_gem_fb_create()
  drm/stm: Use drm_gem_fb_create()
  drm/sun4i: Use drm_gem_fb_create()
  drm/tilcdc: Use drm_gem_fb_create()
  drm/vc4: Use drm_gem_fb_create()
  drm/zte: Use drm_gem_fb_create()
  drm/fb-cma-helper: Remove unused functions

 Documentation/gpu/drm-kms-helpers.rst           |   9 +
 drivers/gpu/drm/Makefile                        |   2 +-
 drivers/gpu/drm/arc/arcpgu_drv.c                |   3 +-
 drivers/gpu/drm/arm/hdlcd_drv.c                 |   3 +-
 drivers/gpu/drm/arm/malidp_drv.c                |   3 +-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    |   2 +-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h    |   1 +
 drivers/gpu/drm/drm_fb_cma_helper.c             | 239 +++-------------------
 drivers/gpu/drm/drm_gem_framebuffer_helper.c    | 252 ++++++++++++++++++++++++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c       |   3 +-
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |   3 +-
 drivers/gpu/drm/imx/imx-drm-core.c              |   3 +-
 drivers/gpu/drm/imx/ipuv3-plane.c               |   3 +-
 drivers/gpu/drm/meson/meson_drv.c               |   3 +-
 drivers/gpu/drm/mxsfb/mxsfb_drv.c               |   5 +-
 drivers/gpu/drm/pl111/pl111_display.c           |   3 +-
 drivers/gpu/drm/pl111/pl111_drv.c               |   3 +-
 drivers/gpu/drm/rcar-du/rcar_du_kms.c           |   3 +-
 drivers/gpu/drm/shmobile/shmob_drm_kms.c        |   3 +-
 drivers/gpu/drm/sti/sti_drv.c                   |   3 +-
 drivers/gpu/drm/stm/drv.c                       |   3 +-
 drivers/gpu/drm/sun4i/sun4i_framebuffer.c       |   3 +-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c             |   3 +-
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c     |   3 +-
 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c     |   5 +-
 drivers/gpu/drm/tinydrm/mipi-dbi.c              |   5 +-
 drivers/gpu/drm/tinydrm/repaper.c               |   5 +-
 drivers/gpu/drm/vc4/vc4_kms.c                   |   3 +-
 drivers/gpu/drm/zte/zx_drm_drv.c                |   3 +-
 include/drm/drm_fb_cma_helper.h                 |  13 --
 include/drm/drm_framebuffer.h                   |   4 +
 include/drm/drm_gem_framebuffer_helper.h        |  35 ++++
 32 files changed, 381 insertions(+), 253 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_gem_framebuffer_helper.c
 create mode 100644 include/drm/drm_gem_framebuffer_helper.h

-- 
2.7.4

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

[PATCH v2 01/22] drm: Add GEM backed framebuffer library

[Noralf Trønnes]

This library provides helpers for drivers that don't subclass
drm_framebuffer and are backed by drm_gem_object. The code is
taken from drm_fb_cma_helper.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 Documentation/gpu/drm-kms-helpers.rst        |   9 +
 drivers/gpu/drm/Makefile                     |   2 +-
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 252 +++++++++++++++++++++++++++
 include/drm/drm_framebuffer.h                |   4 +
 include/drm/drm_gem_framebuffer_helper.h     |  35 ++++
 5 files changed, 301 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_gem_framebuffer_helper.c
 create mode 100644 include/drm/drm_gem_framebuffer_helper.h

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 7c5e254..13dd237 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -296,3 +296,12 @@ Auxiliary Modeset Helpers
 
 .. kernel-doc:: drivers/gpu/drm/drm_modeset_helper.c
    :export:
+
+Framebuffer GEM Helper Reference
+================================
+
+.. kernel-doc:: drivers/gpu/drm/drm_gem_framebuffer_helper.c
+   :doc: overview
+
+.. kernel-doc:: drivers/gpu/drm/drm_gem_framebuffer_helper.c
+   :export:
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 24a066e..a8acc19 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -33,7 +33,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
 		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
 		drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
 		drm_simple_kms_helper.o drm_modeset_helper.o \
-		drm_scdc_helper.o
+		drm_scdc_helper.o drm_gem_framebuffer_helper.o
 
 drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
 drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
new file mode 100644
index 0000000..41a506c
--- /dev/null
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -0,0 +1,252 @@
+/*
+ * drm gem framebuffer helper functions
+ *
+ * Copyright (C) 2017 Noralf Trønnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/dma-buf.h>
+#include <linux/dma-fence.h>
+#include <linux/reservation.h>
+#include <linux/slab.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_gem.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_modeset_helper.h>
+
+/**
+ * DOC: overview
+ *
+ * This library provides helpers for drivers that don't subclass
+ * &drm_framebuffer and are backed by &drm_gem_object.
+ */
+
+/**
+ * drm_gem_fb_get_obj() - Get GEM object for framebuffer
+ * @fb: The framebuffer
+ * @plane: Which plane
+ *
+ * Returns the GEM object for given framebuffer.
+ */
+struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
+					  unsigned int plane)
+{
+	if (plane >= 4)
+		return NULL;
+
+	return fb->obj[plane];
+}
+EXPORT_SYMBOL_GPL(drm_gem_fb_get_obj);
+
+/**
+ * drm_gem_fb_alloc - Allocate GEM backed framebuffer
+ * @dev: DRM device
+ * @mode_cmd: metadata from the userspace fb creation request
+ * @obj: GEM object(s) backing the framebuffer
+ * @num_planes: Number of planes
+ * @funcs: vtable to be used for the new framebuffer object
+ *
+ * Returns:
+ * Allocated struct drm_framebuffer * or error encoded pointer.
+ */
+struct drm_framebuffer *
+drm_gem_fb_alloc(struct drm_device *dev,
+		 const struct drm_mode_fb_cmd2 *mode_cmd,
+		 struct drm_gem_object **obj, unsigned int num_planes,
+		 const struct drm_framebuffer_funcs *funcs)
+{
+	struct drm_framebuffer *fb;
+	int ret, i;
+
+	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
+	if (!fb)
+		return ERR_PTR(-ENOMEM);
+
+	drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
+
+	for (i = 0; i < num_planes; i++)
+		fb->obj[i] = obj[i];
+
+	ret = drm_framebuffer_init(dev, fb, funcs);
+	if (ret) {
+		DRM_DEV_ERROR(dev->dev, "Failed to init framebuffer: %d\n",
+			      ret);
+		kfree(fb);
+		return ERR_PTR(ret);
+	}
+
+	return fb;
+}
+EXPORT_SYMBOL(drm_gem_fb_alloc);
+
+/**
+ * drm_gem_fb_destroy - Free GEM backed framebuffer
+ * @fb: DRM framebuffer
+ *
+ * Frees a GEM backed framebuffer with it's backing buffer(s) and the structure
+ * itself. Drivers can use this as their &drm_framebuffer_funcs->destroy
+ * callback.
+ */
+void drm_gem_fb_destroy(struct drm_framebuffer *fb)
+{
+	int i;
+
+	for (i = 0; i < 4; i++) {
+		if (fb->obj[i])
+			drm_gem_object_put_unlocked(fb->obj[i]);
+	}
+
+	drm_framebuffer_cleanup(fb);
+	kfree(fb);
+}
+EXPORT_SYMBOL(drm_gem_fb_destroy);
+
+/**
+ * drm_gem_fb_create_handle - Create handle for GEM backed framebuffer
+ * @fb: DRM framebuffer
+ * @file: drm file
+ * @handle: handle created
+ *
+ * Drivers can use this as their &drm_framebuffer_funcs->create_handle
+ * callback.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
+			     unsigned int *handle)
+{
+	return drm_gem_handle_create(file, fb->obj[0], handle);
+}
+EXPORT_SYMBOL(drm_gem_fb_create_handle);
+
+/**
+ * drm_gem_fb_create_with_funcs() - helper function for the
+ *                                  &drm_mode_config_funcs.fb_create
+ *                                  callback
+ * @dev: DRM device
+ * @file: drm file for the ioctl call
+ * @mode_cmd: metadata from the userspace fb creation request
+ * @funcs: vtable to be used for the new framebuffer object
+ *
+ * This can be used to set &drm_framebuffer_funcs for drivers that need the
+ * &drm_framebuffer_funcs.dirty callback. Use drm_gem_fb_create() if you don't
+ * need to change &drm_framebuffer_funcs.
+ */
+struct drm_framebuffer *
+drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
+			     const struct drm_mode_fb_cmd2 *mode_cmd,
+			     const struct drm_framebuffer_funcs *funcs)
+{
+	const struct drm_format_info *info;
+	struct drm_gem_object *objs[4];
+	struct drm_framebuffer *fb;
+	int ret, i;
+
+	info = drm_get_format_info(dev, mode_cmd);
+	if (!info)
+		return ERR_PTR(-EINVAL);
+
+	for (i = 0; i < info->num_planes; i++) {
+		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
+		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
+		unsigned int min_size;
+
+		objs[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]);
+		if (!objs[i]) {
+			DRM_DEV_ERROR(dev->dev, "Failed to lookup GEM\n");
+			ret = -ENOENT;
+			goto err_gem_object_put;
+		}
+
+		min_size = (height - 1) * mode_cmd->pitches[i]
+			 + width * info->cpp[i]
+			 + mode_cmd->offsets[i];
+
+		if (objs[i]->size < min_size) {
+			drm_gem_object_put_unlocked(objs[i]);
+			ret = -EINVAL;
+			goto err_gem_object_put;
+		}
+	}
+
+	fb = drm_gem_fb_alloc(dev, mode_cmd, objs, i, funcs);
+	if (IS_ERR(fb)) {
+		ret = PTR_ERR(fb);
+		goto err_gem_object_put;
+	}
+
+	return fb;
+
+err_gem_object_put:
+	for (i--; i >= 0; i--)
+		drm_gem_object_put_unlocked(objs[i]);
+
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_funcs);
+
+static const struct drm_framebuffer_funcs drm_gem_fb_funcs = {
+	.destroy	= drm_gem_fb_destroy,
+	.create_handle	= drm_gem_fb_create_handle,
+};
+
+/**
+ * drm_gem_fb_create() - &drm_mode_config_funcs.fb_create callback function
+ * @dev: DRM device
+ * @file: drm file for the ioctl call
+ * @mode_cmd: metadata from the userspace fb creation request
+ *
+ * If your hardware has special alignment or pitch requirements these should be
+ * checked before calling this function. Use drm_gem_fb_create_with_funcs() if
+ * you need to set &drm_framebuffer_funcs.dirty.
+ */
+struct drm_framebuffer *
+drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
+		  const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+	return drm_gem_fb_create_with_funcs(dev, file, mode_cmd,
+					    &drm_gem_fb_funcs);
+}
+EXPORT_SYMBOL_GPL(drm_gem_fb_create);
+
+/**
+ * drm_gem_fb_prepare_fb() - Prepare gem framebuffer
+ * @plane: Which plane
+ * @state: Plane state attach fence to
+ *
+ * This should be set as the &struct drm_plane_helper_funcs.prepare_fb hook.
+ *
+ * This function checks if the plane FB has an dma-buf attached, extracts
+ * the exclusive fence and attaches it to plane state for the atomic helper
+ * to wait on.
+ *
+ * There is no need for cleanup_fb for gem based framebuffer drivers.
+ */
+int drm_gem_fb_prepare_fb(struct drm_plane *plane,
+			  struct drm_plane_state *state)
+{
+	struct dma_buf *dma_buf;
+	struct dma_fence *fence;
+
+	if ((plane->state->fb == state->fb) || !state->fb)
+		return 0;
+
+	dma_buf = drm_gem_fb_get_obj(state->fb, 0)->dma_buf;
+	if (dma_buf) {
+		fence = reservation_object_get_excl_rcu(dma_buf->resv);
+		drm_atomic_set_fence_for_plane(state, fence);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb);
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
index 5244f05..50def22 100644
--- a/include/drm/drm_framebuffer.h
+++ b/include/drm/drm_framebuffer.h
@@ -190,6 +190,10 @@ struct drm_framebuffer {
 	 * @filp_head: Placed on &drm_file.fbs, protected by &drm_file.fbs_lock.
 	 */
 	struct list_head filp_head;
+	/**
+	 * @obj: GEM objects backing the framebuffer, one per plane (optional).
+	 */
+	struct drm_gem_object *obj[4];
 };
 
 #define obj_to_fb(x) container_of(x, struct drm_framebuffer, base)
diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
new file mode 100644
index 0000000..af3d1ee
--- /dev/null
+++ b/include/drm/drm_gem_framebuffer_helper.h
@@ -0,0 +1,35 @@
+#ifndef __DRM_GEM_FB_HELPER_H__
+#define __DRM_GEM_FB_HELPER_H__
+
+struct drm_device;
+struct drm_file;
+struct drm_framebuffer;
+struct drm_framebuffer_funcs;
+struct drm_gem_object;
+struct drm_mode_fb_cmd2;
+struct drm_plane;
+struct drm_plane_state;
+
+struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
+					  unsigned int plane);
+struct drm_framebuffer *
+drm_gem_fb_alloc(struct drm_device *dev,
+		 const struct drm_mode_fb_cmd2 *mode_cmd,
+		 struct drm_gem_object **obj, unsigned int num_planes,
+		 const struct drm_framebuffer_funcs *funcs);
+void drm_gem_fb_destroy(struct drm_framebuffer *fb);
+int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
+			     unsigned int *handle);
+
+struct drm_framebuffer *
+drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
+			     const struct drm_mode_fb_cmd2 *mode_cmd,
+			     const struct drm_framebuffer_funcs *funcs);
+struct drm_framebuffer *
+drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
+		  const struct drm_mode_fb_cmd2 *mode_cmd);
+
+int drm_gem_fb_prepare_fb(struct drm_plane *plane,
+			  struct drm_plane_state *state);
+
+#endif
-- 
2.7.4

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

[PATCH v2 02/22] drm/fb-cma-helper: Use drm_gem_framebuffer_helper

[Noralf Trønnes]

Use the new drm_gem_framebuffer_helper who's code was copied
from this helper.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_fb_cma_helper.c | 172 +++++++-----------------------------
 1 file changed, 30 insertions(+), 142 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index ade319d..d0c0110 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -18,27 +18,17 @@
  */
 
 #include <drm/drmP.h>
-#include <drm/drm_atomic.h>
-#include <drm/drm_crtc.h>
 #include <drm/drm_fb_helper.h>
-#include <drm/drm_crtc_helper.h>
+#include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_fb_cma_helper.h>
-#include <linux/dma-buf.h>
-#include <linux/dma-mapping.h>
 #include <linux/module.h>
-#include <linux/reservation.h>
 
 #define DEFAULT_FBDEFIO_DELAY_MS 50
 
-struct drm_fb_cma {
-	struct drm_framebuffer		fb;
-	struct drm_gem_cma_object	*obj[4];
-};
-
 struct drm_fbdev_cma {
 	struct drm_fb_helper	fb_helper;
-	struct drm_fb_cma	*fb;
 	const struct drm_framebuffer_funcs *fb_funcs;
 };
 
@@ -90,69 +80,19 @@ static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper)
 	return container_of(helper, struct drm_fbdev_cma, fb_helper);
 }
 
-static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb)
-{
-	return container_of(fb, struct drm_fb_cma, fb);
-}
-
 void drm_fb_cma_destroy(struct drm_framebuffer *fb)
 {
-	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
-	int i;
-
-	for (i = 0; i < 4; i++) {
-		if (fb_cma->obj[i])
-			drm_gem_object_put_unlocked(&fb_cma->obj[i]->base);
-	}
-
-	drm_framebuffer_cleanup(fb);
-	kfree(fb_cma);
+	drm_gem_fb_destroy(fb);
 }
 EXPORT_SYMBOL(drm_fb_cma_destroy);
 
 int drm_fb_cma_create_handle(struct drm_framebuffer *fb,
 	struct drm_file *file_priv, unsigned int *handle)
 {
-	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
-
-	return drm_gem_handle_create(file_priv,
-			&fb_cma->obj[0]->base, handle);
+	return drm_gem_fb_create_handle(fb, file_priv, handle);
 }
 EXPORT_SYMBOL(drm_fb_cma_create_handle);
 
-static struct drm_framebuffer_funcs drm_fb_cma_funcs = {
-	.destroy	= drm_fb_cma_destroy,
-	.create_handle	= drm_fb_cma_create_handle,
-};
-
-static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
-	const struct drm_mode_fb_cmd2 *mode_cmd,
-	struct drm_gem_cma_object **obj,
-	unsigned int num_planes, const struct drm_framebuffer_funcs *funcs)
-{
-	struct drm_fb_cma *fb_cma;
-	int ret;
-	int i;
-
-	fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);
-	if (!fb_cma)
-		return ERR_PTR(-ENOMEM);
-
-	drm_helper_mode_fill_fb_struct(dev, &fb_cma->fb, mode_cmd);
-
-	for (i = 0; i < num_planes; i++)
-		fb_cma->obj[i] = obj[i];
-
-	ret = drm_framebuffer_init(dev, &fb_cma->fb, funcs);
-	if (ret) {
-		dev_err(dev->dev, "Failed to initialize framebuffer: %d\n", ret);
-		kfree(fb_cma);
-		return ERR_PTR(ret);
-	}
-
-	return fb_cma;
-}
-
 /**
  * drm_fb_cma_create_with_funcs() - helper function for the
  *                                  &drm_mode_config_funcs.fb_create
@@ -170,53 +110,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
 	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
 	const struct drm_framebuffer_funcs *funcs)
 {
-	const struct drm_format_info *info;
-	struct drm_fb_cma *fb_cma;
-	struct drm_gem_cma_object *objs[4];
-	struct drm_gem_object *obj;
-	int ret;
-	int i;
-
-	info = drm_get_format_info(dev, mode_cmd);
-	if (!info)
-		return ERR_PTR(-EINVAL);
-
-	for (i = 0; i < info->num_planes; i++) {
-		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
-		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
-		unsigned int min_size;
-
-		obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[i]);
-		if (!obj) {
-			dev_err(dev->dev, "Failed to lookup GEM object\n");
-			ret = -ENOENT;
-			goto err_gem_object_put;
-		}
-
-		min_size = (height - 1) * mode_cmd->pitches[i]
-			 + width * info->cpp[i]
-			 + mode_cmd->offsets[i];
-
-		if (obj->size < min_size) {
-			drm_gem_object_put_unlocked(obj);
-			ret = -EINVAL;
-			goto err_gem_object_put;
-		}
-		objs[i] = to_drm_gem_cma_obj(obj);
-	}
-
-	fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, funcs);
-	if (IS_ERR(fb_cma)) {
-		ret = PTR_ERR(fb_cma);
-		goto err_gem_object_put;
-	}
-
-	return &fb_cma->fb;
-
-err_gem_object_put:
-	for (i--; i >= 0; i--)
-		drm_gem_object_put_unlocked(&objs[i]->base);
-	return ERR_PTR(ret);
+	return drm_gem_fb_create_with_funcs(dev, file_priv, mode_cmd, funcs);
 }
 EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
 
@@ -233,8 +127,7 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
 struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
 	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)
 {
-	return drm_fb_cma_create_with_funcs(dev, file_priv, mode_cmd,
-					    &drm_fb_cma_funcs);
+	return drm_gem_fb_create(dev, file_priv, mode_cmd);
 }
 EXPORT_SYMBOL_GPL(drm_fb_cma_create);
 
@@ -250,12 +143,14 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_create);
 struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb,
 						  unsigned int plane)
 {
-	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
+	struct drm_gem_object *gem;
 
-	if (plane >= 4)
+	gem = drm_gem_fb_get_obj(fb, plane);
+	if (!gem)
 		return NULL;
 
-	return fb_cma->obj[plane];
+	return to_drm_gem_cma_obj(gem);
+
 }
 EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj);
 
@@ -272,13 +167,14 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb,
 				   struct drm_plane_state *state,
 				   unsigned int plane)
 {
-	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
+	struct drm_gem_cma_object *obj;
 	dma_addr_t paddr;
 
-	if (plane >= 4)
+	obj = drm_fb_cma_get_gem_obj(fb, plane);
+	if (!obj)
 		return 0;
 
-	paddr = fb_cma->obj[plane]->paddr + fb->offsets[plane];
+	paddr = obj->paddr + fb->offsets[plane];
 	paddr += fb->format->cpp[plane] * (state->src_x >> 16);
 	paddr += fb->pitches[plane] * (state->src_y >> 16);
 
@@ -302,26 +198,13 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_addr);
 int drm_fb_cma_prepare_fb(struct drm_plane *plane,
 			  struct drm_plane_state *state)
 {
-	struct dma_buf *dma_buf;
-	struct dma_fence *fence;
-
-	if ((plane->state->fb == state->fb) || !state->fb)
-		return 0;
-
-	dma_buf = drm_fb_cma_get_gem_obj(state->fb, 0)->base.dma_buf;
-	if (dma_buf) {
-		fence = reservation_object_get_excl_rcu(dma_buf->resv);
-		drm_atomic_set_fence_for_plane(state, fence);
-	}
-
-	return 0;
+	return drm_gem_fb_prepare_fb(plane, state);
 }
 EXPORT_SYMBOL_GPL(drm_fb_cma_prepare_fb);
 
 #ifdef CONFIG_DEBUG_FS
 static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
 {
-	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
 	int i;
 
 	seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height,
@@ -330,7 +213,7 @@ static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
 	for (i = 0; i < fb->format->num_planes; i++) {
 		seq_printf(m, "   %d: offset=%d pitch=%d, obj: ",
 				i, fb->offsets[i], fb->pitches[i]);
-		drm_gem_cma_describe(fb_cma->obj[i], m);
+		drm_gem_cma_describe(drm_fb_cma_get_gem_obj(fb, i), m);
 	}
 }
 
@@ -434,6 +317,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
 	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
 	struct drm_device *dev = helper->dev;
 	struct drm_gem_cma_object *obj;
+	struct drm_gem_object *gem;
 	struct drm_framebuffer *fb;
 	unsigned int bytes_per_pixel;
 	unsigned long offset;
@@ -464,15 +348,14 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
 		goto err_gem_free_object;
 	}
 
-	fbdev_cma->fb = drm_fb_cma_alloc(dev, &mode_cmd, &obj, 1,
-					 fbdev_cma->fb_funcs);
-	if (IS_ERR(fbdev_cma->fb)) {
+	gem = &obj->base;
+	fb = drm_gem_fb_alloc(dev, &mode_cmd, &gem, 1, fbdev_cma->fb_funcs);
+	if (IS_ERR(fb)) {
 		dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
-		ret = PTR_ERR(fbdev_cma->fb);
+		ret = PTR_ERR(fb);
 		goto err_fb_info_destroy;
 	}
 
-	fb = &fbdev_cma->fb->fb;
 	helper->fb = fb;
 
 	fbi->par = helper;
@@ -500,7 +383,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
 	return 0;
 
 err_cma_destroy:
-	drm_framebuffer_remove(&fbdev_cma->fb->fb);
+	drm_framebuffer_remove(fb);
 err_fb_info_destroy:
 	drm_fb_helper_fini(helper);
 err_gem_free_object:
@@ -570,6 +453,11 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
 }
 EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs);
 
+static const struct drm_framebuffer_funcs drm_fb_cma_funcs = {
+	.destroy	= drm_gem_fb_destroy,
+	.create_handle	= drm_gem_fb_create_handle,
+};
+
 /**
  * drm_fbdev_cma_init() - Allocate and initializes a drm_fbdev_cma struct
  * @dev: DRM device
@@ -597,8 +485,8 @@ void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
 	if (fbdev_cma->fb_helper.fbdev)
 		drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
 
-	if (fbdev_cma->fb)
-		drm_framebuffer_remove(&fbdev_cma->fb->fb);
+	if (fbdev_cma->fb_helper.fb)
+		drm_framebuffer_remove(fbdev_cma->fb_helper.fb);
 
 	drm_fb_helper_fini(&fbdev_cma->fb_helper);
 	kfree(fbdev_cma);
-- 
2.7.4

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

[PATCH v2 03/22] drm/tinydrm: Use drm_gem_framebuffer_helper

[Noralf Trønnes]

Use drm_gem_framebuffer_helper directly instead of the cma
library wrappers.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 3 ++-
 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 5 +++--
 drivers/gpu/drm/tinydrm/mipi-dbi.c          | 5 +++--
 drivers/gpu/drm/tinydrm/repaper.c           | 5 +++--
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
index 551709e..1a8a57c 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
@@ -10,6 +10,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/tinydrm/tinydrm.h>
 #include <linux/device.h>
 #include <linux/dma-buf.h>
@@ -128,7 +129,7 @@ tinydrm_fb_create(struct drm_device *drm, struct drm_file *file_priv,
 {
 	struct tinydrm_device *tdev = drm->dev_private;
 
-	return drm_fb_cma_create_with_funcs(drm, file_priv, mode_cmd,
+	return drm_gem_fb_create_with_funcs(drm, file_priv, mode_cmd,
 					    tdev->fb_funcs);
 }
 
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
index ec43fb7..f4fddec 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
@@ -9,6 +9,7 @@
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_modes.h>
 #include <drm/tinydrm/tinydrm.h>
 
@@ -145,7 +146,7 @@ EXPORT_SYMBOL(tinydrm_display_pipe_update);
  * @pipe: Simple display pipe
  * @plane_state: Plane state
  *
- * This function uses drm_fb_cma_prepare_fb() to check if the plane FB has an
+ * This function uses drm_gem_fb_prepare_fb() to check if the plane FB has an
  * dma-buf attached, extracts the exclusive fence and attaches it to plane
  * state for the atomic helper to wait on. Drivers can use this as their
  * &drm_simple_display_pipe_funcs->prepare_fb callback.
@@ -153,7 +154,7 @@ EXPORT_SYMBOL(tinydrm_display_pipe_update);
 int tinydrm_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
 				    struct drm_plane_state *plane_state)
 {
-	return drm_fb_cma_prepare_fb(&pipe->plane, plane_state);
+	return drm_gem_fb_prepare_fb(&pipe->plane, plane_state);
 }
 EXPORT_SYMBOL(tinydrm_display_pipe_prepare_fb);
 
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index c83eeb7..466fab7 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -9,6 +9,7 @@
  * (at your option) any later version.
  */
 
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/tinydrm/mipi-dbi.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
 #include <linux/debugfs.h>
@@ -253,8 +254,8 @@ static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
 }
 
 static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs = {
-	.destroy	= drm_fb_cma_destroy,
-	.create_handle	= drm_fb_cma_create_handle,
+	.destroy	= drm_gem_fb_destroy,
+	.create_handle	= drm_gem_fb_create_handle,
 	.dirty		= mipi_dbi_fb_dirty,
 };
 
diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
index 3343d3f..bdbc1c7 100644
--- a/drivers/gpu/drm/tinydrm/repaper.c
+++ b/drivers/gpu/drm/tinydrm/repaper.c
@@ -25,6 +25,7 @@
 #include <linux/spi/spi.h>
 #include <linux/thermal.h>
 
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/tinydrm/tinydrm.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
 
@@ -614,8 +615,8 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb,
 }
 
 static const struct drm_framebuffer_funcs repaper_fb_funcs = {
-	.destroy	= drm_fb_cma_destroy,
-	.create_handle	= drm_fb_cma_create_handle,
+	.destroy	= drm_gem_fb_destroy,
+	.create_handle	= drm_gem_fb_create_handle,
 	.dirty		= repaper_fb_dirty,
 };
 
-- 
2.7.4

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

[PATCH v2 04/22] drm/arc: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Alexey Brodkin <abrodkin@synopsys.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/arc/arcpgu_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index 415b7da..088fd51 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -18,6 +18,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_atomic_helper.h>
 #include <linux/of_reserved_mem.h>
 
@@ -32,7 +33,7 @@ static void arcpgu_fb_output_poll_changed(struct drm_device *dev)
 }
 
 static struct drm_mode_config_funcs arcpgu_drm_modecfg_funcs = {
-	.fb_create  = drm_fb_cma_create,
+	.fb_create  = drm_gem_fb_create,
 	.output_poll_changed = arcpgu_fb_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
-- 
2.7.4

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

[PATCH v2 05/22] drm/arm/hdlcd: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Brian Starkey <brian.starkey@arm.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/arm/hdlcd_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index f9bda7b..764d0c8 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -25,6 +25,7 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_of.h>
 
 #include "hdlcd_drv.h"
@@ -106,7 +107,7 @@ static void hdlcd_fb_output_poll_changed(struct drm_device *drm)
 }
 
 static const struct drm_mode_config_funcs hdlcd_mode_config_funcs = {
-	.fb_create = drm_fb_cma_create,
+	.fb_create = drm_gem_fb_create,
 	.output_poll_changed = hdlcd_fb_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
-- 
2.7.4

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

[PATCH v2 06/22] drm/arm/mali: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Brian Starkey <brian.starkey@arm.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/arm/malidp_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 1a57cc2..b894466 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -26,6 +26,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_of.h>
 
 #include "malidp_drv.h"
@@ -249,7 +250,7 @@ static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = {
 };
 
 static const struct drm_mode_config_funcs malidp_mode_config_funcs = {
-	.fb_create = drm_fb_cma_create,
+	.fb_create = drm_gem_fb_create,
 	.output_poll_changed = malidp_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
-- 
2.7.4

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

[PATCH v2 07/22] drm/atmel-hlcdc: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 74d66e1..c6e8061 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -458,7 +458,7 @@ static irqreturn_t atmel_hlcdc_dc_irq_handler(int irq, void *data)
 static struct drm_framebuffer *atmel_hlcdc_fb_create(struct drm_device *dev,
 		struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)
 {
-	return drm_fb_cma_create(dev, file_priv, mode_cmd);
+	return drm_gem_fb_create(dev, file_priv, mode_cmd);
 }
 
 static void atmel_hlcdc_fb_output_poll_changed(struct drm_device *dev)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index 4237b04..6833ee2 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -34,6 +34,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drmP.h>
-- 
2.7.4

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

[PATCH v2 08/22] drm/fsl-dcu: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Stefan Agner <stefan@agner.ch>
Cc: Alison Wang <alison.wang@freescale.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c
index d9d6cc1..ddc68e4 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c
@@ -13,6 +13,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 
 #include "fsl_dcu_drm_crtc.h"
 #include "fsl_dcu_drm_drv.h"
@@ -20,7 +21,7 @@
 static const struct drm_mode_config_funcs fsl_dcu_drm_mode_config_funcs = {
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
-	.fb_create = drm_fb_cma_create,
+	.fb_create = drm_gem_fb_create,
 };
 
 int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev)
-- 
2.7.4

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

[PATCH v2 09/22] drm/hisilicon/kirin: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Xinliang Liu <z.liuxinliang@hisilicon.com>
Cc: Rongrong Zou <zourongrong@gmail.com>
Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Cc: Chen Feng <puck.chen@hisilicon.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index 79fcce7..7a06d3b 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -22,6 +22,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_of.h>
@@ -56,7 +57,7 @@ static void kirin_fbdev_output_poll_changed(struct drm_device *dev)
 }
 
 static const struct drm_mode_config_funcs kirin_drm_mode_config_funcs = {
-	.fb_create = drm_fb_cma_create,
+	.fb_create = drm_gem_fb_create,
 	.output_poll_changed = kirin_fbdev_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
-- 
2.7.4

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

[PATCH v2 10/22] drm/imx: Use drm_gem_fb_create() and drm_gem_fb_prepare_fb()

[Noralf Trønnes]

drm_fb_cma_create() and drm_fb_cma_prepare_fb() are just wrappers now,
use drm_gem_fb_create() and drm_gem_fb_prepare_fb() directly.

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/imx/imx-drm-core.c | 3 ++-
 drivers/gpu/drm/imx/ipuv3-plane.c  | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index f91cb72..93c7e3f 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -24,6 +24,7 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_of.h>
@@ -105,7 +106,7 @@ static int imx_drm_atomic_check(struct drm_device *dev,
 }
 
 static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = {
-	.fb_create = drm_fb_cma_create,
+	.fb_create = drm_gem_fb_create,
 	.output_poll_changed = imx_drm_output_poll_changed,
 	.atomic_check = imx_drm_atomic_check,
 	.atomic_commit = drm_atomic_helper_commit,
diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c
index 0847cc0..86e86df 100644
--- a/drivers/gpu/drm/imx/ipuv3-plane.c
+++ b/drivers/gpu/drm/imx/ipuv3-plane.c
@@ -18,6 +18,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_plane_helper.h>
 
 #include "video/imx-ipu-v3.h"
@@ -661,7 +662,7 @@ static void ipu_plane_atomic_update(struct drm_plane *plane,
 }
 
 static const struct drm_plane_helper_funcs ipu_plane_helper_funcs = {
-	.prepare_fb = drm_fb_cma_prepare_fb,
+	.prepare_fb = drm_gem_fb_prepare_fb,
 	.atomic_check = ipu_plane_atomic_check,
 	.atomic_disable = ipu_plane_atomic_disable,
 	.atomic_update = ipu_plane_atomic_update,
-- 
2.7.4

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

[PATCH v2 11/22] drm/meson: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/meson/meson_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 5375e6d..5eaf9a4 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -34,6 +34,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_rect.h>
 #include <drm/drm_fb_helper.h>
@@ -78,7 +79,7 @@ static const struct drm_mode_config_funcs meson_mode_config_funcs = {
 	.output_poll_changed = meson_fb_output_poll_changed,
 	.atomic_check        = drm_atomic_helper_check,
 	.atomic_commit       = drm_atomic_helper_commit,
-	.fb_create           = drm_fb_cma_create,
+	.fb_create           = drm_gem_fb_create,
 };
 
 static irqreturn_t meson_irq(int irq, void *arg)
-- 
2.7.4

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

[PATCH v2 12/22] drm/mxsfb: Use drm_gem_fb_create() and drm_gem_fb_prepare_fb()

[Noralf Trønnes]

drm_fb_cma_create() and drm_fb_cma_prepare_fb() are just wrappers now,
use drm_gem_fb_create() and drm_gem_fb_prepare_fb() directly.

Cc: Marek Vasut <marex@denx.de>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index a34f41c..cbdca43 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -35,6 +35,7 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_of.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_simple_kms_helper.h>
@@ -92,7 +93,7 @@ void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb)
 }
 
 static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = {
-	.fb_create		= drm_fb_cma_create,
+	.fb_create		= drm_gem_fb_create,
 	.atomic_check		= drm_atomic_helper_check,
 	.atomic_commit		= drm_atomic_helper_commit,
 };
@@ -127,7 +128,7 @@ static void mxsfb_pipe_update(struct drm_simple_display_pipe *pipe,
 static int mxsfb_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
 				 struct drm_plane_state *plane_state)
 {
-	return drm_fb_cma_prepare_fb(&pipe->plane, plane_state);
+	return drm_gem_fb_prepare_fb(&pipe->plane, plane_state);
 }
 
 static struct drm_simple_display_pipe_funcs mxsfb_funcs = {
-- 
2.7.4

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

[PATCH v2 13/22] drm/pl111: Use drm_gem_fb_create() and drm_gem_fb_prepare_fb()

[Noralf Trønnes]

drm_fb_cma_create() and drm_fb_cma_prepare_fb() are just wrappers now,
use drm_gem_fb_create() and drm_gem_fb_prepare_fb() directly.

Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/pl111/pl111_display.c | 3 ++-
 drivers/gpu/drm/pl111/pl111_drv.c     | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_display.c b/drivers/gpu/drm/pl111/pl111_display.c
index c6ca4f1b..1ca2630 100644
--- a/drivers/gpu/drm/pl111/pl111_display.c
+++ b/drivers/gpu/drm/pl111/pl111_display.c
@@ -23,6 +23,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_panel.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 
 #include "pl111_drm.h"
@@ -274,7 +275,7 @@ void pl111_disable_vblank(struct drm_device *drm, unsigned int crtc)
 static int pl111_display_prepare_fb(struct drm_simple_display_pipe *pipe,
 				    struct drm_plane_state *plane_state)
 {
-	return drm_fb_cma_prepare_fb(&pipe->plane, plane_state);
+	return drm_gem_fb_prepare_fb(&pipe->plane, plane_state);
 }
 
 static const struct drm_simple_display_pipe_funcs pl111_display_funcs = {
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index 29653fe..34e8f11 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -66,6 +66,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 
 #include "pl111_drm.h"
@@ -73,7 +74,7 @@
 #define DRIVER_DESC      "DRM module for PL111"
 
 static struct drm_mode_config_funcs mode_config_funcs = {
-	.fb_create = drm_fb_cma_create,
+	.fb_create = drm_gem_fb_create,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
-- 
2.7.4

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

[PATCH v2 14/22] drm/rcar-du: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/rcar-du/rcar_du_kms.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index 5d681ea..de0691a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -18,6 +18,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 
 #include <linux/of_graph.h>
 #include <linux/wait.h>
@@ -211,7 +212,7 @@ rcar_du_fb_create(struct drm_device *dev, struct drm_file *file_priv,
 		}
 	}
 
-	return drm_fb_cma_create(dev, file_priv, mode_cmd);
+	return drm_gem_fb_create(dev, file_priv, mode_cmd);
 }
 
 static void rcar_du_output_poll_changed(struct drm_device *dev)
-- 
2.7.4

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

[PATCH v2 15/22] drm/shmobile: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/shmobile/shmob_drm_kms.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/shmobile/shmob_drm_kms.c b/drivers/gpu/drm/shmobile/shmob_drm_kms.c
index 388a0fc..d36919b 100644
--- a/drivers/gpu/drm/shmobile/shmob_drm_kms.c
+++ b/drivers/gpu/drm/shmobile/shmob_drm_kms.c
@@ -16,6 +16,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 
 #include <video/sh_mobile_meram.h>
 
@@ -131,7 +132,7 @@ shmob_drm_fb_create(struct drm_device *dev, struct drm_file *file_priv,
 		}
 	}
 
-	return drm_fb_cma_create(dev, file_priv, mode_cmd);
+	return drm_gem_fb_create(dev, file_priv, mode_cmd);
 }
 
 static const struct drm_mode_config_funcs shmob_drm_mode_config_funcs = {
-- 
2.7.4

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

[PATCH v2 16/22] drm/sti: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Vincent Abriou <vincent.abriou@st.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/sti/sti_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index 06ef1e38..4d417a9 100644
--- a/drivers/gpu/drm/sti/sti_drv.c
+++ b/drivers/gpu/drm/sti/sti_drv.c
@@ -16,6 +16,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_of.h>
 
@@ -145,7 +146,7 @@ static void sti_output_poll_changed(struct drm_device *ddev)
 }
 
 static const struct drm_mode_config_funcs sti_mode_config_funcs = {
-	.fb_create = drm_fb_cma_create,
+	.fb_create = drm_gem_fb_create,
 	.output_poll_changed = sti_output_poll_changed,
 	.atomic_check = sti_atomic_check,
 	.atomic_commit = drm_atomic_helper_commit,
-- 
2.7.4

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

[PATCH v2 17/22] drm/stm: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Yannick Fertre <yannick.fertre@st.com>
Cc: Philippe Cornu <philippe.cornu@st.com>
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Vincent Abriou <vincent.abriou@st.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/stm/drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
index b333b37..c857663 100644
--- a/drivers/gpu/drm/stm/drv.c
+++ b/drivers/gpu/drm/stm/drv.c
@@ -17,6 +17,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 
 #include "ltdc.h"
 
@@ -31,7 +32,7 @@ static void drv_output_poll_changed(struct drm_device *ddev)
 }
 
 static const struct drm_mode_config_funcs drv_mode_config_funcs = {
-	.fb_create = drm_fb_cma_create,
+	.fb_create = drm_gem_fb_create,
 	.output_poll_changed = drv_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
-- 
2.7.4

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

[PATCH v2 18/22] drm/sun4i: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/sun4i/sun4i_framebuffer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
index 9872e0f..2992f0a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
@@ -12,6 +12,7 @@
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drmP.h>
 
 #include "sun4i_drv.h"
@@ -28,7 +29,7 @@ static const struct drm_mode_config_funcs sun4i_de_mode_config_funcs = {
 	.output_poll_changed	= sun4i_de_output_poll_changed,
 	.atomic_check		= drm_atomic_helper_check,
 	.atomic_commit		= drm_atomic_helper_commit,
-	.fb_create		= drm_fb_cma_create,
+	.fb_create		= drm_gem_fb_create,
 };
 
 struct drm_fbdev_cma *sun4i_framebuffer_init(struct drm_device *drm)
-- 
2.7.4

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

[PATCH v2 19/22] drm/tilcdc: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Jyri Sarha <jsarha@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index b0d70f9..146ac9a 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -23,6 +23,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_fb_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 
 #include "tilcdc_drv.h"
 #include "tilcdc_regs.h"
@@ -65,7 +66,7 @@ static struct of_device_id tilcdc_of_match[];
 static struct drm_framebuffer *tilcdc_fb_create(struct drm_device *dev,
 		struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)
 {
-	return drm_fb_cma_create(dev, file_priv, mode_cmd);
+	return drm_gem_fb_create(dev, file_priv, mode_cmd);
 }
 
 static void tilcdc_fb_output_poll_changed(struct drm_device *dev)
-- 
2.7.4

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

[PATCH v2 20/22] drm/vc4: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/vc4/vc4_kms.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index aeec6e8..88b293b 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -20,6 +20,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include "vc4_drv.h"
 
 static void vc4_output_poll_changed(struct drm_device *dev)
@@ -189,7 +190,7 @@ static struct drm_framebuffer *vc4_fb_create(struct drm_device *dev,
 		mode_cmd = &mode_cmd_local;
 	}
 
-	return drm_fb_cma_create(dev, file_priv, mode_cmd);
+	return drm_gem_fb_create(dev, file_priv, mode_cmd);
 }
 
 static const struct drm_mode_config_funcs vc4_mode_funcs = {
-- 
2.7.4

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

[PATCH v2 21/22] drm/zte: Use drm_gem_fb_create()

[Noralf Trønnes]

drm_fb_cma_create() is just a wrapper around drm_gem_fb_create() now,
so use the function directly.

Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/zte/zx_drm_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c
index 4524482..e8b8266 100644
--- a/drivers/gpu/drm/zte/zx_drm_drv.c
+++ b/drivers/gpu/drm/zte/zx_drm_drv.c
@@ -22,6 +22,7 @@
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_of.h>
 #include <drm/drmP.h>
 
@@ -40,7 +41,7 @@ static void zx_drm_fb_output_poll_changed(struct drm_device *drm)
 }
 
 static const struct drm_mode_config_funcs zx_drm_mode_config_funcs = {
-	.fb_create = drm_fb_cma_create,
+	.fb_create = drm_gem_fb_create,
 	.output_poll_changed = zx_drm_fb_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
-- 
2.7.4

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

[PATCH v2 22/22] drm/fb-cma-helper: Remove unused functions

[Noralf Trønnes]

The cma drivers use the drm_gem_framebuffer_helper functions now,
so remove drm_fb_cma_destroy, drm_fb_cma_create_handle,
drm_fb_cma_create_with_funcs, drm_fb_cma_create and
drm_fb_cma_prepare_fb.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_fb_cma_helper.c | 77 ++-----------------------------------
 include/drm/drm_fb_cma_helper.h     | 13 -------
 2 files changed, 3 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index d0c0110..ab6edcc 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -38,7 +38,7 @@ struct drm_fbdev_cma {
  * Provides helper functions for creating a cma (contiguous memory allocator)
  * backed framebuffer.
  *
- * drm_fb_cma_create() is used in the &drm_mode_config_funcs.fb_create
+ * drm_gem_fb_create() is used in the &drm_mode_config_funcs.fb_create
  * callback function to create a cma backed framebuffer.
  *
  * An fbdev framebuffer backed by cma is also available by calling
@@ -61,8 +61,8 @@ struct drm_fbdev_cma {
  *     }
  *
  *     static struct drm_framebuffer_funcs driver_fb_funcs = {
- *         .destroy       = drm_fb_cma_destroy,
- *         .create_handle = drm_fb_cma_create_handle,
+ *         .destroy       = drm_gem_fb_destroy,
+ *         .create_handle = drm_gem_fb_create_handle,
  *         .dirty         = driver_fb_dirty,
  *     };
  *
@@ -80,57 +80,6 @@ static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper)
 	return container_of(helper, struct drm_fbdev_cma, fb_helper);
 }
 
-void drm_fb_cma_destroy(struct drm_framebuffer *fb)
-{
-	drm_gem_fb_destroy(fb);
-}
-EXPORT_SYMBOL(drm_fb_cma_destroy);
-
-int drm_fb_cma_create_handle(struct drm_framebuffer *fb,
-	struct drm_file *file_priv, unsigned int *handle)
-{
-	return drm_gem_fb_create_handle(fb, file_priv, handle);
-}
-EXPORT_SYMBOL(drm_fb_cma_create_handle);
-
-/**
- * drm_fb_cma_create_with_funcs() - helper function for the
- *                                  &drm_mode_config_funcs.fb_create
- *                                  callback
- * @dev: DRM device
- * @file_priv: drm file for the ioctl call
- * @mode_cmd: metadata from the userspace fb creation request
- * @funcs: vtable to be used for the new framebuffer object
- *
- * This can be used to set &drm_framebuffer_funcs for drivers that need the
- * &drm_framebuffer_funcs.dirty callback. Use drm_fb_cma_create() if you don't
- * need to change &drm_framebuffer_funcs.
- */
-struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
-	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
-	const struct drm_framebuffer_funcs *funcs)
-{
-	return drm_gem_fb_create_with_funcs(dev, file_priv, mode_cmd, funcs);
-}
-EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
-
-/**
- * drm_fb_cma_create() - &drm_mode_config_funcs.fb_create callback function
- * @dev: DRM device
- * @file_priv: drm file for the ioctl call
- * @mode_cmd: metadata from the userspace fb creation request
- *
- * If your hardware has special alignment or pitch requirements these should be
- * checked before calling this function. Use drm_fb_cma_create_with_funcs() if
- * you need to set &drm_framebuffer_funcs.dirty.
- */
-struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
-	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)
-{
-	return drm_gem_fb_create(dev, file_priv, mode_cmd);
-}
-EXPORT_SYMBOL_GPL(drm_fb_cma_create);
-
 /**
  * drm_fb_cma_get_gem_obj() - Get CMA GEM object for framebuffer
  * @fb: The framebuffer
@@ -182,26 +131,6 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb,
 }
 EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_addr);
 
-/**
- * drm_fb_cma_prepare_fb() - Prepare CMA framebuffer
- * @plane: Which plane
- * @state: Plane state attach fence to
- *
- * This should be set as the &struct drm_plane_helper_funcs.prepare_fb hook.
- *
- * This function checks if the plane FB has an dma-buf attached, extracts
- * the exclusive fence and attaches it to plane state for the atomic helper
- * to wait on.
- *
- * There is no need for cleanup_fb for CMA based framebuffer drivers.
- */
-int drm_fb_cma_prepare_fb(struct drm_plane *plane,
-			  struct drm_plane_state *state)
-{
-	return drm_gem_fb_prepare_fb(plane, state);
-}
-EXPORT_SYMBOL_GPL(drm_fb_cma_prepare_fb);
-
 #ifdef CONFIG_DEBUG_FS
 static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
 {
diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h
index a323781..023f052 100644
--- a/include/drm/drm_fb_cma_helper.h
+++ b/include/drm/drm_fb_cma_helper.h
@@ -28,16 +28,6 @@ void drm_fbdev_cma_set_suspend(struct drm_fbdev_cma *fbdev_cma, bool state);
 void drm_fbdev_cma_set_suspend_unlocked(struct drm_fbdev_cma *fbdev_cma,
 					bool state);
 
-void drm_fb_cma_destroy(struct drm_framebuffer *fb);
-int drm_fb_cma_create_handle(struct drm_framebuffer *fb,
-	struct drm_file *file_priv, unsigned int *handle);
-
-struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
-	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
-	const struct drm_framebuffer_funcs *funcs);
-struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
-	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd);
-
 struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb,
 	unsigned int plane);
 
@@ -45,9 +35,6 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb,
 				   struct drm_plane_state *state,
 				   unsigned int plane);
 
-int drm_fb_cma_prepare_fb(struct drm_plane *plane,
-			  struct drm_plane_state *state);
-
 #ifdef CONFIG_DEBUG_FS
 struct seq_file;
 
-- 
2.7.4

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

Re: [PATCH v2 01/22] drm: Add GEM backed framebuffer library

[Daniel Vetter]

On Wed, Aug 09, 2017 at 12:11:04PM +0200, Noralf Trønnes wrote:
> This library provides helpers for drivers that don't subclass
> drm_framebuffer and are backed by drm_gem_object. The code is
> taken from drm_fb_cma_helper.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

lgtm, a few nits below to polish the documentation. With those addressed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  Documentation/gpu/drm-kms-helpers.rst        |   9 +
>  drivers/gpu/drm/Makefile                     |   2 +-
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 252 +++++++++++++++++++++++++++
>  include/drm/drm_framebuffer.h                |   4 +
>  include/drm/drm_gem_framebuffer_helper.h     |  35 ++++
>  5 files changed, 301 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/drm_gem_framebuffer_helper.c
>  create mode 100644 include/drm/drm_gem_framebuffer_helper.h
> 
> diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
> index 7c5e254..13dd237 100644
> --- a/Documentation/gpu/drm-kms-helpers.rst
> +++ b/Documentation/gpu/drm-kms-helpers.rst
> @@ -296,3 +296,12 @@ Auxiliary Modeset Helpers
>  
>  .. kernel-doc:: drivers/gpu/drm/drm_modeset_helper.c
>     :export:
> +
> +Framebuffer GEM Helper Reference
> +================================
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +   :doc: overview
> +
> +.. kernel-doc:: drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +   :export:
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 24a066e..a8acc19 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -33,7 +33,7 @@ drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
>  		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
>  		drm_kms_helper_common.o drm_dp_dual_mode_helper.o \
>  		drm_simple_kms_helper.o drm_modeset_helper.o \
> -		drm_scdc_helper.o
> +		drm_scdc_helper.o drm_gem_framebuffer_helper.o
>  
>  drm_kms_helper-$(CONFIG_DRM_PANEL_BRIDGE) += bridge/panel.o
>  drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> new file mode 100644
> index 0000000..41a506c
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -0,0 +1,252 @@
> +/*
> + * drm gem framebuffer helper functions
> + *
> + * Copyright (C) 2017 Noralf Trønnes
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/dma-buf.h>
> +#include <linux/dma-fence.h>
> +#include <linux/reservation.h>
> +#include <linux/slab.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_fb_helper.h>
> +#include <drm/drm_fourcc.h>
> +#include <drm/drm_framebuffer.h>
> +#include <drm/drm_gem.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_modeset_helper.h>
> +
> +/**
> + * DOC: overview
> + *
> + * This library provides helpers for drivers that don't subclass
> + * &drm_framebuffer and are backed by &drm_gem_object.

I'd rephrase this as "and use &drm_gem_object for their backing storage".

Maybe also drop a hint for the main entry point for drivers into this
library:

"Drivers without addition needs to validate framebuffers can simply use
drm_gem_fb_create() and everything is wired up automatically. But all
parts can be used individually, too."

> + */
> +
> +/**
> + * drm_gem_fb_get_obj() - Get GEM object for framebuffer
> + * @fb: The framebuffer
> + * @plane: Which plane
> + *
> + * Returns the GEM object for given framebuffer.
> + */
> +struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
> +					  unsigned int plane)
> +{
> +	if (plane >= 4)
> +		return NULL;
> +
> +	return fb->obj[plane];
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_get_obj);
> +
> +/**
> + * drm_gem_fb_alloc - Allocate GEM backed framebuffer
> + * @dev: DRM device
> + * @mode_cmd: metadata from the userspace fb creation request
> + * @obj: GEM object(s) backing the framebuffer
> + * @num_planes: Number of planes
> + * @funcs: vtable to be used for the new framebuffer object

This kinda puts all the warnings from drm_framebuffer_init() under the
rug, so not enough text for such a tricky function. Proposal

"This allocates a &struct drm_framebuffer and publishes it by calling
drm_framebuffer_init().

IMPORTANT:
All checking and validation must be done before calling this function.
Framebuffers are supposed to be invariant over their lifetime, which means
evil userspace could otherwise trick the kernel into using unvalidated
framebuffer objects."

Not sure whether that kills your use-cases for this, in which case we
probably need to open-code this in all callers. Or remove the call to
drm_framebuffer_init() and place the driver's validation code between the
call to _alloc() and _init().

Maybe we should even change _init() to be called _register() to avoid this
bug in the future.

But that's just all aside, from your create_with_funcs example below it
all looks good.

> + *
> + * Returns:
> + * Allocated struct drm_framebuffer * or error encoded pointer.
> + */
> +struct drm_framebuffer *
> +drm_gem_fb_alloc(struct drm_device *dev,
> +		 const struct drm_mode_fb_cmd2 *mode_cmd,
> +		 struct drm_gem_object **obj, unsigned int num_planes,
> +		 const struct drm_framebuffer_funcs *funcs)
> +{
> +	struct drm_framebuffer *fb;
> +	int ret, i;
> +
> +	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> +	if (!fb)
> +		return ERR_PTR(-ENOMEM);
> +
> +	drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
> +
> +	for (i = 0; i < num_planes; i++)
> +		fb->obj[i] = obj[i];
> +
> +	ret = drm_framebuffer_init(dev, fb, funcs);
> +	if (ret) {
> +		DRM_DEV_ERROR(dev->dev, "Failed to init framebuffer: %d\n",
> +			      ret);
> +		kfree(fb);
> +		return ERR_PTR(ret);
> +	}
> +
> +	return fb;
> +}
> +EXPORT_SYMBOL(drm_gem_fb_alloc);
> +
> +/**
> + * drm_gem_fb_destroy - Free GEM backed framebuffer
> + * @fb: DRM framebuffer
> + *
> + * Frees a GEM backed framebuffer with it's backing buffer(s) and the structure
> + * itself. Drivers can use this as their &drm_framebuffer_funcs->destroy
> + * callback.
> + */
> +void drm_gem_fb_destroy(struct drm_framebuffer *fb)
> +{
> +	int i;
> +
> +	for (i = 0; i < 4; i++) {
> +		if (fb->obj[i])
> +			drm_gem_object_put_unlocked(fb->obj[i]);
> +	}
> +
> +	drm_framebuffer_cleanup(fb);
> +	kfree(fb);
> +}
> +EXPORT_SYMBOL(drm_gem_fb_destroy);
> +
> +/**
> + * drm_gem_fb_create_handle - Create handle for GEM backed framebuffer
> + * @fb: DRM framebuffer
> + * @file: drm file
> + * @handle: handle created
> + *
> + * Drivers can use this as their &drm_framebuffer_funcs->create_handle
> + * callback.
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
> +			     unsigned int *handle)
> +{
> +	return drm_gem_handle_create(file, fb->obj[0], handle);
> +}
> +EXPORT_SYMBOL(drm_gem_fb_create_handle);
> +
> +/**
> + * drm_gem_fb_create_with_funcs() - helper function for the
> + *                                  &drm_mode_config_funcs.fb_create
> + *                                  callback
> + * @dev: DRM device
> + * @file: drm file for the ioctl call
> + * @mode_cmd: metadata from the userspace fb creation request
> + * @funcs: vtable to be used for the new framebuffer object
> + *
> + * This can be used to set &drm_framebuffer_funcs for drivers that need the
> + * &drm_framebuffer_funcs.dirty callback. Use drm_gem_fb_create() if you don't
> + * need to change &drm_framebuffer_funcs.

Maybe mention here and below that this already validates against the
buffer size (which is what most drivers need to do beyond the basic checks
the drm core already does).

One thing this doesn't do is validate the pixel format and modifiers. We
should only accept framebuffers which can be used for this driver (on any
drm_plane). Not sure this is something current drivers get right even, and
maybe we should try to check that in the core perhaps ... So different
patch series, just something that crossed my mind.

> + */
> +struct drm_framebuffer *
> +drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> +			     const struct drm_mode_fb_cmd2 *mode_cmd,
> +			     const struct drm_framebuffer_funcs *funcs)
> +{
> +	const struct drm_format_info *info;
> +	struct drm_gem_object *objs[4];
> +	struct drm_framebuffer *fb;
> +	int ret, i;
> +
> +	info = drm_get_format_info(dev, mode_cmd);
> +	if (!info)
> +		return ERR_PTR(-EINVAL);
> +
> +	for (i = 0; i < info->num_planes; i++) {
> +		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> +		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
> +		unsigned int min_size;
> +
> +		objs[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]);
> +		if (!objs[i]) {
> +			DRM_DEV_ERROR(dev->dev, "Failed to lookup GEM\n");
> +			ret = -ENOENT;
> +			goto err_gem_object_put;
> +		}
> +
> +		min_size = (height - 1) * mode_cmd->pitches[i]
> +			 + width * info->cpp[i]
> +			 + mode_cmd->offsets[i];
> +
> +		if (objs[i]->size < min_size) {
> +			drm_gem_object_put_unlocked(objs[i]);
> +			ret = -EINVAL;
> +			goto err_gem_object_put;
> +		}
> +	}
> +
> +	fb = drm_gem_fb_alloc(dev, mode_cmd, objs, i, funcs);
> +	if (IS_ERR(fb)) {
> +		ret = PTR_ERR(fb);
> +		goto err_gem_object_put;
> +	}
> +
> +	return fb;
> +
> +err_gem_object_put:
> +	for (i--; i >= 0; i--)
> +		drm_gem_object_put_unlocked(objs[i]);
> +
> +	return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_funcs);
> +
> +static const struct drm_framebuffer_funcs drm_gem_fb_funcs = {
> +	.destroy	= drm_gem_fb_destroy,
> +	.create_handle	= drm_gem_fb_create_handle,
> +};
> +
> +/**
> + * drm_gem_fb_create() - &drm_mode_config_funcs.fb_create callback function
> + * @dev: DRM device
> + * @file: drm file for the ioctl call
> + * @mode_cmd: metadata from the userspace fb creation request
> + *
> + * If your hardware has special alignment or pitch requirements these should be
> + * checked before calling this function. Use drm_gem_fb_create_with_funcs() if
> + * you need to set &drm_framebuffer_funcs.dirty.
> + */
> +struct drm_framebuffer *
> +drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
> +		  const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> +	return drm_gem_fb_create_with_funcs(dev, file, mode_cmd,
> +					    &drm_gem_fb_funcs);
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_create);
> +
> +/**
> + * drm_gem_fb_prepare_fb() - Prepare gem framebuffer
> + * @plane: Which plane
> + * @state: Plane state attach fence to
> + *
> + * This should be set as the &struct drm_plane_helper_funcs.prepare_fb hook.

We tend to not use the struct label when referencing members, so just
&drm_plane_helper_funcs.prepare_fb hook.

> + *
> + * This function checks if the plane FB has an dma-buf attached, extracts
> + * the exclusive fence and attaches it to plane state for the atomic helper
> + * to wait on.
> + *
> + * There is no need for cleanup_fb for gem based framebuffer drivers.

A bit too simple:

"There is no need for &drm_plane_helper_funcs.cleanup_fb hook for simple
gem based framebuffer drivers which have their buffers always pinned in
memory."


> + */
> +int drm_gem_fb_prepare_fb(struct drm_plane *plane,
> +			  struct drm_plane_state *state)
> +{
> +	struct dma_buf *dma_buf;
> +	struct dma_fence *fence;
> +
> +	if ((plane->state->fb == state->fb) || !state->fb)
> +		return 0;
> +
> +	dma_buf = drm_gem_fb_get_obj(state->fb, 0)->dma_buf;
> +	if (dma_buf) {
> +		fence = reservation_object_get_excl_rcu(dma_buf->resv);
> +		drm_atomic_set_fence_for_plane(state, fence);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb);
> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
> index 5244f05..50def22 100644
> --- a/include/drm/drm_framebuffer.h
> +++ b/include/drm/drm_framebuffer.h
> @@ -190,6 +190,10 @@ struct drm_framebuffer {
>  	 * @filp_head: Placed on &drm_file.fbs, protected by &drm_file.fbs_lock.
>  	 */
>  	struct list_head filp_head;
> +	/**
> +	 * @obj: GEM objects backing the framebuffer, one per plane (optional).

I'd elaborate a bit more:

"This is used by the GEM framebuffer helpers, see e.g.
drm_gem_fb_create()."

> +	 */
> +	struct drm_gem_object *obj[4];
>  };
>  
>  #define obj_to_fb(x) container_of(x, struct drm_framebuffer, base)
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> new file mode 100644
> index 0000000..af3d1ee
> --- /dev/null
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -0,0 +1,35 @@
> +#ifndef __DRM_GEM_FB_HELPER_H__
> +#define __DRM_GEM_FB_HELPER_H__
> +
> +struct drm_device;
> +struct drm_file;
> +struct drm_framebuffer;
> +struct drm_framebuffer_funcs;
> +struct drm_gem_object;
> +struct drm_mode_fb_cmd2;
> +struct drm_plane;
> +struct drm_plane_state;
> +
> +struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
> +					  unsigned int plane);
> +struct drm_framebuffer *
> +drm_gem_fb_alloc(struct drm_device *dev,
> +		 const struct drm_mode_fb_cmd2 *mode_cmd,
> +		 struct drm_gem_object **obj, unsigned int num_planes,
> +		 const struct drm_framebuffer_funcs *funcs);
> +void drm_gem_fb_destroy(struct drm_framebuffer *fb);
> +int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
> +			     unsigned int *handle);
> +
> +struct drm_framebuffer *
> +drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> +			     const struct drm_mode_fb_cmd2 *mode_cmd,
> +			     const struct drm_framebuffer_funcs *funcs);
> +struct drm_framebuffer *
> +drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
> +		  const struct drm_mode_fb_cmd2 *mode_cmd);
> +
> +int drm_gem_fb_prepare_fb(struct drm_plane *plane,
> +			  struct drm_plane_state *state);
> +
> +#endif
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 02/22] drm/fb-cma-helper: Use drm_gem_framebuffer_helper

[Daniel Vetter]

On Wed, Aug 09, 2017 at 12:11:05PM +0200, Noralf Trønnes wrote:
> Use the new drm_gem_framebuffer_helper who's code was copied
> from this helper.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/drm_fb_cma_helper.c | 172 +++++++-----------------------------
>  1 file changed, 30 insertions(+), 142 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
> index ade319d..d0c0110 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -18,27 +18,17 @@
>   */
>  
>  #include <drm/drmP.h>
> -#include <drm/drm_atomic.h>
> -#include <drm/drm_crtc.h>
>  #include <drm/drm_fb_helper.h>
> -#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_framebuffer.h>
>  #include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_gem_framebuffer_helper.h>
>  #include <drm/drm_fb_cma_helper.h>
> -#include <linux/dma-buf.h>
> -#include <linux/dma-mapping.h>
>  #include <linux/module.h>
> -#include <linux/reservation.h>
>  
>  #define DEFAULT_FBDEFIO_DELAY_MS 50
>  
> -struct drm_fb_cma {
> -	struct drm_framebuffer		fb;
> -	struct drm_gem_cma_object	*obj[4];
> -};
> -
>  struct drm_fbdev_cma {
>  	struct drm_fb_helper	fb_helper;
> -	struct drm_fb_cma	*fb;
>  	const struct drm_framebuffer_funcs *fb_funcs;
>  };
>  
> @@ -90,69 +80,19 @@ static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper)
>  	return container_of(helper, struct drm_fbdev_cma, fb_helper);
>  }
>  
> -static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb)
> -{
> -	return container_of(fb, struct drm_fb_cma, fb);
> -}
> -
>  void drm_fb_cma_destroy(struct drm_framebuffer *fb)
>  {
> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> -	int i;
> -
> -	for (i = 0; i < 4; i++) {
> -		if (fb_cma->obj[i])
> -			drm_gem_object_put_unlocked(&fb_cma->obj[i]->base);
> -	}
> -
> -	drm_framebuffer_cleanup(fb);
> -	kfree(fb_cma);
> +	drm_gem_fb_destroy(fb);
>  }
>  EXPORT_SYMBOL(drm_fb_cma_destroy);
>  
>  int drm_fb_cma_create_handle(struct drm_framebuffer *fb,
>  	struct drm_file *file_priv, unsigned int *handle)
>  {
> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> -
> -	return drm_gem_handle_create(file_priv,
> -			&fb_cma->obj[0]->base, handle);
> +	return drm_gem_fb_create_handle(fb, file_priv, handle);
>  }
>  EXPORT_SYMBOL(drm_fb_cma_create_handle);
>  
> -static struct drm_framebuffer_funcs drm_fb_cma_funcs = {
> -	.destroy	= drm_fb_cma_destroy,
> -	.create_handle	= drm_fb_cma_create_handle,
> -};
> -
> -static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
> -	const struct drm_mode_fb_cmd2 *mode_cmd,
> -	struct drm_gem_cma_object **obj,
> -	unsigned int num_planes, const struct drm_framebuffer_funcs *funcs)
> -{
> -	struct drm_fb_cma *fb_cma;
> -	int ret;
> -	int i;
> -
> -	fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);
> -	if (!fb_cma)
> -		return ERR_PTR(-ENOMEM);
> -
> -	drm_helper_mode_fill_fb_struct(dev, &fb_cma->fb, mode_cmd);
> -
> -	for (i = 0; i < num_planes; i++)
> -		fb_cma->obj[i] = obj[i];
> -
> -	ret = drm_framebuffer_init(dev, &fb_cma->fb, funcs);
> -	if (ret) {
> -		dev_err(dev->dev, "Failed to initialize framebuffer: %d\n", ret);
> -		kfree(fb_cma);
> -		return ERR_PTR(ret);
> -	}
> -
> -	return fb_cma;
> -}
> -
>  /**
>   * drm_fb_cma_create_with_funcs() - helper function for the
>   *                                  &drm_mode_config_funcs.fb_create
> @@ -170,53 +110,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
>  	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
>  	const struct drm_framebuffer_funcs *funcs)
>  {
> -	const struct drm_format_info *info;
> -	struct drm_fb_cma *fb_cma;
> -	struct drm_gem_cma_object *objs[4];
> -	struct drm_gem_object *obj;
> -	int ret;
> -	int i;
> -
> -	info = drm_get_format_info(dev, mode_cmd);
> -	if (!info)
> -		return ERR_PTR(-EINVAL);
> -
> -	for (i = 0; i < info->num_planes; i++) {
> -		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> -		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
> -		unsigned int min_size;
> -
> -		obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[i]);
> -		if (!obj) {
> -			dev_err(dev->dev, "Failed to lookup GEM object\n");
> -			ret = -ENOENT;
> -			goto err_gem_object_put;
> -		}
> -
> -		min_size = (height - 1) * mode_cmd->pitches[i]
> -			 + width * info->cpp[i]
> -			 + mode_cmd->offsets[i];
> -
> -		if (obj->size < min_size) {
> -			drm_gem_object_put_unlocked(obj);
> -			ret = -EINVAL;
> -			goto err_gem_object_put;
> -		}
> -		objs[i] = to_drm_gem_cma_obj(obj);
> -	}
> -
> -	fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, funcs);
> -	if (IS_ERR(fb_cma)) {
> -		ret = PTR_ERR(fb_cma);
> -		goto err_gem_object_put;
> -	}
> -
> -	return &fb_cma->fb;
> -
> -err_gem_object_put:
> -	for (i--; i >= 0; i--)
> -		drm_gem_object_put_unlocked(&objs[i]->base);
> -	return ERR_PTR(ret);
> +	return drm_gem_fb_create_with_funcs(dev, file_priv, mode_cmd, funcs);
>  }
>  EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
>  
> @@ -233,8 +127,7 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
>  struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
>  	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)
>  {
> -	return drm_fb_cma_create_with_funcs(dev, file_priv, mode_cmd,
> -					    &drm_fb_cma_funcs);
> +	return drm_gem_fb_create(dev, file_priv, mode_cmd);
>  }
>  EXPORT_SYMBOL_GPL(drm_fb_cma_create);
>  
> @@ -250,12 +143,14 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_create);
>  struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb,
>  						  unsigned int plane)
>  {
> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> +	struct drm_gem_object *gem;
>  
> -	if (plane >= 4)
> +	gem = drm_gem_fb_get_obj(fb, plane);
> +	if (!gem)
>  		return NULL;
>  
> -	return fb_cma->obj[plane];
> +	return to_drm_gem_cma_obj(gem);
> +
>  }
>  EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj);
>  
> @@ -272,13 +167,14 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb,
>  				   struct drm_plane_state *state,
>  				   unsigned int plane)
>  {
> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> +	struct drm_gem_cma_object *obj;
>  	dma_addr_t paddr;
>  
> -	if (plane >= 4)
> +	obj = drm_fb_cma_get_gem_obj(fb, plane);
> +	if (!obj)
>  		return 0;
>  
> -	paddr = fb_cma->obj[plane]->paddr + fb->offsets[plane];
> +	paddr = obj->paddr + fb->offsets[plane];
>  	paddr += fb->format->cpp[plane] * (state->src_x >> 16);
>  	paddr += fb->pitches[plane] * (state->src_y >> 16);
>  
> @@ -302,26 +198,13 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_addr);
>  int drm_fb_cma_prepare_fb(struct drm_plane *plane,
>  			  struct drm_plane_state *state)
>  {
> -	struct dma_buf *dma_buf;
> -	struct dma_fence *fence;
> -
> -	if ((plane->state->fb == state->fb) || !state->fb)
> -		return 0;
> -
> -	dma_buf = drm_fb_cma_get_gem_obj(state->fb, 0)->base.dma_buf;
> -	if (dma_buf) {
> -		fence = reservation_object_get_excl_rcu(dma_buf->resv);
> -		drm_atomic_set_fence_for_plane(state, fence);
> -	}
> -
> -	return 0;
> +	return drm_gem_fb_prepare_fb(plane, state);
>  }
>  EXPORT_SYMBOL_GPL(drm_fb_cma_prepare_fb);
>  
>  #ifdef CONFIG_DEBUG_FS
>  static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
>  {
> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
>  	int i;
>  
>  	seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height,
> @@ -330,7 +213,7 @@ static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
>  	for (i = 0; i < fb->format->num_planes; i++) {
>  		seq_printf(m, "   %d: offset=%d pitch=%d, obj: ",
>  				i, fb->offsets[i], fb->pitches[i]);
> -		drm_gem_cma_describe(fb_cma->obj[i], m);
> +		drm_gem_cma_describe(drm_fb_cma_get_gem_obj(fb, i), m);
>  	}
>  }
>  
> @@ -434,6 +317,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
>  	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
>  	struct drm_device *dev = helper->dev;
>  	struct drm_gem_cma_object *obj;
> +	struct drm_gem_object *gem;
>  	struct drm_framebuffer *fb;
>  	unsigned int bytes_per_pixel;
>  	unsigned long offset;
> @@ -464,15 +348,14 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
>  		goto err_gem_free_object;
>  	}
>  
> -	fbdev_cma->fb = drm_fb_cma_alloc(dev, &mode_cmd, &obj, 1,
> -					 fbdev_cma->fb_funcs);
> -	if (IS_ERR(fbdev_cma->fb)) {
> +	gem = &obj->base;
> +	fb = drm_gem_fb_alloc(dev, &mode_cmd, &gem, 1, fbdev_cma->fb_funcs);
> +	if (IS_ERR(fb)) {
>  		dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
> -		ret = PTR_ERR(fbdev_cma->fb);
> +		ret = PTR_ERR(fb);
>  		goto err_fb_info_destroy;
>  	}
>  
> -	fb = &fbdev_cma->fb->fb;
>  	helper->fb = fb;
>  
>  	fbi->par = helper;
> @@ -500,7 +383,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
>  	return 0;
>  
>  err_cma_destroy:
> -	drm_framebuffer_remove(&fbdev_cma->fb->fb);
> +	drm_framebuffer_remove(fb);
>  err_fb_info_destroy:
>  	drm_fb_helper_fini(helper);
>  err_gem_free_object:
> @@ -570,6 +453,11 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs);
>  
> +static const struct drm_framebuffer_funcs drm_fb_cma_funcs = {
> +	.destroy	= drm_gem_fb_destroy,
> +	.create_handle	= drm_gem_fb_create_handle,
> +};

I don't see you removing this in the very last patch. It exactly matches
the function table the gem fb helper provides, why do we need this dupe
here?
-Daniel

> +
>  /**
>   * drm_fbdev_cma_init() - Allocate and initializes a drm_fbdev_cma struct
>   * @dev: DRM device
> @@ -597,8 +485,8 @@ void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
>  	if (fbdev_cma->fb_helper.fbdev)
>  		drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
>  
> -	if (fbdev_cma->fb)
> -		drm_framebuffer_remove(&fbdev_cma->fb->fb);
> +	if (fbdev_cma->fb_helper.fb)
> +		drm_framebuffer_remove(fbdev_cma->fb_helper.fb);
>  
>  	drm_fb_helper_fini(&fbdev_cma->fb_helper);
>  	kfree(fbdev_cma);
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 01/22] drm: Add GEM backed framebuffer library

[Noralf Trønnes]

Den 09.08.2017 22.04, skrev Daniel Vetter:
> On Wed, Aug 09, 2017 at 12:11:04PM +0200, Noralf Trønnes wrote:
>> This library provides helpers for drivers that don't subclass
>> drm_framebuffer and are backed by drm_gem_object. The code is
>> taken from drm_fb_cma_helper.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> lgtm, a few nits below to polish the documentation. With those addressed:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

[...]

>> +/**
>> + * drm_gem_fb_alloc - Allocate GEM backed framebuffer
>> + * @dev: DRM device
>> + * @mode_cmd: metadata from the userspace fb creation request
>> + * @obj: GEM object(s) backing the framebuffer
>> + * @num_planes: Number of planes
>> + * @funcs: vtable to be used for the new framebuffer object
> This kinda puts all the warnings from drm_framebuffer_init() under the
> rug, so not enough text for such a tricky function. Proposal
>
> "This allocates a &struct drm_framebuffer and publishes it by calling
> drm_framebuffer_init().
>
> IMPORTANT:
> All checking and validation must be done before calling this function.
> Framebuffers are supposed to be invariant over their lifetime, which means
> evil userspace could otherwise trick the kernel into using unvalidated
> framebuffer objects."
>
> Not sure whether that kills your use-cases for this, in which case we
> probably need to open-code this in all callers. Or remove the call to
> drm_framebuffer_init() and place the driver's validation code between the
> call to _alloc() and _init().
>
> Maybe we should even change _init() to be called _register() to avoid this
> bug in the future.

The only reason drm_gem_fb_alloc() is exported is to add framebuffers
for fbdev emulation. So maybe it's better to hide that function and make
an explicit one:

/**
  * drm_gem_fbdev_fb_create - Create a drm_framebuffer for fbdev emulation
  * @dev: DRM device
  * @sizes: fbdev size description
  * @pitch_align: optional pitch alignment
  * @obj: GEM object backing the framebuffer
  * @funcs: vtable to be used for the new framebuffer object
  *
  * This function creates a framebuffer for use with fbdev emulation.
  *
  * Returns:
  * Pointer to a drm_framebuffer on success or an error pointer on failure.
  */
struct drm_framebuffer *
drm_gem_fbdev_fb_create(struct drm_device *dev,
             struct drm_fb_helper_surface_size *sizes,
             unsigned int pitch_align, struct drm_gem_object *obj,
             const struct drm_framebuffer_funcs *funcs)
{
     struct drm_mode_fb_cmd2 mode_cmd = { 0 };

     mode_cmd.width = sizes->surface_width;
     mode_cmd.height = sizes->surface_height;
     mode_cmd.pitches[0] = sizes->surface_width *
                   DIV_ROUND_UP(sizes->surface_bpp, 8);
     if (pitch_align)
         mode_cmd.pitches[0] = roundup(mode_cmd.pitches[0],
                           pitch_align);
     mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
                             sizes->surface_depth);
     if (obj->size < mode_cmd.pitches[0] * mode_cmd.height)
         return ERR_PTR(-EINVAL);

     return drm_gem_fb_alloc(dev, &mode_cmd, &obj, 1, funcs);
}
EXPORT_SYMBOL(drm_gem_fbdev_fb_create);


Noralf.

> But that's just all aside, from your create_with_funcs example below it
> all looks good.
>
>> + *
>> + * Returns:
>> + * Allocated struct drm_framebuffer * or error encoded pointer.
>> + */
>> +struct drm_framebuffer *
>> +drm_gem_fb_alloc(struct drm_device *dev,
>> +		 const struct drm_mode_fb_cmd2 *mode_cmd,
>> +		 struct drm_gem_object **obj, unsigned int num_planes,
>> +		 const struct drm_framebuffer_funcs *funcs)
>> +{
>> +	struct drm_framebuffer *fb;
>> +	int ret, i;
>> +
>> +	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
>> +	if (!fb)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
>> +
>> +	for (i = 0; i < num_planes; i++)
>> +		fb->obj[i] = obj[i];
>> +
>> +	ret = drm_framebuffer_init(dev, fb, funcs);
>> +	if (ret) {
>> +		DRM_DEV_ERROR(dev->dev, "Failed to init framebuffer: %d\n",
>> +			      ret);
>> +		kfree(fb);
>> +		return ERR_PTR(ret);
>> +	}
>> +
>> +	return fb;
>> +}
>> +EXPORT_SYMBOL(drm_gem_fb_alloc);
>> +
>> +/**
>> + * drm_gem_fb_destroy - Free GEM backed framebuffer
>> + * @fb: DRM framebuffer
>> + *
>> + * Frees a GEM backed framebuffer with it's backing buffer(s) and the structure
>> + * itself. Drivers can use this as their &drm_framebuffer_funcs->destroy
>> + * callback.
>> + */
>> +void drm_gem_fb_destroy(struct drm_framebuffer *fb)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < 4; i++) {
>> +		if (fb->obj[i])
>> +			drm_gem_object_put_unlocked(fb->obj[i]);
>> +	}
>> +
>> +	drm_framebuffer_cleanup(fb);
>> +	kfree(fb);
>> +}
>> +EXPORT_SYMBOL(drm_gem_fb_destroy);
>> +
>> +/**
>> + * drm_gem_fb_create_handle - Create handle for GEM backed framebuffer
>> + * @fb: DRM framebuffer
>> + * @file: drm file
>> + * @handle: handle created
>> + *
>> + * Drivers can use this as their &drm_framebuffer_funcs->create_handle
>> + * callback.
>> + *
>> + * Returns:
>> + * 0 on success or a negative error code on failure.
>> + */
>> +int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
>> +			     unsigned int *handle)
>> +{
>> +	return drm_gem_handle_create(file, fb->obj[0], handle);
>> +}
>> +EXPORT_SYMBOL(drm_gem_fb_create_handle);
>> +
>> +/**
>> + * drm_gem_fb_create_with_funcs() - helper function for the
>> + *                                  &drm_mode_config_funcs.fb_create
>> + *                                  callback
>> + * @dev: DRM device
>> + * @file: drm file for the ioctl call
>> + * @mode_cmd: metadata from the userspace fb creation request
>> + * @funcs: vtable to be used for the new framebuffer object
>> + *
>> + * This can be used to set &drm_framebuffer_funcs for drivers that need the
>> + * &drm_framebuffer_funcs.dirty callback. Use drm_gem_fb_create() if you don't
>> + * need to change &drm_framebuffer_funcs.
> Maybe mention here and below that this already validates against the
> buffer size (which is what most drivers need to do beyond the basic checks
> the drm core already does).
>
> One thing this doesn't do is validate the pixel format and modifiers. We
> should only accept framebuffers which can be used for this driver (on any
> drm_plane). Not sure this is something current drivers get right even, and
> maybe we should try to check that in the core perhaps ... So different
> patch series, just something that crossed my mind.
>
>> + */
>> +struct drm_framebuffer *
>> +drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
>> +			     const struct drm_mode_fb_cmd2 *mode_cmd,
>> +			     const struct drm_framebuffer_funcs *funcs)
>> +{
>> +	const struct drm_format_info *info;
>> +	struct drm_gem_object *objs[4];
>> +	struct drm_framebuffer *fb;
>> +	int ret, i;
>> +
>> +	info = drm_get_format_info(dev, mode_cmd);
>> +	if (!info)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	for (i = 0; i < info->num_planes; i++) {
>> +		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
>> +		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
>> +		unsigned int min_size;
>> +
>> +		objs[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]);
>> +		if (!objs[i]) {
>> +			DRM_DEV_ERROR(dev->dev, "Failed to lookup GEM\n");
>> +			ret = -ENOENT;
>> +			goto err_gem_object_put;
>> +		}
>> +
>> +		min_size = (height - 1) * mode_cmd->pitches[i]
>> +			 + width * info->cpp[i]
>> +			 + mode_cmd->offsets[i];
>> +
>> +		if (objs[i]->size < min_size) {
>> +			drm_gem_object_put_unlocked(objs[i]);
>> +			ret = -EINVAL;
>> +			goto err_gem_object_put;
>> +		}
>> +	}
>> +
>> +	fb = drm_gem_fb_alloc(dev, mode_cmd, objs, i, funcs);
>> +	if (IS_ERR(fb)) {
>> +		ret = PTR_ERR(fb);
>> +		goto err_gem_object_put;
>> +	}
>> +
>> +	return fb;
>> +
>> +err_gem_object_put:
>> +	for (i--; i >= 0; i--)
>> +		drm_gem_object_put_unlocked(objs[i]);
>> +
>> +	return ERR_PTR(ret);
>> +}
>> +EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_funcs);
>> +
>> +static const struct drm_framebuffer_funcs drm_gem_fb_funcs = {
>> +	.destroy	= drm_gem_fb_destroy,
>> +	.create_handle	= drm_gem_fb_create_handle,
>> +};
>> +
>> +/**
>> + * drm_gem_fb_create() - &drm_mode_config_funcs.fb_create callback function
>> + * @dev: DRM device
>> + * @file: drm file for the ioctl call
>> + * @mode_cmd: metadata from the userspace fb creation request
>> + *
>> + * If your hardware has special alignment or pitch requirements these should be
>> + * checked before calling this function. Use drm_gem_fb_create_with_funcs() if
>> + * you need to set &drm_framebuffer_funcs.dirty.
>> + */
>> +struct drm_framebuffer *
>> +drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
>> +		  const struct drm_mode_fb_cmd2 *mode_cmd)
>> +{
>> +	return drm_gem_fb_create_with_funcs(dev, file, mode_cmd,
>> +					    &drm_gem_fb_funcs);
>> +}
>> +EXPORT_SYMBOL_GPL(drm_gem_fb_create);
>> +
>> +/**
>> + * drm_gem_fb_prepare_fb() - Prepare gem framebuffer
>> + * @plane: Which plane
>> + * @state: Plane state attach fence to
>> + *
>> + * This should be set as the &struct drm_plane_helper_funcs.prepare_fb hook.
> We tend to not use the struct label when referencing members, so just
> &drm_plane_helper_funcs.prepare_fb hook.
>
>> + *
>> + * This function checks if the plane FB has an dma-buf attached, extracts
>> + * the exclusive fence and attaches it to plane state for the atomic helper
>> + * to wait on.
>> + *
>> + * There is no need for cleanup_fb for gem based framebuffer drivers.
> A bit too simple:
>
> "There is no need for &drm_plane_helper_funcs.cleanup_fb hook for simple
> gem based framebuffer drivers which have their buffers always pinned in
> memory."
>
>
>> + */
>> +int drm_gem_fb_prepare_fb(struct drm_plane *plane,
>> +			  struct drm_plane_state *state)
>> +{
>> +	struct dma_buf *dma_buf;
>> +	struct dma_fence *fence;
>> +
>> +	if ((plane->state->fb == state->fb) || !state->fb)
>> +		return 0;
>> +
>> +	dma_buf = drm_gem_fb_get_obj(state->fb, 0)->dma_buf;
>> +	if (dma_buf) {
>> +		fence = reservation_object_get_excl_rcu(dma_buf->resv);
>> +		drm_atomic_set_fence_for_plane(state, fence);
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb);
>> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
>> index 5244f05..50def22 100644
>> --- a/include/drm/drm_framebuffer.h
>> +++ b/include/drm/drm_framebuffer.h
>> @@ -190,6 +190,10 @@ struct drm_framebuffer {
>>   	 * @filp_head: Placed on &drm_file.fbs, protected by &drm_file.fbs_lock.
>>   	 */
>>   	struct list_head filp_head;
>> +	/**
>> +	 * @obj: GEM objects backing the framebuffer, one per plane (optional).
> I'd elaborate a bit more:
>
> "This is used by the GEM framebuffer helpers, see e.g.
> drm_gem_fb_create()."
>
>> +	 */
>> +	struct drm_gem_object *obj[4];
>>   };
>>   
>>   #define obj_to_fb(x) container_of(x, struct drm_framebuffer, base)
>> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
>> new file mode 100644
>> index 0000000..af3d1ee
>> --- /dev/null
>> +++ b/include/drm/drm_gem_framebuffer_helper.h
>> @@ -0,0 +1,35 @@
>> +#ifndef __DRM_GEM_FB_HELPER_H__
>> +#define __DRM_GEM_FB_HELPER_H__
>> +
>> +struct drm_device;
>> +struct drm_file;
>> +struct drm_framebuffer;
>> +struct drm_framebuffer_funcs;
>> +struct drm_gem_object;
>> +struct drm_mode_fb_cmd2;
>> +struct drm_plane;
>> +struct drm_plane_state;
>> +
>> +struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
>> +					  unsigned int plane);
>> +struct drm_framebuffer *
>> +drm_gem_fb_alloc(struct drm_device *dev,
>> +		 const struct drm_mode_fb_cmd2 *mode_cmd,
>> +		 struct drm_gem_object **obj, unsigned int num_planes,
>> +		 const struct drm_framebuffer_funcs *funcs);
>> +void drm_gem_fb_destroy(struct drm_framebuffer *fb);
>> +int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
>> +			     unsigned int *handle);
>> +
>> +struct drm_framebuffer *
>> +drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
>> +			     const struct drm_mode_fb_cmd2 *mode_cmd,
>> +			     const struct drm_framebuffer_funcs *funcs);
>> +struct drm_framebuffer *
>> +drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
>> +		  const struct drm_mode_fb_cmd2 *mode_cmd);
>> +
>> +int drm_gem_fb_prepare_fb(struct drm_plane *plane,
>> +			  struct drm_plane_state *state);
>> +
>> +#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: [PATCH v2 02/22] drm/fb-cma-helper: Use drm_gem_framebuffer_helper

[Noralf Trønnes]

Den 09.08.2017 22.07, skrev Daniel Vetter:
> On Wed, Aug 09, 2017 at 12:11:05PM +0200, Noralf Trønnes wrote:
>> Use the new drm_gem_framebuffer_helper who's code was copied
>> from this helper.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   drivers/gpu/drm/drm_fb_cma_helper.c | 172 +++++++-----------------------------
>>   1 file changed, 30 insertions(+), 142 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
>> index ade319d..d0c0110 100644
>> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
>> @@ -18,27 +18,17 @@
>>    */
>>   
>>   #include <drm/drmP.h>
>> -#include <drm/drm_atomic.h>
>> -#include <drm/drm_crtc.h>
>>   #include <drm/drm_fb_helper.h>
>> -#include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_framebuffer.h>
>>   #include <drm/drm_gem_cma_helper.h>
>> +#include <drm/drm_gem_framebuffer_helper.h>
>>   #include <drm/drm_fb_cma_helper.h>
>> -#include <linux/dma-buf.h>
>> -#include <linux/dma-mapping.h>
>>   #include <linux/module.h>
>> -#include <linux/reservation.h>
>>   
>>   #define DEFAULT_FBDEFIO_DELAY_MS 50
>>   
>> -struct drm_fb_cma {
>> -	struct drm_framebuffer		fb;
>> -	struct drm_gem_cma_object	*obj[4];
>> -};
>> -
>>   struct drm_fbdev_cma {
>>   	struct drm_fb_helper	fb_helper;
>> -	struct drm_fb_cma	*fb;
>>   	const struct drm_framebuffer_funcs *fb_funcs;
>>   };
>>   
>> @@ -90,69 +80,19 @@ static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper)
>>   	return container_of(helper, struct drm_fbdev_cma, fb_helper);
>>   }
>>   
>> -static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb)
>> -{
>> -	return container_of(fb, struct drm_fb_cma, fb);
>> -}
>> -
>>   void drm_fb_cma_destroy(struct drm_framebuffer *fb)
>>   {
>> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
>> -	int i;
>> -
>> -	for (i = 0; i < 4; i++) {
>> -		if (fb_cma->obj[i])
>> -			drm_gem_object_put_unlocked(&fb_cma->obj[i]->base);
>> -	}
>> -
>> -	drm_framebuffer_cleanup(fb);
>> -	kfree(fb_cma);
>> +	drm_gem_fb_destroy(fb);
>>   }
>>   EXPORT_SYMBOL(drm_fb_cma_destroy);
>>   
>>   int drm_fb_cma_create_handle(struct drm_framebuffer *fb,
>>   	struct drm_file *file_priv, unsigned int *handle)
>>   {
>> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
>> -
>> -	return drm_gem_handle_create(file_priv,
>> -			&fb_cma->obj[0]->base, handle);
>> +	return drm_gem_fb_create_handle(fb, file_priv, handle);
>>   }
>>   EXPORT_SYMBOL(drm_fb_cma_create_handle);
>>   
>> -static struct drm_framebuffer_funcs drm_fb_cma_funcs = {
>> -	.destroy	= drm_fb_cma_destroy,
>> -	.create_handle	= drm_fb_cma_create_handle,
>> -};
>> -
>> -static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
>> -	const struct drm_mode_fb_cmd2 *mode_cmd,
>> -	struct drm_gem_cma_object **obj,
>> -	unsigned int num_planes, const struct drm_framebuffer_funcs *funcs)
>> -{
>> -	struct drm_fb_cma *fb_cma;
>> -	int ret;
>> -	int i;
>> -
>> -	fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);
>> -	if (!fb_cma)
>> -		return ERR_PTR(-ENOMEM);
>> -
>> -	drm_helper_mode_fill_fb_struct(dev, &fb_cma->fb, mode_cmd);
>> -
>> -	for (i = 0; i < num_planes; i++)
>> -		fb_cma->obj[i] = obj[i];
>> -
>> -	ret = drm_framebuffer_init(dev, &fb_cma->fb, funcs);
>> -	if (ret) {
>> -		dev_err(dev->dev, "Failed to initialize framebuffer: %d\n", ret);
>> -		kfree(fb_cma);
>> -		return ERR_PTR(ret);
>> -	}
>> -
>> -	return fb_cma;
>> -}
>> -
>>   /**
>>    * drm_fb_cma_create_with_funcs() - helper function for the
>>    *                                  &drm_mode_config_funcs.fb_create
>> @@ -170,53 +110,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
>>   	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
>>   	const struct drm_framebuffer_funcs *funcs)
>>   {
>> -	const struct drm_format_info *info;
>> -	struct drm_fb_cma *fb_cma;
>> -	struct drm_gem_cma_object *objs[4];
>> -	struct drm_gem_object *obj;
>> -	int ret;
>> -	int i;
>> -
>> -	info = drm_get_format_info(dev, mode_cmd);
>> -	if (!info)
>> -		return ERR_PTR(-EINVAL);
>> -
>> -	for (i = 0; i < info->num_planes; i++) {
>> -		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
>> -		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
>> -		unsigned int min_size;
>> -
>> -		obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[i]);
>> -		if (!obj) {
>> -			dev_err(dev->dev, "Failed to lookup GEM object\n");
>> -			ret = -ENOENT;
>> -			goto err_gem_object_put;
>> -		}
>> -
>> -		min_size = (height - 1) * mode_cmd->pitches[i]
>> -			 + width * info->cpp[i]
>> -			 + mode_cmd->offsets[i];
>> -
>> -		if (obj->size < min_size) {
>> -			drm_gem_object_put_unlocked(obj);
>> -			ret = -EINVAL;
>> -			goto err_gem_object_put;
>> -		}
>> -		objs[i] = to_drm_gem_cma_obj(obj);
>> -	}
>> -
>> -	fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, funcs);
>> -	if (IS_ERR(fb_cma)) {
>> -		ret = PTR_ERR(fb_cma);
>> -		goto err_gem_object_put;
>> -	}
>> -
>> -	return &fb_cma->fb;
>> -
>> -err_gem_object_put:
>> -	for (i--; i >= 0; i--)
>> -		drm_gem_object_put_unlocked(&objs[i]->base);
>> -	return ERR_PTR(ret);
>> +	return drm_gem_fb_create_with_funcs(dev, file_priv, mode_cmd, funcs);
>>   }
>>   EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
>>   
>> @@ -233,8 +127,7 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
>>   struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
>>   	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)
>>   {
>> -	return drm_fb_cma_create_with_funcs(dev, file_priv, mode_cmd,
>> -					    &drm_fb_cma_funcs);
>> +	return drm_gem_fb_create(dev, file_priv, mode_cmd);
>>   }
>>   EXPORT_SYMBOL_GPL(drm_fb_cma_create);
>>   
>> @@ -250,12 +143,14 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_create);
>>   struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb,
>>   						  unsigned int plane)
>>   {
>> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
>> +	struct drm_gem_object *gem;
>>   
>> -	if (plane >= 4)
>> +	gem = drm_gem_fb_get_obj(fb, plane);
>> +	if (!gem)
>>   		return NULL;
>>   
>> -	return fb_cma->obj[plane];
>> +	return to_drm_gem_cma_obj(gem);
>> +
>>   }
>>   EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj);
>>   
>> @@ -272,13 +167,14 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb,
>>   				   struct drm_plane_state *state,
>>   				   unsigned int plane)
>>   {
>> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
>> +	struct drm_gem_cma_object *obj;
>>   	dma_addr_t paddr;
>>   
>> -	if (plane >= 4)
>> +	obj = drm_fb_cma_get_gem_obj(fb, plane);
>> +	if (!obj)
>>   		return 0;
>>   
>> -	paddr = fb_cma->obj[plane]->paddr + fb->offsets[plane];
>> +	paddr = obj->paddr + fb->offsets[plane];
>>   	paddr += fb->format->cpp[plane] * (state->src_x >> 16);
>>   	paddr += fb->pitches[plane] * (state->src_y >> 16);
>>   
>> @@ -302,26 +198,13 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_addr);
>>   int drm_fb_cma_prepare_fb(struct drm_plane *plane,
>>   			  struct drm_plane_state *state)
>>   {
>> -	struct dma_buf *dma_buf;
>> -	struct dma_fence *fence;
>> -
>> -	if ((plane->state->fb == state->fb) || !state->fb)
>> -		return 0;
>> -
>> -	dma_buf = drm_fb_cma_get_gem_obj(state->fb, 0)->base.dma_buf;
>> -	if (dma_buf) {
>> -		fence = reservation_object_get_excl_rcu(dma_buf->resv);
>> -		drm_atomic_set_fence_for_plane(state, fence);
>> -	}
>> -
>> -	return 0;
>> +	return drm_gem_fb_prepare_fb(plane, state);
>>   }
>>   EXPORT_SYMBOL_GPL(drm_fb_cma_prepare_fb);
>>   
>>   #ifdef CONFIG_DEBUG_FS
>>   static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
>>   {
>> -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
>>   	int i;
>>   
>>   	seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height,
>> @@ -330,7 +213,7 @@ static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
>>   	for (i = 0; i < fb->format->num_planes; i++) {
>>   		seq_printf(m, "   %d: offset=%d pitch=%d, obj: ",
>>   				i, fb->offsets[i], fb->pitches[i]);
>> -		drm_gem_cma_describe(fb_cma->obj[i], m);
>> +		drm_gem_cma_describe(drm_fb_cma_get_gem_obj(fb, i), m);
>>   	}
>>   }
>>   
>> @@ -434,6 +317,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
>>   	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
>>   	struct drm_device *dev = helper->dev;
>>   	struct drm_gem_cma_object *obj;
>> +	struct drm_gem_object *gem;
>>   	struct drm_framebuffer *fb;
>>   	unsigned int bytes_per_pixel;
>>   	unsigned long offset;
>> @@ -464,15 +348,14 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
>>   		goto err_gem_free_object;
>>   	}
>>   
>> -	fbdev_cma->fb = drm_fb_cma_alloc(dev, &mode_cmd, &obj, 1,
>> -					 fbdev_cma->fb_funcs);
>> -	if (IS_ERR(fbdev_cma->fb)) {
>> +	gem = &obj->base;
>> +	fb = drm_gem_fb_alloc(dev, &mode_cmd, &gem, 1, fbdev_cma->fb_funcs);
>> +	if (IS_ERR(fb)) {
>>   		dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
>> -		ret = PTR_ERR(fbdev_cma->fb);
>> +		ret = PTR_ERR(fb);
>>   		goto err_fb_info_destroy;
>>   	}
>>   
>> -	fb = &fbdev_cma->fb->fb;
>>   	helper->fb = fb;
>>   
>>   	fbi->par = helper;
>> @@ -500,7 +383,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
>>   	return 0;
>>   
>>   err_cma_destroy:
>> -	drm_framebuffer_remove(&fbdev_cma->fb->fb);
>> +	drm_framebuffer_remove(fb);
>>   err_fb_info_destroy:
>>   	drm_fb_helper_fini(helper);
>>   err_gem_free_object:
>> @@ -570,6 +453,11 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
>>   }
>>   EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs);
>>   
>> +static const struct drm_framebuffer_funcs drm_fb_cma_funcs = {
>> +	.destroy	= drm_gem_fb_destroy,
>> +	.create_handle	= drm_gem_fb_create_handle,
>> +};
> I don't see you removing this in the very last patch. It exactly matches
> the function table the gem fb helper provides, why do we need this dupe
> here?

It's used by drm_fbdev_cma_init(). To avoid this I either have to export
the structure from drm_gem_framebuffer_helper and use that, or let the
funcs argument to drm_gem_fb_create_with_funcs() be optional and through
that end up with the drm_gem_framebuffer_helper version as a default.

I really hope that we can find a way to give fb_helper a struct drm_file,
so we don't need to have these special functions for fbdev emulation.
We could just call .dumb_create and .fb_create.

Noralf.


> -Daniel
>
>> +
>>   /**
>>    * drm_fbdev_cma_init() - Allocate and initializes a drm_fbdev_cma struct
>>    * @dev: DRM device
>> @@ -597,8 +485,8 @@ void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
>>   	if (fbdev_cma->fb_helper.fbdev)
>>   		drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
>>   
>> -	if (fbdev_cma->fb)
>> -		drm_framebuffer_remove(&fbdev_cma->fb->fb);
>> +	if (fbdev_cma->fb_helper.fb)
>> +		drm_framebuffer_remove(fbdev_cma->fb_helper.fb);
>>   
>>   	drm_fb_helper_fini(&fbdev_cma->fb_helper);
>>   	kfree(fbdev_cma);
>> -- 
>> 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: [PATCH v2 01/22] drm: Add GEM backed framebuffer library

[Daniel Vetter]

On Thu, Aug 10, 2017 at 08:55:59PM +0200, Noralf Trønnes wrote:
> 
> Den 09.08.2017 22.04, skrev Daniel Vetter:
> > On Wed, Aug 09, 2017 at 12:11:04PM +0200, Noralf Trønnes wrote:
> > > This library provides helpers for drivers that don't subclass
> > > drm_framebuffer and are backed by drm_gem_object. The code is
> > > taken from drm_fb_cma_helper.
> > > 
> > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > lgtm, a few nits below to polish the documentation. With those addressed:
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> [...]
> 
> > > +/**
> > > + * drm_gem_fb_alloc - Allocate GEM backed framebuffer
> > > + * @dev: DRM device
> > > + * @mode_cmd: metadata from the userspace fb creation request
> > > + * @obj: GEM object(s) backing the framebuffer
> > > + * @num_planes: Number of planes
> > > + * @funcs: vtable to be used for the new framebuffer object
> > This kinda puts all the warnings from drm_framebuffer_init() under the
> > rug, so not enough text for such a tricky function. Proposal
> > 
> > "This allocates a &struct drm_framebuffer and publishes it by calling
> > drm_framebuffer_init().
> > 
> > IMPORTANT:
> > All checking and validation must be done before calling this function.
> > Framebuffers are supposed to be invariant over their lifetime, which means
> > evil userspace could otherwise trick the kernel into using unvalidated
> > framebuffer objects."
> > 
> > Not sure whether that kills your use-cases for this, in which case we
> > probably need to open-code this in all callers. Or remove the call to
> > drm_framebuffer_init() and place the driver's validation code between the
> > call to _alloc() and _init().
> > 
> > Maybe we should even change _init() to be called _register() to avoid this
> > bug in the future.
> 
> The only reason drm_gem_fb_alloc() is exported is to add framebuffers
> for fbdev emulation. So maybe it's better to hide that function and make
> an explicit one:

Yeah if you can just use this one here instead of the other, that looks
like the safer interface.

Aside: Do we really need to export this if it's only used for fbdev
allocation? Or is your plan that drivers will call this (like the cma
helpers already do)?

Anyway, my r-b stands if you add this function and unexport the alloc one
(and then I don't think it needs kerneldoc).
-Daniel

> 
> /**
>  * drm_gem_fbdev_fb_create - Create a drm_framebuffer for fbdev emulation
>  * @dev: DRM device
>  * @sizes: fbdev size description
>  * @pitch_align: optional pitch alignment
>  * @obj: GEM object backing the framebuffer
>  * @funcs: vtable to be used for the new framebuffer object
>  *
>  * This function creates a framebuffer for use with fbdev emulation.
>  *
>  * Returns:
>  * Pointer to a drm_framebuffer on success or an error pointer on failure.
>  */
> struct drm_framebuffer *
> drm_gem_fbdev_fb_create(struct drm_device *dev,
>             struct drm_fb_helper_surface_size *sizes,
>             unsigned int pitch_align, struct drm_gem_object *obj,
>             const struct drm_framebuffer_funcs *funcs)
> {
>     struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> 
>     mode_cmd.width = sizes->surface_width;
>     mode_cmd.height = sizes->surface_height;
>     mode_cmd.pitches[0] = sizes->surface_width *
>                   DIV_ROUND_UP(sizes->surface_bpp, 8);
>     if (pitch_align)
>         mode_cmd.pitches[0] = roundup(mode_cmd.pitches[0],
>                           pitch_align);
>     mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
>                             sizes->surface_depth);
>     if (obj->size < mode_cmd.pitches[0] * mode_cmd.height)
>         return ERR_PTR(-EINVAL);
> 
>     return drm_gem_fb_alloc(dev, &mode_cmd, &obj, 1, funcs);
> }
> EXPORT_SYMBOL(drm_gem_fbdev_fb_create);
> 
> 
> Noralf.
> 
> > But that's just all aside, from your create_with_funcs example below it
> > all looks good.
> > 
> > > + *
> > > + * Returns:
> > > + * Allocated struct drm_framebuffer * or error encoded pointer.
> > > + */
> > > +struct drm_framebuffer *
> > > +drm_gem_fb_alloc(struct drm_device *dev,
> > > +		 const struct drm_mode_fb_cmd2 *mode_cmd,
> > > +		 struct drm_gem_object **obj, unsigned int num_planes,
> > > +		 const struct drm_framebuffer_funcs *funcs)
> > > +{
> > > +	struct drm_framebuffer *fb;
> > > +	int ret, i;
> > > +
> > > +	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> > > +	if (!fb)
> > > +		return ERR_PTR(-ENOMEM);
> > > +
> > > +	drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
> > > +
> > > +	for (i = 0; i < num_planes; i++)
> > > +		fb->obj[i] = obj[i];
> > > +
> > > +	ret = drm_framebuffer_init(dev, fb, funcs);
> > > +	if (ret) {
> > > +		DRM_DEV_ERROR(dev->dev, "Failed to init framebuffer: %d\n",
> > > +			      ret);
> > > +		kfree(fb);
> > > +		return ERR_PTR(ret);
> > > +	}
> > > +
> > > +	return fb;
> > > +}
> > > +EXPORT_SYMBOL(drm_gem_fb_alloc);
> > > +
> > > +/**
> > > + * drm_gem_fb_destroy - Free GEM backed framebuffer
> > > + * @fb: DRM framebuffer
> > > + *
> > > + * Frees a GEM backed framebuffer with it's backing buffer(s) and the structure
> > > + * itself. Drivers can use this as their &drm_framebuffer_funcs->destroy
> > > + * callback.
> > > + */
> > > +void drm_gem_fb_destroy(struct drm_framebuffer *fb)
> > > +{
> > > +	int i;
> > > +
> > > +	for (i = 0; i < 4; i++) {
> > > +		if (fb->obj[i])
> > > +			drm_gem_object_put_unlocked(fb->obj[i]);
> > > +	}
> > > +
> > > +	drm_framebuffer_cleanup(fb);
> > > +	kfree(fb);
> > > +}
> > > +EXPORT_SYMBOL(drm_gem_fb_destroy);
> > > +
> > > +/**
> > > + * drm_gem_fb_create_handle - Create handle for GEM backed framebuffer
> > > + * @fb: DRM framebuffer
> > > + * @file: drm file
> > > + * @handle: handle created
> > > + *
> > > + * Drivers can use this as their &drm_framebuffer_funcs->create_handle
> > > + * callback.
> > > + *
> > > + * Returns:
> > > + * 0 on success or a negative error code on failure.
> > > + */
> > > +int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
> > > +			     unsigned int *handle)
> > > +{
> > > +	return drm_gem_handle_create(file, fb->obj[0], handle);
> > > +}
> > > +EXPORT_SYMBOL(drm_gem_fb_create_handle);
> > > +
> > > +/**
> > > + * drm_gem_fb_create_with_funcs() - helper function for the
> > > + *                                  &drm_mode_config_funcs.fb_create
> > > + *                                  callback
> > > + * @dev: DRM device
> > > + * @file: drm file for the ioctl call
> > > + * @mode_cmd: metadata from the userspace fb creation request
> > > + * @funcs: vtable to be used for the new framebuffer object
> > > + *
> > > + * This can be used to set &drm_framebuffer_funcs for drivers that need the
> > > + * &drm_framebuffer_funcs.dirty callback. Use drm_gem_fb_create() if you don't
> > > + * need to change &drm_framebuffer_funcs.
> > Maybe mention here and below that this already validates against the
> > buffer size (which is what most drivers need to do beyond the basic checks
> > the drm core already does).
> > 
> > One thing this doesn't do is validate the pixel format and modifiers. We
> > should only accept framebuffers which can be used for this driver (on any
> > drm_plane). Not sure this is something current drivers get right even, and
> > maybe we should try to check that in the core perhaps ... So different
> > patch series, just something that crossed my mind.
> > 
> > > + */
> > > +struct drm_framebuffer *
> > > +drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> > > +			     const struct drm_mode_fb_cmd2 *mode_cmd,
> > > +			     const struct drm_framebuffer_funcs *funcs)
> > > +{
> > > +	const struct drm_format_info *info;
> > > +	struct drm_gem_object *objs[4];
> > > +	struct drm_framebuffer *fb;
> > > +	int ret, i;
> > > +
> > > +	info = drm_get_format_info(dev, mode_cmd);
> > > +	if (!info)
> > > +		return ERR_PTR(-EINVAL);
> > > +
> > > +	for (i = 0; i < info->num_planes; i++) {
> > > +		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> > > +		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
> > > +		unsigned int min_size;
> > > +
> > > +		objs[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]);
> > > +		if (!objs[i]) {
> > > +			DRM_DEV_ERROR(dev->dev, "Failed to lookup GEM\n");
> > > +			ret = -ENOENT;
> > > +			goto err_gem_object_put;
> > > +		}
> > > +
> > > +		min_size = (height - 1) * mode_cmd->pitches[i]
> > > +			 + width * info->cpp[i]
> > > +			 + mode_cmd->offsets[i];
> > > +
> > > +		if (objs[i]->size < min_size) {
> > > +			drm_gem_object_put_unlocked(objs[i]);
> > > +			ret = -EINVAL;
> > > +			goto err_gem_object_put;
> > > +		}
> > > +	}
> > > +
> > > +	fb = drm_gem_fb_alloc(dev, mode_cmd, objs, i, funcs);
> > > +	if (IS_ERR(fb)) {
> > > +		ret = PTR_ERR(fb);
> > > +		goto err_gem_object_put;
> > > +	}
> > > +
> > > +	return fb;
> > > +
> > > +err_gem_object_put:
> > > +	for (i--; i >= 0; i--)
> > > +		drm_gem_object_put_unlocked(objs[i]);
> > > +
> > > +	return ERR_PTR(ret);
> > > +}
> > > +EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_funcs);
> > > +
> > > +static const struct drm_framebuffer_funcs drm_gem_fb_funcs = {
> > > +	.destroy	= drm_gem_fb_destroy,
> > > +	.create_handle	= drm_gem_fb_create_handle,
> > > +};
> > > +
> > > +/**
> > > + * drm_gem_fb_create() - &drm_mode_config_funcs.fb_create callback function
> > > + * @dev: DRM device
> > > + * @file: drm file for the ioctl call
> > > + * @mode_cmd: metadata from the userspace fb creation request
> > > + *
> > > + * If your hardware has special alignment or pitch requirements these should be
> > > + * checked before calling this function. Use drm_gem_fb_create_with_funcs() if
> > > + * you need to set &drm_framebuffer_funcs.dirty.
> > > + */
> > > +struct drm_framebuffer *
> > > +drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
> > > +		  const struct drm_mode_fb_cmd2 *mode_cmd)
> > > +{
> > > +	return drm_gem_fb_create_with_funcs(dev, file, mode_cmd,
> > > +					    &drm_gem_fb_funcs);
> > > +}
> > > +EXPORT_SYMBOL_GPL(drm_gem_fb_create);
> > > +
> > > +/**
> > > + * drm_gem_fb_prepare_fb() - Prepare gem framebuffer
> > > + * @plane: Which plane
> > > + * @state: Plane state attach fence to
> > > + *
> > > + * This should be set as the &struct drm_plane_helper_funcs.prepare_fb hook.
> > We tend to not use the struct label when referencing members, so just
> > &drm_plane_helper_funcs.prepare_fb hook.
> > 
> > > + *
> > > + * This function checks if the plane FB has an dma-buf attached, extracts
> > > + * the exclusive fence and attaches it to plane state for the atomic helper
> > > + * to wait on.
> > > + *
> > > + * There is no need for cleanup_fb for gem based framebuffer drivers.
> > A bit too simple:
> > 
> > "There is no need for &drm_plane_helper_funcs.cleanup_fb hook for simple
> > gem based framebuffer drivers which have their buffers always pinned in
> > memory."
> > 
> > 
> > > + */
> > > +int drm_gem_fb_prepare_fb(struct drm_plane *plane,
> > > +			  struct drm_plane_state *state)
> > > +{
> > > +	struct dma_buf *dma_buf;
> > > +	struct dma_fence *fence;
> > > +
> > > +	if ((plane->state->fb == state->fb) || !state->fb)
> > > +		return 0;
> > > +
> > > +	dma_buf = drm_gem_fb_get_obj(state->fb, 0)->dma_buf;
> > > +	if (dma_buf) {
> > > +		fence = reservation_object_get_excl_rcu(dma_buf->resv);
> > > +		drm_atomic_set_fence_for_plane(state, fence);
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(drm_gem_fb_prepare_fb);
> > > diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
> > > index 5244f05..50def22 100644
> > > --- a/include/drm/drm_framebuffer.h
> > > +++ b/include/drm/drm_framebuffer.h
> > > @@ -190,6 +190,10 @@ struct drm_framebuffer {
> > >   	 * @filp_head: Placed on &drm_file.fbs, protected by &drm_file.fbs_lock.
> > >   	 */
> > >   	struct list_head filp_head;
> > > +	/**
> > > +	 * @obj: GEM objects backing the framebuffer, one per plane (optional).
> > I'd elaborate a bit more:
> > 
> > "This is used by the GEM framebuffer helpers, see e.g.
> > drm_gem_fb_create()."
> > 
> > > +	 */
> > > +	struct drm_gem_object *obj[4];
> > >   };
> > >   #define obj_to_fb(x) container_of(x, struct drm_framebuffer, base)
> > > diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> > > new file mode 100644
> > > index 0000000..af3d1ee
> > > --- /dev/null
> > > +++ b/include/drm/drm_gem_framebuffer_helper.h
> > > @@ -0,0 +1,35 @@
> > > +#ifndef __DRM_GEM_FB_HELPER_H__
> > > +#define __DRM_GEM_FB_HELPER_H__
> > > +
> > > +struct drm_device;
> > > +struct drm_file;
> > > +struct drm_framebuffer;
> > > +struct drm_framebuffer_funcs;
> > > +struct drm_gem_object;
> > > +struct drm_mode_fb_cmd2;
> > > +struct drm_plane;
> > > +struct drm_plane_state;
> > > +
> > > +struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
> > > +					  unsigned int plane);
> > > +struct drm_framebuffer *
> > > +drm_gem_fb_alloc(struct drm_device *dev,
> > > +		 const struct drm_mode_fb_cmd2 *mode_cmd,
> > > +		 struct drm_gem_object **obj, unsigned int num_planes,
> > > +		 const struct drm_framebuffer_funcs *funcs);
> > > +void drm_gem_fb_destroy(struct drm_framebuffer *fb);
> > > +int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file *file,
> > > +			     unsigned int *handle);
> > > +
> > > +struct drm_framebuffer *
> > > +drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> > > +			     const struct drm_mode_fb_cmd2 *mode_cmd,
> > > +			     const struct drm_framebuffer_funcs *funcs);
> > > +struct drm_framebuffer *
> > > +drm_gem_fb_create(struct drm_device *dev, struct drm_file *file,
> > > +		  const struct drm_mode_fb_cmd2 *mode_cmd);
> > > +
> > > +int drm_gem_fb_prepare_fb(struct drm_plane *plane,
> > > +			  struct drm_plane_state *state);
> > > +
> > > +#endif
> > > -- 
> > > 2.7.4
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 02/22] drm/fb-cma-helper: Use drm_gem_framebuffer_helper

[Daniel Vetter]

On Thu, Aug 10, 2017 at 09:04:34PM +0200, Noralf Trønnes wrote:
> 
> Den 09.08.2017 22.07, skrev Daniel Vetter:
> > On Wed, Aug 09, 2017 at 12:11:05PM +0200, Noralf Trønnes wrote:
> > > Use the new drm_gem_framebuffer_helper who's code was copied
> > > from this helper.
> > > 
> > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > > ---
> > >   drivers/gpu/drm/drm_fb_cma_helper.c | 172 +++++++-----------------------------
> > >   1 file changed, 30 insertions(+), 142 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > index ade319d..d0c0110 100644
> > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> > > @@ -18,27 +18,17 @@
> > >    */
> > >   #include <drm/drmP.h>
> > > -#include <drm/drm_atomic.h>
> > > -#include <drm/drm_crtc.h>
> > >   #include <drm/drm_fb_helper.h>
> > > -#include <drm/drm_crtc_helper.h>
> > > +#include <drm/drm_framebuffer.h>
> > >   #include <drm/drm_gem_cma_helper.h>
> > > +#include <drm/drm_gem_framebuffer_helper.h>
> > >   #include <drm/drm_fb_cma_helper.h>
> > > -#include <linux/dma-buf.h>
> > > -#include <linux/dma-mapping.h>
> > >   #include <linux/module.h>
> > > -#include <linux/reservation.h>
> > >   #define DEFAULT_FBDEFIO_DELAY_MS 50
> > > -struct drm_fb_cma {
> > > -	struct drm_framebuffer		fb;
> > > -	struct drm_gem_cma_object	*obj[4];
> > > -};
> > > -
> > >   struct drm_fbdev_cma {
> > >   	struct drm_fb_helper	fb_helper;
> > > -	struct drm_fb_cma	*fb;
> > >   	const struct drm_framebuffer_funcs *fb_funcs;
> > >   };
> > > @@ -90,69 +80,19 @@ static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper)
> > >   	return container_of(helper, struct drm_fbdev_cma, fb_helper);
> > >   }
> > > -static inline struct drm_fb_cma *to_fb_cma(struct drm_framebuffer *fb)
> > > -{
> > > -	return container_of(fb, struct drm_fb_cma, fb);
> > > -}
> > > -
> > >   void drm_fb_cma_destroy(struct drm_framebuffer *fb)
> > >   {
> > > -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> > > -	int i;
> > > -
> > > -	for (i = 0; i < 4; i++) {
> > > -		if (fb_cma->obj[i])
> > > -			drm_gem_object_put_unlocked(&fb_cma->obj[i]->base);
> > > -	}
> > > -
> > > -	drm_framebuffer_cleanup(fb);
> > > -	kfree(fb_cma);
> > > +	drm_gem_fb_destroy(fb);
> > >   }
> > >   EXPORT_SYMBOL(drm_fb_cma_destroy);
> > >   int drm_fb_cma_create_handle(struct drm_framebuffer *fb,
> > >   	struct drm_file *file_priv, unsigned int *handle)
> > >   {
> > > -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> > > -
> > > -	return drm_gem_handle_create(file_priv,
> > > -			&fb_cma->obj[0]->base, handle);
> > > +	return drm_gem_fb_create_handle(fb, file_priv, handle);
> > >   }
> > >   EXPORT_SYMBOL(drm_fb_cma_create_handle);
> > > -static struct drm_framebuffer_funcs drm_fb_cma_funcs = {
> > > -	.destroy	= drm_fb_cma_destroy,
> > > -	.create_handle	= drm_fb_cma_create_handle,
> > > -};
> > > -
> > > -static struct drm_fb_cma *drm_fb_cma_alloc(struct drm_device *dev,
> > > -	const struct drm_mode_fb_cmd2 *mode_cmd,
> > > -	struct drm_gem_cma_object **obj,
> > > -	unsigned int num_planes, const struct drm_framebuffer_funcs *funcs)
> > > -{
> > > -	struct drm_fb_cma *fb_cma;
> > > -	int ret;
> > > -	int i;
> > > -
> > > -	fb_cma = kzalloc(sizeof(*fb_cma), GFP_KERNEL);
> > > -	if (!fb_cma)
> > > -		return ERR_PTR(-ENOMEM);
> > > -
> > > -	drm_helper_mode_fill_fb_struct(dev, &fb_cma->fb, mode_cmd);
> > > -
> > > -	for (i = 0; i < num_planes; i++)
> > > -		fb_cma->obj[i] = obj[i];
> > > -
> > > -	ret = drm_framebuffer_init(dev, &fb_cma->fb, funcs);
> > > -	if (ret) {
> > > -		dev_err(dev->dev, "Failed to initialize framebuffer: %d\n", ret);
> > > -		kfree(fb_cma);
> > > -		return ERR_PTR(ret);
> > > -	}
> > > -
> > > -	return fb_cma;
> > > -}
> > > -
> > >   /**
> > >    * drm_fb_cma_create_with_funcs() - helper function for the
> > >    *                                  &drm_mode_config_funcs.fb_create
> > > @@ -170,53 +110,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct drm_device *dev,
> > >   	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
> > >   	const struct drm_framebuffer_funcs *funcs)
> > >   {
> > > -	const struct drm_format_info *info;
> > > -	struct drm_fb_cma *fb_cma;
> > > -	struct drm_gem_cma_object *objs[4];
> > > -	struct drm_gem_object *obj;
> > > -	int ret;
> > > -	int i;
> > > -
> > > -	info = drm_get_format_info(dev, mode_cmd);
> > > -	if (!info)
> > > -		return ERR_PTR(-EINVAL);
> > > -
> > > -	for (i = 0; i < info->num_planes; i++) {
> > > -		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> > > -		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
> > > -		unsigned int min_size;
> > > -
> > > -		obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[i]);
> > > -		if (!obj) {
> > > -			dev_err(dev->dev, "Failed to lookup GEM object\n");
> > > -			ret = -ENOENT;
> > > -			goto err_gem_object_put;
> > > -		}
> > > -
> > > -		min_size = (height - 1) * mode_cmd->pitches[i]
> > > -			 + width * info->cpp[i]
> > > -			 + mode_cmd->offsets[i];
> > > -
> > > -		if (obj->size < min_size) {
> > > -			drm_gem_object_put_unlocked(obj);
> > > -			ret = -EINVAL;
> > > -			goto err_gem_object_put;
> > > -		}
> > > -		objs[i] = to_drm_gem_cma_obj(obj);
> > > -	}
> > > -
> > > -	fb_cma = drm_fb_cma_alloc(dev, mode_cmd, objs, i, funcs);
> > > -	if (IS_ERR(fb_cma)) {
> > > -		ret = PTR_ERR(fb_cma);
> > > -		goto err_gem_object_put;
> > > -	}
> > > -
> > > -	return &fb_cma->fb;
> > > -
> > > -err_gem_object_put:
> > > -	for (i--; i >= 0; i--)
> > > -		drm_gem_object_put_unlocked(&objs[i]->base);
> > > -	return ERR_PTR(ret);
> > > +	return drm_gem_fb_create_with_funcs(dev, file_priv, mode_cmd, funcs);
> > >   }
> > >   EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
> > > @@ -233,8 +127,7 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_create_with_funcs);
> > >   struct drm_framebuffer *drm_fb_cma_create(struct drm_device *dev,
> > >   	struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd)
> > >   {
> > > -	return drm_fb_cma_create_with_funcs(dev, file_priv, mode_cmd,
> > > -					    &drm_fb_cma_funcs);
> > > +	return drm_gem_fb_create(dev, file_priv, mode_cmd);
> > >   }
> > >   EXPORT_SYMBOL_GPL(drm_fb_cma_create);
> > > @@ -250,12 +143,14 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_create);
> > >   struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb,
> > >   						  unsigned int plane)
> > >   {
> > > -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> > > +	struct drm_gem_object *gem;
> > > -	if (plane >= 4)
> > > +	gem = drm_gem_fb_get_obj(fb, plane);
> > > +	if (!gem)
> > >   		return NULL;
> > > -	return fb_cma->obj[plane];
> > > +	return to_drm_gem_cma_obj(gem);
> > > +
> > >   }
> > >   EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj);
> > > @@ -272,13 +167,14 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb,
> > >   				   struct drm_plane_state *state,
> > >   				   unsigned int plane)
> > >   {
> > > -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> > > +	struct drm_gem_cma_object *obj;
> > >   	dma_addr_t paddr;
> > > -	if (plane >= 4)
> > > +	obj = drm_fb_cma_get_gem_obj(fb, plane);
> > > +	if (!obj)
> > >   		return 0;
> > > -	paddr = fb_cma->obj[plane]->paddr + fb->offsets[plane];
> > > +	paddr = obj->paddr + fb->offsets[plane];
> > >   	paddr += fb->format->cpp[plane] * (state->src_x >> 16);
> > >   	paddr += fb->pitches[plane] * (state->src_y >> 16);
> > > @@ -302,26 +198,13 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_addr);
> > >   int drm_fb_cma_prepare_fb(struct drm_plane *plane,
> > >   			  struct drm_plane_state *state)
> > >   {
> > > -	struct dma_buf *dma_buf;
> > > -	struct dma_fence *fence;
> > > -
> > > -	if ((plane->state->fb == state->fb) || !state->fb)
> > > -		return 0;
> > > -
> > > -	dma_buf = drm_fb_cma_get_gem_obj(state->fb, 0)->base.dma_buf;
> > > -	if (dma_buf) {
> > > -		fence = reservation_object_get_excl_rcu(dma_buf->resv);
> > > -		drm_atomic_set_fence_for_plane(state, fence);
> > > -	}
> > > -
> > > -	return 0;
> > > +	return drm_gem_fb_prepare_fb(plane, state);
> > >   }
> > >   EXPORT_SYMBOL_GPL(drm_fb_cma_prepare_fb);
> > >   #ifdef CONFIG_DEBUG_FS
> > >   static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
> > >   {
> > > -	struct drm_fb_cma *fb_cma = to_fb_cma(fb);
> > >   	int i;
> > >   	seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height,
> > > @@ -330,7 +213,7 @@ static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
> > >   	for (i = 0; i < fb->format->num_planes; i++) {
> > >   		seq_printf(m, "   %d: offset=%d pitch=%d, obj: ",
> > >   				i, fb->offsets[i], fb->pitches[i]);
> > > -		drm_gem_cma_describe(fb_cma->obj[i], m);
> > > +		drm_gem_cma_describe(drm_fb_cma_get_gem_obj(fb, i), m);
> > >   	}
> > >   }
> > > @@ -434,6 +317,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
> > >   	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> > >   	struct drm_device *dev = helper->dev;
> > >   	struct drm_gem_cma_object *obj;
> > > +	struct drm_gem_object *gem;
> > >   	struct drm_framebuffer *fb;
> > >   	unsigned int bytes_per_pixel;
> > >   	unsigned long offset;
> > > @@ -464,15 +348,14 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
> > >   		goto err_gem_free_object;
> > >   	}
> > > -	fbdev_cma->fb = drm_fb_cma_alloc(dev, &mode_cmd, &obj, 1,
> > > -					 fbdev_cma->fb_funcs);
> > > -	if (IS_ERR(fbdev_cma->fb)) {
> > > +	gem = &obj->base;
> > > +	fb = drm_gem_fb_alloc(dev, &mode_cmd, &gem, 1, fbdev_cma->fb_funcs);
> > > +	if (IS_ERR(fb)) {
> > >   		dev_err(dev->dev, "Failed to allocate DRM framebuffer.\n");
> > > -		ret = PTR_ERR(fbdev_cma->fb);
> > > +		ret = PTR_ERR(fb);
> > >   		goto err_fb_info_destroy;
> > >   	}
> > > -	fb = &fbdev_cma->fb->fb;
> > >   	helper->fb = fb;
> > >   	fbi->par = helper;
> > > @@ -500,7 +383,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
> > >   	return 0;
> > >   err_cma_destroy:
> > > -	drm_framebuffer_remove(&fbdev_cma->fb->fb);
> > > +	drm_framebuffer_remove(fb);
> > >   err_fb_info_destroy:
> > >   	drm_fb_helper_fini(helper);
> > >   err_gem_free_object:
> > > @@ -570,6 +453,11 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
> > >   }
> > >   EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs);
> > > +static const struct drm_framebuffer_funcs drm_fb_cma_funcs = {
> > > +	.destroy	= drm_gem_fb_destroy,
> > > +	.create_handle	= drm_gem_fb_create_handle,
> > > +};
> > I don't see you removing this in the very last patch. It exactly matches
> > the function table the gem fb helper provides, why do we need this dupe
> > here?
> 
> It's used by drm_fbdev_cma_init(). To avoid this I either have to export
> the structure from drm_gem_framebuffer_helper and use that, or let the
> funcs argument to drm_gem_fb_create_with_funcs() be optional and through
> that end up with the drm_gem_framebuffer_helper version as a default.
> 
> I really hope that we can find a way to give fb_helper a struct drm_file,
> so we don't need to have these special functions for fbdev emulation.
> We could just call .dumb_create and .fb_create.

Not sure we discussed this, but iirc the prep work from David Herrmann to
allow creating a drm_file without a struct file backing it has landed. But
that's definitely long-term plans, I just wanted to know why we need this.
I don't think over-polishing this stuff already makes sense, better to get
your big patch series in first.
-Daniel

> 
> Noralf.
> 
> 
> > -Daniel
> > 
> > > +
> > >   /**
> > >    * drm_fbdev_cma_init() - Allocate and initializes a drm_fbdev_cma struct
> > >    * @dev: DRM device
> > > @@ -597,8 +485,8 @@ void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
> > >   	if (fbdev_cma->fb_helper.fbdev)
> > >   		drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
> > > -	if (fbdev_cma->fb)
> > > -		drm_framebuffer_remove(&fbdev_cma->fb->fb);
> > > +	if (fbdev_cma->fb_helper.fb)
> > > +		drm_framebuffer_remove(fbdev_cma->fb_helper.fb);
> > >   	drm_fb_helper_fini(&fbdev_cma->fb_helper);
> > >   	kfree(fbdev_cma);
> > > -- 
> > > 2.7.4
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel