[Intel-gfx] [PATCH] drm/i915: Fix DisplayPort Hotplug
Ville Syrjälä
ville.syrjala at linux.intel.com
Tue Feb 14 19:00:54 UTC 2017
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 at linux.intel.com 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.
> 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.
> 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 at intel.com>
> >> 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 at dabbelt.com>
> >> ---
> >> 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 at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
More information about the Intel-gfx
mailing list