Re: [Intel-gfx] [RFC v4 21/25] drm/fb-helper: Add drm_fb_helper_fb_open/release()
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> > > --- > > > 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(_helper->lock); > > > @@ -368,6 +368,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, > > > struct drm_fb_helper *helper, > > > INIT_WORK(>dirty_work, drm_fb_helper_dirty_work); > > > helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; > > > mutex_init(>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 _ops.fb_open > > > + * @info: fbdev registered by the helper > > > + * @user: 1=userspace, 0=fbcon > > > + * > > > + * Increase fbdev use count. > > > + * If _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 _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 _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
Re: [Intel-gfx] [RFC v4 21/25] drm/fb-helper: Add drm_fb_helper_fb_open/release()
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--- 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(_helper->lock); @@ -368,6 +368,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, INIT_WORK(>dirty_work, drm_fb_helper_dirty_work); helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; mutex_init(>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 _ops.fb_open + * @info: fbdev registered by the helper + * @user: 1=userspace, 0=fbcon + * + * Increase fbdev use count. + * If _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 _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 _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
Re: [Intel-gfx] [RFC v4 21/25] drm/fb-helper: Add drm_fb_helper_fb_open/release()
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> --- > 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(_helper->lock); > @@ -368,6 +368,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct > drm_fb_helper *helper, > INIT_WORK(>dirty_work, drm_fb_helper_dirty_work); > helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; > mutex_init(>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 _ops.fb_open > + * @info: fbdev registered by the helper > + * @user: 1=userspace, 0=fbcon > + * > + * Increase fbdev use count. > + * If _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 _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 _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
[RFC v4 21/25] drm/fb-helper: Add drm_fb_helper_fb_open/release()
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--- 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(_helper->lock); @@ -368,6 +368,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, INIT_WORK(>dirty_work, drm_fb_helper_dirty_work); helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; mutex_init(>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 _ops.fb_open + * @info: fbdev registered by the helper + * @user: 1=userspace, 0=fbcon + * + * Increase fbdev use count. + * If _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 _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 _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 _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
[RFC v4 21/25] drm/fb-helper: Add drm_fb_helper_fb_open/release()
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--- 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(_helper->lock); @@ -368,6 +368,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, INIT_WORK(>dirty_work, drm_fb_helper_dirty_work); helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; mutex_init(>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 _ops.fb_open + * @info: fbdev registered by the helper + * @user: 1=userspace, 0=fbcon + * + * Increase fbdev use count. + * If _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 _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 _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 _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
[RFC v4 21/25] drm/fb-helper: Add drm_fb_helper_fb_open/release()
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--- 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(_helper->lock); @@ -368,6 +368,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, INIT_WORK(>dirty_work, drm_fb_helper_dirty_work); helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; mutex_init(>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 _ops.fb_open + * @info: fbdev registered by the helper + * @user: 1=userspace, 0=fbcon + * + * Increase fbdev use count. + * If _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 _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 _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 _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