| Message ID | 1503940668-25883-4-git-send-email-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 28, 2017 at 07:17:45PM +0200, Noralf Trønnes wrote: > Add drm_fbdev_cma_dev_unplug() and use the drm_fb_helper device unplug > support. Pin driver module on fb_open(). > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > drivers/gpu/drm/drm_fb_cma_helper.c | 139 +++++++++++++++++------------------- > include/drm/drm_fb_cma_helper.h | 1 + > 2 files changed, 68 insertions(+), 72 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c > index f2ee883..2b044be 100644 > --- a/drivers/gpu/drm/drm_fb_cma_helper.c > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > @@ -25,8 +25,6 @@ > #include <drm/drm_fb_cma_helper.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; > @@ -238,6 +236,34 @@ int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg) > EXPORT_SYMBOL_GPL(drm_fb_cma_debugfs_show); > #endif > > +static int drm_fbdev_cma_fb_open(struct fb_info *info, int user) > +{ > + struct drm_fb_helper *fb_helper = info->par; > + struct drm_device *dev = fb_helper->dev; > + > + /* > + * The fb_ops definition resides in this library, meaning fb_open() > + * will take a ref on the library instead of the driver. Make sure the > + * driver module is pinned. Skip fbcon (user==0) since it can detach > + * itself on unregister_framebuffer(). > + */ > + if (user && !try_module_get(dev->driver->fops->owner)) > + return -ENODEV; I thought we've fixed this by redoing the fb_ops as a macro? Maybe tinydrm isn't fixed yet, but then we need to fix up tinydrm. This here otoh kinda smells a bit like a hack ... If we need it, then I'd vote to put it into the fbdev helpers, not the cma specific parts. > + > + return 0; > +} > + > +static int drm_fbdev_cma_fb_release(struct fb_info *info, int user) > +{ > + struct drm_fb_helper *fb_helper = info->par; > + struct drm_device *dev = fb_helper->dev; > + > + if (user) > + module_put(dev->driver->fops->owner); > + > + return 0; > +} > + > static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma) > { > return dma_mmap_writecombine(info->device, vma, info->screen_base, > @@ -247,10 +273,13 @@ static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma) > static struct fb_ops drm_fbdev_cma_ops = { > .owner = THIS_MODULE, > DRM_FB_HELPER_DEFAULT_OPS, > + .fb_open = drm_fbdev_cma_fb_open, > + .fb_release = drm_fbdev_cma_fb_release, > .fb_fillrect = drm_fb_helper_sys_fillrect, > .fb_copyarea = drm_fb_helper_sys_copyarea, > .fb_imageblit = drm_fb_helper_sys_imageblit, > .fb_mmap = drm_fb_cma_mmap, > + .fb_destroy = drm_fb_helper_fb_destroy, > }; > > static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info, > @@ -262,52 +291,26 @@ static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info, > return 0; > } > > -static int drm_fbdev_cma_defio_init(struct fb_info *fbi, > +static int drm_fbdev_cma_defio_init(struct drm_fb_helper *helper, > struct drm_gem_cma_object *cma_obj) > { > - struct fb_deferred_io *fbdefio; > - struct fb_ops *fbops; > + struct fb_info *fbi = helper->fbdev; > + int ret; > > - /* > - * 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; > - } > + ret = drm_fb_helper_defio_init(helper); > + if (ret) > + return ret; > > /* 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); > -} > - Above changes look like unrelated cleanup? Should be a separate patch I think. > static int > drm_fbdev_cma_create(struct drm_fb_helper *helper, > struct drm_fb_helper_surface_size *sizes) > @@ -365,7 +368,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper, > fbi->fix.smem_len = size; > > if (fbdev_cma->fb_funcs->dirty) { > - ret = drm_fbdev_cma_defio_init(fbi, obj); > + ret = drm_fbdev_cma_defio_init(helper, obj); > if (ret) > goto err_cma_destroy; > } > @@ -399,7 +402,6 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, > const struct drm_framebuffer_funcs *funcs) > { > struct drm_fbdev_cma *fbdev_cma; > - struct drm_fb_helper *helper; > int ret; > > fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL); > @@ -409,37 +411,15 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, > } > fbdev_cma->fb_funcs = funcs; > > - helper = &fbdev_cma->fb_helper; > - > - drm_fb_helper_prepare(dev, helper, &drm_fb_cma_helper_funcs); > - > - ret = drm_fb_helper_init(dev, helper, max_conn_count); > - if (ret < 0) { > - dev_err(dev->dev, "Failed to initialize drm fb helper.\n"); > - goto err_free; > - } > - > - ret = drm_fb_helper_single_add_all_connectors(helper); > - if (ret < 0) { > - dev_err(dev->dev, "Failed to add connectors.\n"); > - goto err_drm_fb_helper_fini; > - > - } > - > - ret = drm_fb_helper_initial_config(helper, preferred_bpp); > - if (ret < 0) { > - dev_err(dev->dev, "Failed to set initial hw configuration.\n"); > - goto err_drm_fb_helper_fini; > + ret = drm_fb_helper_simple_init(dev, &fbdev_cma->fb_helper, > + preferred_bpp, max_conn_count, > + &drm_fb_cma_helper_funcs); Yeah the conversion to simple* should be split out I think. But then I thought more of this will be unified with the overall fbdev helpers? > + if (ret) { > + kfree(fbdev_cma); > + return ERR_PTR(ret); > } > > return fbdev_cma; > - > -err_drm_fb_helper_fini: > - drm_fb_helper_fini(helper); > -err_free: > - kfree(fbdev_cma); > - > - return ERR_PTR(ret); > } > EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs); > > @@ -468,17 +448,15 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_init); > /** > * drm_fbdev_cma_fini() - Free drm_fbdev_cma struct > * @fbdev_cma: The drm_fbdev_cma struct > + * > + * This function calls drm_fb_helper_simple_fini() and frees @fbdev_cma. > + * > + * Don't use this function together with drm_fbdev_cma_dev_unplug(). > */ > 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); > - > - drm_fb_helper_fini(&fbdev_cma->fb_helper); > + if (fbdev_cma) > + drm_fb_helper_simple_fini(&fbdev_cma->fb_helper); > kfree(fbdev_cma); > } > EXPORT_SYMBOL_GPL(drm_fbdev_cma_fini); > @@ -542,3 +520,20 @@ void drm_fbdev_cma_set_suspend_unlocked(struct drm_fbdev_cma *fbdev_cma, > state); > } > EXPORT_SYMBOL(drm_fbdev_cma_set_suspend_unlocked); > + > +/** > + * drm_fbdev_cma_dev_unplug - wrapper around drm_fb_helper_dev_unplug > + * @fbdev_cma: The drm_fbdev_cma struct, may be NULL > + * > + * This unplugs the fbdev emulation for a hotpluggable DRM device. See > + * drm_fb_helper_dev_unplug() for details. > + * > + * Drivers must call drm_mode_config_cleanup() and free @fbdev_cma in their > + * &drm_driver->release callback. > + */ > +void drm_fbdev_cma_dev_unplug(struct drm_fbdev_cma *fbdev_cma) > +{ > + if (fbdev_cma) > + drm_fb_helper_dev_unplug(&fbdev_cma->fb_helper); > +} I thought we're trying to phase out the cma helpers, why do we need a wrapper for a wrapper? > +EXPORT_SYMBOL(drm_fbdev_cma_dev_unplug); > diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h > index a323781..a6aafae 100644 > --- a/include/drm/drm_fb_cma_helper.h > +++ b/include/drm/drm_fb_cma_helper.h > @@ -27,6 +27,7 @@ void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma); > void drm_fbdev_cma_set_suspend(struct drm_fbdev_cma *fbdev_cma, bool state); > void drm_fbdev_cma_set_suspend_unlocked(struct drm_fbdev_cma *fbdev_cma, > bool state); > +void drm_fbdev_cma_dev_unplug(struct drm_fbdev_cma *fbdev_cma); > > void drm_fb_cma_destroy(struct drm_framebuffer *fb); > int drm_fb_cma_create_handle(struct drm_framebuffer *fb, > -- > 2.7.4 >
Den 28.08.2017 23.46, skrev Daniel Vetter: > On Mon, Aug 28, 2017 at 07:17:45PM +0200, Noralf Trønnes wrote: >> Add drm_fbdev_cma_dev_unplug() and use the drm_fb_helper device unplug >> support. Pin driver module on fb_open(). >> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> --- >> drivers/gpu/drm/drm_fb_cma_helper.c | 139 +++++++++++++++++------------------- >> include/drm/drm_fb_cma_helper.h | 1 + >> 2 files changed, 68 insertions(+), 72 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c >> index f2ee883..2b044be 100644 >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c >> @@ -25,8 +25,6 @@ >> #include <drm/drm_fb_cma_helper.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; >> @@ -238,6 +236,34 @@ int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg) >> EXPORT_SYMBOL_GPL(drm_fb_cma_debugfs_show); >> #endif >> >> +static int drm_fbdev_cma_fb_open(struct fb_info *info, int user) >> +{ >> + struct drm_fb_helper *fb_helper = info->par; >> + struct drm_device *dev = fb_helper->dev; >> + >> + /* >> + * The fb_ops definition resides in this library, meaning fb_open() >> + * will take a ref on the library instead of the driver. Make sure the >> + * driver module is pinned. Skip fbcon (user==0) since it can detach >> + * itself on unregister_framebuffer(). >> + */ >> + if (user && !try_module_get(dev->driver->fops->owner)) >> + return -ENODEV; > I thought we've fixed this by redoing the fb_ops as a macro? Maybe tinydrm > isn't fixed yet, but then we need to fix up tinydrm. This here otoh kinda > smells a bit like a hack ... > > If we need it, then I'd vote to put it into the fbdev helpers, not the cma > specific parts. This is only necessary in this library atm because it contains this: static struct fb_ops drm_fbdev_cma_ops = { .owner = THIS_MODULE, Other drivers have this definition in their own module, so fb_open() takes a ref on the right module. My shmem gem library will also wrap struct fb_ops and needs to do this. I can add them to drm_fb_helper with a note that they should only be used by libraries. Yes it's kind of a hack, but I find the cma library is very nice in this respect, that it makes fbdev emulation very easy for drivers. It wraps up basic fbdev in 3 function calls. If we were to do this strictly by the book, we would have to add something like this to every cma driver: DEFINE_DRM_FBDEV_CMA_FB_OPS(driver_fb_ops); static int driver_fb_probe(struct drm_fb_helper *helper, struct drm_fb_helper_surface_size *sizes) { return drm_fbdev_cma_create(helper, sizes, &driver_fb_ops); } static const struct drm_fb_helper_funcs driver_helper_funcs = { .fb_probe = driver_fb_probe, }; static int driver_fbdev_init() { - fbdev_cma = drm_fbdev_cma_init(dev, preferred_bpp, max_conn_count); + fbdev_cma = drm_fbdev_cma_init(dev, preferred_bpp, max_conn_count, + &driver_helper_funcs); } Well, maybe not so bad after all :-) Maybe I should do it like this in the shmem gem library. I can't do refactoring like this, this time around. I need to do that shmem gem library before I run out of steam. A refactoring like this will have to wait to later. >> + >> + return 0; >> +} >> + >> +static int drm_fbdev_cma_fb_release(struct fb_info *info, int user) >> +{ >> + struct drm_fb_helper *fb_helper = info->par; >> + struct drm_device *dev = fb_helper->dev; >> + >> + if (user) >> + module_put(dev->driver->fops->owner); >> + >> + return 0; >> +} >> + >> static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma) >> { >> return dma_mmap_writecombine(info->device, vma, info->screen_base, >> @@ -247,10 +273,13 @@ static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma) >> static struct fb_ops drm_fbdev_cma_ops = { >> .owner = THIS_MODULE, >> DRM_FB_HELPER_DEFAULT_OPS, >> + .fb_open = drm_fbdev_cma_fb_open, >> + .fb_release = drm_fbdev_cma_fb_release, >> .fb_fillrect = drm_fb_helper_sys_fillrect, >> .fb_copyarea = drm_fb_helper_sys_copyarea, >> .fb_imageblit = drm_fb_helper_sys_imageblit, >> .fb_mmap = drm_fb_cma_mmap, >> + .fb_destroy = drm_fb_helper_fb_destroy, >> }; >> >> static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info, >> @@ -262,52 +291,26 @@ static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info, >> return 0; >> } >> >> -static int drm_fbdev_cma_defio_init(struct fb_info *fbi, >> +static int drm_fbdev_cma_defio_init(struct drm_fb_helper *helper, >> struct drm_gem_cma_object *cma_obj) >> { >> - struct fb_deferred_io *fbdefio; >> - struct fb_ops *fbops; >> + struct fb_info *fbi = helper->fbdev; >> + int ret; >> >> - /* >> - * 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; >> - } >> + ret = drm_fb_helper_defio_init(helper); >> + if (ret) >> + return ret; >> >> /* 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); >> -} >> - > Above changes look like unrelated cleanup? Should be a separate patch I > think. > >> static int >> drm_fbdev_cma_create(struct drm_fb_helper *helper, >> struct drm_fb_helper_surface_size *sizes) >> @@ -365,7 +368,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper, >> fbi->fix.smem_len = size; >> >> if (fbdev_cma->fb_funcs->dirty) { >> - ret = drm_fbdev_cma_defio_init(fbi, obj); >> + ret = drm_fbdev_cma_defio_init(helper, obj); >> if (ret) >> goto err_cma_destroy; >> } >> @@ -399,7 +402,6 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, >> const struct drm_framebuffer_funcs *funcs) >> { >> struct drm_fbdev_cma *fbdev_cma; >> - struct drm_fb_helper *helper; >> int ret; >> >> fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL); >> @@ -409,37 +411,15 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, >> } >> fbdev_cma->fb_funcs = funcs; >> >> - helper = &fbdev_cma->fb_helper; >> - >> - drm_fb_helper_prepare(dev, helper, &drm_fb_cma_helper_funcs); >> - >> - ret = drm_fb_helper_init(dev, helper, max_conn_count); >> - if (ret < 0) { >> - dev_err(dev->dev, "Failed to initialize drm fb helper.\n"); >> - goto err_free; >> - } >> - >> - ret = drm_fb_helper_single_add_all_connectors(helper); >> - if (ret < 0) { >> - dev_err(dev->dev, "Failed to add connectors.\n"); >> - goto err_drm_fb_helper_fini; >> - >> - } >> - >> - ret = drm_fb_helper_initial_config(helper, preferred_bpp); >> - if (ret < 0) { >> - dev_err(dev->dev, "Failed to set initial hw configuration.\n"); >> - goto err_drm_fb_helper_fini; >> + ret = drm_fb_helper_simple_init(dev, &fbdev_cma->fb_helper, >> + preferred_bpp, max_conn_count, >> + &drm_fb_cma_helper_funcs); > Yeah the conversion to simple* should be split out I think. But then I > thought more of this will be unified with the overall fbdev helpers? I agree, if we find a way to support unplug within the existing helpers. >> + if (ret) { >> + kfree(fbdev_cma); >> + return ERR_PTR(ret); >> } >> >> return fbdev_cma; >> - >> -err_drm_fb_helper_fini: >> - drm_fb_helper_fini(helper); >> -err_free: >> - kfree(fbdev_cma); >> - >> - return ERR_PTR(ret); >> } >> EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs); >> >> @@ -468,17 +448,15 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_init); >> /** >> * drm_fbdev_cma_fini() - Free drm_fbdev_cma struct >> * @fbdev_cma: The drm_fbdev_cma struct >> + * >> + * This function calls drm_fb_helper_simple_fini() and frees @fbdev_cma. >> + * >> + * Don't use this function together with drm_fbdev_cma_dev_unplug(). >> */ >> 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); >> - >> - drm_fb_helper_fini(&fbdev_cma->fb_helper); >> + if (fbdev_cma) >> + drm_fb_helper_simple_fini(&fbdev_cma->fb_helper); >> kfree(fbdev_cma); >> } >> EXPORT_SYMBOL_GPL(drm_fbdev_cma_fini); >> @@ -542,3 +520,20 @@ void drm_fbdev_cma_set_suspend_unlocked(struct drm_fbdev_cma *fbdev_cma, >> state); >> } >> EXPORT_SYMBOL(drm_fbdev_cma_set_suspend_unlocked); >> + >> +/** >> + * drm_fbdev_cma_dev_unplug - wrapper around drm_fb_helper_dev_unplug >> + * @fbdev_cma: The drm_fbdev_cma struct, may be NULL >> + * >> + * This unplugs the fbdev emulation for a hotpluggable DRM device. See >> + * drm_fb_helper_dev_unplug() for details. >> + * >> + * Drivers must call drm_mode_config_cleanup() and free @fbdev_cma in their >> + * &drm_driver->release callback. >> + */ >> +void drm_fbdev_cma_dev_unplug(struct drm_fbdev_cma *fbdev_cma) >> +{ >> + if (fbdev_cma) >> + drm_fb_helper_dev_unplug(&fbdev_cma->fb_helper); >> +} > I thought we're trying to phase out the cma helpers, why do we need a > wrapper for a wrapper? Because struct drm_fbdev_cma is private to this file. When tinydrm switches to shmem, we can remove the dirtyfb stuff from this helper and we will end up with this: struct drm_fbdev_cma { struct drm_fb_helper fb_helper; }; Now it is possible to refactor and switch to using drm_fb_helper directly and drop the fbdev wrappers. We can ofc avoid adding a wrapper by making struct drm_fbdev_cma public. Noralf. >> +EXPORT_SYMBOL(drm_fbdev_cma_dev_unplug); >> diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h >> index a323781..a6aafae 100644 >> --- a/include/drm/drm_fb_cma_helper.h >> +++ b/include/drm/drm_fb_cma_helper.h >> @@ -27,6 +27,7 @@ void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma); >> void drm_fbdev_cma_set_suspend(struct drm_fbdev_cma *fbdev_cma, bool state); >> void drm_fbdev_cma_set_suspend_unlocked(struct drm_fbdev_cma *fbdev_cma, >> bool state); >> +void drm_fbdev_cma_dev_unplug(struct drm_fbdev_cma *fbdev_cma); >> >> void drm_fb_cma_destroy(struct drm_framebuffer *fb); >> int drm_fb_cma_create_handle(struct drm_framebuffer *fb, >> -- >> 2.7.4 >>
On Tue, Aug 29, 2017 at 07:23:14PM +0200, Noralf Trønnes wrote: > > Den 28.08.2017 23.46, skrev Daniel Vetter: > > On Mon, Aug 28, 2017 at 07:17:45PM +0200, Noralf Trønnes wrote: > > > Add drm_fbdev_cma_dev_unplug() and use the drm_fb_helper device unplug > > > support. Pin driver module on fb_open(). > > > > > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > > --- > > > drivers/gpu/drm/drm_fb_cma_helper.c | 139 +++++++++++++++++------------------- > > > include/drm/drm_fb_cma_helper.h | 1 + > > > 2 files changed, 68 insertions(+), 72 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c > > > index f2ee883..2b044be 100644 > > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c > > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > > > @@ -25,8 +25,6 @@ > > > #include <drm/drm_fb_cma_helper.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; > > > @@ -238,6 +236,34 @@ int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg) > > > EXPORT_SYMBOL_GPL(drm_fb_cma_debugfs_show); > > > #endif > > > +static int drm_fbdev_cma_fb_open(struct fb_info *info, int user) > > > +{ > > > + struct drm_fb_helper *fb_helper = info->par; > > > + struct drm_device *dev = fb_helper->dev; > > > + > > > + /* > > > + * The fb_ops definition resides in this library, meaning fb_open() > > > + * will take a ref on the library instead of the driver. Make sure the > > > + * driver module is pinned. Skip fbcon (user==0) since it can detach > > > + * itself on unregister_framebuffer(). > > > + */ > > > + if (user && !try_module_get(dev->driver->fops->owner)) > > > + return -ENODEV; > > I thought we've fixed this by redoing the fb_ops as a macro? Maybe tinydrm > > isn't fixed yet, but then we need to fix up tinydrm. This here otoh kinda > > smells a bit like a hack ... > > > > If we need it, then I'd vote to put it into the fbdev helpers, not the cma > > specific parts. > > This is only necessary in this library atm because it contains this: > > static struct fb_ops drm_fbdev_cma_ops = { > .owner = THIS_MODULE, > > Other drivers have this definition in their own module, so fb_open() > takes a ref on the right module. > > My shmem gem library will also wrap struct fb_ops and needs to do this. > I can add them to drm_fb_helper with a note that they should only be > used by libraries. > > Yes it's kind of a hack, but I find the cma library is very nice in this > respect, that it makes fbdev emulation very easy for drivers. It wraps > up basic fbdev in 3 function calls. > > If we were to do this strictly by the book, we would have to add > something like this to every cma driver: > > DEFINE_DRM_FBDEV_CMA_FB_OPS(driver_fb_ops); > > static int driver_fb_probe(struct drm_fb_helper *helper, > struct drm_fb_helper_surface_size *sizes) > { > return drm_fbdev_cma_create(helper, sizes, &driver_fb_ops); > } > > static const struct drm_fb_helper_funcs driver_helper_funcs = { > .fb_probe = driver_fb_probe, > }; > > static int driver_fbdev_init() > { > - fbdev_cma = drm_fbdev_cma_init(dev, preferred_bpp, max_conn_count); > + fbdev_cma = drm_fbdev_cma_init(dev, preferred_bpp, max_conn_count, > + &driver_helper_funcs); > } > > Well, maybe not so bad after all :-) > Maybe I should do it like this in the shmem gem library. Hm, this does add considerable amounts of boilerplate everywhere, so not sure it's a good idea. > I can't do refactoring like this, this time around. I need to do that > shmem gem library before I run out of steam. A refactoring like this > will have to wait to later. I think your fb_open/close hooks are just fine, but instead of putting them into cma helpers only let's just put them into the core helpers (and add them to DRM_FB_HELPER_DEFAULT_OPS). Them being a special case for cma only was what really irked me, not so much what they do. And doing the get/put module by default shouldn't really hurt (for drivers where the fb_ops and the drm_driver ops match it'll mean double refcounting, but that doesn't matter really). > > > > + > > > + return 0; > > > +} > > > + > > > +static int drm_fbdev_cma_fb_release(struct fb_info *info, int user) > > > +{ > > > + struct drm_fb_helper *fb_helper = info->par; > > > + struct drm_device *dev = fb_helper->dev; > > > + > > > + if (user) > > > + module_put(dev->driver->fops->owner); > > > + > > > + return 0; > > > +} > > > + > > > static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma) > > > { > > > return dma_mmap_writecombine(info->device, vma, info->screen_base, > > > @@ -247,10 +273,13 @@ static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma) > > > static struct fb_ops drm_fbdev_cma_ops = { > > > .owner = THIS_MODULE, > > > DRM_FB_HELPER_DEFAULT_OPS, > > > + .fb_open = drm_fbdev_cma_fb_open, > > > + .fb_release = drm_fbdev_cma_fb_release, > > > .fb_fillrect = drm_fb_helper_sys_fillrect, > > > .fb_copyarea = drm_fb_helper_sys_copyarea, > > > .fb_imageblit = drm_fb_helper_sys_imageblit, > > > .fb_mmap = drm_fb_cma_mmap, > > > + .fb_destroy = drm_fb_helper_fb_destroy, > > > }; > > > static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info, > > > @@ -262,52 +291,26 @@ static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info, > > > return 0; > > > } > > > -static int drm_fbdev_cma_defio_init(struct fb_info *fbi, > > > +static int drm_fbdev_cma_defio_init(struct drm_fb_helper *helper, > > > struct drm_gem_cma_object *cma_obj) > > > { > > > - struct fb_deferred_io *fbdefio; > > > - struct fb_ops *fbops; > > > + struct fb_info *fbi = helper->fbdev; > > > + int ret; > > > - /* > > > - * 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; > > > - } > > > + ret = drm_fb_helper_defio_init(helper); > > > + if (ret) > > > + return ret; > > > /* 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); > > > -} > > > - > > Above changes look like unrelated cleanup? Should be a separate patch I > > think. > > > > > static int > > > drm_fbdev_cma_create(struct drm_fb_helper *helper, > > > struct drm_fb_helper_surface_size *sizes) > > > @@ -365,7 +368,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper, > > > fbi->fix.smem_len = size; > > > if (fbdev_cma->fb_funcs->dirty) { > > > - ret = drm_fbdev_cma_defio_init(fbi, obj); > > > + ret = drm_fbdev_cma_defio_init(helper, obj); > > > if (ret) > > > goto err_cma_destroy; > > > } > > > @@ -399,7 +402,6 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, > > > const struct drm_framebuffer_funcs *funcs) > > > { > > > struct drm_fbdev_cma *fbdev_cma; > > > - struct drm_fb_helper *helper; > > > int ret; > > > fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL); > > > @@ -409,37 +411,15 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, > > > } > > > fbdev_cma->fb_funcs = funcs; > > > - helper = &fbdev_cma->fb_helper; > > > - > > > - drm_fb_helper_prepare(dev, helper, &drm_fb_cma_helper_funcs); > > > - > > > - ret = drm_fb_helper_init(dev, helper, max_conn_count); > > > - if (ret < 0) { > > > - dev_err(dev->dev, "Failed to initialize drm fb helper.\n"); > > > - goto err_free; > > > - } > > > - > > > - ret = drm_fb_helper_single_add_all_connectors(helper); > > > - if (ret < 0) { > > > - dev_err(dev->dev, "Failed to add connectors.\n"); > > > - goto err_drm_fb_helper_fini; > > > - > > > - } > > > - > > > - ret = drm_fb_helper_initial_config(helper, preferred_bpp); > > > - if (ret < 0) { > > > - dev_err(dev->dev, "Failed to set initial hw configuration.\n"); > > > - goto err_drm_fb_helper_fini; > > > + ret = drm_fb_helper_simple_init(dev, &fbdev_cma->fb_helper, > > > + preferred_bpp, max_conn_count, > > > + &drm_fb_cma_helper_funcs); > > Yeah the conversion to simple* should be split out I think. But then I > > thought more of this will be unified with the overall fbdev helpers? > > I agree, if we find a way to support unplug within the existing helpers. > > > > + if (ret) { > > > + kfree(fbdev_cma); > > > + return ERR_PTR(ret); > > > } > > > return fbdev_cma; > > > - > > > -err_drm_fb_helper_fini: > > > - drm_fb_helper_fini(helper); > > > -err_free: > > > - kfree(fbdev_cma); > > > - > > > - return ERR_PTR(ret); > > > } > > > EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs); > > > @@ -468,17 +448,15 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_init); > > > /** > > > * drm_fbdev_cma_fini() - Free drm_fbdev_cma struct > > > * @fbdev_cma: The drm_fbdev_cma struct > > > + * > > > + * This function calls drm_fb_helper_simple_fini() and frees @fbdev_cma. > > > + * > > > + * Don't use this function together with drm_fbdev_cma_dev_unplug(). > > > */ > > > 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); > > > - > > > - drm_fb_helper_fini(&fbdev_cma->fb_helper); > > > + if (fbdev_cma) > > > + drm_fb_helper_simple_fini(&fbdev_cma->fb_helper); > > > kfree(fbdev_cma); > > > } > > > EXPORT_SYMBOL_GPL(drm_fbdev_cma_fini); > > > @@ -542,3 +520,20 @@ void drm_fbdev_cma_set_suspend_unlocked(struct drm_fbdev_cma *fbdev_cma, > > > state); > > > } > > > EXPORT_SYMBOL(drm_fbdev_cma_set_suspend_unlocked); > > > + > > > +/** > > > + * drm_fbdev_cma_dev_unplug - wrapper around drm_fb_helper_dev_unplug > > > + * @fbdev_cma: The drm_fbdev_cma struct, may be NULL > > > + * > > > + * This unplugs the fbdev emulation for a hotpluggable DRM device. See > > > + * drm_fb_helper_dev_unplug() for details. > > > + * > > > + * Drivers must call drm_mode_config_cleanup() and free @fbdev_cma in their > > > + * &drm_driver->release callback. > > > + */ > > > +void drm_fbdev_cma_dev_unplug(struct drm_fbdev_cma *fbdev_cma) > > > +{ > > > + if (fbdev_cma) > > > + drm_fb_helper_dev_unplug(&fbdev_cma->fb_helper); > > > +} > > I thought we're trying to phase out the cma helpers, why do we need a > > wrapper for a wrapper? > > Because struct drm_fbdev_cma is private to this file. > When tinydrm switches to shmem, we can remove the dirtyfb stuff from > this helper and we will end up with this: > > struct drm_fbdev_cma { > struct drm_fb_helper fb_helper; > }; > > Now it is possible to refactor and switch to using drm_fb_helper > directly and drop the fbdev wrappers. > > We can ofc avoid adding a wrapper by making struct drm_fbdev_cma public. Hm, no I understand. I'd vote to make struct drm_fbdev_cma public. We generally don't try to hide stuff in structs only in .c files, makes it easier to up/downcast and have static inline helpers. Moving the struct should be a separate patch I think. -Daniel > > > Noralf. > > > > +EXPORT_SYMBOL(drm_fbdev_cma_dev_unplug); > > > diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h > > > index a323781..a6aafae 100644 > > > --- a/include/drm/drm_fb_cma_helper.h > > > +++ b/include/drm/drm_fb_cma_helper.h > > > @@ -27,6 +27,7 @@ void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma); > > > void drm_fbdev_cma_set_suspend(struct drm_fbdev_cma *fbdev_cma, bool state); > > > void drm_fbdev_cma_set_suspend_unlocked(struct drm_fbdev_cma *fbdev_cma, > > > bool state); > > > +void drm_fbdev_cma_dev_unplug(struct drm_fbdev_cma *fbdev_cma); > > > void drm_fb_cma_destroy(struct drm_framebuffer *fb); > > > int drm_fb_cma_create_handle(struct drm_framebuffer *fb, > > > -- > > > 2.7.4 > > > >
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index f2ee883..2b044be 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -25,8 +25,6 @@ #include <drm/drm_fb_cma_helper.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; @@ -238,6 +236,34 @@ int drm_fb_cma_debugfs_show(struct seq_file *m, void *arg) EXPORT_SYMBOL_GPL(drm_fb_cma_debugfs_show); #endif +static int drm_fbdev_cma_fb_open(struct fb_info *info, int user) +{ + struct drm_fb_helper *fb_helper = info->par; + struct drm_device *dev = fb_helper->dev; + + /* + * The fb_ops definition resides in this library, meaning fb_open() + * will take a ref on the library instead of the driver. Make sure the + * driver module is pinned. Skip fbcon (user==0) since it can detach + * itself on unregister_framebuffer(). + */ + if (user && !try_module_get(dev->driver->fops->owner)) + return -ENODEV; + + return 0; +} + +static int drm_fbdev_cma_fb_release(struct fb_info *info, int user) +{ + struct drm_fb_helper *fb_helper = info->par; + struct drm_device *dev = fb_helper->dev; + + if (user) + module_put(dev->driver->fops->owner); + + return 0; +} + static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma) { return dma_mmap_writecombine(info->device, vma, info->screen_base, @@ -247,10 +273,13 @@ static int drm_fb_cma_mmap(struct fb_info *info, struct vm_area_struct *vma) static struct fb_ops drm_fbdev_cma_ops = { .owner = THIS_MODULE, DRM_FB_HELPER_DEFAULT_OPS, + .fb_open = drm_fbdev_cma_fb_open, + .fb_release = drm_fbdev_cma_fb_release, .fb_fillrect = drm_fb_helper_sys_fillrect, .fb_copyarea = drm_fb_helper_sys_copyarea, .fb_imageblit = drm_fb_helper_sys_imageblit, .fb_mmap = drm_fb_cma_mmap, + .fb_destroy = drm_fb_helper_fb_destroy, }; static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info, @@ -262,52 +291,26 @@ static int drm_fbdev_cma_deferred_io_mmap(struct fb_info *info, return 0; } -static int drm_fbdev_cma_defio_init(struct fb_info *fbi, +static int drm_fbdev_cma_defio_init(struct drm_fb_helper *helper, struct drm_gem_cma_object *cma_obj) { - struct fb_deferred_io *fbdefio; - struct fb_ops *fbops; + struct fb_info *fbi = helper->fbdev; + int ret; - /* - * 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; - } + ret = drm_fb_helper_defio_init(helper); + if (ret) + return ret; /* 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) @@ -365,7 +368,7 @@ drm_fbdev_cma_create(struct drm_fb_helper *helper, fbi->fix.smem_len = size; if (fbdev_cma->fb_funcs->dirty) { - ret = drm_fbdev_cma_defio_init(fbi, obj); + ret = drm_fbdev_cma_defio_init(helper, obj); if (ret) goto err_cma_destroy; } @@ -399,7 +402,6 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, const struct drm_framebuffer_funcs *funcs) { struct drm_fbdev_cma *fbdev_cma; - struct drm_fb_helper *helper; int ret; fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL); @@ -409,37 +411,15 @@ struct drm_fbdev_cma *drm_fbdev_cma_init_with_funcs(struct drm_device *dev, } fbdev_cma->fb_funcs = funcs; - helper = &fbdev_cma->fb_helper; - - drm_fb_helper_prepare(dev, helper, &drm_fb_cma_helper_funcs); - - ret = drm_fb_helper_init(dev, helper, max_conn_count); - if (ret < 0) { - dev_err(dev->dev, "Failed to initialize drm fb helper.\n"); - goto err_free; - } - - ret = drm_fb_helper_single_add_all_connectors(helper); - if (ret < 0) { - dev_err(dev->dev, "Failed to add connectors.\n"); - goto err_drm_fb_helper_fini; - - } - - ret = drm_fb_helper_initial_config(helper, preferred_bpp); - if (ret < 0) { - dev_err(dev->dev, "Failed to set initial hw configuration.\n"); - goto err_drm_fb_helper_fini; + ret = drm_fb_helper_simple_init(dev, &fbdev_cma->fb_helper, + preferred_bpp, max_conn_count, + &drm_fb_cma_helper_funcs); + if (ret) { + kfree(fbdev_cma); + return ERR_PTR(ret); } return fbdev_cma; - -err_drm_fb_helper_fini: - drm_fb_helper_fini(helper); -err_free: - kfree(fbdev_cma); - - return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(drm_fbdev_cma_init_with_funcs); @@ -468,17 +448,15 @@ EXPORT_SYMBOL_GPL(drm_fbdev_cma_init); /** * drm_fbdev_cma_fini() - Free drm_fbdev_cma struct * @fbdev_cma: The drm_fbdev_cma struct + * + * This function calls drm_fb_helper_simple_fini() and frees @fbdev_cma. + * + * Don't use this function together with drm_fbdev_cma_dev_unplug(). */ 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); - - drm_fb_helper_fini(&fbdev_cma->fb_helper); + if (fbdev_cma) + drm_fb_helper_simple_fini(&fbdev_cma->fb_helper); kfree(fbdev_cma); } EXPORT_SYMBOL_GPL(drm_fbdev_cma_fini); @@ -542,3 +520,20 @@ void drm_fbdev_cma_set_suspend_unlocked(struct drm_fbdev_cma *fbdev_cma, state); } EXPORT_SYMBOL(drm_fbdev_cma_set_suspend_unlocked); + +/** + * drm_fbdev_cma_dev_unplug - wrapper around drm_fb_helper_dev_unplug + * @fbdev_cma: The drm_fbdev_cma struct, may be NULL + * + * This unplugs the fbdev emulation for a hotpluggable DRM device. See + * drm_fb_helper_dev_unplug() for details. + * + * Drivers must call drm_mode_config_cleanup() and free @fbdev_cma in their + * &drm_driver->release callback. + */ +void drm_fbdev_cma_dev_unplug(struct drm_fbdev_cma *fbdev_cma) +{ + if (fbdev_cma) + drm_fb_helper_dev_unplug(&fbdev_cma->fb_helper); +} +EXPORT_SYMBOL(drm_fbdev_cma_dev_unplug); diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h index a323781..a6aafae 100644 --- a/include/drm/drm_fb_cma_helper.h +++ b/include/drm/drm_fb_cma_helper.h @@ -27,6 +27,7 @@ void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma); void drm_fbdev_cma_set_suspend(struct drm_fbdev_cma *fbdev_cma, bool state); void drm_fbdev_cma_set_suspend_unlocked(struct drm_fbdev_cma *fbdev_cma, bool state); +void drm_fbdev_cma_dev_unplug(struct drm_fbdev_cma *fbdev_cma); void drm_fb_cma_destroy(struct drm_framebuffer *fb); int drm_fb_cma_create_handle(struct drm_framebuffer *fb,
Add drm_fbdev_cma_dev_unplug() and use the drm_fb_helper device unplug support. Pin driver module on fb_open(). Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- drivers/gpu/drm/drm_fb_cma_helper.c | 139 +++++++++++++++++------------------- include/drm/drm_fb_cma_helper.h | 1 + 2 files changed, 68 insertions(+), 72 deletions(-)