Skip to content

K
Kernel

openeuler / Kernel
1 年多 前同步成功

通知 8
Star 0
Fork 0
K
Kernel
切换分支/标签
  1. 28 4月, 2020 1 次提交
    • L
      drm/dp_mst: Kill the second sideband tx slot, save the world · d308a881
      由 Lyude Paul 提交于 
      While we support using both tx slots for sideband transmissions, it
appears that DisplayPort devices in the field didn't end up doing a very
good job of supporting it. From section 5.2.1 of the DP 2.0
specification:

  There are MST Sink/Branch devices in the field that do not handle
  interleaved message transactions.

  To facilitate message transaction handling by downstream devices, an
  MST Source device shall generate message transactions in an atomic
  manner (i.e., the MST Source device shall not concurrently interleave
  multiple message transactions). Therefore, an MST Source device shall
  clear the Message_Sequence_No value in the Sideband_MSG_Header to 0.

This might come as a bit of a surprise since the vast majority of hubs
will support using both tx slots even if they don't support interleaved
message transactions, and we've also been using both tx slots since MST
was introduced into the kernel.

However, there is one device we've had trouble getting working
consistently with MST for so long that we actually assumed it was just
broken: the infamous Dell P2415Qb. Previously this monitor would appear
to work sometimes, but in most situations would end up timing out
LINK_ADDRESS messages almost at random until you power cycled the whole
display. After reading section 5.2.1 in the DP 2.0 spec, some closer
investigation into this infamous display revealed it was only ever
timing out on sideband messages in the second TX slot.

Sure enough, avoiding the second TX slot has suddenly made this monitor
function perfectly for the first time in five years. And since they
explicitly mention this in the specification, I doubt this is the only
monitor out there with this issue. This might even explain explain the
seemingly harmless garbage sideband responses we would occasionally see
with MST hubs!

So - rewrite our sideband TX handlers to only support one TX slot. In
order to simplify our sideband handling now that we don't support
transmitting to multiple MSTBs at once, we also move all state tracking
for down replies from mstbs to the topology manager.
Signed-off-by: NLyude Paul <lyude@redhat.com>
Fixes: ad7f8a1f ("drm/helper: add Displayport multi-stream helper (v0.6)")
Cc: Sean Paul <sean@poorly.run>
Cc: "Lin, Wayne" <Wayne.Lin@amd.com>
Cc: <stable@vger.kernel.org> # v3.17+
Reviewed-by: NSean Paul <sean@poorly.run>
Link: https://patchwork.freedesktop.org/patch/msgid/20200424181308.770749-1-lyude@redhat.com
      d308a881
  3. 24 4月, 2020 1 次提交
    • L
      Revert "drm/dp_mst: Remove single tx msg restriction." · 973a5909
      由 Lyude Paul 提交于 
      This reverts commit 6bb0942e.

Unfortunately it would appear that the rumors we've heard of sideband
message interleaving not being very well supported are true. On the
Lenovo ThinkPad Thunderbolt 3 dock that I have, interleaved messages
appear to just get dropped:

  [drm:drm_dp_mst_wait_tx_reply [drm_kms_helper]] timedout msg send
  00000000571ddfd0 2 1
  [dp_mst] txmsg cur_offset=2 cur_len=2 seqno=1 state=SENT path_msg=1 dst=00
  [dp_mst] 	type=ENUM_PATH_RESOURCES contents:
  [dp_mst] 		port=2

DP descriptor for this hub:
  OUI 90-cc-24 dev-ID SYNA3  HW-rev 1.0 SW-rev 3.12 quirks 0x0008

It would seem like as well that this is a somewhat well known issue in
the field. From section 5.4.2 of the DisplayPort 2.0 specification:

  There are MST Sink/Branch devices in the field that do not handle
  interleaved message transactions.

  To facilitate message transaction handling by downstream devices, an
  MST Source device shall generate message transactions in an atomic
  manner (i.e., the MST Source device shall not concurrently interleave
  multiple message transactions). Therefore, an MST Source device shall
  clear the Message_Sequence_No value in the Sideband_MSG_Header to 0.

  MST Source devices that support field policy updates by way of
  software should update the policy to forego the generation of
  interleaved message transactions.

This is a bit disappointing, as features like HDCP require that we send
a sideband request every ~2 seconds for each active stream. However,
there isn't really anything in the specification that allows us to
accurately probe for interleaved messages.

If it ends up being that we -really- need this in the future, we might
be able to whitelist hubs where interleaving is known to work-or maybe
try some sort of heuristics. But for now, let's just play it safe and
not use it.
Signed-off-by: NLyude Paul <lyude@redhat.com>
Fixes: 6bb0942e ("drm/dp_mst: Remove single tx msg restriction.")
Cc: Wayne Lin <Wayne.Lin@amd.com>
Cc: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200423164225.680178-1-lyude@redhat.comReviewed-by: NSean Paul <sean@poorly.run>
      973a5909
  5. 10 4月, 2020 4 次提交
  7. 08 4月, 2020 1 次提交
    • L
      drm/dp_mst: Remove drm_dp_mst_has_audio() · 20c22ad3
      由 Lyude Paul 提交于 
      Drive-by fix I noticed the other day - drm_dp_mst_has_audio() only ever
