On Wed, Jan 03, 2018 at 11:21:08PM +0100, Noralf Trønnes wrote:
> Prepare for generic fbdev emulation by letting DRM core work directly
> with the fbdev compatibility layer. This is done by adding new fbdev
> helper vtable callbacks for restore, hotplug_event, unregister and
> release.
>
> Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
No clue whether my idea is any good, but inspired by the bootsplash
discussion, and the prospect that we might get other in-kernel drm/kms
clients I'm wondering whether we should make this a bit more generic and
tie it to drm_file?
The idea would be to have a list of internal drm clients by putting all
the internal drm_files onto a list (same way we do with the userspace
ones). Then we'd just walk that list and call ->hotplug, ->unregister and
->release at the right places.
->restore would be a bit different, I guess for that we'd only call the
->restore callback of the very first kernel-internal client.
With that we could then add/remove kernel-internal drm clients like normal
userspace clients, which should make integration of emergency consoles,
boot splashes and whatever else real easy. And I think it would be a lot
cleaner than leaking fb_helper knowledge into the drm core, which feels a
rather backwards.
Otoh that feels a bit overengineered (but at least it shouldn't be a lot
more code really). If the list is too much work we could start with 1
drm_file * pointer for internal stuff (but would still need locking, so
list_head is probably easier).
Thoughts?
-Daniel
> ---
> drivers/gpu/drm/drm_file.c | 12 +++++++++++-
> drivers/gpu/drm/drm_mode_config.c | 10 ++++++++++
> drivers/gpu/drm/drm_probe_helper.c | 4 ++++
> include/drm/drm_fb_helper.h | 33 +++++++++++++++++++++++++++++++++
> 4 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 400d44437e93..7ec09fb83135 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -35,6 +35,7 @@
> #include <linux/slab.h>
> #include <linux/module.h>
>
> +#include <drm/drm_fb_helper.h>
> #include <drm/drm_file.h>
> #include <drm/drmP.h>
>
> @@ -441,10 +442,19 @@ static void drm_legacy_dev_reinit(struct drm_device *dev)
>
> void drm_lastclose(struct drm_device * dev)
> {
> + struct drm_fb_helper *fb_helper = dev->fb_helper;
> + int ret;
> +
> DRM_DEBUG("\n");
>
> - if (dev->driver->lastclose)
> + if (dev->driver->lastclose) {
> dev->driver->lastclose(dev);
> + } else if (fb_helper && fb_helper->funcs && fb_helper->funcs->restore) {
> + ret = fb_helper->funcs->restore(fb_helper);
> + if (ret)
> + DRM_ERROR("Failed to restore fbdev: %d\n", ret);
> + }
> +
> DRM_DEBUG("driver lastclose completed\n");
>
> if (drm_core_check_feature(dev, DRIVER_LEGACY))
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index bc5c46306b3d..260eb1730244 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -21,6 +21,7 @@
> */
>
> #include <drm/drm_encoder.h>
> +#include <drm/drm_fb_helper.h>
> #include <drm/drm_mode_config.h>
> #include <drm/drmP.h>
>
> @@ -61,6 +62,11 @@ int drm_modeset_register_all(struct drm_device *dev)
>
> void drm_modeset_unregister_all(struct drm_device *dev)
> {
> + struct drm_fb_helper *fb_helper = dev->fb_helper;
> +
> + if (fb_helper && fb_helper->funcs && fb_helper->funcs->unregister)
> + fb_helper->funcs->unregister(fb_helper);
> +
> drm_connector_unregister_all(dev);
> drm_encoder_unregister_all(dev);
> drm_crtc_unregister_all(dev);
> @@ -408,6 +414,7 @@ EXPORT_SYMBOL(drm_mode_config_init);
> */
> void drm_mode_config_cleanup(struct drm_device *dev)
> {
> + struct drm_fb_helper *fb_helper = dev->fb_helper;
> struct drm_connector *connector;
> struct drm_connector_list_iter conn_iter;
> struct drm_crtc *crtc, *ct;
> @@ -417,6 +424,9 @@ void drm_mode_config_cleanup(struct drm_device *dev)
> struct drm_property_blob *blob, *bt;
> struct drm_plane *plane, *plt;
>
> + if (fb_helper && fb_helper->funcs && fb_helper->funcs->release)
> + fb_helper->funcs->release(fb_helper);
> +
> list_for_each_entry_safe(encoder, enct, &dev->mode_config.encoder_list,
> head) {
> encoder->funcs->destroy(encoder);
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 555fbe54d6e2..9d8b0ba54173 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -559,10 +559,14 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
> */
> void drm_kms_helper_hotplug_event(struct drm_device *dev)
> {
> + struct drm_fb_helper *fb_helper = dev->fb_helper;
> +
> /* send a uevent + call fbdev */
> drm_sysfs_hotplug_event(dev);
> if (dev->mode_config.funcs->output_poll_changed)
> dev->mode_config.funcs->output_poll_changed(dev);
> + else if (fb_helper && fb_helper->funcs && fb_helper->funcs->hotplug_event)
> + fb_helper->funcs->hotplug_event(fb_helper);
> }
> EXPORT_SYMBOL(drm_kms_helper_hotplug_event);
>
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 16d8773b60e3..385f967c3552 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -125,6 +125,39 @@ struct drm_fb_helper_funcs {
> struct drm_display_mode **modes,
> struct drm_fb_offset *offsets,
> bool *enabled, int width, int height);
> +
> + /**
> + * @restore:
> + *
> + * Optional callback for restoring fbdev emulation.
> + * Called by drm_lastclose() if &drm_driver->lastclose is not set.
> + */
> + int (*restore)(struct drm_fb_helper *fb_helper);
> +
> + /**
> + * @hotplug_event:
> + *
> + * Optional callback for hotplug events.
> + * Called by drm_kms_helper_hotplug_event() if
> + * &drm_mode_config_funcs->output_poll_changed is not set.
> + */
> + int (*hotplug_event)(struct drm_fb_helper *fb_helper);
> +
> + /**
> + * @unregister:
> + *
> + * Optional callback for unregistrering fbdev emulation.
> + * Called by drm_dev_unregister().
> + */
> + void (*unregister)(struct drm_fb_helper *fb_helper);
> +
> + /**
> + * @release:
> + *
> + * Optional callback for releasing fbdev emulation resources.
> + * Called by drm_mode_config_cleanup().
> + */
> + void (*release)(struct drm_fb_helper *fb_helper);
> };
>
> struct drm_fb_helper_connector {
> --
> 2.14.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
