| Message ID | 1350267038-3599-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> The console lock can be contended, so rather than prevent other drivers > after us from being held up, queue the console suspend into the global > work queue that can happen anytime. I've measured this to take around > 200ms on my T420. Combined with the ring freq/turbo change, we should > save almost 1/2 a second on resume. Did you measure booting with quiet? I can't think what else could be contending on this lock other than printk, now maybe we should reduce the printk noise! This will make debugging things even worse than they are, since you don't see any printks on the screen until after we do this. Dave.
On Mon, 15 Oct 2012 12:32:05 +1000 Dave Airlie <airlied@gmail.com> wrote: > > The console lock can be contended, so rather than prevent other drivers > > after us from being held up, queue the console suspend into the global > > work queue that can happen anytime. I've measured this to take around > > 200ms on my T420. Combined with the ring freq/turbo change, we should > > save almost 1/2 a second on resume. > > Did you measure booting with quiet? I can't think what else could be > contending on this lock other than printk, now maybe we should reduce > the printk noise! > > This will make debugging things even worse than they are, since you > don't see any printks on the screen until after we do this. I thought I measured with quiet, but I'll double check. Not sure about debugging; it's pretty bad to begin with. I generally use netconsole or the "stare really hard at the code" method when it comes to suspend/resume or init time anyway. But yeah, some investigation into the console lock contention is probably a good idea at any rate. A typical distro config probably also has some other fbcon driver and probably vgacon built-in too, and I haven't checked what they do. Jesse
On Mon, Oct 15, 2012 at 2:49 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > On Mon, 15 Oct 2012 12:32:05 +1000 > Dave Airlie <airlied@gmail.com> wrote: > >> > The console lock can be contended, so rather than prevent other drivers >> > after us from being held up, queue the console suspend into the global >> > work queue that can happen anytime. I've measured this to take around >> > 200ms on my T420. Combined with the ring freq/turbo change, we should >> > save almost 1/2 a second on resume. >> >> Did you measure booting with quiet? I can't think what else could be >> contending on this lock other than printk, now maybe we should reduce >> the printk noise! >> >> This will make debugging things even worse than they are, since you >> don't see any printks on the screen until after we do this. > > I thought I measured with quiet, but I'll double check. > > Not sure about debugging; it's pretty bad to begin with. I generally > use netconsole or the "stare really hard at the code" method when it > comes to suspend/resume or init time anyway. > > But yeah, some investigation into the console lock contention is > probably a good idea at any rate. A typical distro config probably > also has some other fbcon driver and probably vgacon built-in too, and > I haven't checked what they do. > If they are still there at s/r time you now have two problems. Dave.
On Sun, 14 Oct 2012 19:10:36 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > The console lock can be contended, so rather than prevent other drivers > after us from being held up, queue the console suspend into the global > work queue that can happen anytime. I've measured this to take around > 200ms on my T420. Combined with the ring freq/turbo change, we should > save almost 1/2 a second on resume. In gneral it looks like the first couple of patches are reflections of the async-domains work, and would probably be better if we looked more closely to integrating into that async init/resume infrastructure. The first patches floating around were to offload attaching inteldrmfb to a separate thread. -Chris
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 491394f..f88bfa6 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1327,6 +1327,8 @@ static int i915_load_modeset_init(struct drm_device *dev) intel_modeset_gem_init(dev); + INIT_WORK(&dev_priv->console_resume_work, intel_console_resume); + ret = drm_irq_install(dev); if (ret) goto cleanup_gem; @@ -1721,6 +1723,7 @@ int i915_driver_unload(struct drm_device *dev) if (drm_core_check_feature(dev, DRIVER_MODESET)) { intel_fbdev_fini(dev); intel_modeset_cleanup(dev); + cancel_work_sync(&dev_priv->console_resume_work); /* * free the memory space allocated for the child device diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index a7837e5..824e3c8 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -520,6 +520,18 @@ int i915_suspend(struct drm_device *dev, pm_message_t state) return 0; } +void intel_console_resume(struct work_struct *work) +{ + struct drm_i915_private *dev_priv = + container_of(work, struct drm_i915_private, + console_resume_work); + struct drm_device *dev = dev_priv->dev; + + console_lock(); + intel_fbdev_set_suspend(dev, 0); + console_unlock(); +} + static int i915_drm_thaw(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -555,9 +567,8 @@ static int i915_drm_thaw(struct drm_device *dev) dev_priv->modeset_on_lid = 0; - console_lock(); - intel_fbdev_set_suspend(dev, 0); - console_unlock(); + schedule_work(&dev_priv->console_resume_work); + return error; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9e446b6..e22b9e3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -453,6 +453,12 @@ typedef struct drm_i915_private { u32 hotplug_supported_mask; struct work_struct hotplug_work; + /* + * The console may be contended at resume, but we don't + * want it to block on it. + */ + struct work_struct console_resume_work; + int num_pipe; int num_pch_pll; @@ -1257,6 +1263,7 @@ extern unsigned long i915_mch_val(struct drm_i915_private *dev_priv); extern unsigned long i915_gfx_val(struct drm_i915_private *dev_priv); extern void i915_update_gfx_val(struct drm_i915_private *dev_priv); +extern void intel_console_resume(struct work_struct *work); /* i915_irq.c */ void i915_hangcheck_elapsed(unsigned long data);
The console lock can be contended, so rather than prevent other drivers after us from being held up, queue the console suspend into the global work queue that can happen anytime. I've measured this to take around 200ms on my T420. Combined with the ring freq/turbo change, we should save almost 1/2 a second on resume. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/i915_dma.c | 3 +++ drivers/gpu/drm/i915/i915_drv.c | 17 ++++++++++++++--- drivers/gpu/drm/i915/i915_drv.h | 7 +++++++ 3 files changed, 24 insertions(+), 3 deletions(-)