made sense back when we still had to validate ports before accessing
them in order to (attempt to) avoid NULL dereferences. Since we have
proper reference counting that guarantees we always can safely access
the MST port, there's no use in keeping this function around as all it
does is validate the port pointer before checking the audio status.

Note - drm_dp_mst_port->has_audio is technically protected by
drm_device->mode_config.connection_mutex, since it's only ever updated
from drm_dp_mst_get_edid(). Additionally, we change the declaration for
port in struct intel_connector to be properly typed, so we can directly
access it.

Changes since v1:
* Change type of intel_connector->port in a separate patch - Sean Paul

Cc: "Lee, Shawn C" <shawn.c.lee@intel.com>
Reviewed-by: NSean Paul <sean@poorly.run>
Signed-off-by: NLyude Paul <lyude@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200406200646.1263435-2-lyude@redhat.com
      20c22ad3
  9. 07 4月, 2020 2 次提交
    • L
      drm/dp_mst: Don't drop NAKs for down responses · 61272e47
      由 Lyude Paul 提交于 
      It looks like that when we introduced the ability to handle multiple
down requests at once, we accidentally started dropping NAK replies -
causing sideband messages which got NAK'd to seemingly timeout and cause
all sorts of weirdness.

So, fix this by making sure we don't return from
drm_dp_mst_handle_down_rep() early, but instead treat NAKs like any
other message.
Signed-off-by: NLyude Paul <lyude@redhat.com>
Fixes: fbc821c4 ("drm/mst: Support simultaneous down replies")
Cc: Wayne Lin <Wayne.Lin@amd.com>
Cc: Wayne Lin <waynelin@amd.com>
Cc: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200403200325.885628-1-lyude@redhat.comReviewed-by: NSean Paul <sean@poorly.run>
      61272e47
    • L
      drm/dp_mst: Fix NULL deref in drm_dp_get_one_sb_msg() · cbfb1b74
      由 Lyude Paul 提交于 
      While we don't need this function to store an mstb anywhere for UP
requests since we process them asynchronously, we do need to make sure
that we don't try to write to **mstb for UP requests otherwise we'll
cause a NULL pointer deref:

    RIP: 0010:drm_dp_get_one_sb_msg+0x4b/0x460 [drm_kms_helper]
    Call Trace:
     ? vprintk_emit+0x16a/0x230
     ? drm_dp_mst_hpd_irq+0x133/0x1010 [drm_kms_helper]
     drm_dp_mst_hpd_irq+0x133/0x1010 [drm_kms_helper]
     ? __drm_dbg+0x87/0x90 [drm]
     ? intel_dp_hpd_pulse+0x24b/0x400 [i915]
     intel_dp_hpd_pulse+0x24b/0x400 [i915]
     i915_digport_work_func+0xd6/0x160 [i915]
     process_one_work+0x1a9/0x370
     worker_thread+0x4d/0x3a0
     kthread+0xf9/0x130
     ? process_one_work+0x370/0x370
     ? kthread_park+0x90/0x90
     ret_from_fork+0x35/0x40

So, fix this.
Signed-off-by: NLyude Paul <lyude@redhat.com>
Fixes: fbc821c4 ("drm/mst: Support simultaneous down replies")
Cc: Wayne Lin <Wayne.Lin@amd.com>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Wayne Lin <waynelin@amd.com>
Cc: Sean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200406193352.1245985-1-lyude@redhat.comReviewed-by: NSean Paul <sean@poorly.run>
      cbfb1b74
  11. 04 4月, 2020 1 次提交
  13. 01 4月, 2020 1 次提交
  15. 28 3月, 2020 3 次提交
  17. 13 3月, 2020 4 次提交
    • L
      drm/dp_mst: Rewrite and fix bandwidth limit checks · 047d4cd2
      由 Lyude Paul 提交于 
      Sigh, this is mostly my fault for not giving commit cd82d82c
("drm/dp_mst: Add branch bandwidth validation to MST atomic check")
enough scrutiny during review. The way we're checking bandwidth
limitations here is mostly wrong:

For starters, drm_dp_mst_atomic_check_bw_limit() determines the
pbn_limit of a branch by simply scanning each port on the current branch
device, then uses the last non-zero full_pbn value that it finds. It
then counts the sum of the PBN used on each branch device for that
level, and compares against the full_pbn value it found before.

This is wrong because ports can and will have different PBN limitations
on many hubs, especially since a number of DisplayPort hubs out there
will be clever and only use the smallest link rate required for each
downstream sink - potentially giving every port a different full_pbn
value depending on what link rate it's trained at. This means with our
current code, which max PBN value we end up with is not well defined.

Additionally, we also need to remember when checking bandwidth
limitations that the top-most device in any MST topology is a branch
device, not a port. This means that the first level of a topology
doesn't technically have a full_pbn value that needs to be checked.
Instead, we should assume that so long as our VCPI allocations fit we're
within the bandwidth limitations of the primary MSTB.

We do however, want to check full_pbn on every port including those of
the primary MSTB. However, it's important to keep in mind that this
value represents the minimum link rate /between a port's sink or mstb,
and the mstb itself/. A quick diagram to explain:

                                MSTB #1
                               /       \
                              /         \
                           Port #1    Port #2
       full_pbn for Port #1 → |          | ← full_pbn for Port #2
                           Sink #1    MSTB #2
                                         |
                                       etc...

