| Message ID | 1471953246-29602-1-git-send-email-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 23, 2016 at 01:54:06PM +0200, Noralf Trønnes wrote: > This adds a function that also takes the console lock before calling > fb_set_suspend() in contrast to drm_fb_helper_set_suspend() which is > a plain wrapper around fb_set_suspend(). > Resume is run asynchronously using a worker if the console lock is > already taken. This is modelled after the i915 driver. > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > > Daniel, > > I haven't got time to convert drivers to use this and now that simpledrm > won't be using it we can wait with this if you prefer, and I'll bundle it > with tinydrm when the time comes. Well it's ready, and maybe someone else has a need for it already. Applied it already, thanks a lot for the quick respin. -Daniel > > Noralf. > > > Changes from version 1: > - Change function name to drm_fb_helper_set_suspend_unlocked() > - Use suspend as argument name to increase readability > > drivers/gpu/drm/drm_fb_helper.c | 62 ++++++++++++++++++++++++++++++++++++++++- > include/drm/drm_fb_helper.h | 9 ++++++ > 2 files changed, 70 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index ce54e98..2886df5 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -29,6 +29,7 @@ > */ > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > +#include <linux/console.h> > #include <linux/kernel.h> > #include <linux/sysrq.h> > #include <linux/slab.h> > @@ -618,6 +619,16 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) > kfree(helper->crtc_info); > } > > +static void drm_fb_helper_resume_worker(struct work_struct *work) > +{ > + struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper, > + resume_work); > + > + console_lock(); > + fb_set_suspend(helper->fbdev, 0); > + console_unlock(); > +} > + > static void drm_fb_helper_dirty_work(struct work_struct *work) > { > struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper, > @@ -649,6 +660,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, > { > INIT_LIST_HEAD(&helper->kernel_fb_list); > spin_lock_init(&helper->dirty_lock); > + INIT_WORK(&helper->resume_work, drm_fb_helper_resume_worker); > INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work); > helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; > helper->funcs = funcs; > @@ -1026,7 +1038,9 @@ EXPORT_SYMBOL(drm_fb_helper_cfb_imageblit); > * @fb_helper: driver-allocated fbdev helper > * @state: desired state, zero to resume, non-zero to suspend > * > - * A wrapper around fb_set_suspend implemented by fbdev core > + * A wrapper around fb_set_suspend implemented by fbdev core. > + * Use drm_fb_helper_set_suspend_unlocked() if you don't need to take > + * the lock yourself > */ > void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper, int state) > { > @@ -1035,6 +1049,52 @@ void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper, int state) > } > EXPORT_SYMBOL(drm_fb_helper_set_suspend); > > +/** > + * drm_fb_helper_set_suspend_unlocked - wrapper around fb_set_suspend that also > + * takes the console lock > + * @fb_helper: driver-allocated fbdev helper > + * @state: desired state, zero to resume, non-zero to suspend > + * > + * A wrapper around fb_set_suspend() that takes the console lock. If the lock > + * isn't available on resume, a worker is tasked with waiting for the lock > + * to become available. The console lock can be pretty contented on resume > + * due to all the printk activity. > + * > + * This function can be called multiple times with the same state since > + * &fb_info->state is checked to see if fbdev is running or not before locking. > + * > + * Use drm_fb_helper_set_suspend() if you need to take the lock yourself. > + */ > +void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper, > + int suspend) > +{ > + if (!fb_helper || !fb_helper->fbdev) > + return; > + > + /* make sure there's no pending/ongoing resume */ > + flush_work(&fb_helper->resume_work); > + > + if (suspend) { > + if (fb_helper->fbdev->state != FBINFO_STATE_RUNNING) > + return; > + > + console_lock(); > + > + } else { > + if (fb_helper->fbdev->state == FBINFO_STATE_RUNNING) > + return; > + > + if (!console_trylock()) { > + schedule_work(&fb_helper->resume_work); > + return; > + } > + } > + > + fb_set_suspend(fb_helper->fbdev, suspend); > + console_unlock(); > +} > +EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked); > + > static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green, > u16 blue, u16 regno, struct fb_info *info) > { > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h > index db8d478..5bde508 100644 > --- a/include/drm/drm_fb_helper.h > +++ b/include/drm/drm_fb_helper.h > @@ -176,6 +176,7 @@ struct drm_fb_helper_connector { > * the screen buffer > * @dirty_lock: spinlock protecting @dirty_clip > * @dirty_work: worker used to flush the framebuffer > + * @resume_work: worker used during resume if the console lock is already taken > * > * This is the main structure used by the fbdev helpers. Drivers supporting > * fbdev emulation should embedded this into their overall driver structure. > @@ -196,6 +197,7 @@ struct drm_fb_helper { > struct drm_clip_rect dirty_clip; > spinlock_t dirty_lock; > struct work_struct dirty_work; > + struct work_struct resume_work; > > /** > * @kernel_fb_list: > @@ -264,6 +266,8 @@ void drm_fb_helper_cfb_imageblit(struct fb_info *info, > const struct fb_image *image); > > void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper, int state); > +void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper, > + int suspend); > > int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info); > > @@ -421,6 +425,11 @@ static inline void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper, > { > } > > +static inline void > +drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper, int suspend) > +{ > +} > + > static inline int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) > { > return 0; > -- > 2.8.2 >
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index ce54e98..2886df5 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -29,6 +29,7 @@ */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/console.h> #include <linux/kernel.h> #include <linux/sysrq.h> #include <linux/slab.h> @@ -618,6 +619,16 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) kfree(helper->crtc_info); } +static void drm_fb_helper_resume_worker(struct work_struct *work) +{ + struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper, + resume_work); + + console_lock(); + fb_set_suspend(helper->fbdev, 0); + console_unlock(); +} + static void drm_fb_helper_dirty_work(struct work_struct *work) { struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper, @@ -649,6 +660,7 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper, { INIT_LIST_HEAD(&helper->kernel_fb_list); spin_lock_init(&helper->dirty_lock); + INIT_WORK(&helper->resume_work, drm_fb_helper_resume_worker); INIT_WORK(&helper->dirty_work, drm_fb_helper_dirty_work); helper->dirty_clip.x1 = helper->dirty_clip.y1 = ~0; helper->funcs = funcs; @@ -1026,7 +1038,9 @@ EXPORT_SYMBOL(drm_fb_helper_cfb_imageblit); * @fb_helper: driver-allocated fbdev helper * @state: desired state, zero to resume, non-zero to suspend * - * A wrapper around fb_set_suspend implemented by fbdev core + * A wrapper around fb_set_suspend implemented by fbdev core. + * Use drm_fb_helper_set_suspend_unlocked() if you don't need to take + * the lock yourself */ void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper, int state) { @@ -1035,6 +1049,52 @@ void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper, int state) } EXPORT_SYMBOL(drm_fb_helper_set_suspend); +/** + * drm_fb_helper_set_suspend_unlocked - wrapper around fb_set_suspend that also + * takes the console lock + * @fb_helper: driver-allocated fbdev helper + * @state: desired state, zero to resume, non-zero to suspend + * + * A wrapper around fb_set_suspend() that takes the console lock. If the lock + * isn't available on resume, a worker is tasked with waiting for the lock + * to become available. The console lock can be pretty contented on resume + * due to all the printk activity. + * + * This function can be called multiple times with the same state since + * &fb_info->state is checked to see if fbdev is running or not before locking. + * + * Use drm_fb_helper_set_suspend() if you need to take the lock yourself. + */ +void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper, + int suspend) +{ + if (!fb_helper || !fb_helper->fbdev) + return; + + /* make sure there's no pending/ongoing resume */ + flush_work(&fb_helper->resume_work); + + if (suspend) { + if (fb_helper->fbdev->state != FBINFO_STATE_RUNNING) + return; + + console_lock(); + + } else { + if (fb_helper->fbdev->state == FBINFO_STATE_RUNNING) + return; + + if (!console_trylock()) { + schedule_work(&fb_helper->resume_work); + return; + } + } + + fb_set_suspend(fb_helper->fbdev, suspend); + console_unlock(); +} +EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked); + static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green, u16 blue, u16 regno, struct fb_info *info) { diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index db8d478..5bde508 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -176,6 +176,7 @@ struct drm_fb_helper_connector { * the screen buffer * @dirty_lock: spinlock protecting @dirty_clip * @dirty_work: worker used to flush the framebuffer + * @resume_work: worker used during resume if the console lock is already taken * * This is the main structure used by the fbdev helpers. Drivers supporting * fbdev emulation should embedded this into their overall driver structure. @@ -196,6 +197,7 @@ struct drm_fb_helper { struct drm_clip_rect dirty_clip; spinlock_t dirty_lock; struct work_struct dirty_work; + struct work_struct resume_work; /** * @kernel_fb_list: @@ -264,6 +266,8 @@ void drm_fb_helper_cfb_imageblit(struct fb_info *info, const struct fb_image *image); void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper, int state); +void drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper, + int suspend); int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info); @@ -421,6 +425,11 @@ static inline void drm_fb_helper_set_suspend(struct drm_fb_helper *fb_helper, { } +static inline void +drm_fb_helper_set_suspend_unlocked(struct drm_fb_helper *fb_helper, int suspend) +{ +} + static inline int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) { return 0;
This adds a function that also takes the console lock before calling fb_set_suspend() in contrast to drm_fb_helper_set_suspend() which is a plain wrapper around fb_set_suspend(). Resume is run asynchronously using a worker if the console lock is already taken. This is modelled after the i915 driver. Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- Daniel, I haven't got time to convert drivers to use this and now that simpledrm won't be using it we can wait with this if you prefer, and I'll bundle it with tinydrm when the time comes. Noralf. Changes from version 1: - Change function name to drm_fb_helper_set_suspend_unlocked() - Use suspend as argument name to increase readability drivers/gpu/drm/drm_fb_helper.c | 62 ++++++++++++++++++++++++++++++++++++++++- include/drm/drm_fb_helper.h | 9 ++++++ 2 files changed, 70 insertions(+), 1 deletion(-) -- 2.8.2