[PATCH 1/3] drm/i915: drop encoder hot_plug calls at resume

[PATCH 1/3] drm/i915: drop encoder hot_plug calls at resume

[Jesse Barnes]

We really just want to go detect displays again and fire off a hotplug
event if things have changed, not go through full hotplug processing.

Requested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b948c71..302495f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -551,24 +551,6 @@ void intel_console_resume(struct work_struct *work)
 	console_unlock();
 }
 
-static void intel_resume_hotplug(struct drm_device *dev)
-{
-	struct drm_mode_config *mode_config = &dev->mode_config;
-	struct intel_encoder *encoder;
-
-	mutex_lock(&mode_config->mutex);
-	DRM_DEBUG_KMS("running encoder hotplug functions\n");
-
-	list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
-		if (encoder->hot_plug)
-			encoder->hot_plug(encoder);
-
-	mutex_unlock(&mode_config->mutex);
-
-	/* Just fire off a uevent and let userspace tell us what to do */
-	drm_helper_hpd_irq_event(dev);
-}
-
 static int i915_drm_thaw_early(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -624,7 +606,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 		intel_hpd_init(dev);
 		dev_priv->enable_hotplug_processing = true;
 		/* Config may have changed between suspend and resume */
-		intel_resume_hotplug(dev);
+		drm_helper_hpd_irq_event(dev);
 	}
 
 	intel_opregion_init(dev);
-- 
1.8.4.2

[PATCH 2/3] drm: add async version of hpd_irq_event

[Jesse Barnes]

In some cases, the callers of this function may not need the return
value and delaying the uevent is ok.  So add an async version of the
function for use in those cases.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/drm_probe_helper.c | 8 ++++++++
 include/drm/drm_crtc_helper.h      | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 79f07f2..f3aee4a 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -446,3 +446,11 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
 	return changed;
 }
 EXPORT_SYMBOL(drm_helper_hpd_irq_event);
+
+void drm_helper_hpd_irq_event_async(void *data, async_cookie_t cookie)
+{
+	struct drm_device *dev = data;
+
+	drm_helper_hpd_irq_event(dev);
+}
+EXPORT_SYMBOL(drm_helper_hpd_irq_event_async);
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index a3d75fe..4f4ed9c 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -33,6 +33,7 @@
 #ifndef __DRM_CRTC_HELPER_H__
 #define __DRM_CRTC_HELPER_H__
 
+#include <linux/async.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/idr.h>
@@ -172,6 +173,7 @@ extern int drm_helper_probe_single_connector_modes_nomerge(struct drm_connector
 extern void drm_kms_helper_poll_init(struct drm_device *dev);
 extern void drm_kms_helper_poll_fini(struct drm_device *dev);
 extern bool drm_helper_hpd_irq_event(struct drm_device *dev);
+extern void drm_helper_hpd_irq_event_async(void *data, async_cookie_t cookie);
 extern void drm_kms_helper_hotplug_event(struct drm_device *dev);
 
 extern void drm_kms_helper_poll_disable(struct drm_device *dev);
-- 
1.8.4.2

[PATCH 3/3] drm/i915: use async hpd_irq_event function on resume

[Jesse Barnes]

Gets the detect code (which may take awhile) out of the resume path,
speeding things up a bit.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 302495f..571f688 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -606,7 +606,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 		intel_hpd_init(dev);
 		dev_priv->enable_hotplug_processing = true;
 		/* Config may have changed between suspend and resume */
-		drm_helper_hpd_irq_event(dev);
+		async_schedule(drm_helper_hpd_irq_event_async, dev);
 	}
 
 	intel_opregion_init(dev);
-- 
1.8.4.2

Re: [PATCH 2/3] drm: add async version of hpd_irq_event

[Daniel Vetter]

On Tue, May 20, 2014 at 03:25:34PM -0700, Jesse Barnes wrote:
> In some cases, the callers of this function may not need the return
> value and delaying the uevent is ok.  So add an async version of the
> function for use in those cases.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/drm_probe_helper.c | 8 ++++++++
>  include/drm/drm_crtc_helper.h      | 2 ++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 79f07f2..f3aee4a 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -446,3 +446,11 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
>  	return changed;
>  }
>  EXPORT_SYMBOL(drm_helper_hpd_irq_event);
> +

Kerneldoc is missing.
-Daniel

> +void drm_helper_hpd_irq_event_async(void *data, async_cookie_t cookie)
> +{
> +	struct drm_device *dev = data;
> +
> +	drm_helper_hpd_irq_event(dev);
> +}
> +EXPORT_SYMBOL(drm_helper_hpd_irq_event_async);
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index a3d75fe..4f4ed9c 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -33,6 +33,7 @@
>  #ifndef __DRM_CRTC_HELPER_H__
>  #define __DRM_CRTC_HELPER_H__
>  
> +#include <linux/async.h>
>  #include <linux/spinlock.h>
>  #include <linux/types.h>
>  #include <linux/idr.h>
> @@ -172,6 +173,7 @@ extern int drm_helper_probe_single_connector_modes_nomerge(struct drm_connector
>  extern void drm_kms_helper_poll_init(struct drm_device *dev);
>  extern void drm_kms_helper_poll_fini(struct drm_device *dev);
>  extern bool drm_helper_hpd_irq_event(struct drm_device *dev);
> +extern void drm_helper_hpd_irq_event_async(void *data, async_cookie_t cookie);
>  extern void drm_kms_helper_hotplug_event(struct drm_device *dev);
>  
>  extern void drm_kms_helper_poll_disable(struct drm_device *dev);
> -- 
> 1.8.4.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/3] drm/i915: use async hpd_irq_event function on resume

