Re: [PATCH] drm/i915/dp: clear and restore MSTM_CTRL around MST DPMS off/on

[Marco Nenciarini <mnencia@kcore.it>]

I hit the problem the automated review pointed out. It did not cause a
black screen here, but the MST topology was torn down during DPMS off.

Same setup as my Tested-by (Dell Pro Max 16, WD19DC, two monitors, i915
over TBT-alt, drm.debug=0x10e), now on Debian kernel 7.2.9 with the
patch applied.

I ran one DPMS off/on cycle each of 30 s, 60 s and 120 s. The 30 s and
60 s runs were clean: no connector probe during the off period, link
and topology kept. In the 120 s run i915 runtime-suspended 14 s into
the DPMS off and resumed 5 s later. The resume probed every connector,
with no user space probe at that point. intel_dp_detect() read
MSTM_CTRL as 0 (the value written by the new clear in
intel_ddi_post_disable_dp()):

  intel_dp_detect: MST support: port: yes, sink: MST, modparam: yes
    -> enable: MST
  drm_dp_dpcd_read: AUX USBC4/DDI TC4/PHY TC4: 0x00111 AUX
    -> (ret= 1) 00
  intel_dp_mst_verify_dpcd_state: [CONNECTOR:554:DP-4]
    [ENCODER:553:DDI TC4/PHY TC4] MST mode got reset, removing
    topology (ret=1, ctrl=0x00)

The topology was removed and the three MST connectors DP-5, DP-6 and
DP-7 were unregistered. The hotplug event made user space probe again
4 s later. That wrote 0x111 <- 07, sent CLEAR_PAYLOAD_ID_TABLE and
LINK_ADDRESS, and the topology came back as DP-5, DP-10 and DP-11.
MSTM_CTRL then stayed set while the hub was in D3, for the remaining
98 s, and the wake was still normal: link training passed at 810000 x4
on the sink and both LTTPRs, payloads allocated, both monitors
enabled. GNOME dropped the external monitors and added them back while
blanked, and logged logical monitor assertion failures.

My earlier 120 s run for the Tested-by did not hit this. It needs a
connector probe while MSTM_CTRL is 0. The patch does not change
intel_dp_mst_verify_dpcd_state(). Its only caller is
intel_dp_detect(), and it expects MST_EN | UP_REQ_EN |
UPSTREAM_IS_SRC whenever is_mst is set.

One way to avoid it: accept MSTM_CTRL == 0 in that function when no
MST stream is active, since that is the window where the patch clears
it on purpose, and the re-enable writes it again. No new flag is
needed. I have not tried it. I can test a v2 with that or any other
approach.

The log of the 120 s run is available if you want it.

Marco