On Tue, Feb 14, 2017 at 11:00 AM, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > On Tue, Feb 14, 2017 at 10:48:10AM -0800, Palmer Dabbelt wrote: >> On Tue, 14 Feb 2017 07:01:54 PST (-0800), ville.syrjala@xxxxxxxxxxxxxxx wrote: >> > On Fri, Feb 10, 2017 at 02:44:20PM -0800, Palmer Dabbelt wrote: >> >> DisplayPort no longer hotplugs on my machine (a 2014 MacBook Pro >> >> attached to an ASUS PB287Q). I believe the problem is that long pulses >> >> are no longer triggering intel_dp_check_link_status. >> > >> > That shouldn't itsefl cause problems with hotplugs. It could cause >> > problems if you hotplug displays without a proper randr client running >> > in which case the link is left running when you unplug the displays and >> > then gets retrained when you plug a display back in. >> >> Maybe it's a problem with my setup, as I don't think I have any randr client >> running: I just run xrandr via my xinitrc and via some scripts linked from udev >> for DRM hotplug events for when I move the laptop around. > > That should more or less work. Well, depends on what you do in your > udev scripts. But that's pretty much what your typical xrandr clients > do, except the udev event gets first converted into a randr event by the > X server. > >> Is it required that >> I have more stuff running in order to make DPMS events work? > > Not sure what you mean by DPMS events. "xset dpms" just turns things > on/off, there are no events involved really. I guess my "dpms event" I meant "the monitor going on and off". >> IIRC my setup has >> looked like this for years now, but if it's broken because it's wacky then I >> can change it. >> >> Regardless, I think something is still broken here. I don't need to actaully >> unplug anything for this to break, if I just run "xset dpms force off" to turn >> the screen off and then wake it back up my eDP laptop display comes back fine >> but my DP monitor doesn't. > > That is definitely a bug somewhere. Can you boot your machine with > drm.debug=0xe passed to the kernel, then reproduce this dpms problem, > and then post the full dmesg. Maybe the log will show something > interesting. Attached. There's two copies: one from after I boot and start X, and the second from after I do a DPMS cycle: $ xset dpms force off $ sleep 10s # wait for the monitor to go to sleep # press ctrl to wake the monitor back up One item of interest: when I ran this on my work monitor (a newer Dell DisplayPort monitor) I didn't have any DPMS problems, but when I ran it on my ASUS PB287Q the monitor doesn't come back from sleep. >> I'd expect eDP and DP to behave the same here. >> IIRC the link doesn't come back up even if I twiddle some xrandr stuff, but I >> might getting this confused with the previous similar bug. If you want I can >> boot back into the broken kernel and run some experiments, but it'd be great if >> you could suggest some for me. >> >> > That doesn't explain why intel_dp_check_link_status() wouldn't get >> > called though. It should be called via intel_dp_detect() -> >> > intel_dp_long_pulse(). >> >> Sorry, I don't actually know anything about DRM so I'm not sure what's going >> on. I just generated this patch by looking at the bug from last time my DP >> monitor stopped working, looking at the last few things that touched that >> function, and then adding back in the thing that looks like it would make link >> detection work again :). > > Well, it should already work. Somehting is hinky. > >> >> The last time I did this I got it wrong as well -- looking at >> <https://bugs.freedesktop.org/show_bug.cgi?id=89453> I see that I suggested >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index d714a4b5711e..274bd293d9e9 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -4625,7 +4625,9 @@ static const struct drm_encoder_funcs intel_dp_enc_funcs = { >> void >> intel_dp_hot_plug(struct intel_encoder *intel_encoder) >> { >> - return; >> + struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base); >> + >> + intel_dp_check_link_status(intel_dp); >> } >> >> enum irqreturn >> >> but the correct solution (d14e7b6d1 "drm/i915: Check DP link status on long hpd >> too") was very different >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 3781cd3..94686cb 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -4961,9 +4961,12 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) >> >> intel_dp_probe_oui(intel_dp); >> >> - if (!intel_dp_probe_mst(intel_dp)) >> + if (!intel_dp_probe_mst(intel_dp)) { >> + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); >> + intel_dp_check_link_status(intel_dp); >> + drm_modeset_unlock(&dev->mode_config.connection_mutex); >> goto mst_fail; >> - >> + } >> >> my fix for this new bug (which might just be another workaround) was copied >> from the correct fix from last time. >> >> > >> >> I bisected the >> >> problem down to (7d23e3c37 "drm/i915: Cleaning up intel_dp_hpd_pulse") >> >> and it appears the only material change there was to remove one of those >> >> calls. >> >> >> >> This commit adds the check_link_status call back in, which causes hotplug to >> >> work again for me. I test this via a "xset dpms force off". Note that this is >> >> very similar to <https://bugs.freedesktop.org/show_bug.cgi?id=89453>, but it's >> >> back again. >> >> >> >> I've tested this against 4.9, and it applies against the current head. >> >> >> >> See <https://bugs.freedesktop.org/show_bug.cgi?id=99766>. >> >> >> >> Here's the commit that triggered the regression: >> >> >> >> commit 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c >> >> Author: Shubhangi Shrivastava <shubhangi.shrivastava@xxxxxxxxx> >> >> Date: Wed Mar 30 18:05:23 2016 +0530 >> >> >> >> drm/i915: Cleaning up intel_dp_hpd_pulse >> >> >> >> Current DP detection has DPCD operations split across >> >> intel_dp_hpd_pulse and intel_dp_detect which contains >> >> duplicates as well. Also intel_dp_detect is called >> >> during modes enumeration as well which will result >> >> in multiple dpcd operations. So this patch tries >> >> to solve both these by bringing all DPCD operations >> >> in one single function and make intel_dp_detect >> >> use existing values instead of repeating same steps. >> >> >> >> Signed-off-by: Palmer Dabbelt <palmer@xxxxxxxxxxx> >> >> --- >> >> drivers/gpu/drm/i915/intel_dp.c | 4 ++++ >> >> 1 file changed, 4 insertions(+) >> >> >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> >> index 0b8e8eb..32ca4be 100644 >> >> --- a/drivers/gpu/drm/i915/intel_dp.c >> >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> >> @@ -4831,6 +4831,10 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) >> >> long_hpd ? "long" : "short"); >> >> >> >> if (long_hpd) { >> >> + drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); >> >> + intel_dp_check_link_status(intel_dp); >> >> + drm_modeset_unlock(&dev->mode_config.connection_mutex); >> >> + >> >> intel_dp->detect_done = false; >> >> return IRQ_NONE; >> >> } >> >> -- >> >> 2.10.2 >> >> >> >> _______________________________________________ >> >> Intel-gfx mailing list >> >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC
Attachment:
dmesg-001-boot.gz
Description: GNU Zip compressed data
Attachment:
dmesg-002-xset_dpms_force_off.gz
Description: GNU Zip compressed data
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