[Daniel Vetter]

On Tue, May 20, 2014 at 03:25:35PM -0700, Jesse Barnes wrote:
> Gets the detect code (which may take awhile) out of the resume path,
> speeding things up a bit.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 302495f..571f688 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -606,7 +606,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
>  		intel_hpd_init(dev);
>  		dev_priv->enable_hotplug_processing = true;
>  		/* Config may have changed between suspend and resume */
> -		drm_helper_hpd_irq_event(dev);
> +		async_schedule(drm_helper_hpd_irq_event_async, dev);

Does that really help all that much? I've thought the driver core
sychnronizes all the async workers again once resume is done. I'm better
to schedule this as a fully async work with e.g. a 1s delay, like we do
with the rps resume work.
-Daniel

>  	}
>  
>  	intel_opregion_init(dev);
> -- 
> 1.8.4.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/3] drm/i915: drop encoder hot_plug calls at resume

[Daniel Vetter]

On Tue, May 20, 2014 at 03:25:33PM -0700, Jesse Barnes wrote:
> We really just want to go detect displays again and fire off a hotplug
> event if things have changed, not go through full hotplug processing.
> 
> Requested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Yeah, still can't remember why we've done this, and the commit message is
silent.

Queued for -next, thanks for the patch.
-Daniel


> ---
>  drivers/gpu/drm/i915/i915_drv.c | 20 +-------------------
>  1 file changed, 1 insertion(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b948c71..302495f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -551,24 +551,6 @@ void intel_console_resume(struct work_struct *work)
>  	console_unlock();
>  }
>  
> -static void intel_resume_hotplug(struct drm_device *dev)
> -{
> -	struct drm_mode_config *mode_config = &dev->mode_config;
> -	struct intel_encoder *encoder;
> -
> -	mutex_lock(&mode_config->mutex);
> -	DRM_DEBUG_KMS("running encoder hotplug functions\n");
> -
> -	list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
> -		if (encoder->hot_plug)
> -			encoder->hot_plug(encoder);
> -
> -	mutex_unlock(&mode_config->mutex);
> -
> -	/* Just fire off a uevent and let userspace tell us what to do */
> -	drm_helper_hpd_irq_event(dev);
> -}
> -
>  static int i915_drm_thaw_early(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -624,7 +606,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
>  		intel_hpd_init(dev);
>  		dev_priv->enable_hotplug_processing = true;
>  		/* Config may have changed between suspend and resume */
> -		intel_resume_hotplug(dev);
> +		drm_helper_hpd_irq_event(dev);
>  	}
>  
>  	intel_opregion_init(dev);
> -- 
> 1.8.4.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/3] drm/i915: use async hpd_irq_event function on resume

[Jesse Barnes]

On Wed, 21 May 2014 08:52:34 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Tue, May 20, 2014 at 03:25:35PM -0700, Jesse Barnes wrote:
> > Gets the detect code (which may take awhile) out of the resume path,
> > speeding things up a bit.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 302495f..571f688 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -606,7 +606,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
> >  		intel_hpd_init(dev);
> >  		dev_priv->enable_hotplug_processing = true;
> >  		/* Config may have changed between suspend and resume */
> > -		drm_helper_hpd_irq_event(dev);
> > +		async_schedule(drm_helper_hpd_irq_event_async, dev);
> 
> Does that really help all that much? I've thought the driver core
> sychnronizes all the async workers again once resume is done. I'm better
> to schedule this as a fully async work with e.g. a 1s delay, like we do
> with the rps resume work.

That might be better, I'll check on the synchronization.  I thought
async_schedule was the new hotness we were supposed to use everywhere...

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH 3/3] drm/i915: use async hpd_irq_event function on resume

[Daniel Vetter]

On Wed, May 21, 2014 at 08:00:22AM -0700, Jesse Barnes wrote:
> On Wed, 21 May 2014 08:52:34 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Tue, May 20, 2014 at 03:25:35PM -0700, Jesse Barnes wrote:
> > > Gets the detect code (which may take awhile) out of the resume path,
> > > speeding things up a bit.
> > > 
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > > index 302495f..571f688 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -606,7 +606,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
> > >  		intel_hpd_init(dev);
> > >  		dev_priv->enable_hotplug_processing = true;
> > >  		/* Config may have changed between suspend and resume */
> > > -		drm_helper_hpd_irq_event(dev);
> > > +		async_schedule(drm_helper_hpd_irq_event_async, dev);
> > 
> > Does that really help all that much? I've thought the driver core
> > sychnronizes all the async workers again once resume is done. I'm better
> > to schedule this as a fully async work with e.g. a 1s delay, like we do
> > with the rps resume work.
> 
> That might be better, I'll check on the synchronization.  I thought
> async_schedule was the new hotness we were supposed to use everywhere...

It's pretty cool for easy async in driver load/resume since it autosyncs.
You can even create more async domains if you need finer-grained sync
points. But if we know that we can be more lenient than full sync before
our driver load/resume completes we need to use classical work queues.

But for stuff like doing paralle modeset restore async domains look like
the right thing.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch