Den 06.11.2017 10.04, skrev Daniel Vetter:
On Sat, Nov 04, 2017 at 02:03:56PM +0100, Noralf Trønnes wrote:
Add functions drm_fb_cma_fbdev_init(), drm_fb_cma_fbdev_fini() and
drm_fb_cma_fbdev_init_with_funcs(). These functions relies on the fact
that the drm_fb_helper struct is stored in dev->drm_fb_helper_private
so drivers don't need to store it.
Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
I guess you've proven me wrong, since at the end of this patch series we
do have some neat functions that do the same thing an earlier patch series
did, expect there's a _cma_ in the name. But the function itself is 100%
generic.
Sry for all the meandering, I guess this once again shows that any problem
in software can indeed be solved by adding another abstraction layer on
top. You'll probably be slightly mad, but I'd vote for the patch to drop
the _cma_ infix again (once everything has landed, no need to respin all
the patches again).
The purpose here is not to add a layer, but to change an existing one.
I want to get rid of struct drm_fbdev_cma and I have 2 options:
- One patch that touches all drivers
- Add new functions, switch over drivers one patch at a time
In effect this is what I do in this patchset:
include/drm/drm_fb_cma_helper.h:
+int drm_fb_cma_fbdev_init_with_funcs(struct drm_device *dev,
+ unsigned int preferred_bpp, unsigned int max_conn_count,
+ const struct drm_framebuffer_funcs *funcs);
+int drm_fb_cma_fbdev_init(struct drm_device *dev, unsigned int
preferred_bpp,
+ unsigned int max_conn_count);
+void drm_fb_cma_fbdev_fini(struct drm_device *dev);
+
-struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
- unsigned int preferred_bpp, unsigned int max_conn_count,
- const struct drm_framebuffer_funcs *funcs);
-struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
- unsigned int preferred_bpp, unsigned int max_conn_count);
-void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma);
-
How about this commit message:
drm/cma-helper: Add drm_fb_cma_fbdev_init/fini()
There is no need for drivers to carry a pointer to the the fbdev
emulation structure now that drm_device has one. Currently drivers has
a pointer to struct drm_fbdev_cma in their driver structure. This
pointer is returned by drm_fbdev_cma_init().
In order to get rid of struct drm_fbdev_cma, add functions
drm_fb_cma_fbdev_init(), drm_fb_cma_fbdev_fini() and
drm_fb_cma_fbdev_init_with_funcs(). These functions relies on the fact
that the drm_fb_helper struct is stored in drm_device->fb_helper so
drivers don't need to store it. When drivers have been moved over to
these functions, the old init/fini functions can be removed.
Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
Noralf.
---
drivers/gpu/drm/drm_fb_cma_helper.c | 116 +++++++++++++++++++++++++++++++++++-
include/drm/drm_fb_cma_helper.h | 7 +++
2 files changed, 121 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 0e3c14174d08..267c04216281 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -23,6 +23,7 @@
#include <drm/drm_gem_cma_helper.h>
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_print.h>
#include <linux/module.h>
#define DEFAULT_FBDEFIO_DELAY_MS 50
@@ -42,7 +43,7 @@ struct drm_fbdev_cma {
* callback function to create a cma backed framebuffer.
*
* An fbdev framebuffer backed by cma is also available by calling
- * drm_fbdev_cma_init(). drm_fbdev_cma_fini() tears it down.
+ * drm_fb_cma_fbdev_init(). drm_fb_cma_fbdev_fini() tears it down.
* If the &drm_framebuffer_funcs.dirty callback is set, fb_deferred_io will be
* set up automatically. &drm_framebuffer_funcs.dirty is called by
* drm_fb_helper_deferred_io() in process context (&struct delayed_work).
@@ -68,7 +69,7 @@ struct drm_fbdev_cma {
*
* Initialize::
*
- * fbdev = drm_fbdev_cma_init_with_funcs(dev, 16,
+ * fbdev = drm_fb_cma_fbdev_init_with_funcs(dev, 16,
* dev->mode_config.num_crtc,
* dev->mode_config.num_connector,
* &driver_fb_funcs);
@@ -314,6 +315,117 @@ static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
.fb_probe = drm_fbdev_cma_create,
};
+/**
+ * drm_fb_cma_fbdev_init_with_funcs() - Allocate and initialize fbdev emulation
+ * @dev: DRM device
+ * @preferred_bpp: Preferred bits per pixel for the device.
+ * @dev->mode_config.preferred_depth is used if this is zero.
+ * @max_conn_count: Maximum number of connectors.
+ * @dev->mode_config.num_connector is used if this is zero.
+ * @funcs: Framebuffer functions, in particular a custom dirty() callback.
With the previous patch this is allowed to be NULL, right? Please
document.
And since that also explains the patch 1, on both patches:
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
But please get an ack from Laurent too.
-Daniel
+ *
+ * Returns:
+ * Zero on success or negative error code on failure.
+ */
+int drm_fb_cma_fbdev_init_with_funcs(struct drm_device *dev,
+ unsigned int preferred_bpp, unsigned int max_conn_count,
+ const struct drm_framebuffer_funcs *funcs)
+{
+ struct drm_fbdev_cma *fbdev_cma;
+ struct drm_fb_helper *fb_helper;
+ int ret;
+
+ if (!preferred_bpp)
+ preferred_bpp = dev->mode_config.preferred_depth;
+ if (!preferred_bpp)
+ preferred_bpp = 32;
+
+ if (!max_conn_count)
+ max_conn_count = dev->mode_config.num_connector;
+
+ fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL);
+ if (!fbdev_cma)
+ return -ENOMEM;
+
+ fbdev_cma->fb_funcs = funcs;
+ fb_helper = &fbdev_cma->fb_helper;
+
+ drm_fb_helper_prepare(dev, fb_helper, &drm_fb_cma_helper_funcs);
+
+ ret = drm_fb_helper_init(dev, fb_helper, max_conn_count);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dev->dev, "Failed to initialize fbdev helper.\n");
+ goto err_free;
+ }
+
+ ret = drm_fb_helper_single_add_all_connectors(fb_helper);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dev->dev, "Failed to add connectors.\n");
+ goto err_drm_fb_helper_fini;
+ }
+
+ ret = drm_fb_helper_initial_config(fb_helper, preferred_bpp);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dev->dev, "Failed to set fbdev configuration.\n");
+ goto err_drm_fb_helper_fini;
+ }
+
+ return 0;
+
+err_drm_fb_helper_fini:
+ drm_fb_helper_fini(fb_helper);
+err_free:
+ kfree(fbdev_cma);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_init_with_funcs);
+
+/**
+ * drm_fb_cma_fbdev_init() - Allocate and initialize fbdev emulation
+ * @dev: DRM device
+ * @preferred_bpp: Preferred bits per pixel for the device.
+ * @dev->mode_config.preferred_depth is used if this is zero.
+ * @max_conn_count: Maximum number of connectors.
+ * @dev->mode_config.num_connector is used if this is zero.
+ *
+ * Returns:
+ * Zero on success or negative error code on failure.
+ */
+int drm_fb_cma_fbdev_init(struct drm_device *dev, unsigned int preferred_bpp,
+ unsigned int max_conn_count)
+{
+ return drm_fb_cma_fbdev_init_with_funcs(dev, preferred_bpp,
+ max_conn_count, NULL);
+}
+EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_init);
+
+/**
+ * drm_fb_cma_fbdev_fini() - Teardown fbdev emulation
+ * @dev: DRM device
+ */
+void drm_fb_cma_fbdev_fini(struct drm_device *dev)
+{
+ struct drm_fb_helper *fb_helper = dev->fb_helper;
+
+ if (!fb_helper)
+ return;
+
+ /* Unregister if it hasn't been done already */
+ if (fb_helper->fbdev && fb_helper->fbdev->dev)
+ drm_fb_helper_unregister_fbi(fb_helper);
+
+ if (fb_helper->fbdev)
+ drm_fbdev_cma_defio_fini(fb_helper->fbdev);
+
+ if (fb_helper->fb)
+ drm_framebuffer_remove(fb_helper->fb);
+
+ drm_fb_helper_fini(fb_helper);
+ kfree(to_fbdev_cma(fb_helper));
+}
+EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_fini);
+
/**
* drm_fbdev_cma_init_with_funcs() - Allocate and initializes a drm_fbdev_cma struct
* @dev: DRM device
diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h
index 023f052a5873..22be22e641a4 100644
--- a/include/drm/drm_fb_cma_helper.h
+++ b/include/drm/drm_fb_cma_helper.h
@@ -15,6 +15,13 @@ struct drm_mode_fb_cmd2;
struct drm_plane;
struct drm_plane_state;
+int drm_fb_cma_fbdev_init_with_funcs(struct drm_device *dev,
+ unsigned int preferred_bpp, unsigned int max_conn_count,
+ const struct drm_framebuffer_funcs *funcs);
+int drm_fb_cma_fbdev_init(struct drm_device *dev, unsigned int preferred_bpp,
+ unsigned int max_conn_count);
+void drm_fb_cma_fbdev_fini(struct drm_device *dev);
+
struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev,
unsigned int preferred_bpp, unsigned int max_conn_count,
const struct drm_framebuffer_funcs *funcs);
--
2.14.2
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
