* [PATCH 1/2] drm/i915: Call intel_dp_mst_resume() before resuming displays [not found] <1457711822-20335-1-git-send-email-cpaul@redhat.com> @ 2016-03-11 15:57 ` Lyude 2016-03-13 18:45 ` [Intel-gfx] " Daniel Vetter 2016-03-11 15:57 ` [PATCH 2/2] drm/i915: Retry after 30ms if we fail to resume DP MST Lyude 1 sibling, 1 reply; 7+ messages in thread From: Lyude @ 2016-03-11 15:57 UTC (permalink / raw) To: intel-gfx Cc: Lyude, stable, Daniel Vetter, Jani Nikula, David Airlie, open list:INTEL DRM DRIVERS (excluding Poulsbo, Moorestow...), linux-kernel@vger.kernel.org (open list) Since we need MST devices ready before we try to resume displays, calling this after intel_display_resume() can result in some issues with various laptop docks where the monitor won't turn back on after suspending the system. This order was originally changed in commit e7d6f7d70829 ("drm/i915: resume MST after reading back hw state") In order to fix some unclaimed register errors, however the actual cause of those has since been fixed. CC: stable@vger.kernel.org Signed-off-by: Lyude <cpaul@redhat.com> --- drivers/gpu/drm/i915/i915_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index f357058..08854ae 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -761,12 +761,12 @@ static int i915_drm_resume(struct drm_device *dev) dev_priv->display.hpd_irq_setup(dev); spin_unlock_irq(&dev_priv->irq_lock); + intel_dp_mst_resume(dev); + drm_modeset_lock_all(dev); intel_display_resume(dev); drm_modeset_unlock_all(dev); - intel_dp_mst_resume(dev); - /* * ... but also need to make sure that hotplug processing * doesn't cause havoc. Like in the driver load code we don't -- 2.5.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Call intel_dp_mst_resume() before resuming displays 2016-03-11 15:57 ` [PATCH 1/2] drm/i915: Call intel_dp_mst_resume() before resuming displays Lyude @ 2016-03-13 18:45 ` Daniel Vetter 2016-03-16 21:49 ` Lyude Paul 0 siblings, 1 reply; 7+ messages in thread From: Daniel Vetter @ 2016-03-13 18:45 UTC (permalink / raw) To: Lyude Cc: intel-gfx, David Airlie, stable, open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list, Daniel Vetter On Fri, Mar 11, 2016 at 10:57:01AM -0500, Lyude wrote: > Since we need MST devices ready before we try to resume displays, > calling this after intel_display_resume() can result in some issues with > various laptop docks where the monitor won't turn back on after > suspending the system. > > This order was originally changed in > > commit e7d6f7d70829 ("drm/i915: resume MST after reading back hw state") > > In order to fix some unclaimed register errors, however the actual cause > of those has since been fixed. > > CC: stable@vger.kernel.org > Signed-off-by: Lyude <cpaul@redhat.com> Don't we need to first apply patch 2/2 to avoid breaking systems in-between? -Daniel > --- > drivers/gpu/drm/i915/i915_drv.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index f357058..08854ae 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -761,12 +761,12 @@ static int i915_drm_resume(struct drm_device *dev) > dev_priv->display.hpd_irq_setup(dev); > spin_unlock_irq(&dev_priv->irq_lock); > > + intel_dp_mst_resume(dev); > + > drm_modeset_lock_all(dev); > intel_display_resume(dev); > drm_modeset_unlock_all(dev); > > - intel_dp_mst_resume(dev); > - > /* > * ... but also need to make sure that hotplug processing > * doesn't cause havoc. Like in the driver load code we don't > -- > 2.5.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Call intel_dp_mst_resume() before resuming displays 2016-03-13 18:45 ` [Intel-gfx] " Daniel Vetter @ 2016-03-16 21:49 ` Lyude Paul 2016-03-29 14:11 ` Lyude Paul 0 siblings, 1 reply; 7+ messages in thread From: Lyude Paul @ 2016-03-16 21:49 UTC (permalink / raw) To: Daniel Vetter Cc: intel-gfx, David Airlie, stable, open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list, Daniel Vetter On Sun, 2016-03-13 at 19:45 +0100, Daniel Vetter wrote: > On Fri, Mar 11, 2016 at 10:57:01AM -0500, Lyude wrote: > > > > Since we need MST devices ready before we try to resume displays, > > calling this after intel_display_resume() can result in some issues with > > various laptop docks where the monitor won't turn back on after > > suspending the system. > > > > This order was originally changed in > > > > commit e7d6f7d70829 ("drm/i915: resume MST after reading back hw state") > > > > In order to fix some unclaimed register errors, however the actual cause > > of those has since been fixed. > > > > CC: stable@vger.kernel.org > > Signed-off-by: Lyude <cpaul@redhat.com> > Don't we need to first apply patch 2/2 to avoid breaking systems > in-between? > -Daniel AFAICT the warns don't appear even with this patch, so no. > > > > > --- > > drivers/gpu/drm/i915/i915_drv.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c > > index f357058..08854ae 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -761,12 +761,12 @@ static int i915_drm_resume(struct drm_device *dev) > > dev_priv->display.hpd_irq_setup(dev); > > spin_unlock_irq(&dev_priv->irq_lock); > > > > + intel_dp_mst_resume(dev); > > + > > drm_modeset_lock_all(dev); > > intel_display_resume(dev); > > drm_modeset_unlock_all(dev); > > > > - intel_dp_mst_resume(dev); > > - > > /* > > * ... but also need to make sure that hotplug processing > > * doesn't cause havoc. Like in the driver load code we don't > > -- > > 2.5.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Call intel_dp_mst_resume() before resuming displays 2016-03-16 21:49 ` Lyude Paul @ 2016-03-29 14:11 ` Lyude Paul 2016-03-30 6:23 ` Daniel Vetter 0 siblings, 1 reply; 7+ messages in thread From: Lyude Paul @ 2016-03-29 14:11 UTC (permalink / raw) To: Daniel Vetter Cc: David Airlie, Daniel Vetter, intel-gfx, open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list, stable bump Could we get a reviewed-by for this patch? It's needed in addition to the patch series I sent for removing intel_dp_dpcd_read_wake() for the T560 to have it's monitors work properly on resume. On Wed, 2016-03-16 at 17:49 -0400, Lyude Paul wrote: > On Sun, 2016-03-13 at 19:45 +0100, Daniel Vetter wrote: > > > > On Fri, Mar 11, 2016 at 10:57:01AM -0500, Lyude wrote: > > > > > > > > > Since we need MST devices ready before we try to resume displays, > > > calling this after intel_display_resume() can result in some issues with > > > various laptop docks where the monitor won't turn back on after > > > suspending the system. > > > > > > This order was originally changed in > > > > > > commit e7d6f7d70829 ("drm/i915: resume MST after reading back hw state") > > > > > > In order to fix some unclaimed register errors, however the actual cause > > > of those has since been fixed. > > > > > > CC: stable@vger.kernel.org > > > Signed-off-by: Lyude <cpaul@redhat.com> > > Don't we need to first apply patch 2/2 to avoid breaking systems > > in-between? > > -Daniel > AFAICT the warns don't appear even with this patch, so no. > > > > > > > > > > > > > --- > > > drivers/gpu/drm/i915/i915_drv.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > b/drivers/gpu/drm/i915/i915_drv.c > > > index f357058..08854ae 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -761,12 +761,12 @@ static int i915_drm_resume(struct drm_device *dev) > > > dev_priv->display.hpd_irq_setup(dev); > > > spin_unlock_irq(&dev_priv->irq_lock); > > > > > > + intel_dp_mst_resume(dev); > > > + > > > drm_modeset_lock_all(dev); > > > intel_display_resume(dev); > > > drm_modeset_unlock_all(dev); > > > > > > - intel_dp_mst_resume(dev); > > > - > > > /* > > > * ... but also need to make sure that hotplug processing > > > * doesn't cause havoc. Like in the driver load code we don't > > > -- > > > 2.5.0 > > > > > > _______________________________________________ > > > 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 -- Cheers, Lyude ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915: Call intel_dp_mst_resume() before resuming displays 2016-03-29 14:11 ` Lyude Paul @ 2016-03-30 6:23 ` Daniel Vetter 0 siblings, 0 replies; 7+ messages in thread From: Daniel Vetter @ 2016-03-30 6:23 UTC (permalink / raw) To: Lyude Paul Cc: Daniel Vetter, David Airlie, Daniel Vetter, intel-gfx, open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list, stable On Tue, Mar 29, 2016 at 10:11:54AM -0400, Lyude Paul wrote: > bump > > Could we get a reviewed-by for this patch? It's needed in addition to the patch > series I sent for removing intel_dp_dpcd_read_wake() for the T560 to have it's > monitors work properly on resume. Applied, thanks. -Daniel > > On Wed, 2016-03-16 at 17:49 -0400, Lyude Paul wrote: > > On Sun, 2016-03-13 at 19:45 +0100, Daniel Vetter wrote: > > > > > > On Fri, Mar 11, 2016 at 10:57:01AM -0500, Lyude wrote: > > > > > > > > > > > > Since we need MST devices ready before we try to resume displays, > > > > calling this after intel_display_resume() can result in some issues with > > > > various laptop docks where the monitor won't turn back on after > > > > suspending the system. > > > > > > > > This order was originally changed in > > > > > > > > commit e7d6f7d70829 ("drm/i915: resume MST after reading back hw state") > > > > > > > > In order to fix some unclaimed register errors, however the actual cause > > > > of those has since been fixed. > > > > > > > > CC: stable@vger.kernel.org > > > > Signed-off-by: Lyude <cpaul@redhat.com> > > > Don't we need to first apply patch 2/2 to avoid breaking systems > > > in-between? > > > -Daniel > > AFAICT the warns don't appear even with this patch, so no. > > > > > > > > > > > > > > > > > > --- > > > > �drivers/gpu/drm/i915/i915_drv.c | 4 ++-- > > > > �1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > > > b/drivers/gpu/drm/i915/i915_drv.c > > > > index f357058..08854ae 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > > @@ -761,12 +761,12 @@ static int i915_drm_resume(struct drm_device *dev) > > > > � dev_priv->display.hpd_irq_setup(dev); > > > > � spin_unlock_irq(&dev_priv->irq_lock); > > > > � > > > > + intel_dp_mst_resume(dev); > > > > + > > > > � drm_modeset_lock_all(dev); > > > > � intel_display_resume(dev); > > > > � drm_modeset_unlock_all(dev); > > > > � > > > > - intel_dp_mst_resume(dev); > > > > - > > > > � /* > > > > � �* ... but also need to make sure that hotplug processing > > > > � �* doesn't cause havoc. Like in the driver load code we don't > > > > --� > > > > 2.5.0 > > > > > > > > _______________________________________________ > > > > 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 > -- > Cheers, > Lyude > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] drm/i915: Retry after 30ms if we fail to resume DP MST [not found] <1457711822-20335-1-git-send-email-cpaul@redhat.com> 2016-03-11 15:57 ` [PATCH 1/2] drm/i915: Call intel_dp_mst_resume() before resuming displays Lyude @ 2016-03-11 15:57 ` Lyude 2016-03-13 18:44 ` Daniel Vetter 1 sibling, 1 reply; 7+ messages in thread From: Lyude @ 2016-03-11 15:57 UTC (permalink / raw) To: intel-gfx Cc: Lyude, stable, Daniel Vetter, Jani Nikula, David Airlie, open list:INTEL DRM DRIVERS (excluding Poulsbo, Moorestow...), linux-kernel@vger.kernel.org (open list) For whatever reason, I've found that some laptops aren't immediately capable of doing aux transactions with their docks when they come out of standby. While I'm still not entirely sure what the cause of this is, sleeping for 30ms and then retrying drm_dp_mst_topology_mgr_resume() should be a sufficient enough workaround until we find a real fix. CC: stable@vger.kernel.org Signed-off-by: Lyude <cpaul@redhat.com> --- drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 1d8de43..8cc5f6f 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -6114,6 +6114,19 @@ void intel_dp_mst_resume(struct drm_device *dev) ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr); if (ret != 0) { + /* + * For some reason, some laptops can't bring + * their MST docks back up immediately after + * resume and need to wait a short period of + * time before aux transactions with the dock + * become functional again. Until we find a + * proper fix for this, this workaround should + * suffice + */ + msleep(30); + ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr); + } + if (ret != 0) { intel_dp_check_mst_status(&intel_dig_port->dp); } } -- 2.5.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] drm/i915: Retry after 30ms if we fail to resume DP MST 2016-03-11 15:57 ` [PATCH 2/2] drm/i915: Retry after 30ms if we fail to resume DP MST Lyude @ 2016-03-13 18:44 ` Daniel Vetter 0 siblings, 0 replies; 7+ messages in thread From: Daniel Vetter @ 2016-03-13 18:44 UTC (permalink / raw) To: Lyude Cc: intel-gfx, stable, open list:INTEL DRM DRIVERS excluding Poulsbo, Moorestow..., linux-kernel@vger.kernel.org open list, Daniel Vetter On Fri, Mar 11, 2016 at 10:57:02AM -0500, Lyude wrote: > For whatever reason, I've found that some laptops aren't immediately > capable of doing aux transactions with their docks when they come out of > standby. While I'm still not entirely sure what the cause of this is, > sleeping for 30ms and then retrying drm_dp_mst_topology_mgr_resume() > should be a sufficient enough workaround until we find a real fix. > > CC: stable@vger.kernel.org > Signed-off-by: Lyude <cpaul@redhat.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 1d8de43..8cc5f6f 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -6114,6 +6114,19 @@ void intel_dp_mst_resume(struct drm_device *dev) > > ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr); > if (ret != 0) { > + /* > + * For some reason, some laptops can't bring > + * their MST docks back up immediately after > + * resume and need to wait a short period of > + * time before aux transactions with the dock > + * become functional again. Until we find a > + * proper fix for this, this workaround should > + * suffice > + */ > + msleep(30); > + ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr); > + } Hm, since it's the dp aux that fails (and not something higher up apparently) shouldnt' we have this massive retry somewhere in the dp aux helpers maybe? DP resume in general is a bit fragile, maybe we're just missing a lot of retries in general? Either way this needs a lot more details. Comment definitely should start out with FIXME, and the commit message should have a protocol of all the experiments you've done thus far. Yes this means a ridiculously long commit message, but in roughly 2 weeks someone else will go wtf on this, and then they must be able to read up the full story. And we need links to bugzillas and mail threads, too. And please Cc: Art with this one too. Thanks, Daniel > + if (ret != 0) { > intel_dp_check_mst_status(&intel_dig_port->dp); > } > } > -- > 2.5.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-03-30 6:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1457711822-20335-1-git-send-email-cpaul@redhat.com>
2016-03-11 15:57 ` [PATCH 1/2] drm/i915: Call intel_dp_mst_resume() before resuming displays Lyude
2016-03-13 18:45 ` [Intel-gfx] " Daniel Vetter
2016-03-16 21:49 ` Lyude Paul
2016-03-29 14:11 ` Lyude Paul
2016-03-30 6:23 ` Daniel Vetter
2016-03-11 15:57 ` [PATCH 2/2] drm/i915: Retry after 30ms if we fail to resume DP MST Lyude
2016-03-13 18:44 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).