Note that in the above diagram, the combined PBN from all VCPI
allocations on said hub should not exceed the full_pbn value of port #2,
and the display configuration on sink #1 should not exceed the full_pbn
value of port #1. However, port #1 and port #2 can otherwise consume as
much bandwidth as they want so long as their VCPI allocations still fit.

And finally - our current bandwidth checking code also makes the mistake
of not checking whether something is an end device or not before trying
to traverse down it.

So, let's fix it by rewriting our bandwidth checking helpers. We split
the function into one part for handling branches which simply adds up
the total PBN on each branch and returns it, and one for checking each
port to ensure we're not going over its PBN limit. Phew.

This should fix regressions seen, where we erroneously reject display
configurations due to thinking they're going over our bandwidth limits
when they're not.

Changes since v1:
* Took an even closer look at how PBN limitations are supposed to be
  handled, and did some experimenting with Sean Paul. Ended up rewriting
  these helpers again, but this time they should actually be correct!
Changes since v2:
* Small indenting fix
* Fix pbn_used check in drm_dp_mst_atomic_check_port_bw_limit()
Signed-off-by: NLyude Paul <lyude@redhat.com>
Fixes: cd82d82c ("drm/dp_mst: Add branch bandwidth validation to MST atomic check")
Cc: Sean Paul <seanpaul@google.com>
Acked-by: NAlex Deucher <alexander.deucher@amd.com>
Reviewed-by: NMikita Lipski <mikita.lipski@amd.com>
Tested-by: NHans de Goede <hdegoede@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200309210131.1497545-1-lyude@redhat.com
      047d4cd2
    • L
      drm/dp_mst: Reprobe path resources in CSN handler · 87212b51
      由 Lyude Paul 提交于 
      We used to punt off reprobing path resources to the link address probe
work, but now that we handle CSNs asynchronously from the driver's HPD
handling we can do whatever the heck we want from the CSN!

So, reprobe the path resources from drm_dp_mst_handle_conn_stat(). Also,
get rid of the path resource reprobing code in
drm_dp_check_and_send_link_address() since it's needlessly complicated
when we already reprobe path resources from
drm_dp_handle_link_address_port(). And finally, teach
drm_dp_send_enum_path_resources() to return 1 on PBN changes so we know
if we need to send another hotplug or not.

This fixes issues where we've indicated to userspace that a port has
just been connected, before we actually probed it's available PBN -
something that results in unexpected atomic check failures.
Signed-off-by: NLyude Paul <lyude@redhat.com>
Fixes: cd82d82c ("drm/dp_mst: Add branch bandwidth validation to MST atomic check")
Cc: Mikita Lipski <mikita.lipski@amd.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Sean Paul <sean@poorly.run>
Link: https://patchwork.freedesktop.org/patch/msgid/20200306234623.547525-4-lyude@redhat.comReviewed-by: NAlex Deucher <alexander.deucher@amd.com>
Tested-by: NHans de Goede <hdegoede@redhat.com>
      87212b51
    • L
      drm/dp_mst: Use full_pbn instead of available_pbn for bandwidth checks · fcf46380
      由 Lyude Paul 提交于 
      DisplayPort specifications are fun. For a while, it's been really
unclear to us what available_pbn actually does. There's a somewhat vague
explanation in the DisplayPort spec (starting from 1.2) that partially
explains it:

  The minimum payload bandwidth number supported by the path. Each node
  updates this number with its available payload bandwidth number if its
  payload bandwidth number is less than that in the Message Transaction
  reply.

So, it sounds like available_pbn represents the smallest link rate in
use between the source and the branch device. Cool, so full_pbn is just
the highest possible PBN that the branch device supports right?

Well, we assumed that for quite a while until Sean Paul noticed that on
some MST hubs, available_pbn will actually get set to 0 whenever there's
any active payloads on the respective branch device. This caused quite a
bit of confusion since clearing the payload ID table would end up fixing
the available_pbn value.

