| Message ID | 20180414115318.14500-22-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Apr 14, 2018 at 01:53:14PM +0200, Noralf Trønnes wrote: > These helpers keep track of fbdev users and drm_driver.last_close will > only restore fbdev when actually in use. Additionally the display is > turned off when the last user is closing. fbcon is a user in this context. > > If struct fb_ops is defined in a library, fb_open() takes a ref on the > library (fb_ops.owner) instead of the driver module. Fix that by ensuring > that the driver module is pinned. > > The functions are not added to the DRM_FB_HELPER_DEFAULT_OPS() macro, > because some of its users do set fb_open/release themselves. > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > drivers/gpu/drm/drm_fb_helper.c | 54 ++++++++++++++++++++++++++++++++++++++++- > include/drm/drm_fb_helper.h | 29 ++++++++++++++++++++++ > 2 files changed, 82 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 98e5bc92c9f2..b1124c08b1ed 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -177,7 +177,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper) > if (!drm_fbdev_emulation || !fb_helper) > return -ENODEV; > > - if (READ_ONCE(fb_helper->deferred_setup)) > + if (READ_ONCE(fb_helper->deferred_setup) || !READ_ONCE(fb_helper->open_count)) > return 0; > > mutex_lock(&fb_helper->lock); > @@ -368,6 +368,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, > INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work); > helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; > mutex_init(&helper->lock); > + helper->open_count = 1; > helper->funcs = funcs; > helper->dev = dev; > } > @@ -620,6 +621,53 @@ int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper) > } > EXPORT_SYMBOL(drm_fb_helper_defio_init); > > +/** > + * drm_fb_helper_fb_open - implementation for &fb_ops.fb_open > + * @info: fbdev registered by the helper > + * @user: 1=userspace, 0=fbcon > + * > + * Increase fbdev use count. > + * If &fb_ops is wrapped in a library, pin the driver module. > + */ > +int drm_fb_helper_fb_open(struct fb_info *info, int user) > +{ > + struct drm_fb_helper *fb_helper = info->par; > + struct drm_device *dev = fb_helper->dev; > + > + if (info->fbops->owner != dev->driver->fops->owner) { > + if (!try_module_get(dev->driver->fops->owner)) > + return -ENODEV; > + } > + > + fb_helper->open_count++; > + > + return 0; > +} > +EXPORT_SYMBOL(drm_fb_helper_fb_open); > + > +/** > + * drm_fb_helper_fb_release - implementation for &fb_ops.fb_release > + * @info: fbdev registered by the helper > + * @user: 1=userspace, 0=fbcon > + * > + * Decrease fbdev use count and turn off if there are no users left. > + * If &fb_ops is wrapped in a library, unpin the driver module. > + */ > +int drm_fb_helper_fb_release(struct fb_info *info, int user) > +{ > + struct drm_fb_helper *fb_helper = info->par; > + struct drm_device *dev = fb_helper->dev; > + > + if (!(--fb_helper->open_count)) > + drm_fb_helper_dpms(info, DRM_MODE_DPMS_OFF); > + > + if (info->fbops->owner != dev->driver->fops->owner) > + module_put(dev->driver->fops->owner); > + > + return 0; > +} > +EXPORT_SYMBOL(drm_fb_helper_fb_release); > + > /** > * drm_fb_helper_sys_read - wrapper around fb_sys_read > * @info: fb_info struct pointer > @@ -1436,6 +1484,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, > if (ret < 0) > return ret; > > + /* Block restore without users if we do track it */ > + if (fb_helper->fbdev->fbops->fb_open == drm_fb_helper_fb_open) > + fb_helper->open_count = 0; Since we kzcalloc, isn't this entirely redundant? Looks confusing to me at least. Otherwise looks all reasonable. -Daniel > + > strcpy(fb_helper->fb->comm, "[fbcon]"); > return 0; > } > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h > index 5f66f253a97b..330983975d5e 100644 > --- a/include/drm/drm_fb_helper.h > +++ b/include/drm/drm_fb_helper.h > @@ -184,6 +184,22 @@ struct drm_fb_helper { > * See also: @deferred_setup > */ > int preferred_bpp; > + > + /** > + * @open_count: > + * > + * Keeps track of fbdev use to know when to not restore fbdev and to > + * disable the pipeline when the last user is gone. > + * > + * Drivers that use drm_fb_helper_fb_open() as their \.fb_open > + * callback will get an initial value of 0 and get restore based on > + * actual use. Others will get an initial value of 1 which means that > + * fbdev will always be restored. Drivers that call > + * drm_fb_helper_fb_open() in their \.fb_open, thus needs to set the > + * initial value to 0 themselves in their &drm_fb_helper_funcs->fb_probe > + * callback. > + */ > + unsigned int open_count; > }; > > /** > @@ -230,6 +246,9 @@ void drm_fb_helper_deferred_io(struct fb_info *info, > struct list_head *pagelist); > int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper); > > +int drm_fb_helper_fb_open(struct fb_info *info, int user); > +int drm_fb_helper_fb_release(struct fb_info *info, int user); > + > ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, > size_t count, loff_t *ppos); > ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, > @@ -376,6 +395,16 @@ static inline int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper) > return -ENODEV; > } > > +static inline int drm_fb_helper_fb_open(struct fb_info *info, int user) > +{ > + return -ENODEV; > +} > + > +static inline int drm_fb_helper_fb_release(struct fb_info *info, int user) > +{ > + return -ENODEV; > +} > + > static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info, > char __user *buf, size_t count, > loff_t *ppos) > -- > 2.15.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Den 16.04.2018 10.46, skrev Daniel Vetter: > On Sat, Apr 14, 2018 at 01:53:14PM +0200, Noralf Trønnes wrote: >> These helpers keep track of fbdev users and drm_driver.last_close will >> only restore fbdev when actually in use. Additionally the display is >> turned off when the last user is closing. fbcon is a user in this context. >> >> If struct fb_ops is defined in a library, fb_open() takes a ref on the >> library (fb_ops.owner) instead of the driver module. Fix that by ensuring >> that the driver module is pinned. >> >> The functions are not added to the DRM_FB_HELPER_DEFAULT_OPS() macro, >> because some of its users do set fb_open/release themselves. >> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> --- >> drivers/gpu/drm/drm_fb_helper.c | 54 ++++++++++++++++++++++++++++++++++++++++- >> include/drm/drm_fb_helper.h | 29 ++++++++++++++++++++++ >> 2 files changed, 82 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >> index 98e5bc92c9f2..b1124c08b1ed 100644 >> --- a/drivers/gpu/drm/drm_fb_helper.c >> +++ b/drivers/gpu/drm/drm_fb_helper.c >> @@ -177,7 +177,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper) >> if (!drm_fbdev_emulation || !fb_helper) >> return -ENODEV; >> >> - if (READ_ONCE(fb_helper->deferred_setup)) >> + if (READ_ONCE(fb_helper->deferred_setup) || !READ_ONCE(fb_helper->open_count)) >> return 0; >> >> mutex_lock(&fb_helper->lock); >> @@ -368,6 +368,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, >> INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work); >> helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; >> mutex_init(&helper->lock); >> + helper->open_count = 1; >> helper->funcs = funcs; >> helper->dev = dev; >> } >> @@ -620,6 +621,53 @@ int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper) >> } >> EXPORT_SYMBOL(drm_fb_helper_defio_init); >> >> +/** >> + * drm_fb_helper_fb_open - implementation for &fb_ops.fb_open >> + * @info: fbdev registered by the helper >> + * @user: 1=userspace, 0=fbcon >> + * >> + * Increase fbdev use count. >> + * If &fb_ops is wrapped in a library, pin the driver module. >> + */ >> +int drm_fb_helper_fb_open(struct fb_info *info, int user) >> +{ >> + struct drm_fb_helper *fb_helper = info->par; >> + struct drm_device *dev = fb_helper->dev; >> + >> + if (info->fbops->owner != dev->driver->fops->owner) { >> + if (!try_module_get(dev->driver->fops->owner)) >> + return -ENODEV; >> + } >> + >> + fb_helper->open_count++; >> + >> + return 0; >> +} >> +EXPORT_SYMBOL(drm_fb_helper_fb_open); >> + >> +/** >> + * drm_fb_helper_fb_release - implementation for &fb_ops.fb_release >> + * @info: fbdev registered by the helper >> + * @user: 1=userspace, 0=fbcon >> + * >> + * Decrease fbdev use count and turn off if there are no users left. >> + * If &fb_ops is wrapped in a library, unpin the driver module. >> + */ >> +int drm_fb_helper_fb_release(struct fb_info *info, int user) >> +{ >> + struct drm_fb_helper *fb_helper = info->par; >> + struct drm_device *dev = fb_helper->dev; >> + >> + if (!(--fb_helper->open_count)) >> + drm_fb_helper_dpms(info, DRM_MODE_DPMS_OFF); >> + >> + if (info->fbops->owner != dev->driver->fops->owner) >> + module_put(dev->driver->fops->owner); >> + >> + return 0; >> +} >> +EXPORT_SYMBOL(drm_fb_helper_fb_release); >> + >> /** >> * drm_fb_helper_sys_read - wrapper around fb_sys_read >> * @info: fb_info struct pointer >> @@ -1436,6 +1484,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, >> if (ret < 0) >> return ret; >> >> + /* Block restore without users if we do track it */ >> + if (fb_helper->fbdev->fbops->fb_open == drm_fb_helper_fb_open) >> + fb_helper->open_count = 0; > Since we kzcalloc, isn't this entirely redundant? Looks confusing to me at > least. I have to keep the existing restore behaviour for those that don't use drm_fb_helper_fb_open(). I do this by setting fb_helper->open_count = 1 in drm_fb_helper_prepare(). I have tried to give a describtion in the @open_count docs. Ofc I could have changed all drivers to use drm_fb_helper_fb_open(), but I think that time is better spent moving drivers over to generic fbdev instead. Noralf. > Otherwise looks all reasonable. > -Daniel > > >> + >> strcpy(fb_helper->fb->comm, "[fbcon]"); >> return 0; >> } >> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h >> index 5f66f253a97b..330983975d5e 100644 >> --- a/include/drm/drm_fb_helper.h >> +++ b/include/drm/drm_fb_helper.h >> @@ -184,6 +184,22 @@ struct drm_fb_helper { >> * See also: @deferred_setup >> */ >> int preferred_bpp; >> + >> + /** >> + * @open_count: >> + * >> + * Keeps track of fbdev use to know when to not restore fbdev and to >> + * disable the pipeline when the last user is gone. >> + * >> + * Drivers that use drm_fb_helper_fb_open() as their \.fb_open >> + * callback will get an initial value of 0 and get restore based on >> + * actual use. Others will get an initial value of 1 which means that >> + * fbdev will always be restored. Drivers that call >> + * drm_fb_helper_fb_open() in their \.fb_open, thus needs to set the >> + * initial value to 0 themselves in their &drm_fb_helper_funcs->fb_probe >> + * callback. >> + */ >> + unsigned int open_count; >> }; >> >> /** >> @@ -230,6 +246,9 @@ void drm_fb_helper_deferred_io(struct fb_info *info, >> struct list_head *pagelist); >> int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper); >> >> +int drm_fb_helper_fb_open(struct fb_info *info, int user); >> +int drm_fb_helper_fb_release(struct fb_info *info, int user); >> + >> ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, >> size_t count, loff_t *ppos); >> ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, >> @@ -376,6 +395,16 @@ static inline int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper) >> return -ENODEV; >> } >> >> +static inline int drm_fb_helper_fb_open(struct fb_info *info, int user) >> +{ >> + return -ENODEV; >> +} >> + >> +static inline int drm_fb_helper_fb_release(struct fb_info *info, int user) >> +{ >> + return -ENODEV; >> +} >> + >> static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info, >> char __user *buf, size_t count, >> loff_t *ppos) >> -- >> 2.15.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Apr 16, 2018 at 06:10:07PM +0200, Noralf Trønnes wrote: > > Den 16.04.2018 10.46, skrev Daniel Vetter: > > On Sat, Apr 14, 2018 at 01:53:14PM +0200, Noralf Trønnes wrote: > > > These helpers keep track of fbdev users and drm_driver.last_close will > > > only restore fbdev when actually in use. Additionally the display is > > > turned off when the last user is closing. fbcon is a user in this context. > > > > > > If struct fb_ops is defined in a library, fb_open() takes a ref on the > > > library (fb_ops.owner) instead of the driver module. Fix that by ensuring > > > that the driver module is pinned. > > > > > > The functions are not added to the DRM_FB_HELPER_DEFAULT_OPS() macro, > > > because some of its users do set fb_open/release themselves. > > > > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > > --- > > > drivers/gpu/drm/drm_fb_helper.c | 54 ++++++++++++++++++++++++++++++++++++++++- > > > include/drm/drm_fb_helper.h | 29 ++++++++++++++++++++++ > > > 2 files changed, 82 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > > > index 98e5bc92c9f2..b1124c08b1ed 100644 > > > --- a/drivers/gpu/drm/drm_fb_helper.c > > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > > @@ -177,7 +177,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper) > > > if (!drm_fbdev_emulation || !fb_helper) > > > return -ENODEV; > > > - if (READ_ONCE(fb_helper->deferred_setup)) > > > + if (READ_ONCE(fb_helper->deferred_setup) || !READ_ONCE(fb_helper->open_count)) > > > return 0; > > > mutex_lock(&fb_helper->lock); > > > @@ -368,6 +368,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, > > > INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work); > > > helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; > > > mutex_init(&helper->lock); > > > + helper->open_count = 1; > > > helper->funcs = funcs; > > > helper->dev = dev; > > > } > > > @@ -620,6 +621,53 @@ int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper) > > > } > > > EXPORT_SYMBOL(drm_fb_helper_defio_init); > > > +/** > > > + * drm_fb_helper_fb_open - implementation for &fb_ops.fb_open > > > + * @info: fbdev registered by the helper > > > + * @user: 1=userspace, 0=fbcon > > > + * > > > + * Increase fbdev use count. > > > + * If &fb_ops is wrapped in a library, pin the driver module. > > > + */ > > > +int drm_fb_helper_fb_open(struct fb_info *info, int user) > > > +{ > > > + struct drm_fb_helper *fb_helper = info->par; > > > + struct drm_device *dev = fb_helper->dev; > > > + > > > + if (info->fbops->owner != dev->driver->fops->owner) { > > > + if (!try_module_get(dev->driver->fops->owner)) > > > + return -ENODEV; > > > + } > > > + > > > + fb_helper->open_count++; > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL(drm_fb_helper_fb_open); > > > + > > > +/** > > > + * drm_fb_helper_fb_release - implementation for &fb_ops.fb_release > > > + * @info: fbdev registered by the helper > > > + * @user: 1=userspace, 0=fbcon > > > + * > > > + * Decrease fbdev use count and turn off if there are no users left. > > > + * If &fb_ops is wrapped in a library, unpin the driver module. > > > + */ > > > +int drm_fb_helper_fb_release(struct fb_info *info, int user) > > > +{ > > > + struct drm_fb_helper *fb_helper = info->par; > > > + struct drm_device *dev = fb_helper->dev; > > > + > > > + if (!(--fb_helper->open_count)) > > > + drm_fb_helper_dpms(info, DRM_MODE_DPMS_OFF); > > > + > > > + if (info->fbops->owner != dev->driver->fops->owner) > > > + module_put(dev->driver->fops->owner); > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL(drm_fb_helper_fb_release); > > > + > > > /** > > > * drm_fb_helper_sys_read - wrapper around fb_sys_read > > > * @info: fb_info struct pointer > > > @@ -1436,6 +1484,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, > > > if (ret < 0) > > > return ret; > > > + /* Block restore without users if we do track it */ > > > + if (fb_helper->fbdev->fbops->fb_open == drm_fb_helper_fb_open) > > > + fb_helper->open_count = 0; > > Since we kzcalloc, isn't this entirely redundant? Looks confusing to me at > > least. > > I have to keep the existing restore behaviour for those that don't use > drm_fb_helper_fb_open(). I do this by setting fb_helper->open_count = 1 > in drm_fb_helper_prepare(). I have tried to give a describtion in the > @open_count docs. Ah I missed that. I think having the fbops->fb_op == drm_fb_helper_fb_open check in the restore function itself, plus a big comment explaining what you're doing, would be clearer. So instead of always checking the open_count, only check it when this helper is in use. > Ofc I could have changed all drivers to use drm_fb_helper_fb_open(), but > I think that time is better spent moving drivers over to generic fbdev > instead. Yeah FIXME (plus maybe todo.rst entry, this is perfect starting task fodder) is totally good enough. -Daniel > > Noralf. > > > Otherwise looks all reasonable. > > -Daniel > > > > > > > + > > > strcpy(fb_helper->fb->comm, "[fbcon]"); > > > return 0; > > > } > > > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h > > > index 5f66f253a97b..330983975d5e 100644 > > > --- a/include/drm/drm_fb_helper.h > > > +++ b/include/drm/drm_fb_helper.h > > > @@ -184,6 +184,22 @@ struct drm_fb_helper { > > > * See also: @deferred_setup > > > */ > > > int preferred_bpp; > > > + > > > + /** > > > + * @open_count: > > > + * > > > + * Keeps track of fbdev use to know when to not restore fbdev and to > > > + * disable the pipeline when the last user is gone. > > > + * > > > + * Drivers that use drm_fb_helper_fb_open() as their \.fb_open > > > + * callback will get an initial value of 0 and get restore based on > > > + * actual use. Others will get an initial value of 1 which means that > > > + * fbdev will always be restored. Drivers that call > > > + * drm_fb_helper_fb_open() in their \.fb_open, thus needs to set the > > > + * initial value to 0 themselves in their &drm_fb_helper_funcs->fb_probe > > > + * callback. > > > + */ > > > + unsigned int open_count; > > > }; > > > /** > > > @@ -230,6 +246,9 @@ void drm_fb_helper_deferred_io(struct fb_info *info, > > > struct list_head *pagelist); > > > int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper); > > > +int drm_fb_helper_fb_open(struct fb_info *info, int user); > > > +int drm_fb_helper_fb_release(struct fb_info *info, int user); > > > + > > > ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, > > > size_t count, loff_t *ppos); > > > ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, > > > @@ -376,6 +395,16 @@ static inline int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper) > > > return -ENODEV; > > > } > > > +static inline int drm_fb_helper_fb_open(struct fb_info *info, int user) > > > +{ > > > + return -ENODEV; > > > +} > > > + > > > +static inline int drm_fb_helper_fb_release(struct fb_info *info, int user) > > > +{ > > > + return -ENODEV; > > > +} > > > + > > > static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info, > > > char __user *buf, size_t count, > > > loff_t *ppos) > > > -- > > > 2.15.1 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx >
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 98e5bc92c9f2..b1124c08b1ed 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -177,7 +177,7 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper) if (!drm_fbdev_emulation || !fb_helper) return -ENODEV; - if (READ_ONCE(fb_helper->deferred_setup)) + if (READ_ONCE(fb_helper->deferred_setup) || !READ_ONCE(fb_helper->open_count)) return 0; mutex_lock(&fb_helper->lock); @@ -368,6 +368,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work); helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; mutex_init(&helper->lock); + helper->open_count = 1; helper->funcs = funcs; helper->dev = dev; } @@ -620,6 +621,53 @@ int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper) } EXPORT_SYMBOL(drm_fb_helper_defio_init); +/** + * drm_fb_helper_fb_open - implementation for &fb_ops.fb_open + * @info: fbdev registered by the helper + * @user: 1=userspace, 0=fbcon + * + * Increase fbdev use count. + * If &fb_ops is wrapped in a library, pin the driver module. + */ +int drm_fb_helper_fb_open(struct fb_info *info, int user) +{ + struct drm_fb_helper *fb_helper = info->par; + struct drm_device *dev = fb_helper->dev; + + if (info->fbops->owner != dev->driver->fops->owner) { + if (!try_module_get(dev->driver->fops->owner)) + return -ENODEV; + } + + fb_helper->open_count++; + + return 0; +} +EXPORT_SYMBOL(drm_fb_helper_fb_open); + +/** + * drm_fb_helper_fb_release - implementation for &fb_ops.fb_release + * @info: fbdev registered by the helper + * @user: 1=userspace, 0=fbcon + * + * Decrease fbdev use count and turn off if there are no users left. + * If &fb_ops is wrapped in a library, unpin the driver module. + */ +int drm_fb_helper_fb_release(struct fb_info *info, int user) +{ + struct drm_fb_helper *fb_helper = info->par; + struct drm_device *dev = fb_helper->dev; + + if (!(--fb_helper->open_count)) + drm_fb_helper_dpms(info, DRM_MODE_DPMS_OFF); + + if (info->fbops->owner != dev->driver->fops->owner) + module_put(dev->driver->fops->owner); + + return 0; +} +EXPORT_SYMBOL(drm_fb_helper_fb_release); + /** * drm_fb_helper_sys_read - wrapper around fb_sys_read * @info: fb_info struct pointer @@ -1436,6 +1484,10 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, if (ret < 0) return ret; + /* Block restore without users if we do track it */ + if (fb_helper->fbdev->fbops->fb_open == drm_fb_helper_fb_open) + fb_helper->open_count = 0; + strcpy(fb_helper->fb->comm, "[fbcon]"); return 0; } diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 5f66f253a97b..330983975d5e 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -184,6 +184,22 @@ struct drm_fb_helper { * See also: @deferred_setup */ int preferred_bpp; + + /** + * @open_count: + * + * Keeps track of fbdev use to know when to not restore fbdev and to + * disable the pipeline when the last user is gone. + * + * Drivers that use drm_fb_helper_fb_open() as their \.fb_open + * callback will get an initial value of 0 and get restore based on + * actual use. Others will get an initial value of 1 which means that + * fbdev will always be restored. Drivers that call + * drm_fb_helper_fb_open() in their \.fb_open, thus needs to set the + * initial value to 0 themselves in their &drm_fb_helper_funcs->fb_probe + * callback. + */ + unsigned int open_count; }; /** @@ -230,6 +246,9 @@ void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head *pagelist); int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper); +int drm_fb_helper_fb_open(struct fb_info *info, int user); +int drm_fb_helper_fb_release(struct fb_info *info, int user); + ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, size_t count, loff_t *ppos); ssize_t drm_fb_helper_sys_write(struct fb_info *info, const char __user *buf, @@ -376,6 +395,16 @@ static inline int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper) return -ENODEV; } +static inline int drm_fb_helper_fb_open(struct fb_info *info, int user) +{ + return -ENODEV; +} + +static inline int drm_fb_helper_fb_release(struct fb_info *info, int user) +{ + return -ENODEV; +} + static inline ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf, size_t count, loff_t *ppos)
These helpers keep track of fbdev users and drm_driver.last_close will only restore fbdev when actually in use. Additionally the display is turned off when the last user is closing. fbcon is a user in this context. If struct fb_ops is defined in a library, fb_open() takes a ref on the library (fb_ops.owner) instead of the driver module. Fix that by ensuring that the driver module is pinned. The functions are not added to the DRM_FB_HELPER_DEFAULT_OPS() macro, because some of its users do set fb_open/release themselves. Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- drivers/gpu/drm/drm_fb_helper.c | 54 ++++++++++++++++++++++++++++++++++++++++- include/drm/drm_fb_helper.h | 29 ++++++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-)