* [PATCH 0/7] Add drm_fb_helper_fbdev_setup/teardown()
@ 2017-12-14 19:10 Noralf Trønnes
2017-12-14 19:10 ` [PATCH 1/7] drm/fb-helper: Set/clear dev->fb_helper in dummy init/fini Noralf Trønnes
` (6 more replies)
0 siblings, 7 replies; 19+ messages in thread
From: Noralf Trønnes @ 2017-12-14 19:10 UTC (permalink / raw)
To: dri-devel; +Cc: daniel.vetter, laurent.pinchart
Hi,
This is a new attempt at simplifying fbdev setup/teardown in drivers.
Hopefully this one is better than my previous attempt. The previous
version allocated the drm_fb_helper struct as well, this time I leave it
to the driver.
I've included an fbdev deferred I/O setup helper as well since it's part
of the teardown.
As always I struggle with documentation so I welcome all help in getting
the words right.
Noralf.
Noralf Trønnes (7):
drm/fb-helper: Set/clear dev->fb_helper in dummy init/fini
drm/fb-helper: Add drm_fb_helper_fbdev_setup/teardown()
drm/docs: Add todo entry for drm_fb_helper_fbdev_setup()
drm/fb-helper: Update DOC with new helpers
drm/fb-helper: Add drm_fb_helper_defio_init()
drm/fb-cma-helper: Use drm_fb_helper_fbdev_setup/teardown()
drm/fb-cma-helper: Honour DRM_FBDEV_EMULATION
Documentation/gpu/todo.rst | 18 ++++
drivers/gpu/drm/Kconfig | 5 +-
drivers/gpu/drm/drm_fb_cma_helper.c | 111 +++-------------------
drivers/gpu/drm/drm_fb_helper.c | 181 +++++++++++++++++++++++++++++++++---
include/drm/drm_fb_helper.h | 38 ++++++++
5 files changed, 241 insertions(+), 112 deletions(-)
--
2.14.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/7] drm/fb-helper: Set/clear dev->fb_helper in dummy init/fini
2017-12-14 19:10 [PATCH 0/7] Add drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
@ 2017-12-14 19:10 ` Noralf Trønnes
2017-12-14 20:10 ` Daniel Vetter
2017-12-14 19:10 ` [PATCH 2/7] drm/fb-helper: Add drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
` (5 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Noralf Trønnes @ 2017-12-14 19:10 UTC (permalink / raw)
To: dri-devel; +Cc: daniel.vetter, laurent.pinchart
Set dev->fb_helper even when fbdev emulation is compiled out,
so drivers can use it to free the structure.
Clear it for consistency.
Fixes: 29ad20b22c8f ("drm: Add drm_device->fb_helper pointer")
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
include/drm/drm_fb_helper.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 1494b12a984a..1bd624579db7 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -33,6 +33,7 @@
struct drm_fb_helper;
#include <drm/drm_crtc.h>
+#include <drm/drm_device.h>
#include <linux/kgdb.h>
enum mode_set_atomic {
@@ -332,11 +333,17 @@ static inline int drm_fb_helper_init(struct drm_device *dev,
struct drm_fb_helper *helper,
int max_conn)
{
+ /* So drivers can use it to free the struct */
+ helper->dev = dev;
+ dev->fb_helper = helper;
+
return 0;
}
static inline void drm_fb_helper_fini(struct drm_fb_helper *helper)
{
+ if (helper && helper->dev)
+ helper->dev->fb_helper = NULL;
}
static inline int drm_fb_helper_blank(int blank, struct fb_info *info)
--
2.14.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/7] drm/fb-helper: Add drm_fb_helper_fbdev_setup/teardown()
2017-12-14 19:10 [PATCH 0/7] Add drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
2017-12-14 19:10 ` [PATCH 1/7] drm/fb-helper: Set/clear dev->fb_helper in dummy init/fini Noralf Trønnes
@ 2017-12-14 19:10 ` Noralf Trønnes
2017-12-14 20:19 ` Daniel Vetter
2017-12-14 19:10 ` [PATCH 3/7] drm/docs: Add todo entry for drm_fb_helper_fbdev_setup() Noralf Trønnes
` (4 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Noralf Trønnes @ 2017-12-14 19:10 UTC (permalink / raw)
To: dri-devel; +Cc: daniel.vetter, laurent.pinchart
Add helpers to setup and teardown fbdev emulation.
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
drivers/gpu/drm/drm_fb_helper.c | 106 ++++++++++++++++++++++++++++++++++++++++
include/drm/drm_fb_helper.h | 25 ++++++++++
2 files changed, 131 insertions(+)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 04a3a5ce370a..823fc8f50d85 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2729,6 +2729,112 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
}
EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
+/**
+ * drm_fb_helper_fbdev_setup() - Setup fbdev emulation
+ * @dev: DRM device
+ * @fb_helper: fbdev helper structure to set up
+ * @funcs: fbdev helper functions
+ * @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.
+ *
+ * This function sets up fbdev emulation and registers fbdev for access by
+ * userspace. If all connectors are disconnected, setup is deferred to the next
+ * time drm_fb_helper_hotplug_event() is called.
+ * The caller must to provide a &drm_fb_helper_funcs->fb_probe callback
+ * function.
+ *
+ * See also: drm_fb_helper_initial_config()
+ *
+ * Returns:
+ * Zero on success or negative error code on failure.
+ */
+int drm_fb_helper_fbdev_setup(struct drm_device *dev,
+ struct drm_fb_helper *fb_helper,
+ const struct drm_fb_helper_funcs *funcs,
+ unsigned int preferred_bpp,
+ unsigned int max_conn_count)
+{
+ 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;
+ if (!max_conn_count) {
+ DRM_DEV_ERROR(dev->dev, "No connectors\n");
+ return -EINVAL;
+ }
+
+ drm_fb_helper_prepare(dev, fb_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");
+ return ret;
+ }
+
+ 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;
+ }
+
+ if (!drm_drv_uses_atomic_modeset(dev))
+ drm_helper_disable_unused_functions(dev);
+
+ 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);
+
+ return ret;
+}
+EXPORT_SYMBOL(drm_fb_helper_fbdev_setup);
+
+/**
+ * drm_fb_helper_fbdev_teardown - Tear down fbdev emulation
+ * @dev: DRM device
+ *
+ * This function unregisters fbdev if not already done and cleans up the
+ * associated resources including the &drm_framebuffer.
+ * The driver is responsible for freeing the &drm_fb_helper structure which is
+ * stored in &drm_device->fb_helper. Do note that this pointer has been cleared
+ * when this function returns.
+ *
+ * In order to support device removal/unplug while file handles are still open,
+ * drm_fb_helper_unregister_fbi() should be called on device removal and
+ * drm_fb_helper_fbdev_teardown() in the &drm_driver->release callback when
+ * file handles are closed.
+ */
+void drm_fb_helper_fbdev_teardown(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);
+
+ drm_fb_helper_fini(fb_helper);
+
+ if (fb_helper->fb)
+ drm_framebuffer_remove(fb_helper->fb);
+}
+EXPORT_SYMBOL(drm_fb_helper_fbdev_teardown);
+
/**
* drm_fb_helper_lastclose - DRM driver lastclose helper for fbdev emulation
* @dev: DRM device
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 1bd624579db7..aa78ac3b8ad0 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -320,6 +320,13 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_
int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
struct drm_connector *connector);
+int drm_fb_helper_fbdev_setup(struct drm_device *dev,
+ struct drm_fb_helper *fb_helper,
+ const struct drm_fb_helper_funcs *funcs,
+ unsigned int preferred_bpp,
+ unsigned int max_conn_count);
+void drm_fb_helper_fbdev_teardown(struct drm_device *dev);
+
void drm_fb_helper_lastclose(struct drm_device *dev);
void drm_fb_helper_output_poll_changed(struct drm_device *dev);
#else
@@ -525,6 +532,24 @@ drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
return 0;
}
+static inline int
+drm_fb_helper_fbdev_setup(struct drm_device *dev,
+ struct drm_fb_helper *fb_helper,
+ const struct drm_fb_helper_funcs *funcs,
+ unsigned int preferred_bpp,
+ unsigned int max_conn_count)
+{
+ /* So drivers can use it to free the struct */
+ dev->fb_helper = fb_helper;
+
+ return 0;
+}
+
+static inline void drm_fb_helper_fbdev_teardown(struct drm_device *dev)
+{
+ dev->fb_helper = NULL;
+}
+
static inline void drm_fb_helper_lastclose(struct drm_device *dev)
{
}
--
2.14.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/7] drm/docs: Add todo entry for drm_fb_helper_fbdev_setup()
2017-12-14 19:10 [PATCH 0/7] Add drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
2017-12-14 19:10 ` [PATCH 1/7] drm/fb-helper: Set/clear dev->fb_helper in dummy init/fini Noralf Trønnes
2017-12-14 19:10 ` [PATCH 2/7] drm/fb-helper: Add drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
@ 2017-12-14 19:10 ` Noralf Trønnes
2017-12-14 19:10 ` [PATCH 4/7] drm/fb-helper: Update DOC with new helpers Noralf Trønnes
` (3 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Noralf Trønnes @ 2017-12-14 19:10 UTC (permalink / raw)
To: dri-devel; +Cc: daniel.vetter, laurent.pinchart
Add entry for conversion of drivers to new helpers.
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
Documentation/gpu/todo.rst | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index f421a54527d2..1e593370f64f 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -194,6 +194,24 @@ drm_mode_config_helper_suspend/resume().
Contact: Maintainer of the driver you plan to convert
+Convert drivers to use drm_fb_helper_fbdev_setup/teardown()
+-----------------------------------------------------------
+
+Most drivers can use drm_fb_helper_fbdev_setup() except maybe:
+
+- amdgpu which has special logic to decide whether to call
+ drm_helper_disable_unused_functions()
+
+- armada which isn't atomic and doesn't call
+ drm_helper_disable_unused_functions()
+
+- i915 which calls drm_fb_helper_initial_config() in a worker
+
+Drivers that use drm_framebuffer_remove() to clean up the fbdev framebuffer can
+probably use drm_fb_helper_fbdev_teardown().
+
+Contact: Maintainer of the driver you plan to convert
+
Core refactorings
=================
--
2.14.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/7] drm/fb-helper: Update DOC with new helpers
2017-12-14 19:10 [PATCH 0/7] Add drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
` (2 preceding siblings ...)
2017-12-14 19:10 ` [PATCH 3/7] drm/docs: Add todo entry for drm_fb_helper_fbdev_setup() Noralf Trønnes
@ 2017-12-14 19:10 ` Noralf Trønnes
2017-12-15 13:17 ` Noralf Trønnes
2017-12-14 19:10 ` [PATCH 5/7] drm/fb-helper: Add drm_fb_helper_defio_init() Noralf Trønnes
` (2 subsequent siblings)
6 siblings, 1 reply; 19+ messages in thread
From: Noralf Trønnes @ 2017-12-14 19:10 UTC (permalink / raw)
To: dri-devel; +Cc: daniel.vetter, laurent.pinchart
Promote new helpers for dealing with fbdev emulation.
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
drivers/gpu/drm/drm_fb_helper.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 823fc8f50d85..14aa83579e76 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -66,19 +66,17 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
* helper functions used by many drivers to implement the kernel mode setting
* interfaces.
*
- * Initialization is done as a four-step process with drm_fb_helper_prepare(),
- * drm_fb_helper_init(), drm_fb_helper_single_add_all_connectors() and
- * drm_fb_helper_initial_config(). Drivers with fancier requirements than the
- * default behaviour can override the third step with their own code.
- * Teardown is done with drm_fb_helper_fini() after the fbdev device is
- * unregisters using drm_fb_helper_unregister_fbi().
+ * Setup fbdev emulation by calling drm_fb_helper_fbdev_setup() and tear it
+ * down by calling drm_fb_helper_fbdev_teardown().
*
- * At runtime drivers should restore the fbdev console by calling
- * drm_fb_helper_restore_fbdev_mode_unlocked() from their &drm_driver.lastclose
- * callback. They should also notify the fb helper code from updates to the
- * output configuration by calling drm_fb_helper_hotplug_event(). For easier
- * integration with the output polling code in drm_crtc_helper.c the modeset
- * code provides a &drm_mode_config_funcs.output_poll_changed callback.
+ * At runtime drivers should restore the fbdev console by using
+ * drm_fb_helper_lastclose() as their &drm_driver.lastclose callback.
+ * They should also notify the fb helper code from updates to the output
+ * configuration by using drm_fb_helper_output_poll_changed() as their
+ * &drm_mode_config_funcs.output_poll_changed callback.
+ *
+ * For suspend/resume consider using drm_mode_config_helper_suspend() and
+ * drm_mode_config_helper_resume() which takes care of fbdev as well.
*
* All other functions exported by the fb helper library can be used to
* implement the fbdev driver interface by the driver.
--
2.14.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 5/7] drm/fb-helper: Add drm_fb_helper_defio_init()
2017-12-14 19:10 [PATCH 0/7] Add drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
` (3 preceding siblings ...)
2017-12-14 19:10 ` [PATCH 4/7] drm/fb-helper: Update DOC with new helpers Noralf Trønnes
@ 2017-12-14 19:10 ` Noralf Trønnes
2017-12-14 20:25 ` Daniel Vetter
2017-12-14 19:10 ` [PATCH 6/7] drm/fb-cma-helper: Use drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
2017-12-14 19:10 ` [PATCH 7/7] drm/fb-cma-helper: Honour DRM_FBDEV_EMULATION Noralf Trønnes
6 siblings, 1 reply; 19+ messages in thread
From: Noralf Trønnes @ 2017-12-14 19:10 UTC (permalink / raw)
To: dri-devel; +Cc: daniel.vetter, laurent.pinchart
Add helper for initializing fbdev deferred I/O.
The cleanup could have happened in drm_fb_helper_fini(), but that would
have required me to set fb_info->fbdefio to NULL in a couple of drivers
before they call _fini() to avoid double defio cleanup. The problem is
that one of those is vboxvideo which lives in Greg's staging tree.
So I put the cleanup in drm_fb_helper_fbdev_teardown(), not perfect
but not that bad either.
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
drivers/gpu/drm/drm_fb_helper.c | 53 +++++++++++++++++++++++++++++++++++++++++
include/drm/drm_fb_helper.h | 6 +++++
2 files changed, 59 insertions(+)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 14aa83579e76..d5eeed1c7749 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1022,6 +1022,48 @@ void drm_fb_helper_deferred_io(struct fb_info *info,
}
EXPORT_SYMBOL(drm_fb_helper_deferred_io);
+/**
+ * drm_fb_helper_defio_init - fbdev deferred I/O initialization
+ * @fb_helper: driver-allocated fbdev helper
+ *
+ * This function allocates &fb_deferred_io, sets callback to
+ * drm_fb_helper_deferred_io(), delay to 50ms and calls fb_deferred_io_init().
+ * drm_fb_helper_fbdev_teardown() cleans up deferred I/O.
+ *
+ * NOTE: A copy of &fb_ops is made and assigned to &info->fbops. This is done
+ * because fb_deferred_io_cleanup() clears &fbops->fb_mmap and would thereby
+ * affect other instances of that &fb_ops.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper)
+{
+ struct fb_info *info = fb_helper->fbdev;
+ struct fb_deferred_io *fbdefio;
+ struct fb_ops *fbops;
+
+ fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
+ fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
+ if (!fbdefio || !fbops) {
+ kfree(fbdefio);
+ kfree(fbops);
+ return -ENOMEM;
+ }
+
+ info->fbdefio = fbdefio;
+ fbdefio->delay = msecs_to_jiffies(50);
+ fbdefio->deferred_io = drm_fb_helper_deferred_io;
+
+ *fbops = *info->fbops;
+ info->fbops = fbops;
+
+ fb_deferred_io_init(info);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_fb_helper_defio_init);
+
/**
* drm_fb_helper_sys_read - wrapper around fb_sys_read
* @info: fb_info struct pointer
@@ -2743,6 +2785,9 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
* The caller must to provide a &drm_fb_helper_funcs->fb_probe callback
* function.
*
+ * Drivers that need fbdev deferred I/O should use drm_fb_helper_defio_init()
+ * to set it up.
+ *
* See also: drm_fb_helper_initial_config()
*
* Returns:
@@ -2818,6 +2863,7 @@ EXPORT_SYMBOL(drm_fb_helper_fbdev_setup);
void drm_fb_helper_fbdev_teardown(struct drm_device *dev)
{
struct drm_fb_helper *fb_helper = dev->fb_helper;
+ struct fb_ops *fbops = NULL;
if (!fb_helper)
return;
@@ -2826,7 +2872,14 @@ void drm_fb_helper_fbdev_teardown(struct drm_device *dev)
if (fb_helper->fbdev && fb_helper->fbdev->dev)
drm_fb_helper_unregister_fbi(fb_helper);
+ if (fb_helper->fbdev && fb_helper->fbdev->fbdefio) {
+ fb_deferred_io_cleanup(fb_helper->fbdev);
+ kfree(fb_helper->fbdev->fbdefio);
+ fbops = fb_helper->fbdev->fbops;
+ }
+
drm_fb_helper_fini(fb_helper);
+ kfree(fbops);
if (fb_helper->fb)
drm_framebuffer_remove(fb_helper->fb);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index aa78ac3b8ad0..b069433e7fc1 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -276,6 +276,7 @@ void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper);
void drm_fb_helper_deferred_io(struct fb_info *info,
struct list_head *pagelist);
+int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper);
ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf,
size_t count, loff_t *ppos);
@@ -423,6 +424,11 @@ static inline void drm_fb_helper_deferred_io(struct fb_info *info,
{
}
+static inline int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper)
+{
+ return -ENODEV;
+}
+
static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info,
char __user *buf, size_t count,
loff_t *ppos)
--
2.14.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 6/7] drm/fb-cma-helper: Use drm_fb_helper_fbdev_setup/teardown()
2017-12-14 19:10 [PATCH 0/7] Add drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
` (4 preceding siblings ...)
2017-12-14 19:10 ` [PATCH 5/7] drm/fb-helper: Add drm_fb_helper_defio_init() Noralf Trønnes
@ 2017-12-14 19:10 ` Noralf Trønnes
2017-12-14 19:10 ` [PATCH 7/7] drm/fb-cma-helper: Honour DRM_FBDEV_EMULATION Noralf Trønnes
6 siblings, 0 replies; 19+ messages in thread
From: Noralf Trønnes @ 2017-12-14 19:10 UTC (permalink / raw)
To: dri-devel; +Cc: daniel.vetter, laurent.pinchart
Use the new setup/teardown helpers as well as drm_fb_helper_defio_init().
Wrap fb_deferred_io_mmap() in ifdef so we can make fbdev optional.
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
I had to touch drm_fbdev_cma_fini() here, but that will be rebased away
when I this patch is unblocked and applied:
[PATCH v3 11/11] drm/cma-helper: Remove drm_fbdev_cma* functions
drivers/gpu/drm/drm_fb_cma_helper.c | 111 +++++-------------------------------
1 file changed, 15 insertions(+), 96 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 186d00adfb5f..9f8210130dc6 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -26,8 +26,6 @@
#include <drm/drm_print.h>
#include <linux/module.h>
-#define DEFAULT_FBDEFIO_DELAY_MS 50
-
struct drm_fbdev_cma {
struct drm_fb_helper fb_helper;
const struct drm_framebuffer_funcs *fb_funcs;
@@ -149,58 +147,14 @@ static struct fb_ops drm_fbdev_cma_ops = {
static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info,
struct vm_area_struct *vma)
{
+#ifdef CONFIG_FB_DEFERRED_IO
fb_deferred_io_mmap(info, vma);
+#endif
vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
return 0;
}
-static int drm_fbdev_cma_defio_init(struct fb_info *fbi,
- struct drm_gem_cma_object *cma_obj)
-{
- struct fb_deferred_io *fbdefio;
- struct fb_ops *fbops;
-
- /*
- * Per device structures are needed because:
- * fbops: fb_deferred_io_cleanup() clears fbops.fb_mmap
- * fbdefio: individual delays
- */
- fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
- fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
- if (!fbdefio || !fbops) {
- kfree(fbdefio);
- kfree(fbops);
- return -ENOMEM;
- }
-
- /* can't be offset from vaddr since dirty() uses cma_obj */
- fbi->screen_buffer = cma_obj->vaddr;
- /* fb_deferred_io_fault() needs a physical address */
- fbi->fix.smem_start = page_to_phys(virt_to_page(fbi->screen_buffer));
-
- *fbops = *fbi->fbops;
- fbi->fbops = fbops;
-
- fbdefio->delay = msecs_to_jiffies(DEFAULT_FBDEFIO_DELAY_MS);
- fbdefio->deferred_io = drm_fb_helper_deferred_io;
- fbi->fbdefio = fbdefio;
- fb_deferred_io_init(fbi);
- fbi->fbops->fb_mmap = drm_fbdev_cma_deferred_io_mmap;
-
- return 0;
-}
-
-static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
-{
- if (!fbi->fbdefio)
- return;
-
- fb_deferred_io_cleanup(fbi);
- kfree(fbi->fbdefio);
- kfree(fbi->fbops);
-}
-
static int
drm_fbdev_cma_create(struct drm_fb_helper *helper,
struct drm_fb_helper_surface_size *sizes)
@@ -258,9 +212,15 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper,
fbi->fix.smem_len = size;
if (fb->funcs->dirty) {
- ret = drm_fbdev_cma_defio_init(fbi, obj);
+ ret = drm_fb_helper_defio_init(helper);
if (ret)
goto err_cma_destroy;
+
+ /* can't be offset from vaddr since dirty() uses cma obj */
+ fbi->screen_buffer = obj->vaddr;
+ /* fb_deferred_io_fault() needs a physical address */
+ fbi->fix.smem_start = page_to_phys(virt_to_page(obj->vaddr));
+ fbi->fbops->fb_mmap = drm_fbdev_cma_deferred_io_mmap;
}
return 0;
@@ -296,52 +256,23 @@ int drm_fb_cma_fbdev_init_with_funcs(struct drm_device *dev,
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);
+ ret = drm_fb_helper_fbdev_setup(dev, &fbdev_cma->fb_helper,
+ &drm_fb_cma_helper_funcs,
+ preferred_bpp, 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;
+ kfree(fbdev_cma);
+ return ret;
}
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);
@@ -375,17 +306,7 @@ void drm_fb_cma_fbdev_fini(struct drm_device *dev)
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);
+ drm_fb_helper_fbdev_teardown(dev);
kfree(to_fbdev_cma(fb_helper));
}
EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_fini);
@@ -477,8 +398,6 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);
void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
{
drm_fb_helper_unregister_fbi(&fbdev_cma->fb_helper);
- if (fbdev_cma->fb_helper.fbdev)
- drm_fbdev_cma_defio_fini(fbdev_cma->fb_helper.fbdev);
if (fbdev_cma->fb_helper.fb)
drm_framebuffer_remove(fbdev_cma->fb_helper.fb);
--
2.14.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 7/7] drm/fb-cma-helper: Honour DRM_FBDEV_EMULATION
2017-12-14 19:10 [PATCH 0/7] Add drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
` (5 preceding siblings ...)
2017-12-14 19:10 ` [PATCH 6/7] drm/fb-cma-helper: Use drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
@ 2017-12-14 19:10 ` Noralf Trønnes
2017-12-14 20:26 ` Daniel Vetter
6 siblings, 1 reply; 19+ messages in thread
From: Noralf Trønnes @ 2017-12-14 19:10 UTC (permalink / raw)
To: dri-devel; +Cc: daniel.vetter, laurent.pinchart
Make it possible to opt out of fbdev emulation.
DRM_FBDEV_EMULATION selects DRM_KMS_FB_HELPER and is default yes so
this doesn't change the default behaviour.
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
drivers/gpu/drm/Kconfig | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index d853989848d6..fe1b6030d3c6 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -143,10 +143,7 @@ config DRM_KMS_CMA_HELPER
bool
depends on DRM
select DRM_GEM_CMA_HELPER
- select DRM_KMS_FB_HELPER
- select FB_SYS_FILLRECT
- select FB_SYS_COPYAREA
- select FB_SYS_IMAGEBLIT
+ select DRM_KMS_HELPER
help
Choose this if you need the KMS CMA helper functions
--
2.14.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/7] drm/fb-helper: Set/clear dev->fb_helper in dummy init/fini
2017-12-14 19:10 ` [PATCH 1/7] drm/fb-helper: Set/clear dev->fb_helper in dummy init/fini Noralf Trønnes
@ 2017-12-14 20:10 ` Daniel Vetter
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2017-12-14 20:10 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: daniel.vetter, laurent.pinchart, dri-devel
On Thu, Dec 14, 2017 at 08:10:02PM +0100, Noralf Trønnes wrote:
> Set dev->fb_helper even when fbdev emulation is compiled out,
> so drivers can use it to free the structure.
> Clear it for consistency.
>
> Fixes: 29ad20b22c8f ("drm: Add drm_device->fb_helper pointer")
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> include/drm/drm_fb_helper.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 1494b12a984a..1bd624579db7 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -33,6 +33,7 @@
> struct drm_fb_helper;
>
> #include <drm/drm_crtc.h>
> +#include <drm/drm_device.h>
> #include <linux/kgdb.h>
>
> enum mode_set_atomic {
> @@ -332,11 +333,17 @@ static inline int drm_fb_helper_init(struct drm_device *dev,
> struct drm_fb_helper *helper,
> int max_conn)
> {
> + /* So drivers can use it to free the struct */
> + helper->dev = dev;
> + dev->fb_helper = helper;
Not sure I forgot it, or it got dropped, but I'd like
dev->mode_config.fb_helper better. That's where all the other kms stuff
resides.
But 100% bikeshed, so feel free to ignore. Definitely only a follow-up
patch.
-Daniel
> +
> return 0;
> }
>
> static inline void drm_fb_helper_fini(struct drm_fb_helper *helper)
> {
> + if (helper && helper->dev)
> + helper->dev->fb_helper = NULL;
> }
>
> static inline int drm_fb_helper_blank(int blank, struct fb_info *info)
> --
> 2.14.2
>
--
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/7] drm/fb-helper: Add drm_fb_helper_fbdev_setup/teardown()
2017-12-14 19:10 ` [PATCH 2/7] drm/fb-helper: Add drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
@ 2017-12-14 20:19 ` Daniel Vetter
2017-12-15 13:19 ` Noralf Trønnes
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2017-12-14 20:19 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: daniel.vetter, laurent.pinchart, dri-devel
On Thu, Dec 14, 2017 at 08:10:03PM +0100, Noralf Trønnes wrote:
> Add helpers to setup and teardown fbdev emulation.
>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 106 ++++++++++++++++++++++++++++++++++++++++
> include/drm/drm_fb_helper.h | 25 ++++++++++
> 2 files changed, 131 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 04a3a5ce370a..823fc8f50d85 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2729,6 +2729,112 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> }
> EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
>
> +/**
> + * drm_fb_helper_fbdev_setup() - Setup fbdev emulation
> + * @dev: DRM device
> + * @fb_helper: fbdev helper structure to set up
> + * @funcs: fbdev helper functions
> + * @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.
> + *
> + * This function sets up fbdev emulation and registers fbdev for access by
> + * userspace. If all connectors are disconnected, setup is deferred to the next
> + * time drm_fb_helper_hotplug_event() is called.
> + * The caller must to provide a &drm_fb_helper_funcs->fb_probe callback
> + * function.
I think we should be a bit clearer when it's not advisable to use this
helper because there are issues:
- when connectors can be hotplugged (e.g. dp mst). For that case you need
to init the fbdev before you enabling hotplugging, but call
initial_config after you've enabled hotplugging. Otherwise there's a
posssible race window.
- when there's anything else that mus be done before we do the fbdev
registration, e.g. defio_init.
So still not 100% sure this will help you all that much, but it's also not
an unreasonable idea.
Assuming we can agree on the ceveats and how do document them:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> + *
> + * See also: drm_fb_helper_initial_config()
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_fb_helper_fbdev_setup(struct drm_device *dev,
> + struct drm_fb_helper *fb_helper,
> + const struct drm_fb_helper_funcs *funcs,
> + unsigned int preferred_bpp,
> + unsigned int max_conn_count)
> +{
> + 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;
> + if (!max_conn_count) {
> + DRM_DEV_ERROR(dev->dev, "No connectors\n");
> + return -EINVAL;
> + }
> +
> + drm_fb_helper_prepare(dev, fb_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");
> + return ret;
> + }
> +
> + 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;
> + }
> +
> + if (!drm_drv_uses_atomic_modeset(dev))
> + drm_helper_disable_unused_functions(dev);
> +
> + 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);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_fb_helper_fbdev_setup);
> +
> +/**
> + * drm_fb_helper_fbdev_teardown - Tear down fbdev emulation
> + * @dev: DRM device
> + *
> + * This function unregisters fbdev if not already done and cleans up the
> + * associated resources including the &drm_framebuffer.
> + * The driver is responsible for freeing the &drm_fb_helper structure which is
> + * stored in &drm_device->fb_helper. Do note that this pointer has been cleared
> + * when this function returns.
> + *
> + * In order to support device removal/unplug while file handles are still open,
> + * drm_fb_helper_unregister_fbi() should be called on device removal and
> + * drm_fb_helper_fbdev_teardown() in the &drm_driver->release callback when
> + * file handles are closed.
> + */
> +void drm_fb_helper_fbdev_teardown(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);
> +
> + drm_fb_helper_fini(fb_helper);
> +
> + if (fb_helper->fb)
> + drm_framebuffer_remove(fb_helper->fb);
> +}
> +EXPORT_SYMBOL(drm_fb_helper_fbdev_teardown);
> +
> /**
> * drm_fb_helper_lastclose - DRM driver lastclose helper for fbdev emulation
> * @dev: DRM device
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 1bd624579db7..aa78ac3b8ad0 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -320,6 +320,13 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_
> int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> struct drm_connector *connector);
>
> +int drm_fb_helper_fbdev_setup(struct drm_device *dev,
> + struct drm_fb_helper *fb_helper,
> + const struct drm_fb_helper_funcs *funcs,
> + unsigned int preferred_bpp,
> + unsigned int max_conn_count);
> +void drm_fb_helper_fbdev_teardown(struct drm_device *dev);
> +
> void drm_fb_helper_lastclose(struct drm_device *dev);
> void drm_fb_helper_output_poll_changed(struct drm_device *dev);
> #else
> @@ -525,6 +532,24 @@ drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> return 0;
> }
>
> +static inline int
> +drm_fb_helper_fbdev_setup(struct drm_device *dev,
> + struct drm_fb_helper *fb_helper,
> + const struct drm_fb_helper_funcs *funcs,
> + unsigned int preferred_bpp,
> + unsigned int max_conn_count)
> +{
> + /* So drivers can use it to free the struct */
> + dev->fb_helper = fb_helper;
> +
> + return 0;
> +}
> +
> +static inline void drm_fb_helper_fbdev_teardown(struct drm_device *dev)
> +{
> + dev->fb_helper = NULL;
> +}
> +
> static inline void drm_fb_helper_lastclose(struct drm_device *dev)
> {
> }
> --
> 2.14.2
>
--
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/7] drm/fb-helper: Add drm_fb_helper_defio_init()
2017-12-14 19:10 ` [PATCH 5/7] drm/fb-helper: Add drm_fb_helper_defio_init() Noralf Trønnes
@ 2017-12-14 20:25 ` Daniel Vetter
2017-12-15 13:18 ` Noralf Trønnes
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2017-12-14 20:25 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: daniel.vetter, laurent.pinchart, dri-devel
On Thu, Dec 14, 2017 at 08:10:06PM +0100, Noralf Trønnes wrote:
> Add helper for initializing fbdev deferred I/O.
>
> The cleanup could have happened in drm_fb_helper_fini(), but that would
> have required me to set fb_info->fbdefio to NULL in a couple of drivers
> before they call _fini() to avoid double defio cleanup. The problem is
> that one of those is vboxvideo which lives in Greg's staging tree.
> So I put the cleanup in drm_fb_helper_fbdev_teardown(), not perfect
> but not that bad either.
>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 53 +++++++++++++++++++++++++++++++++++++++++
> include/drm/drm_fb_helper.h | 6 +++++
> 2 files changed, 59 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 14aa83579e76..d5eeed1c7749 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1022,6 +1022,48 @@ void drm_fb_helper_deferred_io(struct fb_info *info,
> }
> EXPORT_SYMBOL(drm_fb_helper_deferred_io);
>
> +/**
> + * drm_fb_helper_defio_init - fbdev deferred I/O initialization
> + * @fb_helper: driver-allocated fbdev helper
> + *
> + * This function allocates &fb_deferred_io, sets callback to
> + * drm_fb_helper_deferred_io(), delay to 50ms and calls fb_deferred_io_init().
> + * drm_fb_helper_fbdev_teardown() cleans up deferred I/O.
> + *
> + * NOTE: A copy of &fb_ops is made and assigned to &info->fbops. This is done
> + * because fb_deferred_io_cleanup() clears &fbops->fb_mmap and would thereby
> + * affect other instances of that &fb_ops.
Do we need to call this before initial_config? Or after? Should be
documented imo.
Also, did you look into just fixing fb_deferred_io_cleanup to no longer do
this? Changing function tables is definitely not cool (because that means
we can't put them into read-only memory and protect them from attackers -
function tables are a high-value target in the kernel, since usually easy
to trigger them).
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper)
> +{
> + struct fb_info *info = fb_helper->fbdev;
> + struct fb_deferred_io *fbdefio;
> + struct fb_ops *fbops;
> +
> + fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
> + fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
> + if (!fbdefio || !fbops) {
> + kfree(fbdefio);
> + kfree(fbops);
> + return -ENOMEM;
> + }
> +
> + info->fbdefio = fbdefio;
> + fbdefio->delay = msecs_to_jiffies(50);
> + fbdefio->deferred_io = drm_fb_helper_deferred_io;
> +
> + *fbops = *info->fbops;
> + info->fbops = fbops;
> +
> + fb_deferred_io_init(info);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_fb_helper_defio_init);
> +
> /**
> * drm_fb_helper_sys_read - wrapper around fb_sys_read
> * @info: fb_info struct pointer
> @@ -2743,6 +2785,9 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
> * The caller must to provide a &drm_fb_helper_funcs->fb_probe callback
> * function.
> *
> + * Drivers that need fbdev deferred I/O should use drm_fb_helper_defio_init()
> + * to set it up.
Not exactly sure why you put this here ... Maybe throw it into the
overview documentation, in a new paragraph that explains when you have a
non-NULL fb->dirty callback, then you most likely want to setup
defio_init. Except for the shmem fun. Explaining all that would be good
(maybe even put it under a "Framebuffer Flushing" heading).
Otherwise looks all reasonable to me.
Cheers, Daniel
> + *
> * See also: drm_fb_helper_initial_config()
> *
> * Returns:
> @@ -2818,6 +2863,7 @@ EXPORT_SYMBOL(drm_fb_helper_fbdev_setup);
> void drm_fb_helper_fbdev_teardown(struct drm_device *dev)
> {
> struct drm_fb_helper *fb_helper = dev->fb_helper;
> + struct fb_ops *fbops = NULL;
>
> if (!fb_helper)
> return;
> @@ -2826,7 +2872,14 @@ void drm_fb_helper_fbdev_teardown(struct drm_device *dev)
> if (fb_helper->fbdev && fb_helper->fbdev->dev)
> drm_fb_helper_unregister_fbi(fb_helper);
>
> + if (fb_helper->fbdev && fb_helper->fbdev->fbdefio) {
> + fb_deferred_io_cleanup(fb_helper->fbdev);
> + kfree(fb_helper->fbdev->fbdefio);
> + fbops = fb_helper->fbdev->fbops;
> + }
> +
> drm_fb_helper_fini(fb_helper);
> + kfree(fbops);
>
> if (fb_helper->fb)
> drm_framebuffer_remove(fb_helper->fb);
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index aa78ac3b8ad0..b069433e7fc1 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -276,6 +276,7 @@ void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper);
>
> void drm_fb_helper_deferred_io(struct fb_info *info,
> struct list_head *pagelist);
> +int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper);
>
> ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf,
> size_t count, loff_t *ppos);
> @@ -423,6 +424,11 @@ static inline void drm_fb_helper_deferred_io(struct fb_info *info,
> {
> }
>
> +static inline int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper)
> +{
> + return -ENODEV;
> +}
> +
> static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info,
> char __user *buf, size_t count,
> loff_t *ppos)
> --
> 2.14.2
>
--
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 7/7] drm/fb-cma-helper: Honour DRM_FBDEV_EMULATION
2017-12-14 19:10 ` [PATCH 7/7] drm/fb-cma-helper: Honour DRM_FBDEV_EMULATION Noralf Trønnes
@ 2017-12-14 20:26 ` Daniel Vetter
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2017-12-14 20:26 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: daniel.vetter, laurent.pinchart, dri-devel
On Thu, Dec 14, 2017 at 08:10:08PM +0100, Noralf Trønnes wrote:
> Make it possible to opt out of fbdev emulation.
> DRM_FBDEV_EMULATION selects DRM_KMS_FB_HELPER and is default yes so
> this doesn't change the default behaviour.
>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
For all the patches I didn't comment on:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/Kconfig | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index d853989848d6..fe1b6030d3c6 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -143,10 +143,7 @@ config DRM_KMS_CMA_HELPER
> bool
> depends on DRM
> select DRM_GEM_CMA_HELPER
> - select DRM_KMS_FB_HELPER
> - select FB_SYS_FILLRECT
> - select FB_SYS_COPYAREA
> - select FB_SYS_IMAGEBLIT
> + select DRM_KMS_HELPER
> help
> Choose this if you need the KMS CMA helper functions
>
> --
> 2.14.2
>
--
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/7] drm/fb-helper: Update DOC with new helpers
2017-12-14 19:10 ` [PATCH 4/7] drm/fb-helper: Update DOC with new helpers Noralf Trønnes
@ 2017-12-15 13:17 ` Noralf Trønnes
2017-12-15 15:42 ` Daniel Vetter
0 siblings, 1 reply; 19+ messages in thread
From: Noralf Trønnes @ 2017-12-15 13:17 UTC (permalink / raw)
To: dri-devel; +Cc: daniel.vetter, laurent.pinchart
Den 14.12.2017 20.10, skrev Noralf Trønnes:
> Promote new helpers for dealing with fbdev emulation.
>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 823fc8f50d85..14aa83579e76 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -66,19 +66,17 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
> * helper functions used by many drivers to implement the kernel mode setting
> * interfaces.
> *
> - * Initialization is done as a four-step process with drm_fb_helper_prepare(),
> - * drm_fb_helper_init(), drm_fb_helper_single_add_all_connectors() and
> - * drm_fb_helper_initial_config(). Drivers with fancier requirements than the
> - * default behaviour can override the third step with their own code.
> - * Teardown is done with drm_fb_helper_fini() after the fbdev device is
> - * unregisters using drm_fb_helper_unregister_fbi().
> + * Setup fbdev emulation by calling drm_fb_helper_fbdev_setup() and tear it
> + * down by calling drm_fb_helper_fbdev_teardown().
> *
How about this for documenting when _setup() can't be used:
* Setup fbdev emulation by calling drm_fb_helper_fbdev_setup() and tear it
* down by calling drm_fb_helper_fbdev_teardown().
*
* Drivers that need to handle connector hotplugging (e.g. dp mst)
can't use
* the setup helper and will need to do the whole four-step setup
process with
* drm_fb_helper_prepare(), drm_fb_helper_init(),
* drm_fb_helper_single_add_all_connectors(), enable hotplugging and
* drm_fb_helper_initial_config() to avoid a possible race window.
Noralf.
> - * At runtime drivers should restore the fbdev console by calling
> - * drm_fb_helper_restore_fbdev_mode_unlocked() from their &drm_driver.lastclose
> - * callback. They should also notify the fb helper code from updates to the
> - * output configuration by calling drm_fb_helper_hotplug_event(). For easier
> - * integration with the output polling code in drm_crtc_helper.c the modeset
> - * code provides a &drm_mode_config_funcs.output_poll_changed callback.
> + * At runtime drivers should restore the fbdev console by using
> + * drm_fb_helper_lastclose() as their &drm_driver.lastclose callback.
> + * They should also notify the fb helper code from updates to the output
> + * configuration by using drm_fb_helper_output_poll_changed() as their
> + * &drm_mode_config_funcs.output_poll_changed callback.
> + *
> + * For suspend/resume consider using drm_mode_config_helper_suspend() and
> + * drm_mode_config_helper_resume() which takes care of fbdev as well.
> *
> * All other functions exported by the fb helper library can be used to
> * implement the fbdev driver interface by the driver.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/7] drm/fb-helper: Add drm_fb_helper_defio_init()
2017-12-14 20:25 ` Daniel Vetter
@ 2017-12-15 13:18 ` Noralf Trønnes
2017-12-15 15:38 ` Daniel Vetter
2017-12-15 15:43 ` Daniel Vetter
0 siblings, 2 replies; 19+ messages in thread
From: Noralf Trønnes @ 2017-12-15 13:18 UTC (permalink / raw)
To: Daniel Vetter; +Cc: daniel.vetter, laurent.pinchart, dri-devel
Den 14.12.2017 21.25, skrev Daniel Vetter:
> On Thu, Dec 14, 2017 at 08:10:06PM +0100, Noralf Trønnes wrote:
>> Add helper for initializing fbdev deferred I/O.
>>
>> The cleanup could have happened in drm_fb_helper_fini(), but that would
>> have required me to set fb_info->fbdefio to NULL in a couple of drivers
>> before they call _fini() to avoid double defio cleanup. The problem is
>> that one of those is vboxvideo which lives in Greg's staging tree.
>> So I put the cleanup in drm_fb_helper_fbdev_teardown(), not perfect
>> but not that bad either.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>> drivers/gpu/drm/drm_fb_helper.c | 53 +++++++++++++++++++++++++++++++++++++++++
>> include/drm/drm_fb_helper.h | 6 +++++
>> 2 files changed, 59 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index 14aa83579e76..d5eeed1c7749 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -1022,6 +1022,48 @@ void drm_fb_helper_deferred_io(struct fb_info *info,
>> }
>> EXPORT_SYMBOL(drm_fb_helper_deferred_io);
>>
>> +/**
>> + * drm_fb_helper_defio_init - fbdev deferred I/O initialization
>> + * @fb_helper: driver-allocated fbdev helper
>> + *
>> + * This function allocates &fb_deferred_io, sets callback to
>> + * drm_fb_helper_deferred_io(), delay to 50ms and calls fb_deferred_io_init().
>> + * drm_fb_helper_fbdev_teardown() cleans up deferred I/O.
>> + *
>> + * NOTE: A copy of &fb_ops is made and assigned to &info->fbops. This is done
>> + * because fb_deferred_io_cleanup() clears &fbops->fb_mmap and would thereby
>> + * affect other instances of that &fb_ops.
> Do we need to call this before initial_config? Or after? Should be
> documented imo.
Indeed it should be:
* This function allocates &fb_deferred_io, sets callback to
* drm_fb_helper_deferred_io(), delay to 50ms and calls
fb_deferred_io_init().
* It should be called from the &drm_fb_helper_funcs->fb_probe callback.
* drm_fb_helper_fbdev_teardown() cleans up deferred I/O.
> Also, did you look into just fixing fb_deferred_io_cleanup to no longer do
> this? Changing function tables is definitely not cool (because that means
> we can't put them into read-only memory and protect them from attackers -
> function tables are a high-value target in the kernel, since usually easy
> to trigger them).
The fbdev operations isn't const:
struct fb_info {
struct fb_ops *fbops;
};
Fixing that is a project of it's own as a quick grep revealed:
drivers/video/fbdev/mb862xx/mb862xxfb_accel.c: info->fbops->fb_fillrect
= cfb_fillrect;
drivers/video/fbdev/mb862xx/mb862xxfb_accel.c: info->fbops->fb_copyarea
= cfb_copyarea;
drivers/video/fbdev/mb862xx/mb862xxfb_accel.c: info->fbops->fb_imageblit
= cfb_imageblit;
drivers/video/fbdev/mb862xx/mb862xxfb_accel.c: info->fbops->fb_fillrect
= mb86290fb_fillrect;
drivers/video/fbdev/mb862xx/mb862xxfb_accel.c: info->fbops->fb_copyarea
= mb86290fb_copyarea;
drivers/video/fbdev/mb862xx/mb862xxfb_accel.c: info->fbops->fb_imageblit
= mb86290fb_imageblit;
drivers/video/fbdev/uvesafb.c: info->fbops->fb_blank = NULL;
drivers/video/fbdev/uvesafb.c: info->fbops->fb_pan_display = NULL;
drivers/video/fbdev/aty/atyfb_base.c: info->fbops->fb_sync = atyfb_sync;
drivers/video/fbdev/aty/atyfb_base.c: info->fbops->fb_sync = NULL;
drivers/video/fbdev/aty/mach64_cursor.c: info->fbops->fb_cursor =
atyfb_cursor;
drivers/video/fbdev/core/fb_defio.c: info->fbops->fb_mmap =
fb_deferred_io_mmap;
drivers/video/fbdev/core/fb_defio.c: info->fbops->fb_mmap = NULL;
drivers/video/fbdev/vesafb.c: info->fbops->fb_pan_display = NULL;
drivers/video/fbdev/udlfb.c: info->fbops->fb_mmap =
dlfb_ops_mmap;
drivers/video/fbdev/smscufx.c: info->fbops->fb_mmap = ufx_ops_mmap;
drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_imageblit =
nvidiafb_imageblit;
drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_fillrect =
nvidiafb_fillrect;
drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_copyarea =
nvidiafb_copyarea;
drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_sync = nvidiafb_sync;
drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_imageblit =
cfb_imageblit;
drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_fillrect =
cfb_fillrect;
drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_copyarea =
cfb_copyarea;
drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_sync = NULL;
drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_cursor = NULL;
drivers/gpu/drm/udl/udl_fb.c: info->fbops->fb_mmap = udl_fb_mmap;
drivers/gpu/drm/drm_fb_cma_helper.c: fbi->fbops->fb_mmap =
drm_fbdev_cma_deferred_io_mmap;
Noralf.
>
>> + *
>> + * Returns:
>> + * 0 on success or a negative error code on failure.
>> + */
>> +int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper)
>> +{
>> + struct fb_info *info = fb_helper->fbdev;
>> + struct fb_deferred_io *fbdefio;
>> + struct fb_ops *fbops;
>> +
>> + fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
>> + fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
>> + if (!fbdefio || !fbops) {
>> + kfree(fbdefio);
>> + kfree(fbops);
>> + return -ENOMEM;
>> + }
>> +
>> + info->fbdefio = fbdefio;
>> + fbdefio->delay = msecs_to_jiffies(50);
>> + fbdefio->deferred_io = drm_fb_helper_deferred_io;
>> +
>> + *fbops = *info->fbops;
>> + info->fbops = fbops;
>> +
>> + fb_deferred_io_init(info);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(drm_fb_helper_defio_init);
>> +
>> /**
>> * drm_fb_helper_sys_read - wrapper around fb_sys_read
>> * @info: fb_info struct pointer
>> @@ -2743,6 +2785,9 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
>> * The caller must to provide a &drm_fb_helper_funcs->fb_probe callback
>> * function.
>> *
>> + * Drivers that need fbdev deferred I/O should use drm_fb_helper_defio_init()
>> + * to set it up.
> Not exactly sure why you put this here ... Maybe throw it into the
> overview documentation, in a new paragraph that explains when you have a
> non-NULL fb->dirty callback, then you most likely want to setup
> defio_init. Except for the shmem fun. Explaining all that would be good
> (maybe even put it under a "Framebuffer Flushing" heading).
>
> Otherwise looks all reasonable to me.
>
> Cheers, Daniel
>
>> + *
>> * See also: drm_fb_helper_initial_config()
>> *
>> * Returns:
>> @@ -2818,6 +2863,7 @@ EXPORT_SYMBOL(drm_fb_helper_fbdev_setup);
>> void drm_fb_helper_fbdev_teardown(struct drm_device *dev)
>> {
>> struct drm_fb_helper *fb_helper = dev->fb_helper;
>> + struct fb_ops *fbops = NULL;
>>
>> if (!fb_helper)
>> return;
>> @@ -2826,7 +2872,14 @@ void drm_fb_helper_fbdev_teardown(struct drm_device *dev)
>> if (fb_helper->fbdev && fb_helper->fbdev->dev)
>> drm_fb_helper_unregister_fbi(fb_helper);
>>
>> + if (fb_helper->fbdev && fb_helper->fbdev->fbdefio) {
>> + fb_deferred_io_cleanup(fb_helper->fbdev);
>> + kfree(fb_helper->fbdev->fbdefio);
>> + fbops = fb_helper->fbdev->fbops;
>> + }
>> +
>> drm_fb_helper_fini(fb_helper);
>> + kfree(fbops);
>>
>> if (fb_helper->fb)
>> drm_framebuffer_remove(fb_helper->fb);
>> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
>> index aa78ac3b8ad0..b069433e7fc1 100644
>> --- a/include/drm/drm_fb_helper.h
>> +++ b/include/drm/drm_fb_helper.h
>> @@ -276,6 +276,7 @@ void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper);
>>
>> void drm_fb_helper_deferred_io(struct fb_info *info,
>> struct list_head *pagelist);
>> +int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper);
>>
>> ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf,
>> size_t count, loff_t *ppos);
>> @@ -423,6 +424,11 @@ static inline void drm_fb_helper_deferred_io(struct fb_info *info,
>> {
>> }
>>
>> +static inline int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper)
>> +{
>> + return -ENODEV;
>> +}
>> +
>> static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info,
>> char __user *buf, size_t count,
>> loff_t *ppos)
>> --
>> 2.14.2
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/7] drm/fb-helper: Add drm_fb_helper_fbdev_setup/teardown()
2017-12-14 20:19 ` Daniel Vetter
@ 2017-12-15 13:19 ` Noralf Trønnes
0 siblings, 0 replies; 19+ messages in thread
From: Noralf Trønnes @ 2017-12-15 13:19 UTC (permalink / raw)
To: Daniel Vetter; +Cc: daniel.vetter, laurent.pinchart, dri-devel
Den 14.12.2017 21.19, skrev Daniel Vetter:
> On Thu, Dec 14, 2017 at 08:10:03PM +0100, Noralf Trønnes wrote:
>> Add helpers to setup and teardown fbdev emulation.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>> drivers/gpu/drm/drm_fb_helper.c | 106 ++++++++++++++++++++++++++++++++++++++++
>> include/drm/drm_fb_helper.h | 25 ++++++++++
>> 2 files changed, 131 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index 04a3a5ce370a..823fc8f50d85 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -2729,6 +2729,112 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>> }
>> EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
>>
>> +/**
>> + * drm_fb_helper_fbdev_setup() - Setup fbdev emulation
>> + * @dev: DRM device
>> + * @fb_helper: fbdev helper structure to set up
>> + * @funcs: fbdev helper functions
>> + * @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.
>> + *
>> + * This function sets up fbdev emulation and registers fbdev for access by
>> + * userspace. If all connectors are disconnected, setup is deferred to the next
>> + * time drm_fb_helper_hotplug_event() is called.
>> + * The caller must to provide a &drm_fb_helper_funcs->fb_probe callback
>> + * function.
> I think we should be a bit clearer when it's not advisable to use this
> helper because there are issues:
> - when connectors can be hotplugged (e.g. dp mst). For that case you need
> to init the fbdev before you enabling hotplugging, but call
> initial_config after you've enabled hotplugging. Otherwise there's a
> posssible race window.
I have made a propose DOC change in patch 4.
> - when there's anything else that mus be done before we do the fbdev
> registration, e.g. defio_init.
This happens in .fb_probe callback. See my reply to patch 5.
> So still not 100% sure this will help you all that much, but it's also not
> an unreasonable idea.
>
> Assuming we can agree on the ceveats and how do document them:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
>> + *
>> + * See also: drm_fb_helper_initial_config()
>> + *
>> + * Returns:
>> + * Zero on success or negative error code on failure.
>> + */
>> +int drm_fb_helper_fbdev_setup(struct drm_device *dev,
>> + struct drm_fb_helper *fb_helper,
>> + const struct drm_fb_helper_funcs *funcs,
>> + unsigned int preferred_bpp,
>> + unsigned int max_conn_count)
>> +{
>> + 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;
>> + if (!max_conn_count) {
>> + DRM_DEV_ERROR(dev->dev, "No connectors\n");
>> + return -EINVAL;
>> + }
>> +
>> + drm_fb_helper_prepare(dev, fb_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");
>> + return ret;
>> + }
>> +
>> + 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;
>> + }
>> +
>> + if (!drm_drv_uses_atomic_modeset(dev))
>> + drm_helper_disable_unused_functions(dev);
>> +
>> + 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);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(drm_fb_helper_fbdev_setup);
>> +
>> +/**
>> + * drm_fb_helper_fbdev_teardown - Tear down fbdev emulation
>> + * @dev: DRM device
>> + *
>> + * This function unregisters fbdev if not already done and cleans up the
>> + * associated resources including the &drm_framebuffer.
>> + * The driver is responsible for freeing the &drm_fb_helper structure which is
>> + * stored in &drm_device->fb_helper. Do note that this pointer has been cleared
>> + * when this function returns.
>> + *
>> + * In order to support device removal/unplug while file handles are still open,
>> + * drm_fb_helper_unregister_fbi() should be called on device removal and
>> + * drm_fb_helper_fbdev_teardown() in the &drm_driver->release callback when
>> + * file handles are closed.
>> + */
>> +void drm_fb_helper_fbdev_teardown(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);
>> +
>> + drm_fb_helper_fini(fb_helper);
>> +
>> + if (fb_helper->fb)
>> + drm_framebuffer_remove(fb_helper->fb);
>> +}
>> +EXPORT_SYMBOL(drm_fb_helper_fbdev_teardown);
>> +
>> /**
>> * drm_fb_helper_lastclose - DRM driver lastclose helper for fbdev emulation
>> * @dev: DRM device
>> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
>> index 1bd624579db7..aa78ac3b8ad0 100644
>> --- a/include/drm/drm_fb_helper.h
>> +++ b/include/drm/drm_fb_helper.h
>> @@ -320,6 +320,13 @@ int drm_fb_helper_add_one_connector(struct drm_fb_helper *fb_helper, struct drm_
>> int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
>> struct drm_connector *connector);
>>
>> +int drm_fb_helper_fbdev_setup(struct drm_device *dev,
>> + struct drm_fb_helper *fb_helper,
>> + const struct drm_fb_helper_funcs *funcs,
>> + unsigned int preferred_bpp,
>> + unsigned int max_conn_count);
>> +void drm_fb_helper_fbdev_teardown(struct drm_device *dev);
>> +
>> void drm_fb_helper_lastclose(struct drm_device *dev);
>> void drm_fb_helper_output_poll_changed(struct drm_device *dev);
>> #else
>> @@ -525,6 +532,24 @@ drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
>> return 0;
>> }
>>
>> +static inline int
>> +drm_fb_helper_fbdev_setup(struct drm_device *dev,
>> + struct drm_fb_helper *fb_helper,
>> + const struct drm_fb_helper_funcs *funcs,
>> + unsigned int preferred_bpp,
>> + unsigned int max_conn_count)
>> +{
>> + /* So drivers can use it to free the struct */
>> + dev->fb_helper = fb_helper;
>> +
>> + return 0;
>> +}
>> +
>> +static inline void drm_fb_helper_fbdev_teardown(struct drm_device *dev)
>> +{
>> + dev->fb_helper = NULL;
>> +}
>> +
>> static inline void drm_fb_helper_lastclose(struct drm_device *dev)
>> {
>> }
>> --
>> 2.14.2
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/7] drm/fb-helper: Add drm_fb_helper_defio_init()
2017-12-15 13:18 ` Noralf Trønnes
@ 2017-12-15 15:38 ` Daniel Vetter
2017-12-15 15:43 ` Daniel Vetter
1 sibling, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2017-12-15 15:38 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: Laurent Pinchart, dri-devel
On Fri, Dec 15, 2017 at 2:18 PM, Noralf Trønnes <noralf@tronnes.org> wrote:
>
> Den 14.12.2017 21.25, skrev Daniel Vetter:
>>
>> On Thu, Dec 14, 2017 at 08:10:06PM +0100, Noralf Trønnes wrote:
>>>
>>> Add helper for initializing fbdev deferred I/O.
>>>
>>> The cleanup could have happened in drm_fb_helper_fini(), but that would
>>> have required me to set fb_info->fbdefio to NULL in a couple of drivers
>>> before they call _fini() to avoid double defio cleanup. The problem is
>>> that one of those is vboxvideo which lives in Greg's staging tree.
>>> So I put the cleanup in drm_fb_helper_fbdev_teardown(), not perfect
>>> but not that bad either.
>>>
>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>> ---
>>> drivers/gpu/drm/drm_fb_helper.c | 53
>>> +++++++++++++++++++++++++++++++++++++++++
>>> include/drm/drm_fb_helper.h | 6 +++++
>>> 2 files changed, 59 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>>> b/drivers/gpu/drm/drm_fb_helper.c
>>> index 14aa83579e76..d5eeed1c7749 100644
>>> --- a/drivers/gpu/drm/drm_fb_helper.c
>>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>>> @@ -1022,6 +1022,48 @@ void drm_fb_helper_deferred_io(struct fb_info
>>> *info,
>>> }
>>> EXPORT_SYMBOL(drm_fb_helper_deferred_io);
>>> +/**
>>> + * drm_fb_helper_defio_init - fbdev deferred I/O initialization
>>> + * @fb_helper: driver-allocated fbdev helper
>>> + *
>>> + * This function allocates &fb_deferred_io, sets callback to
>>> + * drm_fb_helper_deferred_io(), delay to 50ms and calls
>>> fb_deferred_io_init().
>>> + * drm_fb_helper_fbdev_teardown() cleans up deferred I/O.
>>> + *
>>> + * NOTE: A copy of &fb_ops is made and assigned to &info->fbops. This is
>>> done
>>> + * because fb_deferred_io_cleanup() clears &fbops->fb_mmap and would
>>> thereby
>>> + * affect other instances of that &fb_ops.
>>
>> Do we need to call this before initial_config? Or after? Should be
>> documented imo.
>
>
> Indeed it should be:
>
> * This function allocates &fb_deferred_io, sets callback to
> * drm_fb_helper_deferred_io(), delay to 50ms and calls
> fb_deferred_io_init().
> * It should be called from the &drm_fb_helper_funcs->fb_probe callback.
> * drm_fb_helper_fbdev_teardown() cleans up deferred I/O.
>
>> Also, did you look into just fixing fb_deferred_io_cleanup to no longer do
>> this? Changing function tables is definitely not cool (because that means
>> we can't put them into read-only memory and protect them from attackers -
>> function tables are a high-value target in the kernel, since usually easy
>> to trigger them).
>
>
> The fbdev operations isn't const:
>
> struct fb_info {
> struct fb_ops *fbops;
> };
>
> Fixing that is a project of it's own as a quick grep revealed:
>
> drivers/video/fbdev/mb862xx/mb862xxfb_accel.c: info->fbops->fb_fillrect =
> cfb_fillrect;
> drivers/video/fbdev/mb862xx/mb862xxfb_accel.c: info->fbops->fb_copyarea =
> cfb_copyarea;
> drivers/video/fbdev/mb862xx/mb862xxfb_accel.c: info->fbops->fb_imageblit =
> cfb_imageblit;
> drivers/video/fbdev/mb862xx/mb862xxfb_accel.c: info->fbops->fb_fillrect =
> mb86290fb_fillrect;
> drivers/video/fbdev/mb862xx/mb862xxfb_accel.c: info->fbops->fb_copyarea =
> mb86290fb_copyarea;
> drivers/video/fbdev/mb862xx/mb862xxfb_accel.c: info->fbops->fb_imageblit =
> mb86290fb_imageblit;
>
> drivers/video/fbdev/uvesafb.c: info->fbops->fb_blank = NULL;
> drivers/video/fbdev/uvesafb.c: info->fbops->fb_pan_display = NULL;
>
> drivers/video/fbdev/aty/atyfb_base.c: info->fbops->fb_sync = atyfb_sync;
> drivers/video/fbdev/aty/atyfb_base.c: info->fbops->fb_sync = NULL;
>
> drivers/video/fbdev/aty/mach64_cursor.c: info->fbops->fb_cursor =
> atyfb_cursor;
>
> drivers/video/fbdev/core/fb_defio.c: info->fbops->fb_mmap =
> fb_deferred_io_mmap;
> drivers/video/fbdev/core/fb_defio.c: info->fbops->fb_mmap = NULL;
Ah, it frobs the mmap callback with it's own.
> drivers/video/fbdev/vesafb.c: info->fbops->fb_pan_display = NULL;
>
> drivers/video/fbdev/udlfb.c: info->fbops->fb_mmap =
> dlfb_ops_mmap;
>
> drivers/video/fbdev/smscufx.c: info->fbops->fb_mmap = ufx_ops_mmap;
>
> drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_imageblit =
> nvidiafb_imageblit;
> drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_fillrect =
> nvidiafb_fillrect;
> drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_copyarea =
> nvidiafb_copyarea;
> drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_sync = nvidiafb_sync;
> drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_imageblit =
> cfb_imageblit;
> drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_fillrect =
> cfb_fillrect;
> drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_copyarea =
> cfb_copyarea;
> drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_sync = NULL;
> drivers/video/fbdev/nvidia/nvidia.c: info->fbops->fb_cursor = NULL;
^^ this one we can probably delete, use tegra or nouveau instead.
> drivers/gpu/drm/udl/udl_fb.c: info->fbops->fb_mmap = udl_fb_mmap;
Ugh.
> drivers/gpu/drm/drm_fb_cma_helper.c: fbi->fbops->fb_mmap =
> drm_fbdev_cma_deferred_io_mmap;
Yeah. Maybe a todo.rst entry would be good for this one here.
-Daniel
>
> Noralf.
>
>
>>
>>> + *
>>> + * Returns:
>>> + * 0 on success or a negative error code on failure.
>>> + */
>>> +int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper)
>>> +{
>>> + struct fb_info *info = fb_helper->fbdev;
>>> + struct fb_deferred_io *fbdefio;
>>> + struct fb_ops *fbops;
>>> +
>>> + fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
>>> + fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
>>> + if (!fbdefio || !fbops) {
>>> + kfree(fbdefio);
>>> + kfree(fbops);
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + info->fbdefio = fbdefio;
>>> + fbdefio->delay = msecs_to_jiffies(50);
>>> + fbdefio->deferred_io = drm_fb_helper_deferred_io;
>>> +
>>> + *fbops = *info->fbops;
>>> + info->fbops = fbops;
>>> +
>>> + fb_deferred_io_init(info);
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL(drm_fb_helper_defio_init);
>>> +
>>> /**
>>> * drm_fb_helper_sys_read - wrapper around fb_sys_read
>>> * @info: fb_info struct pointer
>>> @@ -2743,6 +2785,9 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
>>> * The caller must to provide a &drm_fb_helper_funcs->fb_probe callback
>>> * function.
>>> *
>>> + * Drivers that need fbdev deferred I/O should use
>>> drm_fb_helper_defio_init()
>>> + * to set it up.
>>
>> Not exactly sure why you put this here ... Maybe throw it into the
>> overview documentation, in a new paragraph that explains when you have a
>> non-NULL fb->dirty callback, then you most likely want to setup
>> defio_init. Except for the shmem fun. Explaining all that would be good
>> (maybe even put it under a "Framebuffer Flushing" heading).
>>
>> Otherwise looks all reasonable to me.
>>
>> Cheers, Daniel
>>
>>> + *
>>> * See also: drm_fb_helper_initial_config()
>>> *
>>> * Returns:
>>> @@ -2818,6 +2863,7 @@ EXPORT_SYMBOL(drm_fb_helper_fbdev_setup);
>>> void drm_fb_helper_fbdev_teardown(struct drm_device *dev)
>>> {
>>> struct drm_fb_helper *fb_helper = dev->fb_helper;
>>> + struct fb_ops *fbops = NULL;
>>> if (!fb_helper)
>>> return;
>>> @@ -2826,7 +2872,14 @@ void drm_fb_helper_fbdev_teardown(struct
>>> drm_device *dev)
>>> if (fb_helper->fbdev && fb_helper->fbdev->dev)
>>> drm_fb_helper_unregister_fbi(fb_helper);
>>> + if (fb_helper->fbdev && fb_helper->fbdev->fbdefio) {
>>> + fb_deferred_io_cleanup(fb_helper->fbdev);
>>> + kfree(fb_helper->fbdev->fbdefio);
>>> + fbops = fb_helper->fbdev->fbops;
>>> + }
>>> +
>>> drm_fb_helper_fini(fb_helper);
>>> + kfree(fbops);
>>> if (fb_helper->fb)
>>> drm_framebuffer_remove(fb_helper->fb);
>>> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
>>> index aa78ac3b8ad0..b069433e7fc1 100644
>>> --- a/include/drm/drm_fb_helper.h
>>> +++ b/include/drm/drm_fb_helper.h
>>> @@ -276,6 +276,7 @@ void drm_fb_helper_unlink_fbi(struct drm_fb_helper
>>> *fb_helper);
>>> void drm_fb_helper_deferred_io(struct fb_info *info,
>>> struct list_head *pagelist);
>>> +int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper);
>>> ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user
>>> *buf,
>>> size_t count, loff_t *ppos);
>>> @@ -423,6 +424,11 @@ static inline void drm_fb_helper_deferred_io(struct
>>> fb_info *info,
>>> {
>>> }
>>> +static inline int drm_fb_helper_defio_init(struct drm_fb_helper
>>> *fb_helper)
>>> +{
>>> + return -ENODEV;
>>> +}
>>> +
>>> static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info,
>>> char __user *buf, size_t
>>> count,
>>> loff_t *ppos)
>>> --
>>> 2.14.2
>>>
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/7] drm/fb-helper: Update DOC with new helpers
2017-12-15 13:17 ` Noralf Trønnes
@ 2017-12-15 15:42 ` Daniel Vetter
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Vetter @ 2017-12-15 15:42 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: daniel.vetter, laurent.pinchart, dri-devel
On Fri, Dec 15, 2017 at 02:17:49PM +0100, Noralf Trønnes wrote:
>
> Den 14.12.2017 20.10, skrev Noralf Trønnes:
> > Promote new helpers for dealing with fbdev emulation.
> >
> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > ---
> > drivers/gpu/drm/drm_fb_helper.c | 22 ++++++++++------------
> > 1 file changed, 10 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index 823fc8f50d85..14aa83579e76 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -66,19 +66,17 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
> > * helper functions used by many drivers to implement the kernel mode setting
> > * interfaces.
> > *
> > - * Initialization is done as a four-step process with drm_fb_helper_prepare(),
> > - * drm_fb_helper_init(), drm_fb_helper_single_add_all_connectors() and
> > - * drm_fb_helper_initial_config(). Drivers with fancier requirements than the
> > - * default behaviour can override the third step with their own code.
> > - * Teardown is done with drm_fb_helper_fini() after the fbdev device is
> > - * unregisters using drm_fb_helper_unregister_fbi().
> > + * Setup fbdev emulation by calling drm_fb_helper_fbdev_setup() and tear it
> > + * down by calling drm_fb_helper_fbdev_teardown().
> > *
>
> How about this for documenting when _setup() can't be used:
>
> * Setup fbdev emulation by calling drm_fb_helper_fbdev_setup() and tear it
> * down by calling drm_fb_helper_fbdev_teardown().
> *
> * Drivers that need to handle connector hotplugging (e.g. dp mst) can't use
> * the setup helper and will need to do the whole four-step setup process
> with
> * drm_fb_helper_prepare(), drm_fb_helper_init(),
> * drm_fb_helper_single_add_all_connectors(), enable hotplugging and
> * drm_fb_helper_initial_config() to avoid a possible race window.
lgtm, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> on both this an
the other patch that inspired this update.
-Daniel
>
> Noralf.
>
> > - * At runtime drivers should restore the fbdev console by calling
> > - * drm_fb_helper_restore_fbdev_mode_unlocked() from their &drm_driver.lastclose
> > - * callback. They should also notify the fb helper code from updates to the
> > - * output configuration by calling drm_fb_helper_hotplug_event(). For easier
> > - * integration with the output polling code in drm_crtc_helper.c the modeset
> > - * code provides a &drm_mode_config_funcs.output_poll_changed callback.
> > + * At runtime drivers should restore the fbdev console by using
> > + * drm_fb_helper_lastclose() as their &drm_driver.lastclose callback.
> > + * They should also notify the fb helper code from updates to the output
> > + * configuration by using drm_fb_helper_output_poll_changed() as their
> > + * &drm_mode_config_funcs.output_poll_changed callback.
> > + *
> > + * For suspend/resume consider using drm_mode_config_helper_suspend() and
> > + * drm_mode_config_helper_resume() which takes care of fbdev as well.
> > *
> > * All other functions exported by the fb helper library can be used to
> > * implement the fbdev driver interface by the driver.
>
--
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/7] drm/fb-helper: Add drm_fb_helper_defio_init()
2017-12-15 13:18 ` Noralf Trønnes
2017-12-15 15:38 ` Daniel Vetter
@ 2017-12-15 15:43 ` Daniel Vetter
2017-12-15 15:57 ` Noralf Trønnes
1 sibling, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2017-12-15 15:43 UTC (permalink / raw)
To: Noralf Trønnes; +Cc: daniel.vetter, dri-devel, laurent.pinchart
On Fri, Dec 15, 2017 at 02:18:55PM +0100, Noralf Trønnes wrote:
>
> Den 14.12.2017 21.25, skrev Daniel Vetter:
> > On Thu, Dec 14, 2017 at 08:10:06PM +0100, Noralf Trønnes wrote:
> > > Add helper for initializing fbdev deferred I/O.
> > >
> > > The cleanup could have happened in drm_fb_helper_fini(), but that would
> > > have required me to set fb_info->fbdefio to NULL in a couple of drivers
> > > before they call _fini() to avoid double defio cleanup. The problem is
> > > that one of those is vboxvideo which lives in Greg's staging tree.
> > > So I put the cleanup in drm_fb_helper_fbdev_teardown(), not perfect
> > > but not that bad either.
> > >
> > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > > ---
> > > drivers/gpu/drm/drm_fb_helper.c | 53 +++++++++++++++++++++++++++++++++++++++++
> > > include/drm/drm_fb_helper.h | 6 +++++
> > > 2 files changed, 59 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > > index 14aa83579e76..d5eeed1c7749 100644
> > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > @@ -1022,6 +1022,48 @@ void drm_fb_helper_deferred_io(struct fb_info *info,
> > > }
> > > EXPORT_SYMBOL(drm_fb_helper_deferred_io);
> > > +/**
> > > + * drm_fb_helper_defio_init - fbdev deferred I/O initialization
> > > + * @fb_helper: driver-allocated fbdev helper
> > > + *
> > > + * This function allocates &fb_deferred_io, sets callback to
> > > + * drm_fb_helper_deferred_io(), delay to 50ms and calls fb_deferred_io_init().
> > > + * drm_fb_helper_fbdev_teardown() cleans up deferred I/O.
> > > + *
> > > + * NOTE: A copy of &fb_ops is made and assigned to &info->fbops. This is done
> > > + * because fb_deferred_io_cleanup() clears &fbops->fb_mmap and would thereby
> > > + * affect other instances of that &fb_ops.
> > Do we need to call this before initial_config? Or after? Should be
> > documented imo.
>
> Indeed it should be:
>
> * This function allocates &fb_deferred_io, sets callback to
> * drm_fb_helper_deferred_io(), delay to 50ms and calls
> fb_deferred_io_init().
> * It should be called from the &drm_fb_helper_funcs->fb_probe callback.
> * drm_fb_helper_fbdev_teardown() cleans up deferred I/O.
Forgot this part, lgtm, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I think we got them all now?
-Daniel
--
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/7] drm/fb-helper: Add drm_fb_helper_defio_init()
2017-12-15 15:43 ` Daniel Vetter
@ 2017-12-15 15:57 ` Noralf Trønnes
0 siblings, 0 replies; 19+ messages in thread
From: Noralf Trønnes @ 2017-12-15 15:57 UTC (permalink / raw)
To: Daniel Vetter; +Cc: daniel.vetter, laurent.pinchart, dri-devel
Den 15.12.2017 16.43, skrev Daniel Vetter:
> On Fri, Dec 15, 2017 at 02:18:55PM +0100, Noralf Trønnes wrote:
>> Den 14.12.2017 21.25, skrev Daniel Vetter:
>>> On Thu, Dec 14, 2017 at 08:10:06PM +0100, Noralf Trønnes wrote:
>>>> Add helper for initializing fbdev deferred I/O.
>>>>
>>>> The cleanup could have happened in drm_fb_helper_fini(), but that would
>>>> have required me to set fb_info->fbdefio to NULL in a couple of drivers
>>>> before they call _fini() to avoid double defio cleanup. The problem is
>>>> that one of those is vboxvideo which lives in Greg's staging tree.
>>>> So I put the cleanup in drm_fb_helper_fbdev_teardown(), not perfect
>>>> but not that bad either.
>>>>
>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>> ---
>>>> drivers/gpu/drm/drm_fb_helper.c | 53 +++++++++++++++++++++++++++++++++++++++++
>>>> include/drm/drm_fb_helper.h | 6 +++++
>>>> 2 files changed, 59 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>>>> index 14aa83579e76..d5eeed1c7749 100644
>>>> --- a/drivers/gpu/drm/drm_fb_helper.c
>>>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>>>> @@ -1022,6 +1022,48 @@ void drm_fb_helper_deferred_io(struct fb_info *info,
>>>> }
>>>> EXPORT_SYMBOL(drm_fb_helper_deferred_io);
>>>> +/**
>>>> + * drm_fb_helper_defio_init - fbdev deferred I/O initialization
>>>> + * @fb_helper: driver-allocated fbdev helper
>>>> + *
>>>> + * This function allocates &fb_deferred_io, sets callback to
>>>> + * drm_fb_helper_deferred_io(), delay to 50ms and calls fb_deferred_io_init().
>>>> + * drm_fb_helper_fbdev_teardown() cleans up deferred I/O.
>>>> + *
>>>> + * NOTE: A copy of &fb_ops is made and assigned to &info->fbops. This is done
>>>> + * because fb_deferred_io_cleanup() clears &fbops->fb_mmap and would thereby
>>>> + * affect other instances of that &fb_ops.
>>> Do we need to call this before initial_config? Or after? Should be
>>> documented imo.
>> Indeed it should be:
>>
>> * This function allocates &fb_deferred_io, sets callback to
>> * drm_fb_helper_deferred_io(), delay to 50ms and calls
>> fb_deferred_io_init().
>> * It should be called from the &drm_fb_helper_funcs->fb_probe callback.
>> * drm_fb_helper_fbdev_teardown() cleans up deferred I/O.
> Forgot this part, lgtm, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> I think we got them all now?
Indeed, thanks Daniel.
Noralf.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2017-12-15 15:57 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-14 19:10 [PATCH 0/7] Add drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
2017-12-14 19:10 ` [PATCH 1/7] drm/fb-helper: Set/clear dev->fb_helper in dummy init/fini Noralf Trønnes
2017-12-14 20:10 ` Daniel Vetter
2017-12-14 19:10 ` [PATCH 2/7] drm/fb-helper: Add drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
2017-12-14 20:19 ` Daniel Vetter
2017-12-15 13:19 ` Noralf Trønnes
2017-12-14 19:10 ` [PATCH 3/7] drm/docs: Add todo entry for drm_fb_helper_fbdev_setup() Noralf Trønnes
2017-12-14 19:10 ` [PATCH 4/7] drm/fb-helper: Update DOC with new helpers Noralf Trønnes
2017-12-15 13:17 ` Noralf Trønnes
2017-12-15 15:42 ` Daniel Vetter
2017-12-14 19:10 ` [PATCH 5/7] drm/fb-helper: Add drm_fb_helper_defio_init() Noralf Trønnes
2017-12-14 20:25 ` Daniel Vetter
2017-12-15 13:18 ` Noralf Trønnes
2017-12-15 15:38 ` Daniel Vetter
2017-12-15 15:43 ` Daniel Vetter
2017-12-15 15:57 ` Noralf Trønnes
2017-12-14 19:10 ` [PATCH 6/7] drm/fb-cma-helper: Use drm_fb_helper_fbdev_setup/teardown() Noralf Trønnes
2017-12-14 19:10 ` [PATCH 7/7] drm/fb-cma-helper: Honour DRM_FBDEV_EMULATION Noralf Trønnes
2017-12-14 20:26 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.