* [PATCH 0/2] Small fixes to speed up resume @ 2016-11-03 15:42 ` Lyude 0 siblings, 0 replies; 20+ messages in thread From: Lyude @ 2016-11-03 15:42 UTC (permalink / raw) To: intel-gfx; +Cc: Lyude, David Airlie, linux-kernel, dri-devel, Daniel Vetter Recently David Weinehall has been investigating how we can make the resume process in i915 faster, and poked me to take a look at why we were taking so long while reprobing. As it turns out the main issue is just that we hold up the entire resume process while we reprobe connectors, which isn't really necessary. This fixes things so we can actually run the connector reprobing on resume asynchronously. Cc: David Weinehall <david.weinehall@linux.intel.com> Lyude (2): drm/i915: Remove redundant reprobe in i915_drm_resume drm/i915: Reinit polling before hpd when resuming drivers/gpu/drm/i915/i915_drv.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/2] Small fixes to speed up resume @ 2016-11-03 15:42 ` Lyude 0 siblings, 0 replies; 20+ messages in thread From: Lyude @ 2016-11-03 15:42 UTC (permalink / raw) To: intel-gfx Cc: Lyude, David Weinehall, Daniel Vetter, Jani Nikula, David Airlie, dri-devel, linux-kernel Recently David Weinehall has been investigating how we can make the resume process in i915 faster, and poked me to take a look at why we were taking so long while reprobing. As it turns out the main issue is just that we hold up the entire resume process while we reprobe connectors, which isn't really necessary. This fixes things so we can actually run the connector reprobing on resume asynchronously. Cc: David Weinehall <david.weinehall@linux.intel.com> Lyude (2): drm/i915: Remove redundant reprobe in i915_drm_resume drm/i915: Reinit polling before hpd when resuming drivers/gpu/drm/i915/i915_drv.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume 2016-11-03 15:42 ` Lyude (?) @ 2016-11-03 15:42 ` Lyude 2016-11-03 16:02 ` Chris Wilson 2016-11-03 18:50 ` [Intel-gfx] " David Weinehall -1 siblings, 2 replies; 20+ messages in thread From: Lyude @ 2016-11-03 15:42 UTC (permalink / raw) To: intel-gfx Cc: Lyude, David Weinehall, Daniel Vetter, Jani Nikula, David Airlie, dri-devel, linux-kernel Weine's investigation on benchmarking the suspend/resume process pointed out a lot of the time in suspend/resume is being spent reprobing. While the reprobing process is a lengthy one for good reason, we don't need to hold up the entire suspend/resume process while we wait for it to finish. Luckily as it turns out, we already trigger a full connector reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing in i915_drm_resume() entirely. This won't lead to less time spent resuming just yet since now the bottleneck will be waiting for the mode_config lock in drm_kms_helper_poll_enable(), since that will be held as long as i915_hpd_poll_init_work() is reprobing all of the connectors. But we'll address that in the next patch. Signed-off-by: Lyude <lyude@redhat.com> Cc: David Weinehall <david.weinehall@linux.intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index bfb2efd..532cc0f 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device *dev) * notifications. * */ intel_hpd_init(dev_priv); - /* Config may have changed between suspend and resume */ - drm_helper_hpd_irq_event(dev); intel_opregion_register(dev_priv); -- 2.7.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume 2016-11-03 15:42 ` [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume Lyude @ 2016-11-03 16:02 ` Chris Wilson 2016-11-03 18:50 ` [Intel-gfx] " David Weinehall 1 sibling, 0 replies; 20+ messages in thread From: Chris Wilson @ 2016-11-03 16:02 UTC (permalink / raw) To: Lyude; +Cc: Daniel Vetter, intel-gfx, linux-kernel, dri-devel On Thu, Nov 03, 2016 at 11:42:37AM -0400, Lyude wrote: > Weine's investigation on benchmarking the suspend/resume process pointed > out a lot of the time in suspend/resume is being spent reprobing. While > the reprobing process is a lengthy one for good reason, we don't need to > hold up the entire suspend/resume process while we wait for it to > finish. Luckily as it turns out, we already trigger a full connector > reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing in > i915_drm_resume() entirely. > > This won't lead to less time spent resuming just yet since now the > bottleneck will be waiting for the mode_config lock in > drm_kms_helper_poll_enable(), since that will be held as long as > i915_hpd_poll_init_work() is reprobing all of the connectors. But we'll > address that in the next patch. > > Signed-off-by: Lyude <lyude@redhat.com> > Cc: David Weinehall <david.weinehall@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index bfb2efd..532cc0f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device *dev) > * notifications. > * */ > intel_hpd_init(dev_priv); > - /* Config may have changed between suspend and resume */ > - drm_helper_hpd_irq_event(dev); The comment is still apt. This code is known to be broken since it doesn't detect a change in monitors (e.g. a change in external connectors from docking) between suspend and resend. We still have to send the uevent. + drm_kms_helper_hotplug_event(dev); which also depends upon us actually reseting the connector->status to unknown in drm_mode_config_reset(), Daniel! -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume @ 2016-11-03 16:02 ` Chris Wilson 0 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2016-11-03 16:02 UTC (permalink / raw) To: Lyude; +Cc: intel-gfx, David Airlie, linux-kernel, dri-devel, Daniel Vetter On Thu, Nov 03, 2016 at 11:42:37AM -0400, Lyude wrote: > Weine's investigation on benchmarking the suspend/resume process pointed > out a lot of the time in suspend/resume is being spent reprobing. While > the reprobing process is a lengthy one for good reason, we don't need to > hold up the entire suspend/resume process while we wait for it to > finish. Luckily as it turns out, we already trigger a full connector > reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing in > i915_drm_resume() entirely. > > This won't lead to less time spent resuming just yet since now the > bottleneck will be waiting for the mode_config lock in > drm_kms_helper_poll_enable(), since that will be held as long as > i915_hpd_poll_init_work() is reprobing all of the connectors. But we'll > address that in the next patch. > > Signed-off-by: Lyude <lyude@redhat.com> > Cc: David Weinehall <david.weinehall@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index bfb2efd..532cc0f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device *dev) > * notifications. > * */ > intel_hpd_init(dev_priv); > - /* Config may have changed between suspend and resume */ > - drm_helper_hpd_irq_event(dev); The comment is still apt. This code is known to be broken since it doesn't detect a change in monitors (e.g. a change in external connectors from docking) between suspend and resend. We still have to send the uevent. + drm_kms_helper_hotplug_event(dev); which also depends upon us actually reseting the connector->status to unknown in drm_mode_config_reset(), Daniel! -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume 2016-11-03 16:02 ` Chris Wilson @ 2016-11-03 16:11 ` Lyude Paul -1 siblings, 0 replies; 20+ messages in thread From: Lyude Paul @ 2016-11-03 16:11 UTC (permalink / raw) To: Chris Wilson, Lyude; +Cc: Daniel Vetter, intel-gfx, linux-kernel, dri-devel On Thu, 2016-11-03 at 16:02 +0000, Chris Wilson wrote: > On Thu, Nov 03, 2016 at 11:42:37AM -0400, Lyude wrote: > > > > Weine's investigation on benchmarking the suspend/resume process pointed > > out a lot of the time in suspend/resume is being spent reprobing. While > > the reprobing process is a lengthy one for good reason, we don't need to > > hold up the entire suspend/resume process while we wait for it to > > finish. Luckily as it turns out, we already trigger a full connector > > reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing in > > i915_drm_resume() entirely. > > > > This won't lead to less time spent resuming just yet since now the > > bottleneck will be waiting for the mode_config lock in > > drm_kms_helper_poll_enable(), since that will be held as long as > > i915_hpd_poll_init_work() is reprobing all of the connectors. But we'll > > address that in the next patch. > > > > Signed-off-by: Lyude <lyude@redhat.com> > > Cc: David Weinehall <david.weinehall@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index bfb2efd..532cc0f 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device *dev) > > * notifications. > > * */ > > intel_hpd_init(dev_priv); > > - /* Config may have changed between suspend and resume */ > > - drm_helper_hpd_irq_event(dev); > > The comment is still apt. This code is known to be broken since it > doesn't detect a change in monitors (e.g. a change in external connectors > from docking) between suspend and resend. We still have to send the uevent. > > + drm_kms_helper_hotplug_event(dev); I might not have explained myself very well. The way things should look with this patch is like this: i915_drm_resume() -> intel_hpd_init() -> sets dev_priv->hotplug.poll_enabled to true -> schedules dev_priv->hotplug.poll_init_work -> continue resume… at the same time: i915_hpd_poll_init_work() gets scheduled and starts -> since dev_priv->hotplug.poll_enabled == false, drm_helper_hpd_irq_event() is called -> drm_helper_hpd_irq_event() reprobes connectors -> if anything changed, drm_kms_helper_hotplug_event() gets called. So we're still polling the connectors when coming out of resume just like before, except now we're doing it without needlessly making the whole resume process stall until we're done. We're also no longer reprobing display connectors twice… > > which also depends upon us actually reseting the connector->status to > unknown in drm_mode_config_reset(), Daniel! > -Chris > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume @ 2016-11-03 16:11 ` Lyude Paul 0 siblings, 0 replies; 20+ messages in thread From: Lyude Paul @ 2016-11-03 16:11 UTC (permalink / raw) To: Chris Wilson, Lyude; +Cc: Daniel Vetter, intel-gfx, linux-kernel, dri-devel On Thu, 2016-11-03 at 16:02 +0000, Chris Wilson wrote: > On Thu, Nov 03, 2016 at 11:42:37AM -0400, Lyude wrote: > > > > Weine's investigation on benchmarking the suspend/resume process pointed > > out a lot of the time in suspend/resume is being spent reprobing. While > > the reprobing process is a lengthy one for good reason, we don't need to > > hold up the entire suspend/resume process while we wait for it to > > finish. Luckily as it turns out, we already trigger a full connector > > reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing in > > i915_drm_resume() entirely. > > > > This won't lead to less time spent resuming just yet since now the > > bottleneck will be waiting for the mode_config lock in > > drm_kms_helper_poll_enable(), since that will be held as long as > > i915_hpd_poll_init_work() is reprobing all of the connectors. But we'll > > address that in the next patch. > > > > Signed-off-by: Lyude <lyude@redhat.com> > > Cc: David Weinehall <david.weinehall@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index bfb2efd..532cc0f 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device *dev) > > * notifications. > > * */ > > intel_hpd_init(dev_priv); > > - /* Config may have changed between suspend and resume */ > > - drm_helper_hpd_irq_event(dev); > > The comment is still apt. This code is known to be broken since it > doesn't detect a change in monitors (e.g. a change in external connectors > from docking) between suspend and resend. We still have to send the uevent. > > + drm_kms_helper_hotplug_event(dev); I might not have explained myself very well. The way things should look with this patch is like this: i915_drm_resume() -> intel_hpd_init() -> sets dev_priv->hotplug.poll_enabled to true -> schedules dev_priv->hotplug.poll_init_work -> continue resume… at the same time: i915_hpd_poll_init_work() gets scheduled and starts -> since dev_priv->hotplug.poll_enabled == false, drm_helper_hpd_irq_event() is called -> drm_helper_hpd_irq_event() reprobes connectors -> if anything changed, drm_kms_helper_hotplug_event() gets called. So we're still polling the connectors when coming out of resume just like before, except now we're doing it without needlessly making the whole resume process stall until we're done. We're also no longer reprobing display connectors twice… > > which also depends upon us actually reseting the connector->status to > unknown in drm_mode_config_reset(), Daniel! > -Chris > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume 2016-11-03 16:11 ` Lyude Paul @ 2016-11-03 16:11 ` Lyude Paul -1 siblings, 0 replies; 20+ messages in thread From: Lyude Paul @ 2016-11-03 16:11 UTC (permalink / raw) To: Chris Wilson, Lyude; +Cc: Daniel Vetter, intel-gfx, linux-kernel, dri-devel On Thu, 2016-11-03 at 12:11 -0400, Lyude Paul wrote: > On Thu, 2016-11-03 at 16:02 +0000, Chris Wilson wrote: > > > > On Thu, Nov 03, 2016 at 11:42:37AM -0400, Lyude wrote: > > > > > > > > > Weine's investigation on benchmarking the suspend/resume process pointed > > > out a lot of the time in suspend/resume is being spent reprobing. While > > > the reprobing process is a lengthy one for good reason, we don't need to > > > hold up the entire suspend/resume process while we wait for it to > > > finish. Luckily as it turns out, we already trigger a full connector > > > reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing in > > > i915_drm_resume() entirely. > > > > > > This won't lead to less time spent resuming just yet since now the > > > bottleneck will be waiting for the mode_config lock in > > > drm_kms_helper_poll_enable(), since that will be held as long as > > > i915_hpd_poll_init_work() is reprobing all of the connectors. But we'll > > > address that in the next patch. > > > > > > Signed-off-by: Lyude <lyude@redhat.com> > > > Cc: David Weinehall <david.weinehall@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_drv.c | 2 -- > > > 1 file changed, 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > index bfb2efd..532cc0f 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device *dev) > > > * notifications. > > > * */ > > > intel_hpd_init(dev_priv); > > > - /* Config may have changed between suspend and resume */ > > > - drm_helper_hpd_irq_event(dev); > > > > The comment is still apt. This code is known to be broken since it > > doesn't detect a change in monitors (e.g. a change in external connectors > > from docking) between suspend and resend. We still have to send the uevent. > > > > + drm_kms_helper_hotplug_event(dev); > > I might not have explained myself very well. The way things should look with > this patch is like this: > > i915_drm_resume() > -> intel_hpd_init() > -> sets dev_priv->hotplug.poll_enabled to true Whoops, s/true/false/ > -> schedules dev_priv->hotplug.poll_init_work > -> continue resume… > > at the same time: > > i915_hpd_poll_init_work() gets scheduled and starts > -> since dev_priv->hotplug.poll_enabled == false, drm_helper_hpd_irq_event() > is called > -> drm_helper_hpd_irq_event() reprobes connectors > -> if anything changed, drm_kms_helper_hotplug_event() gets called. > > So we're still polling the connectors when coming out of resume just like > before, except now we're doing it without needlessly making the whole resume > process stall until we're done. We're also no longer reprobing display > connectors twice… > > > > > > > which also depends upon us actually reseting the connector->status to > > unknown in drm_mode_config_reset(), Daniel! > > -Chris > > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume @ 2016-11-03 16:11 ` Lyude Paul 0 siblings, 0 replies; 20+ messages in thread From: Lyude Paul @ 2016-11-03 16:11 UTC (permalink / raw) To: Chris Wilson, Lyude; +Cc: Daniel Vetter, intel-gfx, linux-kernel, dri-devel On Thu, 2016-11-03 at 12:11 -0400, Lyude Paul wrote: > On Thu, 2016-11-03 at 16:02 +0000, Chris Wilson wrote: > > > > On Thu, Nov 03, 2016 at 11:42:37AM -0400, Lyude wrote: > > > > > > > > > Weine's investigation on benchmarking the suspend/resume process pointed > > > out a lot of the time in suspend/resume is being spent reprobing. While > > > the reprobing process is a lengthy one for good reason, we don't need to > > > hold up the entire suspend/resume process while we wait for it to > > > finish. Luckily as it turns out, we already trigger a full connector > > > reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing in > > > i915_drm_resume() entirely. > > > > > > This won't lead to less time spent resuming just yet since now the > > > bottleneck will be waiting for the mode_config lock in > > > drm_kms_helper_poll_enable(), since that will be held as long as > > > i915_hpd_poll_init_work() is reprobing all of the connectors. But we'll > > > address that in the next patch. > > > > > > Signed-off-by: Lyude <lyude@redhat.com> > > > Cc: David Weinehall <david.weinehall@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_drv.c | 2 -- > > > 1 file changed, 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > index bfb2efd..532cc0f 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device *dev) > > > * notifications. > > > * */ > > > intel_hpd_init(dev_priv); > > > - /* Config may have changed between suspend and resume */ > > > - drm_helper_hpd_irq_event(dev); > > > > The comment is still apt. This code is known to be broken since it > > doesn't detect a change in monitors (e.g. a change in external connectors > > from docking) between suspend and resend. We still have to send the uevent. > > > > + drm_kms_helper_hotplug_event(dev); > > I might not have explained myself very well. The way things should look with > this patch is like this: > > i915_drm_resume() > -> intel_hpd_init() > -> sets dev_priv->hotplug.poll_enabled to true Whoops, s/true/false/ > -> schedules dev_priv->hotplug.poll_init_work > -> continue resume… > > at the same time: > > i915_hpd_poll_init_work() gets scheduled and starts > -> since dev_priv->hotplug.poll_enabled == false, drm_helper_hpd_irq_event() > is called > -> drm_helper_hpd_irq_event() reprobes connectors > -> if anything changed, drm_kms_helper_hotplug_event() gets called. > > So we're still polling the connectors when coming out of resume just like > before, except now we're doing it without needlessly making the whole resume > process stall until we're done. We're also no longer reprobing display > connectors twice… > > > > > > > which also depends upon us actually reseting the connector->status to > > unknown in drm_mode_config_reset(), Daniel! > > -Chris > > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume 2016-11-03 16:11 ` Lyude Paul @ 2016-11-03 16:25 ` Chris Wilson -1 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2016-11-03 16:25 UTC (permalink / raw) To: Lyude Paul; +Cc: Daniel Vetter, intel-gfx, Lyude, linux-kernel, dri-devel On Thu, Nov 03, 2016 at 12:11:55PM -0400, Lyude Paul wrote: > On Thu, 2016-11-03 at 12:11 -0400, Lyude Paul wrote: > > On Thu, 2016-11-03 at 16:02 +0000, Chris Wilson wrote: > > > > > > On Thu, Nov 03, 2016 at 11:42:37AM -0400, Lyude wrote: > > > > > > > > > > > > Weine's investigation on benchmarking the suspend/resume process pointed > > > > out a lot of the time in suspend/resume is being spent reprobing. While > > > > the reprobing process is a lengthy one for good reason, we don't need to > > > > hold up the entire suspend/resume process while we wait for it to > > > > finish. Luckily as it turns out, we already trigger a full connector > > > > reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing in > > > > i915_drm_resume() entirely. > > > > > > > > This won't lead to less time spent resuming just yet since now the > > > > bottleneck will be waiting for the mode_config lock in > > > > drm_kms_helper_poll_enable(), since that will be held as long as > > > > i915_hpd_poll_init_work() is reprobing all of the connectors. But we'll > > > > address that in the next patch. > > > > > > > > Signed-off-by: Lyude <lyude@redhat.com> > > > > Cc: David Weinehall <david.weinehall@linux.intel.com> > > > > --- > > > > drivers/gpu/drm/i915/i915_drv.c | 2 -- > > > > 1 file changed, 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > index bfb2efd..532cc0f 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device *dev) > > > > * notifications. > > > > * */ > > > > intel_hpd_init(dev_priv); > > > > - /* Config may have changed between suspend and resume */ > > > > - drm_helper_hpd_irq_event(dev); > > > > > > The comment is still apt. This code is known to be broken since it > > > doesn't detect a change in monitors (e.g. a change in external connectors > > > from docking) between suspend and resend. We still have to send the uevent. > > > > > > + drm_kms_helper_hotplug_event(dev); > > > > I might not have explained myself very well. The way things should look with > > this patch is like this: > > > > i915_drm_resume() > > -> intel_hpd_init() > > -> sets dev_priv->hotplug.poll_enabled to true > Whoops, s/true/false/ > > > -> schedules dev_priv->hotplug.poll_init_work > > -> continue resume… > > > > at the same time: > > > > i915_hpd_poll_init_work() gets scheduled and starts > > -> since dev_priv->hotplug.poll_enabled == false, drm_helper_hpd_irq_event() > > is called > > -> drm_helper_hpd_irq_event() reprobes connectors > > -> if anything changed, drm_kms_helper_hotplug_event() gets called. drm_helper_hpd_irq_event() does not detect a change in monitors, for example, so there is no uevent in that case. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume @ 2016-11-03 16:25 ` Chris Wilson 0 siblings, 0 replies; 20+ messages in thread From: Chris Wilson @ 2016-11-03 16:25 UTC (permalink / raw) To: Lyude Paul; +Cc: Lyude, Daniel Vetter, intel-gfx, linux-kernel, dri-devel On Thu, Nov 03, 2016 at 12:11:55PM -0400, Lyude Paul wrote: > On Thu, 2016-11-03 at 12:11 -0400, Lyude Paul wrote: > > On Thu, 2016-11-03 at 16:02 +0000, Chris Wilson wrote: > > > > > > On Thu, Nov 03, 2016 at 11:42:37AM -0400, Lyude wrote: > > > > > > > > > > > > Weine's investigation on benchmarking the suspend/resume process pointed > > > > out a lot of the time in suspend/resume is being spent reprobing. While > > > > the reprobing process is a lengthy one for good reason, we don't need to > > > > hold up the entire suspend/resume process while we wait for it to > > > > finish. Luckily as it turns out, we already trigger a full connector > > > > reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing in > > > > i915_drm_resume() entirely. > > > > > > > > This won't lead to less time spent resuming just yet since now the > > > > bottleneck will be waiting for the mode_config lock in > > > > drm_kms_helper_poll_enable(), since that will be held as long as > > > > i915_hpd_poll_init_work() is reprobing all of the connectors. But we'll > > > > address that in the next patch. > > > > > > > > Signed-off-by: Lyude <lyude@redhat.com> > > > > Cc: David Weinehall <david.weinehall@linux.intel.com> > > > > --- > > > > drivers/gpu/drm/i915/i915_drv.c | 2 -- > > > > 1 file changed, 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > index bfb2efd..532cc0f 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device *dev) > > > > * notifications. > > > > * */ > > > > intel_hpd_init(dev_priv); > > > > - /* Config may have changed between suspend and resume */ > > > > - drm_helper_hpd_irq_event(dev); > > > > > > The comment is still apt. This code is known to be broken since it > > > doesn't detect a change in monitors (e.g. a change in external connectors > > > from docking) between suspend and resend. We still have to send the uevent. > > > > > > + drm_kms_helper_hotplug_event(dev); > > > > I might not have explained myself very well. The way things should look with > > this patch is like this: > > > > i915_drm_resume() > > -> intel_hpd_init() > > -> sets dev_priv->hotplug.poll_enabled to true > Whoops, s/true/false/ > > > -> schedules dev_priv->hotplug.poll_init_work > > -> continue resume… > > > > at the same time: > > > > i915_hpd_poll_init_work() gets scheduled and starts > > -> since dev_priv->hotplug.poll_enabled == false, drm_helper_hpd_irq_event() > > is called > > -> drm_helper_hpd_irq_event() reprobes connectors > > -> if anything changed, drm_kms_helper_hotplug_event() gets called. drm_helper_hpd_irq_event() does not detect a change in monitors, for example, so there is no uevent in that case. -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume 2016-11-03 16:25 ` Chris Wilson @ 2016-11-03 16:42 ` Lyude Paul -1 siblings, 0 replies; 20+ messages in thread From: Lyude Paul @ 2016-11-03 16:42 UTC (permalink / raw) To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx, Lyude, linux-kernel, dri-devel On Thu, 2016-11-03 at 16:25 +0000, Chris Wilson wrote: > On Thu, Nov 03, 2016 at 12:11:55PM -0400, Lyude Paul wrote: > > > > On Thu, 2016-11-03 at 12:11 -0400, Lyude Paul wrote: > > > > > > On Thu, 2016-11-03 at 16:02 +0000, Chris Wilson wrote: > > > > > > > > > > > > On Thu, Nov 03, 2016 at 11:42:37AM -0400, Lyude wrote: > > > > > > > > > > > > > > > > > > > > Weine's investigation on benchmarking the suspend/resume process > > > > > pointed > > > > > out a lot of the time in suspend/resume is being spent reprobing. > > > > > While > > > > > the reprobing process is a lengthy one for good reason, we don't need > > > > > to > > > > > hold up the entire suspend/resume process while we wait for it to > > > > > finish. Luckily as it turns out, we already trigger a full connector > > > > > reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing > > > > > in > > > > > i915_drm_resume() entirely. > > > > > > > > > > This won't lead to less time spent resuming just yet since now the > > > > > bottleneck will be waiting for the mode_config lock in > > > > > drm_kms_helper_poll_enable(), since that will be held as long as > > > > > i915_hpd_poll_init_work() is reprobing all of the connectors. But > > > > > we'll > > > > > address that in the next patch. > > > > > > > > > > Signed-off-by: Lyude <lyude@redhat.com> > > > > > Cc: David Weinehall <david.weinehall@linux.intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/i915_drv.c | 2 -- > > > > > 1 file changed, 2 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > > index bfb2efd..532cc0f 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > > @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device > > > > > *dev) > > > > > * notifications. > > > > > * */ > > > > > intel_hpd_init(dev_priv); > > > > > - /* Config may have changed between suspend and resume */ > > > > > - drm_helper_hpd_irq_event(dev); > > > > > > > > The comment is still apt. This code is known to be broken since it > > > > doesn't detect a change in monitors (e.g. a change in external > > > > connectors > > > > from docking) between suspend and resend. We still have to send the > > > > uevent. > > > > > > > > + drm_kms_helper_hotplug_event(dev); > > > > > > I might not have explained myself very well. The way things should look > > > with > > > this patch is like this: > > > > > > i915_drm_resume() > > > -> intel_hpd_init() > > > -> sets dev_priv->hotplug.poll_enabled to true > > Whoops, s/true/false/ > > > > > > > > -> schedules dev_priv->hotplug.poll_init_work > > > -> continue resume… > > > > > > at the same time: > > > > > > i915_hpd_poll_init_work() gets scheduled and starts > > > -> since dev_priv->hotplug.poll_enabled == false, > > > drm_helper_hpd_irq_event() > > > is called > > > -> drm_helper_hpd_irq_event() reprobes connectors > > > -> if anything changed, drm_kms_helper_hotplug_event() gets called. > > drm_helper_hpd_irq_event() does not detect a change in monitors, for > example, so there is no uevent in that case. I still think you're mistaken (and I wouldn't blame you, drm_helper_hpd_irq_event() has a rather misleading name). Contents of drm_helper_hpd_irq_event() bool drm_helper_hpd_irq_event(struct drm_device *dev) { struct drm_connector *connector; enum drm_connector_status old_status; bool changed = false; if (!dev->mode_config.poll_enabled) return false; mutex_lock(&dev->mode_config.mutex); drm_for_each_connector(connector, dev) { /* Only handle HPD capable connectors. */ if (!(connector->polled & DRM_CONNECTOR_POLL_HPD)) continue; old_status = connector->status; connector->status = connector->funcs->detect(connector, false); DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n", connector->base.id, connector->name, drm_get_connector_status_name(old_status), drm_get_connector_status_name(connector->status)); if (old_status != connector->status) changed = true; } mutex_unlock(&dev->mode_config.mutex); if (changed) drm_kms_helper_hotplug_event(dev); return changed; } Unless I made a mistake somewhere down the line of writing these: poll_enabled will be set to true (due to the change done in the second patch where we move the call for enabling polling up so that it gets called before intel_hpd_init()), which causes it to go through all the connectors and call connector->funcs->detect() (which will be intel_dp_detect(), intel_hdmi_detect(), etc. And then send a uevent if their status changed. This being said there's a couple of cases I know we don't handle correctly from writing up igt tests with the chamelium: we don't simulate a disconnect + connect when the EDID changed from what we had last, so if we suspend the system with monitor A connected to port foo and resume it with monitor B connected to port foo, it will still think we're connected to monitor A. However, we don't handle this properly with the current code either. As for connector->funcs->detect(), it does do a real connector reprobe as opposed to just checking the status of the HPD pins, I'd look at the detect() functions if you're still skeptical. > -Chris > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume @ 2016-11-03 16:42 ` Lyude Paul 0 siblings, 0 replies; 20+ messages in thread From: Lyude Paul @ 2016-11-03 16:42 UTC (permalink / raw) To: Chris Wilson; +Cc: Lyude, Daniel Vetter, intel-gfx, linux-kernel, dri-devel On Thu, 2016-11-03 at 16:25 +0000, Chris Wilson wrote: > On Thu, Nov 03, 2016 at 12:11:55PM -0400, Lyude Paul wrote: > > > > On Thu, 2016-11-03 at 12:11 -0400, Lyude Paul wrote: > > > > > > On Thu, 2016-11-03 at 16:02 +0000, Chris Wilson wrote: > > > > > > > > > > > > On Thu, Nov 03, 2016 at 11:42:37AM -0400, Lyude wrote: > > > > > > > > > > > > > > > > > > > > Weine's investigation on benchmarking the suspend/resume process > > > > > pointed > > > > > out a lot of the time in suspend/resume is being spent reprobing. > > > > > While > > > > > the reprobing process is a lengthy one for good reason, we don't need > > > > > to > > > > > hold up the entire suspend/resume process while we wait for it to > > > > > finish. Luckily as it turns out, we already trigger a full connector > > > > > reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing > > > > > in > > > > > i915_drm_resume() entirely. > > > > > > > > > > This won't lead to less time spent resuming just yet since now the > > > > > bottleneck will be waiting for the mode_config lock in > > > > > drm_kms_helper_poll_enable(), since that will be held as long as > > > > > i915_hpd_poll_init_work() is reprobing all of the connectors. But > > > > > we'll > > > > > address that in the next patch. > > > > > > > > > > Signed-off-by: Lyude <lyude@redhat.com> > > > > > Cc: David Weinehall <david.weinehall@linux.intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/i915_drv.c | 2 -- > > > > > 1 file changed, 2 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > > index bfb2efd..532cc0f 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > > @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device > > > > > *dev) > > > > > * notifications. > > > > > * */ > > > > > intel_hpd_init(dev_priv); > > > > > - /* Config may have changed between suspend and resume */ > > > > > - drm_helper_hpd_irq_event(dev); > > > > > > > > The comment is still apt. This code is known to be broken since it > > > > doesn't detect a change in monitors (e.g. a change in external > > > > connectors > > > > from docking) between suspend and resend. We still have to send the > > > > uevent. > > > > > > > > + drm_kms_helper_hotplug_event(dev); > > > > > > I might not have explained myself very well. The way things should look > > > with > > > this patch is like this: > > > > > > i915_drm_resume() > > > -> intel_hpd_init() > > > -> sets dev_priv->hotplug.poll_enabled to true > > Whoops, s/true/false/ > > > > > > > > -> schedules dev_priv->hotplug.poll_init_work > > > -> continue resume… > > > > > > at the same time: > > > > > > i915_hpd_poll_init_work() gets scheduled and starts > > > -> since dev_priv->hotplug.poll_enabled == false, > > > drm_helper_hpd_irq_event() > > > is called > > > -> drm_helper_hpd_irq_event() reprobes connectors > > > -> if anything changed, drm_kms_helper_hotplug_event() gets called. > > drm_helper_hpd_irq_event() does not detect a change in monitors, for > example, so there is no uevent in that case. I still think you're mistaken (and I wouldn't blame you, drm_helper_hpd_irq_event() has a rather misleading name). Contents of drm_helper_hpd_irq_event() bool drm_helper_hpd_irq_event(struct drm_device *dev) { struct drm_connector *connector; enum drm_connector_status old_status; bool changed = false; if (!dev->mode_config.poll_enabled) return false; mutex_lock(&dev->mode_config.mutex); drm_for_each_connector(connector, dev) { /* Only handle HPD capable connectors. */ if (!(connector->polled & DRM_CONNECTOR_POLL_HPD)) continue; old_status = connector->status; connector->status = connector->funcs->detect(connector, false); DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n", connector->base.id, connector->name, drm_get_connector_status_name(old_status), drm_get_connector_status_name(connector->status)); if (old_status != connector->status) changed = true; } mutex_unlock(&dev->mode_config.mutex); if (changed) drm_kms_helper_hotplug_event(dev); return changed; } Unless I made a mistake somewhere down the line of writing these: poll_enabled will be set to true (due to the change done in the second patch where we move the call for enabling polling up so that it gets called before intel_hpd_init()), which causes it to go through all the connectors and call connector->funcs->detect() (which will be intel_dp_detect(), intel_hdmi_detect(), etc. And then send a uevent if their status changed. This being said there's a couple of cases I know we don't handle correctly from writing up igt tests with the chamelium: we don't simulate a disconnect + connect when the EDID changed from what we had last, so if we suspend the system with monitor A connected to port foo and resume it with monitor B connected to port foo, it will still think we're connected to monitor A. However, we don't handle this properly with the current code either. As for connector->funcs->detect(), it does do a real connector reprobe as opposed to just checking the status of the HPD pins, I'd look at the detect() functions if you're still skeptical. > -Chris > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume 2016-11-03 15:42 ` [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume Lyude @ 2016-11-03 18:50 ` David Weinehall 2016-11-03 18:50 ` [Intel-gfx] " David Weinehall 1 sibling, 0 replies; 20+ messages in thread From: David Weinehall @ 2016-11-03 18:50 UTC (permalink / raw) To: Lyude; +Cc: David Airlie, Daniel Vetter, intel-gfx, linux-kernel, dri-devel On Thu, Nov 03, 2016 at 11:42:37AM -0400, Lyude wrote: > Weine's investigation on benchmarking the suspend/resume process pointed > out a lot of the time in suspend/resume is being spent reprobing. While > the reprobing process is a lengthy one for good reason, we don't need to > hold up the entire suspend/resume process while we wait for it to > finish. Luckily as it turns out, we already trigger a full connector > reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing in > i915_drm_resume() entirely. > > This won't lead to less time spent resuming just yet since now the > bottleneck will be waiting for the mode_config lock in > drm_kms_helper_poll_enable(), since that will be held as long as > i915_hpd_poll_init_work() is reprobing all of the connectors. But we'll > address that in the next patch. > > Signed-off-by: Lyude <lyude@redhat.com> > Cc: David Weinehall <david.weinehall@linux.intel.com> Tested-by: David Weinehall <david.weinehall@linux.intel.com> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> Testcase: analyze_suspend.py -config config/suspend-callgraph.cfg -filter i915 > --- > drivers/gpu/drm/i915/i915_drv.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index bfb2efd..532cc0f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device *dev) > * notifications. > * */ > intel_hpd_init(dev_priv); > - /* Config may have changed between suspend and resume */ > - drm_helper_hpd_irq_event(dev); > > intel_opregion_register(dev_priv); > > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume @ 2016-11-03 18:50 ` David Weinehall 0 siblings, 0 replies; 20+ messages in thread From: David Weinehall @ 2016-11-03 18:50 UTC (permalink / raw) To: Lyude; +Cc: intel-gfx, David Airlie, linux-kernel, dri-devel, Daniel Vetter On Thu, Nov 03, 2016 at 11:42:37AM -0400, Lyude wrote: > Weine's investigation on benchmarking the suspend/resume process pointed > out a lot of the time in suspend/resume is being spent reprobing. While > the reprobing process is a lengthy one for good reason, we don't need to > hold up the entire suspend/resume process while we wait for it to > finish. Luckily as it turns out, we already trigger a full connector > reprobe in i915_hpd_poll_init_work(), so we can just ditch reprobing in > i915_drm_resume() entirely. > > This won't lead to less time spent resuming just yet since now the > bottleneck will be waiting for the mode_config lock in > drm_kms_helper_poll_enable(), since that will be held as long as > i915_hpd_poll_init_work() is reprobing all of the connectors. But we'll > address that in the next patch. > > Signed-off-by: Lyude <lyude@redhat.com> > Cc: David Weinehall <david.weinehall@linux.intel.com> Tested-by: David Weinehall <david.weinehall@linux.intel.com> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> Testcase: analyze_suspend.py -config config/suspend-callgraph.cfg -filter i915 > --- > drivers/gpu/drm/i915/i915_drv.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index bfb2efd..532cc0f 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1602,8 +1602,6 @@ static int i915_drm_resume(struct drm_device *dev) > * notifications. > * */ > intel_hpd_init(dev_priv); > - /* Config may have changed between suspend and resume */ > - drm_helper_hpd_irq_event(dev); > > intel_opregion_register(dev_priv); > > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] drm/i915: Reinit polling before hpd when resuming 2016-11-03 15:42 ` Lyude @ 2016-11-03 15:42 ` Lyude -1 siblings, 0 replies; 20+ messages in thread From: Lyude @ 2016-11-03 15:42 UTC (permalink / raw) To: intel-gfx; +Cc: Lyude, David Airlie, linux-kernel, dri-devel, Daniel Vetter Now that we don't run the connector reprobing from i915_drm_resume(), we need to make it so we don't have to wait for reprobing to finish so that we actually speed things up. In order to do this, we need to make sure that i915_drm_resume() doesn't get blocked by i915_hpd_poll_init_work() while trying to acquire the mode_config lock that drm_kms_helper_poll_enable() needs to acquire. The easiest way to do this is to just enable polling before hpd. This shouldn't break anything since at that point we have everything else we need for polling enabled. As well, this should result in a rather significant improvement in how quickly we can resume the system. Signed-off-by: Lyude <lyude@redhat.com> Cc: David Weinehall <david.weinehall@linux.intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 532cc0f..f605dde 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1595,6 +1595,8 @@ static int i915_drm_resume(struct drm_device *dev) intel_display_resume(dev); + drm_kms_helper_poll_enable(dev); + /* * ... but also need to make sure that hotplug processing * doesn't cause havoc. Like in the driver load code we don't @@ -1614,7 +1616,6 @@ static int i915_drm_resume(struct drm_device *dev) intel_opregion_notify_adapter(dev_priv, PCI_D0); intel_autoenable_gt_powersave(dev_priv); - drm_kms_helper_poll_enable(dev); enable_rpm_wakeref_asserts(dev_priv); -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/2] drm/i915: Reinit polling before hpd when resuming @ 2016-11-03 15:42 ` Lyude 0 siblings, 0 replies; 20+ messages in thread From: Lyude @ 2016-11-03 15:42 UTC (permalink / raw) To: intel-gfx Cc: Lyude, David Weinehall, Daniel Vetter, Jani Nikula, David Airlie, dri-devel, linux-kernel Now that we don't run the connector reprobing from i915_drm_resume(), we need to make it so we don't have to wait for reprobing to finish so that we actually speed things up. In order to do this, we need to make sure that i915_drm_resume() doesn't get blocked by i915_hpd_poll_init_work() while trying to acquire the mode_config lock that drm_kms_helper_poll_enable() needs to acquire. The easiest way to do this is to just enable polling before hpd. This shouldn't break anything since at that point we have everything else we need for polling enabled. As well, this should result in a rather significant improvement in how quickly we can resume the system. Signed-off-by: Lyude <lyude@redhat.com> Cc: David Weinehall <david.weinehall@linux.intel.com> --- drivers/gpu/drm/i915/i915_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 532cc0f..f605dde 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1595,6 +1595,8 @@ static int i915_drm_resume(struct drm_device *dev) intel_display_resume(dev); + drm_kms_helper_poll_enable(dev); + /* * ... but also need to make sure that hotplug processing * doesn't cause havoc. Like in the driver load code we don't @@ -1614,7 +1616,6 @@ static int i915_drm_resume(struct drm_device *dev) intel_opregion_notify_adapter(dev_priv, PCI_D0); intel_autoenable_gt_powersave(dev_priv); - drm_kms_helper_poll_enable(dev); enable_rpm_wakeref_asserts(dev_priv); -- 2.7.4 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] drm/i915: Reinit polling before hpd when resuming 2016-11-03 15:42 ` Lyude @ 2016-11-03 18:50 ` David Weinehall -1 siblings, 0 replies; 20+ messages in thread From: David Weinehall @ 2016-11-03 18:50 UTC (permalink / raw) To: Lyude; +Cc: David Airlie, Daniel Vetter, intel-gfx, linux-kernel, dri-devel On Thu, Nov 03, 2016 at 11:42:38AM -0400, Lyude wrote: > Now that we don't run the connector reprobing from i915_drm_resume(), we > need to make it so we don't have to wait for reprobing to finish so that > we actually speed things up. In order to do this, we need to make sure > that i915_drm_resume() doesn't get blocked by i915_hpd_poll_init_work() > while trying to acquire the mode_config lock that > drm_kms_helper_poll_enable() needs to acquire. > > The easiest way to do this is to just enable polling before hpd. This > shouldn't break anything since at that point we have everything else we > need for polling enabled. > > As well, this should result in a rather significant improvement in how > quickly we can resume the system. > > Signed-off-by: Lyude <lyude@redhat.com> > Cc: David Weinehall <david.weinehall@linux.intel.com> Tested-by: David Weinehall <david.weinehall@linux.intel.com> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> Testcase: analyze_suspend.py -config config/suspend-callgraph.cfg -filter i915 > --- > drivers/gpu/drm/i915/i915_drv.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 532cc0f..f605dde 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1595,6 +1595,8 @@ static int i915_drm_resume(struct drm_device *dev) > > intel_display_resume(dev); > > + drm_kms_helper_poll_enable(dev); > + > /* > * ... but also need to make sure that hotplug processing > * doesn't cause havoc. Like in the driver load code we don't > @@ -1614,7 +1616,6 @@ static int i915_drm_resume(struct drm_device *dev) > intel_opregion_notify_adapter(dev_priv, PCI_D0); > > intel_autoenable_gt_powersave(dev_priv); > - drm_kms_helper_poll_enable(dev); > > enable_rpm_wakeref_asserts(dev_priv); > > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] drm/i915: Reinit polling before hpd when resuming @ 2016-11-03 18:50 ` David Weinehall 0 siblings, 0 replies; 20+ messages in thread From: David Weinehall @ 2016-11-03 18:50 UTC (permalink / raw) To: Lyude; +Cc: intel-gfx, David Airlie, linux-kernel, dri-devel, Daniel Vetter On Thu, Nov 03, 2016 at 11:42:38AM -0400, Lyude wrote: > Now that we don't run the connector reprobing from i915_drm_resume(), we > need to make it so we don't have to wait for reprobing to finish so that > we actually speed things up. In order to do this, we need to make sure > that i915_drm_resume() doesn't get blocked by i915_hpd_poll_init_work() > while trying to acquire the mode_config lock that > drm_kms_helper_poll_enable() needs to acquire. > > The easiest way to do this is to just enable polling before hpd. This > shouldn't break anything since at that point we have everything else we > need for polling enabled. > > As well, this should result in a rather significant improvement in how > quickly we can resume the system. > > Signed-off-by: Lyude <lyude@redhat.com> > Cc: David Weinehall <david.weinehall@linux.intel.com> Tested-by: David Weinehall <david.weinehall@linux.intel.com> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> Testcase: analyze_suspend.py -config config/suspend-callgraph.cfg -filter i915 > --- > drivers/gpu/drm/i915/i915_drv.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 532cc0f..f605dde 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1595,6 +1595,8 @@ static int i915_drm_resume(struct drm_device *dev) > > intel_display_resume(dev); > > + drm_kms_helper_poll_enable(dev); > + > /* > * ... but also need to make sure that hotplug processing > * doesn't cause havoc. Like in the driver load code we don't > @@ -1614,7 +1616,6 @@ static int i915_drm_resume(struct drm_device *dev) > intel_opregion_notify_adapter(dev_priv, PCI_D0); > > intel_autoenable_gt_powersave(dev_priv); > - drm_kms_helper_poll_enable(dev); > > enable_rpm_wakeref_asserts(dev_priv); > > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
* ✓ Fi.CI.BAT: success for Small fixes to speed up resume 2016-11-03 15:42 ` Lyude ` (2 preceding siblings ...) (?) @ 2016-11-03 16:16 ` Patchwork -1 siblings, 0 replies; 20+ messages in thread From: Patchwork @ 2016-11-03 16:16 UTC (permalink / raw) To: Lyude Paul; +Cc: intel-gfx == Series Details == Series: Small fixes to speed up resume URL : https://patchwork.freedesktop.org/series/14787/ State : success == Summary == Series 14787v1 Small fixes to speed up resume https://patchwork.freedesktop.org/api/1.0/series/14787/revisions/1/mbox/ fi-bdw-5557u total:241 pass:226 dwarn:0 dfail:0 fail:0 skip:15 fi-bsw-n3050 total:241 pass:201 dwarn:0 dfail:0 fail:0 skip:40 fi-bxt-t5700 total:241 pass:213 dwarn:0 dfail:0 fail:0 skip:28 fi-byt-j1900 total:241 pass:213 dwarn:0 dfail:0 fail:0 skip:28 fi-byt-n2820 total:241 pass:209 dwarn:0 dfail:0 fail:0 skip:32 fi-hsw-4770 total:241 pass:221 dwarn:0 dfail:0 fail:0 skip:20 fi-hsw-4770r total:241 pass:221 dwarn:0 dfail:0 fail:0 skip:20 fi-ilk-650 total:241 pass:188 dwarn:0 dfail:0 fail:0 skip:53 fi-ivb-3520m total:241 pass:219 dwarn:0 dfail:0 fail:0 skip:22 fi-ivb-3770 total:241 pass:219 dwarn:0 dfail:0 fail:0 skip:22 fi-kbl-7200u total:241 pass:219 dwarn:0 dfail:0 fail:0 skip:22 fi-skl-6260u total:241 pass:227 dwarn:0 dfail:0 fail:0 skip:14 fi-skl-6700hq total:241 pass:220 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6700k total:241 pass:219 dwarn:1 dfail:0 fail:0 skip:21 fi-skl-6770hq total:241 pass:227 dwarn:0 dfail:0 fail:0 skip:14 fi-snb-2520m total:241 pass:209 dwarn:0 dfail:0 fail:0 skip:32 fi-snb-2600 total:241 pass:208 dwarn:0 dfail:0 fail:0 skip:33 8a1c897ee1d0d93b8e70991becb6f9f8488dba1b drm-intel-nightly: 2016y-11m-03d-13h-53m-22s UTC integration manifest 643b3b8 drm/i915: Reinit polling before hpd when resuming 3f36aca drm/i915: Remove redundant reprobe in i915_drm_resume == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2898/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-11-03 18:56 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-11-03 15:42 [PATCH 0/2] Small fixes to speed up resume Lyude 2016-11-03 15:42 ` Lyude 2016-11-03 15:42 ` [PATCH 1/2] drm/i915: Remove redundant reprobe in i915_drm_resume Lyude 2016-11-03 16:02 ` [Intel-gfx] " Chris Wilson 2016-11-03 16:02 ` Chris Wilson 2016-11-03 16:11 ` Lyude Paul 2016-11-03 16:11 ` Lyude Paul 2016-11-03 16:11 ` Lyude Paul 2016-11-03 16:11 ` Lyude Paul 2016-11-03 16:25 ` Chris Wilson 2016-11-03 16:25 ` Chris Wilson 2016-11-03 16:42 ` Lyude Paul 2016-11-03 16:42 ` [Intel-gfx] " Lyude Paul 2016-11-03 18:50 ` David Weinehall 2016-11-03 18:50 ` [Intel-gfx] " David Weinehall 2016-11-03 15:42 ` [PATCH 2/2] drm/i915: Reinit polling before hpd when resuming Lyude 2016-11-03 15:42 ` Lyude 2016-11-03 18:50 ` David Weinehall 2016-11-03 18:50 ` [Intel-gfx] " David Weinehall 2016-11-03 16:16 ` ✓ Fi.CI.BAT: success for Small fixes to speed up resume Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.