Re: [PATCH] drm/i915: use delayed work for resume hotplug v2

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Thu, 9 Oct 2014 11:11:32 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Wed, Oct 08, 2014 at 07:32:12AM -0700, Jesse Barnes wrote:
> > On Wed, 8 Oct 2014 07:43:34 +0100
> > Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > 
> > > On Tue, Oct 07, 2014 at 01:25:23PM -0700, Jesse Barnes wrote:
> > > > Gets the detect code (which may take awhile) out of the resume path,
> > > > speeding things up a bit.
> > > > 
> > > > v2: use a delayed work queue instead (Daniel)
> > > > 
> > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_dma.c | 10 ++++++++++
> > > >  drivers/gpu/drm/i915/i915_drv.c |  8 ++++++--
> > > >  drivers/gpu/drm/i915/i915_drv.h |  1 +
> > > >  3 files changed, 17 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > > > index 85d14e1..633095d 100644
> > > > --- a/drivers/gpu/drm/i915/i915_dma.c
> > > > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > > @@ -1303,6 +1303,15 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
> > > >  	.can_switch = i915_switcheroo_can_switch,
> > > >  };
> > > >  
> > > > +static void intel_resume_hotplug(struct work_struct *work)
> > > > +{
> > > > +	struct drm_i915_private *dev_priv =
> > > > +		container_of(work, struct drm_i915_private,
> > > > +			     hotplug_resume_work.work);
> > > > +
> > > > +	drm_helper_hpd_irq_event(dev_priv->dev);
> > > > +}
> > > > +
> > > >  static int i915_load_modeset_init(struct drm_device *dev)
> > > >  {
> > > >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > > > @@ -1364,6 +1373,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
> > > >  
> > > >  	/* Only enable hotplug handling once the fbdev is fully set up. */
> > > >  	intel_hpd_init(dev_priv);
> > > > +	INIT_DELAYED_WORK(&dev_priv->hotplug_resume_work, intel_resume_hotplug);
> > > >  
> > > >  	/*
> > > >  	 * Some ports require correctly set-up hpd registers for detection to
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > > index a05a1d0..83075f9 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > @@ -726,8 +726,12 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
> > > >  		 * notifications.
> > > >  		 * */
> > > >  		intel_hpd_init(dev_priv);
> > > > -		/* Config may have changed between suspend and resume */
> > > > -		drm_helper_hpd_irq_event(dev);
> > > > +		/* Config may have changed between suspend and resume. Just
> > > > +		 * try to make sure the rest of driver resume is finished
> > > > +		 * before we start probing for config changes.
> 
> /* Config may have changed between suspend and resume, queue a hotplug
>  * notification for userspace to check when it wakes up. Delay the work
>  * slightly so that the resume has time to finish, before userspace
>  * starts pounding at the gates checking for changes. This speeds up the
>  * resume process by reducing contention for resources.
>  */

Yeah looks good and a bit less scary.

Jesse