* [PATCH] drm/i915: use delayed work for resume hotplug v2
@ 2014-10-07 20:25 Jesse Barnes
2014-10-08 6:43 ` Chris Wilson
2014-10-09 15:13 ` [PATCH] drm/i915: use delayed work for resume hotplug v4 Jesse Barnes
0 siblings, 2 replies; 8+ messages in thread
From: Jesse Barnes @ 2014-10-07 20:25 UTC (permalink / raw)
To: intel-gfx; +Cc: shuang.he
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.
+ */
+ schedule_delayed_work(&dev_priv->hotplug_resume_work,
+ msecs_to_jiffies(50));
}
intel_opregion_init(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1e476b5..6273ad6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1521,6 +1521,7 @@ struct drm_i915_private {
} hpd_stats[HPD_NUM_PINS];
u32 hpd_event_bits;
struct delayed_work hotplug_reenable_work;
+ struct delayed_work hotplug_resume_work;
struct i915_fbc fbc;
struct i915_drrs drrs;
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: use delayed work for resume hotplug v2
2014-10-07 20:25 [PATCH] drm/i915: use delayed work for resume hotplug v2 Jesse Barnes
@ 2014-10-08 6:43 ` Chris Wilson
2014-10-08 14:32 ` Jesse Barnes
2014-10-09 15:13 ` [PATCH] drm/i915: use delayed work for resume hotplug v4 Jesse Barnes
1 sibling, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2014-10-08 6:43 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx, shuang.he
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.
This is a nice vague statement that scares me. If half of this comment
is true, using a simple delay is periliously lackadaisical. The commit
log just talks about speeding up resume, but this comment implies to me
something more sinister.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: use delayed work for resume hotplug v2
2014-10-08 6:43 ` Chris Wilson
@ 2014-10-08 14:32 ` Jesse Barnes
2014-10-09 10:11 ` Chris Wilson
0 siblings, 1 reply; 8+ messages in thread
From: Jesse Barnes @ 2014-10-08 14:32 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, shuang.he
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.
>
> This is a nice vague statement that scares me. If half of this comment
> is true, using a simple delay is periliously lackadaisical. The commit
> log just talks about speeding up resume, but this comment implies to me
> something more sinister.
Nothing sinister; nothing should blow up if we end up firing this
before resume completes. We mainly want to avoid hitting lock contention
from the mode set above if it ever becomes async. So a rough estimate
ought to be fine (there's really nothing that should take long after
this point anyway).
Jesse
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: use delayed work for resume hotplug v2
2014-10-08 14:32 ` Jesse Barnes
@ 2014-10-09 10:11 ` Chris Wilson
2014-10-09 13:57 ` Jesse Barnes
0 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2014-10-09 10:11 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx, shuang.he
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.
*/
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: use delayed work for resume hotplug v2
2014-10-09 10:11 ` Chris Wilson
@ 2014-10-09 13:57 ` Jesse Barnes
0 siblings, 0 replies; 8+ messages in thread
From: Jesse Barnes @ 2014-10-09 13:57 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, shuang.he
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] drm/i915: use delayed work for resume hotplug v4
2014-10-07 20:25 [PATCH] drm/i915: use delayed work for resume hotplug v2 Jesse Barnes
2014-10-08 6:43 ` Chris Wilson
@ 2014-10-09 15:13 ` Jesse Barnes
2014-10-10 6:09 ` Chris Wilson
2014-10-19 14:27 ` Daniel Vetter
1 sibling, 2 replies; 8+ messages in thread
From: Jesse Barnes @ 2014-10-09 15:13 UTC (permalink / raw)
To: intel-gfx
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)
v3: cancel delayed work at unload and suspend time (Jesse)
v4: make delayed work comment less scary (Chris)
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
drivers/gpu/drm/i915/i915_dma.c | 12 ++++++++++++
drivers/gpu/drm/i915/i915_drv.c | 19 +++++++++++++++++--
drivers/gpu/drm/i915/i915_drv.h | 1 +
3 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 85d14e1..65001de 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
@@ -1854,6 +1864,8 @@ int i915_driver_unload(struct drm_device *dev)
acpi_video_unregister();
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+ cancel_delayed_work(&dev_priv->hotplug_resume_work);
+
intel_fbdev_fini(dev);
intel_modeset_cleanup(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a05a1d0..74fc289 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -585,6 +585,12 @@ static int i915_drm_freeze(struct drm_device *dev)
}
/*
+ * In case we haven't run this by the time we resume, we may
+ * as well cancel until we resume again.
+ */
+ cancel_delayed_work(&dev_priv->hotplug_resume_work);
+
+ /*
* Disable CRTCs directly since we want to preserve sw state
* for _thaw. Also, power gate the CRTC power wells.
*/
@@ -726,8 +732,17 @@ 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, 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.
+ */
+ schedule_delayed_work(&dev_priv->hotplug_resume_work,
+ msecs_to_jiffies(50));
}
intel_opregion_init(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1e476b5..6273ad6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1521,6 +1521,7 @@ struct drm_i915_private {
} hpd_stats[HPD_NUM_PINS];
u32 hpd_event_bits;
struct delayed_work hotplug_reenable_work;
+ struct delayed_work hotplug_resume_work;
struct i915_fbc fbc;
struct i915_drrs drrs;
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: use delayed work for resume hotplug v4
2014-10-09 15:13 ` [PATCH] drm/i915: use delayed work for resume hotplug v4 Jesse Barnes
@ 2014-10-10 6:09 ` Chris Wilson
2014-10-19 14:27 ` Daniel Vetter
1 sibling, 0 replies; 8+ messages in thread
From: Chris Wilson @ 2014-10-10 6:09 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx
On Thu, Oct 09, 2014 at 08:13:18AM -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)
> v3: cancel delayed work at unload and suspend time (Jesse)
> v4: make delayed work comment less scary (Chris)
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Comment inline, but still
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 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
> @@ -1854,6 +1864,8 @@ int i915_driver_unload(struct drm_device *dev)
> acpi_video_unregister();
>
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> + cancel_delayed_work(&dev_priv->hotplug_resume_work);
> +
I would have put this in modeset_cleanup so that it was symmetric with
modeset_init.
> intel_fbdev_fini(dev);
> intel_modeset_cleanup(dev);
>
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: use delayed work for resume hotplug v4
2014-10-09 15:13 ` [PATCH] drm/i915: use delayed work for resume hotplug v4 Jesse Barnes
2014-10-10 6:09 ` Chris Wilson
@ 2014-10-19 14:27 ` Daniel Vetter
1 sibling, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2014-10-19 14:27 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx
On Thu, Oct 09, 2014 at 08:13:18AM -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)
> v3: cancel delayed work at unload and suspend time (Jesse)
> v4: make delayed work comment less scary (Chris)
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Is this r-b: Chris or not? Two comments below.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_dma.c | 12 ++++++++++++
> drivers/gpu/drm/i915/i915_drv.c | 19 +++++++++++++++++--
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> 3 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 85d14e1..65001de 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
> @@ -1854,6 +1864,8 @@ int i915_driver_unload(struct drm_device *dev)
> acpi_video_unregister();
>
> if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> + cancel_delayed_work(&dev_priv->hotplug_resume_work);
> +
This should be moved to intel_modeset_cleanup to the other hpd quiescent
calls imo. Splattering stuff all over isn't good, and if we need to stop
hpd handling here already then we should move everything.
> intel_fbdev_fini(dev);
> intel_modeset_cleanup(dev);
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a05a1d0..74fc289 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -585,6 +585,12 @@ static int i915_drm_freeze(struct drm_device *dev)
> }
>
> /*
> + * In case we haven't run this by the time we resume, we may
> + * as well cancel until we resume again.
> + */
> + cancel_delayed_work(&dev_priv->hotplug_resume_work);
Again this should be right next to the other hpd quiescence calls, i.e.
next to drm_kms_helper_poll_disable.
> +
> + /*
> * Disable CRTCs directly since we want to preserve sw state
> * for _thaw. Also, power gate the CRTC power wells.
> */
> @@ -726,8 +732,17 @@ 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, 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.
> + */
> + schedule_delayed_work(&dev_priv->hotplug_resume_work,
> + msecs_to_jiffies(50));
> }
>
> intel_opregion_init(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1e476b5..6273ad6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1521,6 +1521,7 @@ struct drm_i915_private {
> } hpd_stats[HPD_NUM_PINS];
> u32 hpd_event_bits;
> struct delayed_work hotplug_reenable_work;
> + struct delayed_work hotplug_resume_work;
>
> struct i915_fbc fbc;
> struct i915_drrs drrs;
> --
> 1.9.1
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-10-19 14:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-07 20:25 [PATCH] drm/i915: use delayed work for resume hotplug v2 Jesse Barnes
2014-10-08 6:43 ` Chris Wilson
2014-10-08 14:32 ` Jesse Barnes
2014-10-09 10:11 ` Chris Wilson
2014-10-09 13:57 ` Jesse Barnes
2014-10-09 15:13 ` [PATCH] drm/i915: use delayed work for resume hotplug v4 Jesse Barnes
2014-10-10 6:09 ` Chris Wilson
2014-10-19 14:27 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox