| Message ID | 1471897525-31118-3-git-send-email-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 22, 2016 at 10:25:22PM +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> > --- > drivers/gpu/drm/drm_fb_helper.c | 57 +++++++++++++++++++++++++++++++++++++++++ > include/drm/drm_fb_helper.h | 9 +++++++ > 2 files changed, 66 insertions(+) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index ce54e98..897e6f4 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; > @@ -1035,6 +1047,51 @@ 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_lock - 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_lock(struct drm_fb_helper *fb_helper, int state) We generally call the functions which take the locks themselves _unlocked (or the version that requires locks to be held _locked). Other nit: I think added a sentence to the kerneldoc of drm_fb_helper_set_suspend to instead use this one (since it avoids lock contention on resume) would be good. Otherwise looks great. And if you're bored, following up with a bunch of patches to roll this out to drivers would be awesome ;-) > +{ > + if (!fb_helper || !fb_helper->fbdev) > + return; > + > + /* make sure there's no pending/ongoing resume */ > + flush_work(&fb_helper->resume_work); > + > + if (state) { /* suspend */ Hm, one more nit: rename state to suspend and you both make the declaration more self-explanatory, and can drop these 2 comments here. Thanks, Daniel > + if (fb_helper->fbdev->state != FBINFO_STATE_RUNNING) > + return; > + > + console_lock(); > + > + } else { /* resume */ > + 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, state); > + console_unlock(); > +} > +EXPORT_SYMBOL(drm_fb_helper_set_suspend_lock); > + > 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..db7e861 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_lock(struct drm_fb_helper *fb_helper, > + int state); > > 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_lock(struct drm_fb_helper *fb_helper, int state) > +{ > +} > + > static inline int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper) > { > return 0; > -- > 2.8.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index ce54e98..897e6f4 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; @@ -1035,6 +1047,51 @@ 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_lock - 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_lock(struct drm_fb_helper *fb_helper, int state) +{ + if (!fb_helper || !fb_helper->fbdev) + return; + + /* make sure there's no pending/ongoing resume */ + flush_work(&fb_helper->resume_work); + + if (state) { /* suspend */ + if (fb_helper->fbdev->state != FBINFO_STATE_RUNNING) + return; + + console_lock(); + + } else { /* resume */ + 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, state); + console_unlock(); +} +EXPORT_SYMBOL(drm_fb_helper_set_suspend_lock); + 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..db7e861 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_lock(struct drm_fb_helper *fb_helper, + int state); 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_lock(struct drm_fb_helper *fb_helper, int state) +{ +} + 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> --- drivers/gpu/drm/drm_fb_helper.c | 57 +++++++++++++++++++++++++++++++++++++++++ include/drm/drm_fb_helper.h | 9 +++++++ 2 files changed, 66 insertions(+)