Re: [PATCH] drm/i915: Fix DisplayPort Hotplug

[Palmer Dabbelt <palmer@dabbelt.com>]

On Tue, 14 Feb 2017 07:01:54 PST (-0800), ville.syrjala@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.  Is it required that
I have more stuff running in order to make DPMS events work?  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.  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 :).

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@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@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@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