On Tue, Jan 05, 2016 at 09:05:49PM +0100, Daniel Vetter wrote: > On Tue, Jan 05, 2016 at 02:53:08PM -0500, Lyude wrote: > > There's no reason to check for an active encoder when setting up hpd, > > especially since a disconnected connector isn't going to have an encoder > > in the first place. This fixes reprobing display connectors on resume. > > > > Signed-off-by: Lyude <cpaul at redhat.com> > > --- > > After some talking with danvet on IRC, I learned that calling > > drm_helper_hpd_irq_event() does actually trigger a full reprobe of each > > connector's status. It turns out this is the actual reason reprobing on > > resume > > hasn't been working (this was observed on a T440s): > > > > - We call hpd_init() > > - We check each connector for a couple of things before marking > > connector->polled with DRM_CONNECTOR_POLL_HPD, one of which is an > > active encoder. Of course, a disconnected port won't have an active > > encoder, so we don't add the flag to any of the connector's. > > - We call drm_helper_hpd_irq_event() > > - drm_helper_irq_event() checks each connector for the > > DRM_CONNECTOR_POLL_HPD flag. The only one that has it is eDP-1, so we > > skip reprobing each connector except that one. > > The above explanation should be part of the commit message, otherwise it's > hard to understand the impact of this fix. > > Fixes: 0e32b39ceed6 ("drm/i915: add DP 1.2 MST support (v0.7)") > Cc: stable at vger.kernel.org > > Unfortunately Dave doesn't explain in his MST commit why he's done this. > Dave? I can't come up with a reason, but better to test that. Lyude? > > Anyway looks good to me. > > Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
Need to retract my r-b. MST connectors don't have a fixed fake encoder, and the intel_connector->encoder link is done at link time and not static. We need to filter out MST connectors here, otherwise this will go boom. While at it, can you please line-break this properly to appease checkpatch? -Daniel > > > > > drivers/gpu/drm/i915/intel_hotplug.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c > > b/drivers/gpu/drm/i915/intel_hotplug.c > > index b177857..18797bc 100644 > > --- a/drivers/gpu/drm/i915/intel_hotplug.c > > +++ b/drivers/gpu/drm/i915/intel_hotplug.c > > @@ -468,7 +468,7 @@ void intel_hpd_init(struct drm_i915_private *dev_priv) > > list_for_each_entry(connector, &mode_config->connector_list, head) { > > struct intel_connector *intel_connector = > > to_intel_connector(connector); > > connector->polled = intel_connector->polled; > > - if (connector->encoder && !connector->polled && > > I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE) > > + if (!connector->polled && I915_HAS_HOTPLUG(dev) && > > intel_connector->encoder->hpd_pin > HPD_NONE) > > connector->polled = DRM_CONNECTOR_POLL_HPD; > > if (intel_connector->mst_port) > > connector->polled = DRM_CONNECTOR_POLL_HPD; > > -- > > 2.5.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx at lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch