| Message ID | 1471897525-31118-6-git-send-email-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 22, 2016 at 10:25:25PM +0200, Noralf Trønnes wrote: > There is currently no non-fbdev mechanism in place to kick out > simpledrm when the real hw-driver is probed. As a stop gap until > that is in place, honour remove_conflicting_framebuffers() and > delete the simple-framebuffer platform device when it's called. > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > > Changes from version 3: > - drm_device.platformdev is deprecated, use to_platform_device(ddev->dev). > - fb_helper might have been released in sdrm_fbdev_fb_destroy(), > so open code drm_fb_helper_release_fbi() > - Strengthen the test in sdrm_fbdev_event_notify() that we're the one. > > Changes from version 2: > - Don't forget to free fb_info when kicked out. > > drivers/gpu/drm/simpledrm/Kconfig | 5 +++ > drivers/gpu/drm/simpledrm/simpledrm.h | 2 + > drivers/gpu/drm/simpledrm/simpledrm_drv.c | 3 ++ > drivers/gpu/drm/simpledrm/simpledrm_fbdev.c | 62 ++++++++++++++++++++++++++++- > 4 files changed, 70 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/simpledrm/Kconfig b/drivers/gpu/drm/simpledrm/Kconfig > index 3257590..4275d13 100644 > --- a/drivers/gpu/drm/simpledrm/Kconfig > +++ b/drivers/gpu/drm/simpledrm/Kconfig > @@ -16,6 +16,11 @@ config DRM_SIMPLEDRM > If fbdev support is enabled, this driver will also provide an fbdev > compatibility layer that supports fbcon, mmap is not supported. > > + WARNING > + fbdev must be enabled for simpledrm to disable itself when a real > + hw-driver is probed. It relies on remove_conflicting_framebuffers() > + to be called by the hw-driver. > + > If unsure, say Y. > > To compile this driver as a module, choose M here: the > diff --git a/drivers/gpu/drm/simpledrm/simpledrm.h b/drivers/gpu/drm/simpledrm/simpledrm.h > index d4eb52c..3cca196 100644 > --- a/drivers/gpu/drm/simpledrm/simpledrm.h > +++ b/drivers/gpu/drm/simpledrm/simpledrm.h > @@ -87,5 +87,7 @@ int sdrm_fb_init(struct drm_device *ddev, struct sdrm_framebuffer *fb, > struct sdrm_gem_object *obj); > void sdrm_fbdev_init(struct sdrm_device *sdrm); > void sdrm_fbdev_cleanup(struct sdrm_device *sdrm); > +void sdrm_fbdev_kickout_init(void); > +void sdrm_fbdev_kickout_exit(void); > > #endif /* SDRM_DRV_H */ > diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c b/drivers/gpu/drm/simpledrm/simpledrm_drv.c > index fe752c6..0750652 100644 > --- a/drivers/gpu/drm/simpledrm/simpledrm_drv.c > +++ b/drivers/gpu/drm/simpledrm/simpledrm_drv.c > @@ -531,12 +531,15 @@ static int __init sdrm_init(void) > } > } > > + sdrm_fbdev_kickout_init(); > + > return 0; > } > module_init(sdrm_init); > > static void __exit sdrm_exit(void) > { > + sdrm_fbdev_kickout_exit(); > platform_driver_unregister(&sdrm_simplefb_driver); > } > module_exit(sdrm_exit); > diff --git a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c > index c6596ad..7c6db2c 100644 > --- a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c > +++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c > @@ -44,6 +44,20 @@ static int sdrm_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) > return -ENODEV; > } > > +/* > + * Releasing has to be done outside the notifier callchain when we're > + * kicked out, since do_unregister_framebuffer() calls put_fb_info() > + * after the notifier has run. > + * Open code drm_fb_helper_release_fbi() since fb_helper is freed at > + * this point when kicked out. > + */ > +static void sdrm_fbdev_fb_destroy(struct fb_info *info) > +{ > + if (info->cmap.len) > + fb_dealloc_cmap(&info->cmap); > + framebuffer_release(info); > +} > + > static struct fb_ops sdrm_fbdev_ops = { > .owner = THIS_MODULE, > .fb_fillrect = drm_fb_helper_sys_fillrect, > @@ -53,6 +67,7 @@ static struct fb_ops sdrm_fbdev_ops = { > .fb_set_par = drm_fb_helper_set_par, > .fb_setcmap = drm_fb_helper_setcmap, > .fb_mmap = sdrm_fb_mmap, > + .fb_destroy = sdrm_fbdev_fb_destroy, > }; > > static int sdrm_fbdev_create(struct drm_fb_helper *helper, > @@ -110,6 +125,9 @@ static int sdrm_fbdev_create(struct drm_fb_helper *helper, > fbi->screen_base = obj->vmapping; > fbi->fix.smem_len = sdrm->fb_size; > > + fbi->apertures->ranges[0].base = sdrm->fb_base; > + fbi->apertures->ranges[0].size = sdrm->fb_size; > + > return 0; > > err_fbi_release: > @@ -188,9 +206,13 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sdrm) > sdrm->fb_helper = NULL; > fbdev = to_sdrm_fbdev(fb_helper); > > - drm_fb_helper_unregister_fbi(fb_helper); > + /* it might have been kicked out */ > + if (registered_fb[fbdev->fb_helper.fbdev->node]) > + drm_fb_helper_unregister_fbi(fb_helper); > + > + /* freeing fb_info is done in fb_ops.fb_destroy() */ > + > cancel_work_sync(&fb_helper->dirty_work); > - drm_fb_helper_release_fbi(fb_helper); > > drm_framebuffer_unregister_private(fb_helper->fb); > drm_framebuffer_cleanup(fb_helper->fb); > @@ -199,3 +221,39 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sdrm) > drm_fb_helper_fini(fb_helper); > kfree(fbdev); > } > + > +static int sdrm_fbdev_event_notify(struct notifier_block *self, > + unsigned long action, void *data) > +{ > + struct sdrm_device *sdrm; > + struct fb_event *event = data; > + struct fb_info *info = event->info; > + struct drm_fb_helper *fb_helper = info->par; > + > + if (action != FB_EVENT_FB_UNREGISTERED) > + return NOTIFY_DONE; > + > + if (!fb_helper || !fb_helper->dev || fb_helper->fbdev != info) > + return NOTIFY_DONE; > + > + sdrm = fb_helper->dev->dev_private; > + > + if (sdrm && sdrm->fb_helper == fb_helper) > + platform_device_del(to_platform_device(fb_helper->dev->dev)); > + > + return NOTIFY_DONE; > +} One problem this leaves behind is that registering of the new fbdev driver is too late - by that point we've already set up the entire driver, including modeset. If fbdev meanwhile does a dpms off or something like that all hell will break loose. I think the only option is to add a new notifier chain for fbdev removal, called from remove_conflicting_framebuffers (even for CONFIG_FB=n, so need a fallback in core/fb_notify.c like with the other notifier I think). That would at least keep things working if fbdev is entirely disabled. Or have I entirely misunderstood how this works? Oh and another one, which we learned the hard way with i915: We probably need to get rid of vgacon too. And again we need to do that _before_ we start touching the hardware. I think simpledrm probably needs that too. But since simpledrm doesn't know on which pci device it sits it probably won't know if the same device is decoding vga signals, so no one knows how that is supposed to work :( One thing at the time probably. -Daniel > + > +static struct notifier_block sdrm_fbdev_event_notifier = { > + .notifier_call = sdrm_fbdev_event_notify, > +}; > + > +void sdrm_fbdev_kickout_init(void) > +{ > + fb_register_client(&sdrm_fbdev_event_notifier); > +} > + > +void sdrm_fbdev_kickout_exit(void) > +{ > + fb_unregister_client(&sdrm_fbdev_event_notifier); > +} > -- > 2.8.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Den 23.08.2016 14:41, skrev Daniel Vetter: > On Mon, Aug 22, 2016 at 10:25:25PM +0200, Noralf Trønnes wrote: >> There is currently no non-fbdev mechanism in place to kick out >> simpledrm when the real hw-driver is probed. As a stop gap until >> that is in place, honour remove_conflicting_framebuffers() and >> delete the simple-framebuffer platform device when it's called. >> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> --- <snip> >> diff --git a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c >> index c6596ad..7c6db2c 100644 >> --- a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c >> +++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c >> @@ -44,6 +44,20 @@ static int sdrm_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) >> return -ENODEV; >> } >> >> +/* >> + * Releasing has to be done outside the notifier callchain when we're >> + * kicked out, since do_unregister_framebuffer() calls put_fb_info() >> + * after the notifier has run. >> + * Open code drm_fb_helper_release_fbi() since fb_helper is freed at >> + * this point when kicked out. >> + */ >> +static void sdrm_fbdev_fb_destroy(struct fb_info *info) >> +{ >> + if (info->cmap.len) >> + fb_dealloc_cmap(&info->cmap); >> + framebuffer_release(info); >> +} >> + >> static struct fb_ops sdrm_fbdev_ops = { >> .owner = THIS_MODULE, >> .fb_fillrect = drm_fb_helper_sys_fillrect, >> @@ -53,6 +67,7 @@ static struct fb_ops sdrm_fbdev_ops = { >> .fb_set_par = drm_fb_helper_set_par, >> .fb_setcmap = drm_fb_helper_setcmap, >> .fb_mmap = sdrm_fb_mmap, >> + .fb_destroy = sdrm_fbdev_fb_destroy, >> }; >> >> static int sdrm_fbdev_create(struct drm_fb_helper *helper, >> @@ -110,6 +125,9 @@ static int sdrm_fbdev_create(struct drm_fb_helper *helper, >> fbi->screen_base = obj->vmapping; >> fbi->fix.smem_len = sdrm->fb_size; >> >> + fbi->apertures->ranges[0].base = sdrm->fb_base; >> + fbi->apertures->ranges[0].size = sdrm->fb_size; >> + >> return 0; >> >> err_fbi_release: >> @@ -188,9 +206,13 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sdrm) >> sdrm->fb_helper = NULL; >> fbdev = to_sdrm_fbdev(fb_helper); >> >> - drm_fb_helper_unregister_fbi(fb_helper); >> + /* it might have been kicked out */ >> + if (registered_fb[fbdev->fb_helper.fbdev->node]) >> + drm_fb_helper_unregister_fbi(fb_helper); >> + >> + /* freeing fb_info is done in fb_ops.fb_destroy() */ >> + >> cancel_work_sync(&fb_helper->dirty_work); >> - drm_fb_helper_release_fbi(fb_helper); >> >> drm_framebuffer_unregister_private(fb_helper->fb); >> drm_framebuffer_cleanup(fb_helper->fb); >> @@ -199,3 +221,39 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sdrm) >> drm_fb_helper_fini(fb_helper); >> kfree(fbdev); >> } >> + >> +static int sdrm_fbdev_event_notify(struct notifier_block *self, >> + unsigned long action, void *data) >> +{ >> + struct sdrm_device *sdrm; >> + struct fb_event *event = data; >> + struct fb_info *info = event->info; >> + struct drm_fb_helper *fb_helper = info->par; >> + >> + if (action != FB_EVENT_FB_UNREGISTERED) >> + return NOTIFY_DONE; >> + >> + if (!fb_helper || !fb_helper->dev || fb_helper->fbdev != info) >> + return NOTIFY_DONE; >> + >> + sdrm = fb_helper->dev->dev_private; >> + >> + if (sdrm && sdrm->fb_helper == fb_helper) >> + platform_device_del(to_platform_device(fb_helper->dev->dev)); >> + >> + return NOTIFY_DONE; >> +} > One problem this leaves behind is that registering of the new fbdev driver > is too late - by that point we've already set up the entire driver, > including modeset. If fbdev meanwhile does a dpms off or something like > that all hell will break loose. I don't understand how fbdev registration comes into play here. Drivers call remove_conflicting_framebuffers very early so simpledrm is gone by the time they register anything. For simpledrm, fbdev doing blank/unblank is a no-op since fb_ops.fb_blank is not implemented. So a fb_blank() just results in fbcon doing a software blank. > I think the only option is to add a new notifier chain for fbdev removal, > called from remove_conflicting_framebuffers (even for CONFIG_FB=n, so need > a fallback in core/fb_notify.c like with the other notifier I think). That > would at least keep things working if fbdev is entirely disabled. Or have > I entirely misunderstood how this works? How about extending the new wrapper you just added. Something like this: static int find_simpledrm_cb(struct device *dev, void *data) { struct platform_device *pdev = to_platform_device(dev); char *name = data; DRM_INFO("Switching to %s from simpledrm\n", name); platform_device_del(pdev); return 0; } /* not sure where this function should go */ void drm_remove_simpledrm(const char *name) { struct device_driver *drv; drv = driver_find("simpledrm", &platform_bus_type); if (drv) driver_for_each_device(drv, NULL, name, find_simpledrm_cb); /* or we can just unregister the driver instead */ if (drv) { struct platform_driver *pdrv = to_platform_driver(drv); DRM_INFO("simpledrm is being removed by %s\n", name); platform_driver_unregister(pdrv); } } static inline int drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a, const char *name, bool primary) { int ret; ret = remove_conflicting_framebuffers(a, name, primary); drm_remove_simpledrm(name); return ret; } #else static inline int drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a, const char *name, bool primary) { drm_remove_simpledrm(name); return 0; } This removes simpledrm unconditionally without looking at apertures_struct though. Not sure if that is needed? Noralf. > Oh and another one, which we learned the hard way with i915: We probably > need to get rid of vgacon too. And again we need to do that _before_ we > start touching the hardware. I think simpledrm probably needs that too. > But since simpledrm doesn't know on which pci device it sits it probably > won't know if the same device is decoding vga signals, so no one knows how > that is supposed to work :( One thing at the time probably. > -Daniel > >> + >> +static struct notifier_block sdrm_fbdev_event_notifier = { >> + .notifier_call = sdrm_fbdev_event_notify, >> +}; >> + >> +void sdrm_fbdev_kickout_init(void) >> +{ >> + fb_register_client(&sdrm_fbdev_event_notifier); >> +} >> + >> +void sdrm_fbdev_kickout_exit(void) >> +{ >> + fb_unregister_client(&sdrm_fbdev_event_notifier); >> +} >> -- >> 2.8.2 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Aug 23, 2016 at 7:52 PM, Noralf Trønnes <noralf@tronnes.org> wrote: >>> +static int sdrm_fbdev_event_notify(struct notifier_block *self, >>> + unsigned long action, void *data) >>> +{ >>> + struct sdrm_device *sdrm; >>> + struct fb_event *event = data; >>> + struct fb_info *info = event->info; >>> + struct drm_fb_helper *fb_helper = info->par; >>> + >>> + if (action != FB_EVENT_FB_UNREGISTERED) >>> + return NOTIFY_DONE; >>> + >>> + if (!fb_helper || !fb_helper->dev || fb_helper->fbdev != info) >>> + return NOTIFY_DONE; >>> + >>> + sdrm = fb_helper->dev->dev_private; >>> + >>> + if (sdrm && sdrm->fb_helper == fb_helper) >>> + >>> platform_device_del(to_platform_device(fb_helper->dev->dev)); >>> + >>> + return NOTIFY_DONE; >>> +} >> >> One problem this leaves behind is that registering of the new fbdev driver >> is too late - by that point we've already set up the entire driver, >> including modeset. If fbdev meanwhile does a dpms off or something like >> that all hell will break loose. > > > I don't understand how fbdev registration comes into play here. Drivers call > remove_conflicting_framebuffers very early so simpledrm is gone by the time > they register anything. > > For simpledrm, fbdev doing blank/unblank is a no-op since fb_ops.fb_blank > is not implemented. So a fb_blank() just results in fbcon doing a > software blank. Maybe my scenario wasn't entirely clear: - prereq: fbdev emulation in drm is disabled 1. simpledrm loads and sets up the firmware fb 2. real driver loads, first calls drm_fb_helper_remove_conflicting_framebuffer. Nothing happens because CONFIG_FB=n. 3. real driver start loading, remapping the gart and what not else 4. something is drawn using fbcon, simplerdrm writes through the now invalid mapping -> BOOM You have code to listen to the framebuffer registration notifier, but I think even that happens way too late. Or at least I didn't spot any code in remove_conflicting_framebuffers which would call down into that notifier. Or maybe I entirely misunderstand your code ... Wrt fixing: Just adding it to the recently added stub is of course also a working solution. -Daniel PS: Can you pls review the 2 patches I submitted with you on cc? I won't merge my own patches without proper review, so without that done they're stuck.
Den 23.08.2016 20:01, skrev Daniel Vetter: > On Tue, Aug 23, 2016 at 7:52 PM, Noralf Trønnes <noralf@tronnes.org> wrote: >>>> +static int sdrm_fbdev_event_notify(struct notifier_block *self, >>>> + unsigned long action, void *data) >>>> +{ >>>> + struct sdrm_device *sdrm; >>>> + struct fb_event *event = data; >>>> + struct fb_info *info = event->info; >>>> + struct drm_fb_helper *fb_helper = info->par; >>>> + >>>> + if (action != FB_EVENT_FB_UNREGISTERED) >>>> + return NOTIFY_DONE; >>>> + >>>> + if (!fb_helper || !fb_helper->dev || fb_helper->fbdev != info) >>>> + return NOTIFY_DONE; >>>> + >>>> + sdrm = fb_helper->dev->dev_private; >>>> + >>>> + if (sdrm && sdrm->fb_helper == fb_helper) >>>> + >>>> platform_device_del(to_platform_device(fb_helper->dev->dev)); >>>> + >>>> + return NOTIFY_DONE; >>>> +} >>> One problem this leaves behind is that registering of the new fbdev driver >>> is too late - by that point we've already set up the entire driver, >>> including modeset. If fbdev meanwhile does a dpms off or something like >>> that all hell will break loose. >> >> I don't understand how fbdev registration comes into play here. Drivers call >> remove_conflicting_framebuffers very early so simpledrm is gone by the time >> they register anything. >> >> For simpledrm, fbdev doing blank/unblank is a no-op since fb_ops.fb_blank >> is not implemented. So a fb_blank() just results in fbcon doing a >> software blank. > Maybe my scenario wasn't entirely clear: > - prereq: fbdev emulation in drm is disabled > 1. simpledrm loads and sets up the firmware fb > 2. real driver loads, first calls > drm_fb_helper_remove_conflicting_framebuffer. Nothing happens because > CONFIG_FB=n. > 3. real driver start loading, remapping the gart and what not else > 4. something is drawn using fbcon, simplerdrm writes through the now > invalid mapping > -> BOOM Yes CONFIG_FB=n is not covered, that's the drawback mentioned in the Kconfig. However, who uses simpledrm without fbdev/fbcon when it shall be handed over to a real hw-driver? But yes, it's not a good stop gap solution, I like my other idea much better. > You have code to listen to the framebuffer registration notifier, but > I think even that happens way too late. Or at least I didn't spot any > code in remove_conflicting_framebuffers which would call down into > that notifier. Or maybe I entirely misunderstand your code ... remove_conflicting_framebuffers unregisters the fbdev framebuffer. sdrm_fbdev_event_notify detects that the framebuffer is being unregistered, and deletes the platform device. Some details: do_remove_conflicting_framebuffers(): for (i = 0 ; i < FB_MAX; i++) { [...] printk(KERN_INFO "fb: switching to %s from %s\n", name, registered_fb[i]->fix.id); ret = do_unregister_framebuffer(registered_fb[i]); do_unregister_framebuffer(): short version { console_lock(); event.info = fb_info; ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event); unlock_fb_info(fb_info); console_unlock(); pm_vt_switch_unregister(fb_info->dev); unlink_framebuffer(fb_info); registered_fb[i] = NULL; num_registered_fb--; event.info = fb_info; console_lock(); fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event); /* at this point simpledrm has been deleted */ console_unlock(); /* this may free fb info */ put_fb_info(fb_info); return 0; } > Wrt fixing: Just adding it to the recently added stub is of course > also a working solution. I actually like this better, because it's so straightforward and easy to understand. The notifier solution is very convoluted and easy to mess up. And it runs with the console lock held... > -Daniel > > PS: Can you pls review the 2 patches I submitted with you on cc? I > won't merge my own patches without proper review, so without that done > they're stuck. Ok. I'll test the simple-helper plane patch tomorrow. In simpledrm I first tried to send the vblank event only in pipe.update(), but I had to do it in enable() and disable() as well to make that vblank timeout go away. Noralf.
diff --git a/drivers/gpu/drm/simpledrm/Kconfig b/drivers/gpu/drm/simpledrm/Kconfig index 3257590..4275d13 100644 --- a/drivers/gpu/drm/simpledrm/Kconfig +++ b/drivers/gpu/drm/simpledrm/Kconfig @@ -16,6 +16,11 @@ config DRM_SIMPLEDRM If fbdev support is enabled, this driver will also provide an fbdev compatibility layer that supports fbcon, mmap is not supported. + WARNING + fbdev must be enabled for simpledrm to disable itself when a real + hw-driver is probed. It relies on remove_conflicting_framebuffers() + to be called by the hw-driver. + If unsure, say Y. To compile this driver as a module, choose M here: the diff --git a/drivers/gpu/drm/simpledrm/simpledrm.h b/drivers/gpu/drm/simpledrm/simpledrm.h index d4eb52c..3cca196 100644 --- a/drivers/gpu/drm/simpledrm/simpledrm.h +++ b/drivers/gpu/drm/simpledrm/simpledrm.h @@ -87,5 +87,7 @@ int sdrm_fb_init(struct drm_device *ddev, struct sdrm_framebuffer *fb, struct sdrm_gem_object *obj); void sdrm_fbdev_init(struct sdrm_device *sdrm); void sdrm_fbdev_cleanup(struct sdrm_device *sdrm); +void sdrm_fbdev_kickout_init(void); +void sdrm_fbdev_kickout_exit(void); #endif /* SDRM_DRV_H */ diff --git a/drivers/gpu/drm/simpledrm/simpledrm_drv.c b/drivers/gpu/drm/simpledrm/simpledrm_drv.c index fe752c6..0750652 100644 --- a/drivers/gpu/drm/simpledrm/simpledrm_drv.c +++ b/drivers/gpu/drm/simpledrm/simpledrm_drv.c @@ -531,12 +531,15 @@ static int __init sdrm_init(void) } } + sdrm_fbdev_kickout_init(); + return 0; } module_init(sdrm_init); static void __exit sdrm_exit(void) { + sdrm_fbdev_kickout_exit(); platform_driver_unregister(&sdrm_simplefb_driver); } module_exit(sdrm_exit); diff --git a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c index c6596ad..7c6db2c 100644 --- a/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c +++ b/drivers/gpu/drm/simpledrm/simpledrm_fbdev.c @@ -44,6 +44,20 @@ static int sdrm_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) return -ENODEV; } +/* + * Releasing has to be done outside the notifier callchain when we're + * kicked out, since do_unregister_framebuffer() calls put_fb_info() + * after the notifier has run. + * Open code drm_fb_helper_release_fbi() since fb_helper is freed at + * this point when kicked out. + */ +static void sdrm_fbdev_fb_destroy(struct fb_info *info) +{ + if (info->cmap.len) + fb_dealloc_cmap(&info->cmap); + framebuffer_release(info); +} + static struct fb_ops sdrm_fbdev_ops = { .owner = THIS_MODULE, .fb_fillrect = drm_fb_helper_sys_fillrect, @@ -53,6 +67,7 @@ static struct fb_ops sdrm_fbdev_ops = { .fb_set_par = drm_fb_helper_set_par, .fb_setcmap = drm_fb_helper_setcmap, .fb_mmap = sdrm_fb_mmap, + .fb_destroy = sdrm_fbdev_fb_destroy, }; static int sdrm_fbdev_create(struct drm_fb_helper *helper, @@ -110,6 +125,9 @@ static int sdrm_fbdev_create(struct drm_fb_helper *helper, fbi->screen_base = obj->vmapping; fbi->fix.smem_len = sdrm->fb_size; + fbi->apertures->ranges[0].base = sdrm->fb_base; + fbi->apertures->ranges[0].size = sdrm->fb_size; + return 0; err_fbi_release: @@ -188,9 +206,13 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sdrm) sdrm->fb_helper = NULL; fbdev = to_sdrm_fbdev(fb_helper); - drm_fb_helper_unregister_fbi(fb_helper); + /* it might have been kicked out */ + if (registered_fb[fbdev->fb_helper.fbdev->node]) + drm_fb_helper_unregister_fbi(fb_helper); + + /* freeing fb_info is done in fb_ops.fb_destroy() */ + cancel_work_sync(&fb_helper->dirty_work); - drm_fb_helper_release_fbi(fb_helper); drm_framebuffer_unregister_private(fb_helper->fb); drm_framebuffer_cleanup(fb_helper->fb); @@ -199,3 +221,39 @@ void sdrm_fbdev_cleanup(struct sdrm_device *sdrm) drm_fb_helper_fini(fb_helper); kfree(fbdev); } + +static int sdrm_fbdev_event_notify(struct notifier_block *self, + unsigned long action, void *data) +{ + struct sdrm_device *sdrm; + struct fb_event *event = data; + struct fb_info *info = event->info; + struct drm_fb_helper *fb_helper = info->par; + + if (action != FB_EVENT_FB_UNREGISTERED) + return NOTIFY_DONE; + + if (!fb_helper || !fb_helper->dev || fb_helper->fbdev != info) + return NOTIFY_DONE; + + sdrm = fb_helper->dev->dev_private; + + if (sdrm && sdrm->fb_helper == fb_helper) + platform_device_del(to_platform_device(fb_helper->dev->dev)); + + return NOTIFY_DONE; +} + +static struct notifier_block sdrm_fbdev_event_notifier = { + .notifier_call = sdrm_fbdev_event_notify, +}; + +void sdrm_fbdev_kickout_init(void) +{ + fb_register_client(&sdrm_fbdev_event_notifier); +} + +void sdrm_fbdev_kickout_exit(void) +{ + fb_unregister_client(&sdrm_fbdev_event_notifier); +}
There is currently no non-fbdev mechanism in place to kick out simpledrm when the real hw-driver is probed. As a stop gap until that is in place, honour remove_conflicting_framebuffers() and delete the simple-framebuffer platform device when it's called. Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- Changes from version 3: - drm_device.platformdev is deprecated, use to_platform_device(ddev->dev). - fb_helper might have been released in sdrm_fbdev_fb_destroy(), so open code drm_fb_helper_release_fbi() - Strengthen the test in sdrm_fbdev_event_notify() that we're the one. Changes from version 2: - Don't forget to free fb_info when kicked out. drivers/gpu/drm/simpledrm/Kconfig | 5 +++ drivers/gpu/drm/simpledrm/simpledrm.h | 2 + drivers/gpu/drm/simpledrm/simpledrm_drv.c | 3 ++ drivers/gpu/drm/simpledrm/simpledrm_fbdev.c | 62 ++++++++++++++++++++++++++++- 4 files changed, 70 insertions(+), 2 deletions(-) -- 2.8.2