Re: [PATCH 2/7] drm/fb-helper: Add drm_fb_helper_fbdev_setup/teardown()
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 --- 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 + * + * 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_hel
Re: [PATCH 2/7] drm/fb-helper: Add drm_fb_helper_fbdev_setup/teardown()
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 > --- > 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 > + * > + * 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_unregi
[PATCH 2/7] drm/fb-helper: Add drm_fb_helper_fbdev_setup/teardown()
Add helpers to setup and teardown fbdev emulation. Signed-off-by: Noralf Trønnes --- 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, +