On Tue, Aug 29, 2017 at 06:17:25PM +0200, Noralf Trønnes wrote:
>
> Den 28.08.2017 23.41, skrev Daniel Vetter:
> > On Mon, Aug 28, 2017 at 07:17:44PM +0200, Noralf Trønnes wrote:
> > > Support device unplug by introducing a new initialization function:
> > > drm_fb_helper_simple_init() which together with
> > > drm_fb_helper_dev_unplug() and drm_fb_helper_fb_destroy() handles open
> > > fbdev fd's after device unplug. There's also a
> > > drm_fb_helper_simple_fini() for drivers who's device won't be removed.
> > >
> > > It turns out that fbdev has the necessary ref counting, so
> > > unregister_framebuffer() together with fb_ops->destroy handles unplug
> > > with open fd's. The ref counting doesn't apply to the fb_info structure
> > > itself, but to use of the fbdev framebuffer.
> > >
> > > Analysis of entry points after unregister_framebuffer():
> > > - fbcon has been unbound (through notifier)
> > > - sysfs files removed
> > >
> > > First we look at possible operations on open fbdev file handles:
> > >
> > > static const struct file_operations fb_fops = {
> > > .read = fb_read,
> > > .write = fb_write,
> > > .unlocked_ioctl = fb_ioctl,
> > > .compat_ioctl = fb_compat_ioctl,
> > > .mmap = fb_mmap,
> > > .open = fb_open,
> > > .release = fb_release,
> > > .get_unmapped_area = get_fb_unmapped_area,
> > > .fsync = fb_deferred_io_fsync,
> > > };
> > >
> > > Not possible after unregister:
> > > fb_open() -> fb_ops.fb_open
> > >
> > > Protected by file_fb_info() (-ENODEV):
> > > fb_read() -> fb_ops.fb_read : drm_fb_helper_sys_read()
> > > fb_write() -> fb_ops.fb_write : drm_fb_helper_sys_write()
> > > fb_ioctl() -> fb_ops.fb_ioctl : drm_fb_helper_ioctl()
> > > fb_compat_ioctl() -> fb_ops.fb_compat_ioctl
> > > fb_mmap() -> fb_ops.fb_mmap
> > >
> > > Safe:
> > > fb_release() -> fb_ops.fb_release
> > > get_fb_unmapped_area() : info->screen_base
> > > fb_deferred_io_fsync() : if (info->fbdefio) &info->deferred_work
> > >
> > > Active mmap's will need the backing buffer to be present.
> > > If deferred IO is used, mmap writes will via a worker generate calls to
> > > drm_fb_helper_deferred_io() which in turn via a worker calls into
> > > drm_fb_helper_dirty_work().
> > >
> > > Secondly we look at the remaining struct fb_ops operations:
> > >
> > > Called by fbcon:
> > > - fb_ops.fb_fillrect : drm_fb_helper_{sys,cfb}_fillrect()
> > > - fb_ops.fb_copyarea : drm_fb_helper_{sys,cfb}_copyarea()
> > > - fb_ops.fb_imageblit : drm_fb_helper_{sys,cfb}_imageblit()
> > >
> > > Called in fb_set_var() which is called from ioctl, sysfs and fbcon:
> > > - fb_ops.fb_check_var : drm_fb_helper_check_var()
> > > - fb_ops.fb_set_par : drm_fb_helper_set_par()
> > > drm_fb_helper_set_par() is also called from drm_fb_helper_hotplug_event().
> > >
> > > Called in fb_set_cmap() which is called from fb_set_var(), ioctl
> > > and fbcon:
> > > - fb_ops.fb_setcolreg
> > > - fb_ops.fb_setcmap : drm_fb_helper_setcmap()
> > >
> > > Called in fb_blank() which is called from ioctl and fbcon:
> > > - fb_ops.fb_blank : drm_fb_helper_blank()
> > >
> > > Called in fb_pan_display() which is called from fb_set_var(), ioctl,
> > > sysfs and fbcon:
> > > - fb_ops.fb_pan_display : drm_fb_helper_pan_display()
> > >
> > > Called by fbcon:
> > > - fb_ops.fb_cursor
> > >
> > > Called in fb_read(), fb_write(), and fb_get_buffer_offset() which is
> > > called by fbcon:
> > > - fb_ops.fb_sync
> > >
> > > Called by fb_set_var():
> > > - fb_ops.fb_get_caps
> > >
> > > Called by fbcon:
> > > - fb_ops.fb_debug_enter : drm_fb_helper_debug_enter()
> > > - fb_ops.fb_debug_leave : drm_fb_helper_debug_leave()
> > >
> > > Destroy is safe
> > > - fb_ops.fb_destroy
> > >
> > > Finally we look at other call paths:
> > >
> > > drm_fb_helper_set_suspend{_unlocked}() and
> > > drm_fb_helper_resume_worker():
> > > Calls into fb_set_suspend(), but that's fine since it just uses the
> > > fbdev notifier.
> > >
> > > drm_fb_helper_restore_fbdev_mode_unlocked():
> > > Called from drm_driver.last_close, possibly after drm_fb_helper_fini().
> > >
> > > drm_fb_helper_force_kernel_mode():
> > > Triggered by sysrq, possibly after unplug, but before
> > > drm_fb_helper_fini().
> > >
> > > drm_fb_helper_hotplug_event():
> > > Called by drm_kms_helper_hotplug_event(). I don't know if this can be
> > > called after drm_dev_unregister(), so add a check to be safe.
> > >
> > > Based on this analysis the following functions get a
> > > drm_dev_is_unplugged() check:
> > > - drm_fb_helper_restore_fbdev_mode_unlocked()
> > > - drm_fb_helper_force_kernel_mode()
> > > - drm_fb_helper_hotplug_event()
> > > - drm_fb_helper_dirty_work()
> > >
> > > Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
> > You're mixing in the new simple fbdev helper helpers as a bonus, that's
> > two things in one patch and makes it harder to review what's really going
> > on.
> >
> > My gut feeling is that when we need a helper for the helper the original
> > helper needs to be improved, but I think that's much easier to decide when
> > it's a separate patch. Splitting things out would definitely make this
> > patch much smaller and easier to understand and review.
> >
> > The only reason for the simple helpers that I could find is the
> > drm_dev_ref/unref, we should be able to do that in one of the existing
> > callbacks.
>
> The reason I did that is because I couldn't put it in the existing
> helpers since the framebuffer is removed after drm_fb_helper_fini() and
> I can't drop the ref on drm_device before the framebuffer is gone.
Why is that impossible? I'd have assumed that we'd first drop all the
drm_device refcounts before we do any of the cleanup steps.
> I can split the patch in two:
> 1. Add drm_dev_is_unplugged() checks
> 2. Add drm_fb_helper_simple_init/fini() and drm_fb_helper_fb_destroy()
>
> Maybe there's a way to remove the framebuffer in drm_fb_helper_fini()
> so we can drop the ref there, I don't know.
No, we can't do that since the references that get dropped in the driver
code is the driver code's private reference. The problem is that a bunch
of drivers still embed the framebuffer for the fbdev emulation into a
larger structure, which means the reference counting is obviously busted.
But all reasonable modern drivers should do that, and if we have to
convert some to do proper unplug behaviour - well that's expected anyway
because the current ->unload hook doesn't work for unplugging.
-Daniel
> Here's how drivers destroy fbdev:
>
>
> static int amdgpu_fbdev_destroy(struct drm_device *dev, struct amdgpu_fbdev
> *rfbdev)
> {
> struct amdgpu_framebuffer *rfb = &rfbdev->rfb;
>
> drm_fb_helper_unregister_fbi(&rfbdev->helper);
>
> if (rfb->obj) {
> amdgpufb_destroy_pinned_object(rfb->obj);
> rfb->obj = NULL;
> }
> drm_fb_helper_fini(&rfbdev->helper);
> drm_framebuffer_unregister_private(&rfb->base);
> drm_framebuffer_cleanup(&rfb->base);
>
> return 0;
> }
>
> void armada_fbdev_fini(struct drm_device *dev)
> {
> struct armada_private *priv = dev->dev_private;
> struct drm_fb_helper *fbh = priv->fbdev;
>
> if (fbh) {
> drm_fb_helper_unregister_fbi(fbh);
>
> drm_fb_helper_fini(fbh);
>
> if (fbh->fb)
> fbh->fb->funcs->destroy(fbh->fb);
>
> priv->fbdev = NULL;
> }
> }
>
> static void ast_fbdev_destroy(struct drm_device *dev,
> struct ast_fbdev *afbdev)
> {
> struct ast_framebuffer *afb = &afbdev->afb;
>
> drm_fb_helper_unregister_fbi(&afbdev->helper);
>
> if (afb->obj) {
> drm_gem_object_unreference_unlocked(afb->obj);
> afb->obj = NULL;
> }
> drm_fb_helper_fini(&afbdev->helper);
>
> vfree(afbdev->sysram);
> drm_framebuffer_unregister_private(&afb->base);
> drm_framebuffer_cleanup(&afb->base);
> }
>
> static int bochs_fbdev_destroy(struct bochs_device *bochs)
> {
> struct bochs_framebuffer *gfb = &bochs->fb.gfb;
>
> DRM_DEBUG_DRIVER("\n");
>
> drm_fb_helper_unregister_fbi(&bochs->fb.helper);
>
> if (gfb->obj) {
> drm_gem_object_unreference_unlocked(gfb->obj);
> gfb->obj = NULL;
> }
>
> drm_framebuffer_unregister_private(&gfb->base);
> drm_framebuffer_cleanup(&gfb->base);
>
> return 0;
> }
>
> static int cirrus_fbdev_destroy(struct drm_device *dev,
> struct cirrus_fbdev *gfbdev)
> {
> struct cirrus_framebuffer *gfb = &gfbdev->gfb;
>
> drm_fb_helper_unregister_fbi(&gfbdev->helper);
>
> if (gfb->obj) {
> drm_gem_object_unreference_unlocked(gfb->obj);
> gfb->obj = NULL;
> }
>
> vfree(gfbdev->sysram);
> drm_fb_helper_fini(&gfbdev->helper);
> drm_framebuffer_unregister_private(&gfb->base);
> drm_framebuffer_cleanup(&gfb->base);
>
> return 0;
> }
>
> 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)
> drm_framebuffer_remove(&fbdev_cma->fb->fb);
>
> drm_fb_helper_fini(&fbdev_cma->fb_helper);
> kfree(fbdev_cma);
> }
>
> static void exynos_drm_fbdev_destroy(struct drm_device *dev,
> struct drm_fb_helper *fb_helper)
> {
> struct exynos_drm_fbdev *exynos_fbd = to_exynos_fbdev(fb_helper);
> struct exynos_drm_gem *exynos_gem = exynos_fbd->exynos_gem;
> struct drm_framebuffer *fb;
>
> vunmap(exynos_gem->kvaddr);
>
> /* release drm framebuffer and real buffer */
> if (fb_helper->fb && fb_helper->fb->funcs) {
> fb = fb_helper->fb;
> if (fb)
> drm_framebuffer_remove(fb);
> }
>
> drm_fb_helper_unregister_fbi(fb_helper);
>
> drm_fb_helper_fini(fb_helper);
> }
>
> static int psb_fbdev_destroy(struct drm_device *dev, struct psb_fbdev
> *fbdev)
> {
> struct psb_framebuffer *psbfb = &fbdev->pfb;
>
> drm_fb_helper_unregister_fbi(&fbdev->psb_fb_helper);
>
> drm_fb_helper_fini(&fbdev->psb_fb_helper);
> drm_framebuffer_unregister_private(&psbfb->base);
> drm_framebuffer_cleanup(&psbfb->base);
>
> if (psbfb->gtt)
> drm_gem_object_unreference_unlocked(&psbfb->gtt->gem);
> return 0;
> }
>
> static void hibmc_fbdev_destroy(struct hibmc_fbdev *fbdev)
> {
> struct hibmc_framebuffer *gfb = fbdev->fb;
> struct drm_fb_helper *fbh = &fbdev->helper;
>
> drm_fb_helper_unregister_fbi(fbh);
>
> drm_fb_helper_fini(fbh);
>
> if (gfb)
> drm_framebuffer_unreference(&gfb->fb);
> }
>
> static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
> {
> /* We rely on the object-free to release the VMA pinning for
> * the info->screen_base mmaping. Leaking the VMA is simpler than
> * trying to rectify all the possible error paths leading here.
> */
>
> drm_fb_helper_unregister_fbi(&ifbdev->helper);
>
> drm_fb_helper_fini(&ifbdev->helper);
>
> if (ifbdev->vma) {
> mutex_lock(&ifbdev->helper.dev->struct_mutex);
> intel_unpin_fb_vma(ifbdev->vma);
> mutex_unlock(&ifbdev->helper.dev->struct_mutex);
> }
>
> if (ifbdev->fb)
> drm_framebuffer_remove(&ifbdev->fb->base);
>
> kfree(ifbdev);
> }
>
> static int mga_fbdev_destroy(struct drm_device *dev,
> struct mga_fbdev *mfbdev)
> {
> struct mga_framebuffer *mfb = &mfbdev->mfb;
>
> drm_fb_helper_unregister_fbi(&mfbdev->helper);
>
> if (mfb->obj) {
> drm_gem_object_unreference_unlocked(mfb->obj);
> mfb->obj = NULL;
> }
> drm_fb_helper_fini(&mfbdev->helper);
> vfree(mfbdev->sysram);
> drm_framebuffer_unregister_private(&mfb->base);
> drm_framebuffer_cleanup(&mfb->base);
>
> return 0;
> }
>
> void msm_fbdev_free(struct drm_device *dev)
> {
> struct msm_drm_private *priv = dev->dev_private;
> struct drm_fb_helper *helper = priv->fbdev;
> struct msm_fbdev *fbdev;
>
> DBG();
>
> drm_fb_helper_unregister_fbi(helper);
>
> drm_fb_helper_fini(helper);
>
> fbdev = to_msm_fbdev(priv->fbdev);
>
> /* this will free the backing object */
> if (fbdev->fb) {
> msm_gem_put_vaddr(fbdev->bo);
> drm_framebuffer_remove(fbdev->fb);
> }
>
> kfree(fbdev);
>
> priv->fbdev = NULL;
> }
>
> static int
> nouveau_fbcon_destroy(struct drm_device *dev, struct nouveau_fbdev *fbcon)
> {
> struct nouveau_framebuffer *nouveau_fb =
> nouveau_framebuffer(fbcon->helper.fb);
>
> drm_fb_helper_unregister_fbi(&fbcon->helper);
> drm_fb_helper_fini(&fbcon->helper);
>
> if (nouveau_fb->nvbo) {
> nouveau_bo_vma_del(nouveau_fb->nvbo, &nouveau_fb->vma);
> nouveau_bo_unmap(nouveau_fb->nvbo);
> nouveau_bo_unpin(nouveau_fb->nvbo);
> drm_framebuffer_unreference(&nouveau_fb->base);
> }
>
> return 0;
> }
>
> void omap_fbdev_free(struct drm_device *dev)
> {
> struct omap_drm_private *priv = dev->dev_private;
> struct drm_fb_helper *helper = priv->fbdev;
> struct omap_fbdev *fbdev;
>
> DBG();
>
> drm_fb_helper_unregister_fbi(helper);
>
> drm_fb_helper_fini(helper);
>
> fbdev = to_omap_fbdev(priv->fbdev);
>
> /* release the ref taken in omap_fbdev_create() */
> omap_gem_put_paddr(fbdev->bo);
>
> /* this will free the backing object */
> if (fbdev->fb)
> drm_framebuffer_remove(fbdev->fb);
>
> kfree(fbdev);
>
> priv->fbdev = NULL;
> }
>
> static int qxl_fbdev_destroy(struct drm_device *dev, struct qxl_fbdev
> *qfbdev)
> {
> struct qxl_framebuffer *qfb = &qfbdev->qfb;
>
> drm_fb_helper_unregister_fbi(&qfbdev->helper);
>
> if (qfb->obj) {
> qxlfb_destroy_pinned_object(qfb->obj);
> qfb->obj = NULL;
> }
> drm_fb_helper_fini(&qfbdev->helper);
> vfree(qfbdev->shadow);
> drm_framebuffer_cleanup(&qfb->base);
>
> return 0;
> }
>
> static int radeon_fbdev_destroy(struct drm_device *dev, struct radeon_fbdev
> *rfbdev)
> {
> struct radeon_framebuffer *rfb = &rfbdev->rfb;
>
> drm_fb_helper_unregister_fbi(&rfbdev->helper);
>
> if (rfb->obj) {
> radeonfb_destroy_pinned_object(rfb->obj);
> rfb->obj = NULL;
> }
> drm_fb_helper_fini(&rfbdev->helper);
> drm_framebuffer_unregister_private(&rfb->base);
> drm_framebuffer_cleanup(&rfb->base);
>
> return 0;
> }
>
> void rockchip_drm_fbdev_fini(struct drm_device *dev)
> {
> struct rockchip_drm_private *private = dev->dev_private;
> struct drm_fb_helper *helper;
>
> helper = &private->fbdev_helper;
>
> drm_fb_helper_unregister_fbi(helper);
>
> if (helper->fb)
> drm_framebuffer_unreference(helper->fb);
>
> drm_fb_helper_fini(helper);
> }
>
> static void tegra_fbdev_exit(struct tegra_fbdev *fbdev)
> {
> drm_fb_helper_unregister_fbi(&fbdev->base);
>
> if (fbdev->fb)
> drm_framebuffer_remove(&fbdev->fb->base);
>
> drm_fb_helper_fini(&fbdev->base);
> tegra_fbdev_free(fbdev);
> }
>
> static void udl_fbdev_destroy(struct drm_device *dev,
> struct udl_fbdev *ufbdev)
> {
> drm_fb_helper_unregister_fbi(&ufbdev->helper);
> drm_fb_helper_fini(&ufbdev->helper);
> drm_framebuffer_unregister_private(&ufbdev->ufb.base);
> drm_framebuffer_cleanup(&ufbdev->ufb.base);
> drm_gem_object_unreference_unlocked(&ufbdev->ufb.obj->base);
> }
>
> static int virtio_gpu_fbdev_destroy(struct drm_device *dev,
> struct virtio_gpu_fbdev *vgfbdev)
> {
> struct virtio_gpu_framebuffer *vgfb = &vgfbdev->vgfb;
>
> drm_fb_helper_unregister_fbi(&vgfbdev->helper);
>
> if (vgfb->obj)
> vgfb->obj = NULL;
> drm_fb_helper_fini(&vgfbdev->helper);
> drm_framebuffer_cleanup(&vgfb->base);
>
> return 0;
> }
>
>
> Noralf.
>
> > > ---
> > > drivers/gpu/drm/drm_fb_helper.c | 204 +++++++++++++++++++++++++++++++++++++++-
> > > include/drm/drm_fb_helper.h | 35 +++++++
> > > 2 files changed, 237 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > > index 2e33467..fc898db 100644
> > > --- a/drivers/gpu/drm/drm_fb_helper.c
> > > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > > @@ -105,6 +105,158 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
> > > * mmap page writes.
> > > */
> > > +/**
> > > + * drm_fb_helper_simple_init - Simple fbdev emulation initialization
> > > + * @dev: drm device
> > > + * @fb_helper: driver-allocated fbdev helper structure to initialize
> > > + * @bpp_sel: bpp value to use for the framebuffer configuration
> > > + * @max_conn_count: max connector count
> > > + * @funcs: pointer to structure of functions associate with this helper
> > > + *
> > > + * Simple fbdev emulation initialization which calls the following functions:
> > > + * drm_fb_helper_prepare(), drm_fb_helper_init(),
> > > + * drm_fb_helper_single_add_all_connectors() and
> > > + * drm_fb_helper_initial_config().
> > > + *
> > > + * This function takes a ref on &drm_device and must be used together with
> > > + * drm_fb_helper_simple_fini() or drm_fb_helper_dev_unplug().
> > > + *
> > > + * fbdev deferred I/O users must use drm_fb_helper_defio_init().
> > > + *
> > > + * Returns:
> > > + * 0 on success or a negative error code on failure.
> > > + */
> > > +int drm_fb_helper_simple_init(struct drm_device *dev,
> > > + struct drm_fb_helper *fb_helper, int bpp_sel,
> > > + int max_conn_count,
> > > + const struct drm_fb_helper_funcs *funcs)
> > > +{
> > > + int ret;
> > > +
> > > + 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 fb helper.\n");
> > > + return ret;
> > > + }
> > > +
> > > + drm_dev_ref(dev);
> > > +
> > > + 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, bpp_sel);
> > > + if (ret < 0) {
> > > + DRM_DEV_ERROR(dev->dev, "Failed to set initial hw config.\n");
> > > + goto err_drm_fb_helper_fini;
> > > + }
> > > +
> > > + return 0;
> > > +
> > > +err_drm_fb_helper_fini:
> > > + drm_fb_helper_fini(fb_helper);
> > > + drm_dev_unref(dev);
> > > +
> > > + return ret;
> > > +}
> > > +EXPORT_SYMBOL_GPL(drm_fb_helper_simple_init);
> > > +
> > > +static void drm_fb_helper_simple_fini_cleanup(struct drm_fb_helper *fb_helper)
> > > +{
> > > + struct fb_info *info = fb_helper->fbdev;
> > > + struct fb_ops *fbops = NULL;
> > > +
> > > + if (info && info->fbdefio) {
> > > + fb_deferred_io_cleanup(info);
> > > + kfree(info->fbdefio);
> > > + info->fbdefio = NULL;
> > > + fbops = info->fbops;
> > > + }
> > > +
> > > + drm_fb_helper_fini(fb_helper);
> > > + kfree(fbops);
> > > + if (fb_helper->fb)
> > > + drm_framebuffer_remove(fb_helper->fb);
> > > + drm_dev_unref(fb_helper->dev);
> > > +}
> > > +
> > > +/**
> > > + * drm_fb_helper_simple_fini - Simple fbdev cleanup
> > > + * @fb_helper: fbdev emulation structure, can be NULL
> > > + *
> > > + * Simple fbdev emulation cleanup. This function unregisters fbdev, cleans up
> > > + * deferred I/O if necessary, finalises @fb_helper and removes the framebuffer.
> > > + * The driver if responsible for freeing the @fb_helper structure.
> > > + *
> > > + * Don't use this function if you use drm_fb_helper_dev_unplug().
> > > + */
> > > +void drm_fb_helper_simple_fini(struct drm_fb_helper *fb_helper)
> > > +{
> > > + struct fb_info *info;
> > > +
> > > + if (!fb_helper)
> > > + return;
> > > +
> > > + info = fb_helper->fbdev;
> > > +
> > > + /* Has drm_fb_helper_dev_unplug() been used? */
> > > + if (info && info->dev)
> > > + drm_fb_helper_unregister_fbi(fb_helper);
> > > +
> > > + if (!(info && info->fbops && info->fbops->fb_destroy))
> > > + drm_fb_helper_simple_fini_cleanup(fb_helper);
> > > +}
> > > +EXPORT_SYMBOL_GPL(drm_fb_helper_simple_fini);
> > > +
> > > +/**
> > > + * drm_fb_helper_dev_unplug - unplug an fbdev device
> > > + * @fb_helper: driver-allocated fbdev helper, can be NULL
> > > + *
> > > + * This unplugs the fbdev emulation for a hotpluggable DRM device, which makes
> > > + * fbdev inaccessible to userspace operations. This essentially unregisters
> > > + * fbdev and can be called while there are still open users of @fb_helper.
> > > + * Entry-points from fbdev into drm core/helpers are protected by the fbdev
> > > + * &fb_info ref count and drm_dev_is_unplugged(). This means that the driver
> > > + * also has to call drm_dev_unplug() to complete the unplugging.
> > > + *
> > > + * Drivers must use drm_fb_helper_fb_destroy() as their &fb_ops.fb_destroy
> > > + * callback and call drm_mode_config_cleanup() and free @fb_helper in their
> > > + * &drm_driver->release callback.
> > > + *
> > > + * @fb_helper is finalized by this function unless there are open fbdev fd's
> > > + * in case this is postponed to the closing of the last fd. Finalizing includes
> > > + * dropping the ref taken on &drm_device in drm_fb_helper_simple_init().
> > > + */
> > > +void drm_fb_helper_dev_unplug(struct drm_fb_helper *fb_helper)
> > > +{
> > > + drm_fb_helper_unregister_fbi(fb_helper);
> > I don't see the value of this wrapper. If you want to explain how to best
> > unplug the fbdev emulation I thinkt that's better done in some dedicated
> > DOC section (referenced by drm_dev_unplug() maybe), than by providing a
> > wrapper that only gives you a different name.
> >
> > _unplug is also a bit misleading, since _unplug is about yanking the
> > backing device. This just unregisters the the fbdev interface.
> >
> > > +}
> > > +EXPORT_SYMBOL(drm_fb_helper_dev_unplug);
> > > +
> > > +/**
> > > + * drm_fb_helper_fb_destroy - implementation for &fb_ops.fb_destroy
> > > + * @info: fbdev registered by the helper
> > > + *
> > > + * This function does the same as drm_fb_helper_simple_fini() except
> > > + * unregistering fbdev which is already done.
> > > + *
> > > + * &fb_ops.fb_destroy is called during unregister_framebuffer() or the last
> > > + * fb_release() which ever comes last.
> > > + */
> > > +void drm_fb_helper_fb_destroy(struct fb_info *info)
> > > +{
> > > + struct drm_fb_helper *helper = info->par;
> > > +
> > > + DRM_DEBUG("\n");
> > > + drm_fb_helper_simple_fini_cleanup(helper);
> > > +}
> > > +EXPORT_SYMBOL(drm_fb_helper_fb_destroy);
> > > +
> > > #define drm_fb_helper_for_each_connector(fbh, i__) \
> > > for (({ lockdep_assert_held(&(fbh)->lock); }), \
> > > i__ = 0; i__ < (fbh)->connector_count; i__++)
> > > @@ -498,7 +650,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> > > bool do_delayed;
> > > int ret;
> > > - if (!drm_fbdev_emulation)
> > > + if (!drm_fbdev_emulation || drm_dev_is_unplugged(fb_helper->dev))
> > > return -ENODEV;
> > > if (READ_ONCE(fb_helper->deferred_setup))
> > > @@ -563,6 +715,9 @@ static bool drm_fb_helper_force_kernel_mode(void)
> > > list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
> > > struct drm_device *dev = helper->dev;
> > > + if (drm_dev_is_unplugged(dev))
> > > + continue;
> > > +
> > > if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> > > continue;
> > > @@ -735,6 +890,9 @@ static void drm_fb_helper_dirty_work(struct work_struct *work)
> > > struct drm_clip_rect clip_copy;
> > > unsigned long flags;
> > > + if (drm_dev_is_unplugged(helper->dev))
> > > + return;
> > > +
> > > spin_lock_irqsave(&helper->dirty_lock, flags);
> > > clip_copy = *clip;
> > > clip->x1 = clip->y1 = ~0;
> > > @@ -949,6 +1107,48 @@ void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper)
> > > }
> > > EXPORT_SYMBOL(drm_fb_helper_unlink_fbi);
> > > +/**
> > > + * 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().
> > > + *
> > > + * 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. This copy is freed by the helper
> > > + * during cleanup.
> > > + *
> > > + * 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);
> > > +
> > > static void drm_fb_helper_dirty(struct fb_info *info, u32 x, u32 y,
> > > u32 width, u32 height)
> > > {
> > > @@ -2591,7 +2791,7 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
> > > {
> > > int err = 0;
> > > - if (!drm_fbdev_emulation)
> > > + if (!drm_fbdev_emulation || drm_dev_is_unplugged(fb_helper->dev))
> > > return 0;
> > > mutex_lock(&fb_helper->lock);
> > > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> > > index 33fe959..1c5a648 100644
> > > --- a/include/drm/drm_fb_helper.h
> > > +++ b/include/drm/drm_fb_helper.h
> > > @@ -242,6 +242,14 @@ struct drm_fb_helper {
> > > .fb_ioctl = drm_fb_helper_ioctl
> > > #ifdef CONFIG_DRM_FBDEV_EMULATION
> > > +int drm_fb_helper_simple_init(struct drm_device *dev,
> > > + struct drm_fb_helper *fb_helper, int bpp_sel,
> > > + int max_conn_count,
> > > + const struct drm_fb_helper_funcs *funcs);
> > > +void drm_fb_helper_simple_fini(struct drm_fb_helper *fb_helper);
> > > +void drm_fb_helper_dev_unplug(struct drm_fb_helper *fb_helper);
> > > +void drm_fb_helper_fb_destroy(struct fb_info *info);
> > > +
> > > void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
> > > const struct drm_fb_helper_funcs *funcs);
> > > int drm_fb_helper_init(struct drm_device *dev,
> > > @@ -265,6 +273,7 @@ void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch,
> > > void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper);
> > > +int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper);
> > > void drm_fb_helper_deferred_io(struct fb_info *info,
> > > struct list_head *pagelist);
> > > @@ -311,6 +320,27 @@ 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);
> > > #else
> > > +static inline int
> > > +drm_fb_helper_simple_init(struct drm_device *dev,
> > > + struct drm_fb_helper *fb_helper, int bpp_sel,
> > > + int max_conn_count,
> > > + const struct drm_fb_helper_funcs *funcs)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline void drm_fb_helper_simple_fini(struct drm_fb_helper *fb_helper)
> > > +{
> > > +}
> > > +
> > > +static inline void drm_fb_helper_dev_unplug(struct drm_fb_helper *fb_helper)
> > > +{
> > > +}
> > > +
> > > +static inline void drm_fb_helper_fb_destroy(struct fb_info *info)
> > > +{
> > > +}
> > > +
> > > static inline void drm_fb_helper_prepare(struct drm_device *dev,
> > > struct drm_fb_helper *helper,
> > > const struct drm_fb_helper_funcs *funcs)
> > > @@ -393,6 +423,11 @@ static inline void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper)
> > > {
> > > }
> > > +static inline int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > static inline void drm_fb_helper_deferred_io(struct fb_info *info,
> > > struct list_head *pagelist)
> > > {
> > > --
> > > 2.7.4
> > >
>
--
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
