Re: [PATCH 2/6] drm/fb-helper: Support device unplug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux