On Wed, 25 Jul 2018, Bartlomiej Zolnierkiewicz wrote:
> > Can unregister_framebuffer() be called when /dev/fb0 is open as a file
> > handle and/or mapped to some process?
>
> It should be OK.
>
> > > Moreover the dlfb <-> fb_info locking scheme seems to be reversed
> > > (+racy) as it is dlfb that should control lifetime of fb_info, then
> > > in dlfb_free() we should just call framebuffer_release() etc.
> >
> > How should in your opinion framebuffer destruction work?
> >
> > Should the driver count the number of users and call
> > unregister_framebuffer() when it drops to zero?
> >
> > Or should the driver call unregister_framebuffer() unconditionally when
> > the device is unplugged and destroy the device in the "fb_destroy"
> > callback? (fb_destroy seems to be called by the framebuffer subsystem when
> > the open count reaches zero)
>
> The driver should call unregister_framebuffer() unconditionally in
> dlfb_usb_disconnect() (instead of calling unlink_framebuffer()).
I reworked the udlfb driver to call unregister_framebuffer unconditionally
and destroy the device from the fb_destroy method and it works very well.
I tried to unplug it at various times and it didn't result in any crashes
or warnings.
I'll send the patch in next email.
> Anyway it seems that this would require major reworking of the driver and
> I think that it would be better to put efforts into fixing udl-kms driver
> instead. For now I have queued your patch (with __unregister_framebuffer()
> change to keep the old behavior for non-udlfb drivers) for v4.19 (patch
> attached at the end of this mail).
Could someone describe what is the architecturely proper way to unload a
KMS driver?
udl_usb_disconnect calls these functions
drm_kms_helper_poll_disable(dev);
udl_fbdev_unplug(dev);
udl_drop_usb(dev);
drm_dev_unplug(dev);
- and if crashes if the device is unplugged while Xserver is running.
And it results in a warning "WARNING: CPU: 0 PID: 21685 at
drivers/gpu/drm/drm_mode_config.c:473 drm_mode_config_cleanup+0x294/0x2b8
[drm]" if the device is unplugged while only console is in use.
> > I tested matrox PCI framebuffer driver on an old computer - and it suffers
> > from the same problem as udlfb. The matrox driver keeps the open count and
> > destroys itself when the open count reaches zero - but the console that is
> > bound to the driver prevents the open count from reaching zero - so if I
> > unbind the PCI device in sysfs, it does nothing and the console is still
> > active and works.
>
> matroxfb is a not a good reference driver as it also defers the call to
> unregister_framebuffer() when the device is unplugged:
>
> static void matroxfb_remove(struct matrox_fb_info *minfo, int dummy)
> {
> ...
> minfo->dead = 1;
> if (minfo->usecount) {
> /* destroy it later */
> --> return;
> }
> matroxfb_unregister_device(minfo);
> unregister_framebuffer(&minfo->fbcon);
> ...
> }
I think that for PCI framebuffer drivers, there's no correct way how to
unload them correctly - so the framebuffer subsystem should prevent PCI
unbind.
If the user unbinds the device, then what?
- either you destroy the framebuffer immediatelly and you'll get crashes
if it is used by some programs
- or you delay destroying the framebuffer - but then, the unbound device
may be re-bound to a different instance of the driver (or a different
driver) and these two instances would clash accessing the videoram and
regsters simultaneously
BTW. only 5 framebuffer drivers currently use the fb_destroy - so most of
them are destroyed improperly.
Mikulas
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