So, we just went with that until commit cd82d82c ("drm/dp_mst: Add
branch bandwidth validation to MST atomic check") started breaking
people's setups due to us getting erroneous available_pbn values. So, we
did some more digging and got confused until we finally looked at the
definition for full_pbn:

  The bandwidth of the link at the trained link rate and lane count
  between the DP Source device and the DP Sink device with no time slots
  allocated to VC Payloads, represented as a Payload Bandwidth Number. As
  with the Available_Payload_Bandwidth_Number, this number is determined
  by the link with the lowest lane count and link rate.

That's what we get for not reading specs closely enough, hehe. So, since
full_pbn is definitely what we want for doing bandwidth restriction
checks - let's start using that instead and ignore available_pbn
entirely.
Signed-off-by: NLyude Paul <lyude@redhat.com>
Fixes: cd82d82c ("drm/dp_mst: Add branch bandwidth validation to MST atomic check")
Cc: Mikita Lipski <mikita.lipski@amd.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Sean Paul <sean@poorly.run>
Reviewed-by: NMikita Lipski <mikita.lipski@amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200306234623.547525-3-lyude@redhat.comReviewed-by: NAlex Deucher <alexander.deucher@amd.com>
Tested-by: NHans de Goede <hdegoede@redhat.com>
      fcf46380
    • L
      drm/dp_mst: Rename drm_dp_mst_is_dp_mst_end_device() to be less redundant · b2feb1d6
      由 Lyude Paul 提交于 
      It's already prefixed by dp_mst, so we don't really need to repeat
ourselves here. One of the changes I should have picked up originally
when reviewing MST DSC support.

There should be no functional changes here

Cc: Mikita Lipski <mikita.lipski@amd.com>
Cc: Sean Paul <seanpaul@google.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: NLyude Paul <lyude@redhat.com>
Reviewed-by: NAlex Deucher <alexander.deucher@amd.com>
Tested-by: NHans de Goede <hdegoede@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200306234623.547525-2-lyude@redhat.com
      b2feb1d6
  19. 12 3月, 2020 2 次提交
  21. 11 3月, 2020 2 次提交
  23. 04 3月, 2020 1 次提交
    • L
      drm/dp: Introduce EDID-based quirks · 0883ce81
      由 Lyude Paul 提交于 
      The whole point of using OUIs is so that we can recognize certain
devices and potentially apply quirks for them. Normally this should work
quite well, but there appears to be quite a number of laptop panels out
there that will fill the OUI but not the device ID. As such, for devices
like this I can't imagine it's a very good idea to try relying on OUIs
for applying quirks. As well, some laptop vendors have confirmed to us
that their panels have this exact issue.

So, let's introduce the ability to apply DP quirks based on EDID
identification. We reuse the same quirk bits for OUI-based quirks, so
that callers can simply check all possible quirks using
drm_dp_has_quirk().
Signed-off-by: NLyude Paul <lyude@redhat.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: NAdam Jackson <ajax@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200211183358.157448-2-lyude@redhat.com
      0883ce81
  25. 28 2月, 2020 2 次提交
  27. 31 1月, 2020 1 次提交
  29. 23 1月, 2020 2 次提交
    • L
      drm/dp_mst: Fix clearing payload state on topology disable · 8732fe46
      由 Lyude Paul 提交于 
      The issues caused by:

commit 64e62bdf ("drm/dp_mst: Remove VCPI while disabling topology
mgr")

Prompted me to take a closer look at how we clear the payload state in
general when disabling the topology, and it turns out there's actually
two subtle issues here.

The first is that we're not grabbing &mgr.payload_lock when clearing the
payloads in drm_dp_mst_topology_mgr_set_mst(). Seeing as the canonical
lock order is &mgr.payload_lock -> &mgr.lock (because we always want
&mgr.lock to be the inner-most lock so topology validation always
works), this makes perfect sense. It also means that -technically- there
could be racing between someone calling
drm_dp_mst_topology_mgr_set_mst() to disable the topology, along with a
modeset occurring that's modifying the payload state at the same time.

The second is the more obvious issue that Wayne Lin discovered, that
we're not clearing proposed_payloads when disabling the topology.

I actually can't see any obvious places where the racing caused by the
first issue would break something, and it could be that some of our
higher-level locks already prevent this by happenstance, but better safe
then sorry. So, let's make it so that drm_dp_mst_topology_mgr_set_mst()
first grabs &mgr.payload_lock followed by &mgr.lock so that we never
race when modifying the payload state. Then, we also clear
proposed_payloads to fix the original issue of enabling a new topology
with a dirty payload state. This doesn't clear any of the drm_dp_vcpi
structures, but those are getting destroyed along with the ports anyway.

Changes since v1:
* Use sizeof(mgr->payloads[0])/sizeof(mgr->proposed_vcpis[0]) instead -
  vsyrjala

Cc: Sean Paul <sean@poorly.run>
Cc: Wayne Lin <Wayne.Lin@amd.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: stable@vger.kernel.org # v4.4+
Signed-off-by: NLyude Paul <lyude@redhat.com>
Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200122194321.14953-1-lyude@redhat.com
      8732fe46
    • L
      drm/dp_mst: Fix indenting in drm_dp_mst_topology_mgr_set_mst() · 3ff4c24b
      由 Lyude Paul 提交于 
      This has always bugged me but somehow I've never remembered to actually
fix it. So let's do that.
Signed-off-by: NLyude Paul <lyude@redhat.com>
Reviewed-by: NVille Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200117224749.128994-1-lyude@redhat.com
      3ff4c24b
  31. 22 1月, 2020 1 次提交
  33. 18 1月, 2020 4 次提交
    • W
      drm/dp_mst: Handle SST-only branch device case · db1a0795
      由 Wayne Lin 提交于 
      [Why]
While handling LINK_ADDRESS reply, current code expects a peer device
can handle sideband message once the peer device type is reported as
DP_PEER_DEVICE_MST_BRANCHING. However, when the connected device is
a SST branch case, it can't handle the sideband message(MST_CAP=0 in
DPCD 00021h).

Current code will try to send LINK_ADDRESS to SST branch device and end
up with message timeout and monitor can't display normally. As the
result of that, we should take SST branch device into account.

[How]
According to DP 1.4 spec, we can use Peer_Device_Type as
DP_PEER_DEVICE_MST_BRANCHING and Message_Capability_Status as 0 to
indicate peer device as a SST-only branch device.

Fix following:
- Add the function drm_dp_mst_is_dp_mst_end_device() to decide whether a
peer device connected to a DFP is mst end device. Which also indicates
if the peer device is capable of handling message or not.
- Take SST-only branch device case into account in
drm_dp_port_set_pdt() and add a new parameter 'new_mcs'. Take sst branch
device case as the same case as DP_PEER_DEVICE_DP_LEGACY_CONV and
DP_PEER_DEVICE_SST_SINK. All original handling logics remain.
- Take SST-only branch device case into account in
drm_dp_mst_port_add_connector().
- Fix some parts in drm_dp_mst_handle_link_address_port() to have SST
branch device case into consideration.
- Fix the arguments of drm_dp_port_set_pdt() in
drm_dp_mst_handle_conn_stat().
- Have SST branch device also report
connector_status_connected when the ddps is true
in drm_dp_mst_detect_port()
- Fix the arguments of drm_dp_port_set_pdt() in
drm_dp_delayed_destroy_port()

Changes since v1:(https://patchwork.kernel.org/patch/11323079/)
* Squash previous patch into one patch and merge the commit message here.
* Combine the if statements mentioned in comments

Fixes: c485e2c9 ("drm/dp_mst: Refactor pdt setup/teardown, add more locking")
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Harry Wentland <hwentlan@amd.com>
Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: NWayne Lin <Wayne.Lin@amd.com>
Reviewed-by: NLyude Paul <lyude@redhat.com>
Signed-off-by: NLyude Paul <lyude@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200117060350.26358-2-Wayne.Lin@amd.com
      db1a0795
    • J
      drm/mst: Some style improvements in drm_dp_mst_topology_mgr_set_mst() · 7a3cbf59
      由 José Roberto de Souza 提交于 
      Removing this lose code block and removing unnecessary bracket.

Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: NJosé Roberto de Souza <jose.souza@intel.com>
Signed-off-by: NLyude Paul <lyude@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200117015837.402239-2-jose.souza@intel.com
      7a3cbf59
    • J
      drm/mst: Don't do atomic checks over disabled managers · 7b199143
      由 José Roberto de Souza 提交于 
      When a main MST port is disconnected drivers should call
drm_dp_mst_topology_mgr_set_mst() disabling the MST manager, this
function will set manager mst_primary to NULL and it will cause the
crash bellow on the next atomic check when trying to access
mst_primary->port.

As there is no use in running checks over managers that are not
active this patch will skip it.

[  305.616450] [drm:drm_dp_mst_atomic_check] [MST PORT:00000000cc2049e9] releases all VCPI slots
[  305.625085] [drm:drm_dp_mst_atomic_check] [MST PORT:00000000020ab43e] releases all VCPI slots
[  305.633729] [drm:drm_dp_mst_atomic_check] [MST MGR:00000000cdd467d4] mst state 00000000b67672eb VCPI avail=63 used=0
[  305.644405] BUG: kernel NULL pointer dereference, address: 0000000000000030
[  305.651448] #PF: supervisor read access in kernel mode
[  305.656640] #PF: error_code(0x0000) - not-present page
[  305.661807] PGD 0 P4D 0
[  305.664396] Oops: 0000 [#1] PREEMPT SMP NOPTI
[  305.668789] CPU: 3 PID: 183 Comm: kworker/3:2 Not tainted 5.5.0-rc6+ #1404
[  305.675703] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP TLC, BIOS ICLSFWR1.R00.3201.A00.1905140358 05/14/2019
[  305.689425] Workqueue: events drm_dp_delayed_destroy_work
[  305.694874] RIP: 0010:drm_dp_mst_atomic_check+0x138/0x2c0
[  305.700306] Code: 00 00 00 41 29 d9 41 89 d8 4c 89 fa 4c 89 f1 48 c7 c6 b0 b1 34 82 bf 10 00 00 00 45 31 ed e8 3f 99 02 00 4d 8b bf 80 04 00 00 <49> 8b 47 30 49 8d 5f 30 4c 8d 60 e8 48 39 c3 74 35 49 8b 7c 24 28
[  305.719169] RSP: 0018:ffffc90001687b58 EFLAGS: 00010246
[  305.724434] RAX: 0000000000000000 RBX: 000000000000003f RCX: 0000000000000000
[  305.731611] RDX: 0000000000000000 RSI: ffff88849fba8cb8 RDI: 00000000ffffffff
[  305.738785] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000001
[  305.745962] R10: ffffc900016879a0 R11: ffffc900016879a5 R12: 0000000000000000
[  305.753139] R13: 0000000000000000 R14: ffff8884905c9bc0 R15: 0000000000000000
[  305.760315] FS:  0000000000000000(0000) GS:ffff88849fb80000(0000) knlGS:0000000000000000
[  305.768452] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  305.774263] CR2: 0000000000000030 CR3: 0000000005610006 CR4: 0000000000760ee0
[  305.781441] PKRU: 55555554
[  305.784228] Call Trace:
[  305.786739]  intel_atomic_check+0xb2e/0x2560 [i915]
[  305.791678]  ? printk+0x53/0x6a
[  305.794856]  ? drm_atomic_check_only+0x3e/0x810
[  305.799417]  ? __drm_dbg+0x82/0x90
[  305.802848]  drm_atomic_check_only+0x56a/0x810
[  305.807322]  drm_atomic_commit+0xe/0x50
[  305.811185]  drm_client_modeset_commit_atomic+0x1e2/0x250
[  305.816619]  drm_client_modeset_commit_force+0x4d/0x180
[  305.821921]  drm_fb_helper_restore_fbdev_mode_unlocked+0x46/0xa0
[  305.827963]  drm_fb_helper_set_par+0x2b/0x40
[  305.832265]  drm_fb_helper_hotplug_event.part.0+0xb2/0xd0
[  305.837755]  drm_kms_helper_hotplug_event+0x21/0x30
[  305.842694]  process_one_work+0x25b/0x5b0
[  305.846735]  worker_thread+0x4b/0x3b0
[  305.850439]  kthread+0x100/0x140
[  305.853690]  ? process_one_work+0x5b0/0x5b0
[  305.857901]  ? kthread_park+0x80/0x80
[  305.861588]  ret_from_fork+0x24/0x50
[  305.865202] Modules linked in: snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic i915 btusb btrtl btbcm btintel bluetooth prime_numbers snd_hda_intel snd_intel_dspcfg snd_hda_codec e1000e snd_hwdep snd_hda_core thunderbolt mei_hdcp mei_me asix cdc_ether x86_pkg_temp_thermal r8152 mei coretemp usbnet snd_pcm mii crct10dif_pclmul ptp crc32_pclmul ecdh_generic ghash_clmulni_intel pps_core ecc i2c_i801 intel_lpss_pci
[  305.903096] CR2: 0000000000000030
[  305.906431] ---[ end trace 70ee364eed801cb0 ]---
[  305.940816] RIP: 0010:drm_dp_mst_atomic_check+0x138/0x2c0
[  305.946261] Code: 00 00 00 41 29 d9 41 89 d8 4c 89 fa 4c 89 f1 48 c7 c6 b0 b1 34 82 bf 10 00 00 00 45 31 ed e8 3f 99 02 00 4d 8b bf 80 04 00 00 <49> 8b 47 30 49 8d 5f 30 4c 8d 60 e8 48 39 c3 74 35 49 8b 7c 24 28
[  305.965125] RSP: 0018:ffffc90001687b58 EFLAGS: 00010246
[  305.970382] RAX: 0000000000000000 RBX: 000000000000003f RCX: 0000000000000000
[  305.977571] RDX: 0000000000000000 RSI: ffff88849fba8cb8 RDI: 00000000ffffffff
[  305.984747] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000001
[  305.991921] R10: ffffc900016879a0 R11: ffffc900016879a5 R12: 0000000000000000
[  305.999099] R13: 0000000000000000 R14: ffff8884905c9bc0 R15: 0000000000000000
[  306.006271] FS:  0000000000000000(0000) GS:ffff88849fb80000(0000) knlGS:0000000000000000
[  306.014407] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  306.020185] CR2: 0000000000000030 CR3: 000000048b3aa003 CR4: 0000000000760ee0
[  306.027404] PKRU: 55555554
[  306.030127] BUG: sleeping function called from invalid context at include/linux/percpu-rwsem.h:38
[  306.039049] in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 183, name: kworker/3:2
[  306.047272] INFO: lockdep is turned off.
[  306.051217] irq event stamp: 77505
[  306.054647] hardirqs last  enabled at (77505): [<ffffffff81a0c147>] _raw_spin_unlock_irqrestore+0x47/0x60
[  306.064270] hardirqs last disabled at (77504): [<ffffffff81a0bedf>] _raw_spin_lock_irqsave+0xf/0x50
[  306.073404] softirqs last  enabled at (77402): [<ffffffff81e00389>] __do_softirq+0x389/0x47f
[  306.081885] softirqs last disabled at (77395): [<ffffffff810b83a9>] irq_exit+0xa9/0xc0
[  306.089859] CPU: 3 PID: 183 Comm: kworker/3:2 Tainted: G      D           5.5.0-rc6+ #1404
[  306.098167] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP TLC, BIOS ICLSFWR1.R00.3201.A00.1905140358 05/14/2019
[  306.111882] Workqueue: events drm_dp_delayed_destroy_work
[  306.117314] Call Trace:
[  306.119780]  dump_stack+0x71/0xa0
[  306.123135]  ___might_sleep.cold+0xf7/0x10b
[  306.127399]  exit_signals+0x2b/0x360
[  306.131014]  do_exit+0xa7/0xc70
[  306.134189]  ? kthread+0x100/0x140
[  306.137615]  rewind_stack_do_exit+0x17/0x20

Fixes: cd82d82c ("drm/dp_mst: Add branch bandwidth validation to MST atomic check")
Cc: Mikita Lipski <mikita.lipski@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Lyude Paul <lyude@redhat.com>
Acked-by: NMikita Lipski <mikita.lipski@amd.com>
Reviewed-by: NLyude Paul <lyude@redhat.com>
Signed-off-by: NJosé Roberto de Souza <jose.souza@intel.com>
Signed-off-by: NAlex Deucher <alexander.deucher@amd.com>
      7b199143
    • L
      Revert "drm/dp_mst: Remove VCPI while disabling topology mgr" · a8667596
      由 Lyude Paul 提交于 
      This reverts commit 64e62bdf.

This commit ends up causing some lockdep splats due to trying to grab the
payload lock while holding the mgr's lock:

[   54.010099]
[   54.011765] ======================================================
[   54.018670] WARNING: possible circular locking dependency detected
[   54.025577] 5.5.0-rc6-02274-g77381c23ee63 #47 Not tainted
[   54.031610] ------------------------------------------------------
[   54.038516] kworker/1:6/1040 is trying to acquire lock:
[   54.044354] ffff888272af3228 (&mgr->payload_lock){+.+.}, at:
drm_dp_mst_topology_mgr_set_mst+0x218/0x2e4
[   54.054957]
[   54.054957] but task is already holding lock:
[   54.061473] ffff888272af3060 (&mgr->lock){+.+.}, at:
drm_dp_mst_topology_mgr_set_mst+0x3c/0x2e4
[   54.071193]
[   54.071193] which lock already depends on the new lock.
[   54.071193]
[   54.080334]
[   54.080334] the existing dependency chain (in reverse order) is:
[   54.088697]
[   54.088697] -> #1 (&mgr->lock){+.+.}:
[   54.094440]        __mutex_lock+0xc3/0x498
[   54.099015]        drm_dp_mst_topology_get_port_validated+0x25/0x80
[   54.106018]        drm_dp_update_payload_part1+0xa2/0x2e2
[   54.112051]        intel_mst_pre_enable_dp+0x144/0x18f
[   54.117791]        intel_encoders_pre_enable+0x63/0x70
[   54.123532]        hsw_crtc_enable+0xa1/0x722
[   54.128396]        intel_update_crtc+0x50/0x194
[   54.133455]        skl_commit_modeset_enables+0x40c/0x540
[   54.139485]        intel_atomic_commit_tail+0x5f7/0x130d
[   54.145418]        intel_atomic_commit+0x2c8/0x2d8
[   54.150770]        drm_atomic_helper_set_config+0x5a/0x70
[   54.156801]        drm_mode_setcrtc+0x2ab/0x833
[   54.161862]        drm_ioctl+0x2e5/0x424
[   54.166242]        vfs_ioctl+0x21/0x2f
[   54.170426]        do_vfs_ioctl+0x5fb/0x61e
[   54.175096]        ksys_ioctl+0x55/0x75
[   54.179377]        __x64_sys_ioctl+0x1a/0x1e
[   54.184146]        do_syscall_64+0x5c/0x6d
[   54.188721]        entry_SYSCALL_64_after_hwframe+0x49/0xbe
[   54.194946]
[   54.194946] -> #0 (&mgr->payload_lock){+.+.}:
[   54.201463]
[   54.201463] other info that might help us debug this:
[   54.201463]
[   54.210410]  Possible unsafe locking scenario:
[   54.210410]
[   54.217025]        CPU0                    CPU1
[   54.222082]        ----                    ----
[   54.227138]   lock(&mgr->lock);
[   54.230643]                                lock(&mgr->payload_lock);
[   54.237742]                                lock(&mgr->lock);
[   54.244062]   lock(&mgr->payload_lock);
[   54.248346]
[   54.248346]  *** DEADLOCK ***
[   54.248346]
[   54.254959] 7 locks held by kworker/1:6/1040:
[   54.259822]  #0: ffff888275c4f528 ((wq_completion)events){+.+.},
at: worker_thread+0x455/0x6e2
[   54.269451]  #1: ffffc9000119beb0
((work_completion)(&(&dev_priv->hotplug.hotplug_work)->work)){+.+.},
at: worker_thread+0x455/0x6e2
[   54.282768]  #2: ffff888272a403f0 (&dev->mode_config.mutex){+.+.},
at: i915_hotplug_work_func+0x4b/0x2be
[   54.293368]  #3: ffffffff824fc6c0 (drm_connector_list_iter){.+.+},
at: i915_hotplug_work_func+0x17e/0x2be
[   54.304061]  #4: ffffc9000119bc58 (crtc_ww_class_acquire){+.+.},
at: drm_helper_probe_detect_ctx+0x40/0xfd
[   54.314855]  #5: ffff888272a40470 (crtc_ww_class_mutex){+.+.}, at:
drm_modeset_lock+0x74/0xe2
[   54.324385]  #6: ffff888272af3060 (&mgr->lock){+.+.}, at:
drm_dp_mst_topology_mgr_set_mst+0x3c/0x2e4
[   54.334597]
[   54.334597] stack backtrace:
[   54.339464] CPU: 1 PID: 1040 Comm: kworker/1:6 Not tainted
5.5.0-rc6-02274-g77381c23ee63 #47
[   54.348893] Hardware name: Google Fizz/Fizz, BIOS
Google_Fizz.10139.39.0 01/04/2018
[   54.357451] Workqueue: events i915_hotplug_work_func
[   54.362995] Call Trace:
[   54.365724]  dump_stack+0x71/0x9c
[   54.369427]  check_noncircular+0x91/0xbc
[   54.373809]  ? __lock_acquire+0xc9e/0xf66
[   54.378286]  ? __lock_acquire+0xc9e/0xf66
[   54.382763]  ? lock_acquire+0x175/0x1ac
[   54.387048]  ? drm_dp_mst_topology_mgr_set_mst+0x218/0x2e4
[   54.393177]  ? __mutex_lock+0xc3/0x498
[   54.397362]  ? drm_dp_mst_topology_mgr_set_mst+0x218/0x2e4
[   54.403492]  ? drm_dp_mst_topology_mgr_set_mst+0x218/0x2e4
[   54.409620]  ? drm_dp_dpcd_access+0xd9/0x101
[   54.414390]  ? drm_dp_mst_topology_mgr_set_mst+0x218/0x2e4
[   54.420517]  ? drm_dp_mst_topology_mgr_set_mst+0x218/0x2e4
[   54.426645]  ? intel_digital_port_connected+0x34d/0x35c
[   54.432482]  ? intel_dp_detect+0x227/0x44e
[   54.437056]  ? ww_mutex_lock+0x49/0x9a
[   54.441242]  ? drm_helper_probe_detect_ctx+0x75/0xfd
[   54.446789]  ? intel_encoder_hotplug+0x4b/0x97
[   54.451752]  ? intel_ddi_hotplug+0x61/0x2e0
[   54.456423]  ? mark_held_locks+0x53/0x68
[   54.460803]  ? _raw_spin_unlock_irqrestore+0x3a/0x51
[   54.466347]  ? lockdep_hardirqs_on+0x187/0x1a4
[   54.471310]  ? drm_connector_list_iter_next+0x89/0x9a
[   54.476953]  ? i915_hotplug_work_func+0x206/0x2be
[   54.482208]  ? worker_thread+0x4d5/0x6e2
[   54.486587]  ? worker_thread+0x455/0x6e2
[   54.490966]  ? queue_work_on+0x64/0x64
[   54.495151]  ? kthread+0x1e9/0x1f1
[   54.498946]  ? queue_work_on+0x64/0x64
[   54.503130]  ? kthread_unpark+0x5e/0x5e
[   54.507413]  ? ret_from_fork+0x3a/0x50

The proper fix for this is probably cleanup the VCPI allocations when we're
enabling the topology, or on the first payload allocation. For now though,
let's just revert.
Signed-off-by: NLyude Paul <lyude@redhat.com>
Fixes: 64e62bdf ("drm/dp_mst: Remove VCPI while disabling topology mgr")
Cc: Sean Paul <sean@poorly.run>
Cc: Wayne Lin <Wayne.Lin@amd.com>
Reviewed-by: NSean Paul <sean@poorly.run>
Link: https://patchwork.freedesktop.org/patch/msgid/20200117205149.97262-1-lyude@redhat.com
      a8667596
  35. 16 1月, 2020 1 次提交
    • W
      drm/dp_mst: Have DP_Tx send one msg at a time · 5a64967a
      由 Wayne Lin 提交于 
      [Why]
Noticed this while testing MST with the 4 ports MST hub from
StarTech.com. Sometimes can't light up monitors normally and get the
error message as 'sideband msg build failed'.

Look into aux transactions, found out that source sometimes will send
out another down request before receiving the down reply of the
previous down request. On the other hand, in drm_dp_get_one_sb_msg(),
current code doesn't handle the interleaved replies case. Hence, source
can't build up message completely and can't light up monitors.

[How]
For good compatibility, enforce source to send out one down request at a
time. Add a flag, is_waiting_for_dwn_reply, to determine if the source
can send out a down request immediately or not.

- Check the flag before calling process_single_down_tx_qlock to send out
a msg
- Set the flag when successfully send out a down request
- Clear the flag when successfully build up a down reply
- Clear the flag when find erros during sending out a down request
- Clear the flag when find errors during building up a down reply
- Clear the flag when timeout occurs during waiting for a down reply
- Use drm_dp_mst_kick_tx() to try to send another down request in queue
at the end of drm_dp_mst_wait_tx_reply() (attempt to send out messages
in queue when errors occur)

Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: NWayne Lin <Wayne.Lin@amd.com>
Signed-off-by: NLyude Paul <lyude@redhat.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200113093649.11755-1-Wayne.Lin@amd.com
      5a64967a
  37. 15 1月, 2020 1 次提交
    • W
      drm/dp_mst: clear time slots for ports invalid · 7617e962
      由 Wayne Lin 提交于 
      [Why]
When change the connection status in a MST topology, mst device
which detect the event will send out CONNECTION_STATUS_NOTIFY messgae.

e.g. src-mst-mst-sst => src-mst (unplug) mst-sst

Currently, under the above case of unplugging device, ports which have
been allocated payloads and are no longer in the topology still occupy
time slots and recorded in proposed_vcpi[] of topology manager.

If we don't clean up the proposed_vcpi[], when code flow goes to try to
update payload table by calling drm_dp_update_payload_part1(), we will
fail at checking port validation due to there are ports with proposed
time slots but no longer in the mst topology. As the result of that, we
will also stop updating the DPCD payload table of down stream port.

[How]
While handling the CONNECTION_STATUS_NOTIFY message, add a detection to
see if the event indicates that a device is unplugged to an output port.
If the detection is true, then iterrate over all proposed_vcpi[] to
see whether a port of the proposed_vcpi[] is still in the topology or
not. If the port is invalid, set its num_slots to 0.

Thereafter, when try to update payload table by calling
drm_dp_update_payload_part1(), we can successfully update the DPCD
payload table of down stream port and clear the proposed_vcpi[] to NULL.

Changes since v1:(https://patchwork.kernel.org/patch/11275801/)
* Invert the conditional to reduce the indenting
Reviewed-by: NLyude Paul <lyude@redhat.com>
Signed-off-by: NWayne Lin <Wayne.Lin@amd.com>
Signed-off-by: NLyude Paul <lyude@redhat.com>
[removed cc for stable - there's too many patches this depends on for
this to backport cleanly]
Link: https://patchwork.freedesktop.org/patch/msgid/20200106102158.28261-1-Wayne.Lin@amd.com
      7617e962
  39. 10 1月, 2020 5 次提交