Hi Florian,
On Sun, 19 September 2010 Florian Tobias Schandinat <FlorianSchandinat@xxxxxx> wrote:Hi,
Bruno PrÃmont schrieb:For USB-attached (or other hot-(un)pluggable) framebuffers the currentI agree. This is a great idea even for non-hot-(un)pluggable framebuffers.
fbdev infrastructure is not very helpful. Each such driver currently
needs to perform the ref-counting on its own in .fbops.fb_open and
.fbops.fb_release callbacks.
Yes, things like drmfb and drivers of multi-head capable framebuffer
drivers would certainly appreciate as well, but they will probably also
want to care about users (of fb_info.screen_base).
If you have concerns regarding the API changes, please let me know.Uhm, I'm not really happy with what we count. With the old method you mentioned we ref-counted framebuffer users, after your patch it's more counting users + uses. This might be okay as we usually are interested whether the ref_count is 0 or not but it doesn't look right if we modify the refcount during nearly every framebuffer operation. Wouldn't it be sufficient to do the refcounting in fb_open & fb_release operation + in fbcon where open&release are done?
Well I'm more for counting the uses, (especially as the aim is to not
force the driver to look exactly when it can free the fb_info struct).
If the driver needs to know about active users (e.g. for handling memory
reorganization on mode change or the like) that would remain driver's job.
diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.cAs far as I know there exist framebuffer drivers which do not call framebuffer_alloc but contain their own fb_info. I guess these would be broken as well.
index 0a08f13..be5f342 100644
--- a/drivers/video/fbsysfs.c
+++ b/drivers/video/fbsysfs.c
@@ -58,6 +58,7 @@ struct fb_info *framebuffer_alloc(size_t size, struct device *dev)
info->par = p + fb_info_size;
info->device = dev;
+ kref_init(&info->refcount);
For those it would be better to switch them to be using framebuffer_alloc.