[OEM5.14/J][SRU][PATCH v2 0/7] Fix DP tunneling issues on AMD Rembrandt

[Stefan Bader]

On 03.08.22 16:42, Mario Limonciello wrote:
> On 8/3/22 09:35, Stefan Bader wrote:
>> On 02.08.22 20:11, Mario Limonciello wrote:
>>> BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1983143
>>>
>>> When a Thunderbolt monitor or Thunderbolt dock are connected to a USB4
>>> port on AMD Yellow Carp (Rembrandt) a software connection manager works
>>> with the hardware to initialize tunnels.
>>>
>>> One of those tunnels is for DP (display port). It was observed that
>>> on both the OEM-5.14 as well as Jammy-5.15 kernel that DP tunneling
>>> specifically when connecting to a Thunderbolt monitor fails.
>>> This circumstance does not happen on OEM-5.17 or mainline kernels.
>>>
>>> This is due to a backport error on the USB4 patch series that went
>>> into OEM-5.14 and moved forward to Jammy-5.15.  Patch 1/7 in this
>>> series corrects that error as minimally as possible as a SAUCE patch.
>>>
>>> It was also observed that DP tunneling is not working for Thunderbolt
>>> monitor connected at boot, but this is a different root cause.
>>> The "Pre-OS" connection manager creates DP tunnels that are expected
>>> to be re-used by the OS connection manager. The remaining patches in
>>> this series enable this flow.
>>>
>>> Patches 2-6 are upstream patches or reverts to closer match upstream.
>>> Patch 7 is a SAUCE patch because it is not landed upstream yet, but
>>> will be needed for patch 2-6 to work properly.  If the Thunderbolt
>>> CM doesn't hand off resources, amdgpu can't possibly re-use them.
>>>
>>> For this whole flow to work an updated AMDGPU DMCUB firmware is also
>>> needed.  This is already in process as part of:
>>> https://bugs.launchpad.net/ubuntu/+source/linux-firmware/+bug/1981922
>>>
>>> v1->v2:
>>> * Fixup [1/4], it didn't work due to a contextual error
>>> * Add a revert of a stable patch before previous patch [3/4] applied
>>>    so that it can re-apply cleanly.
>>> * Add missing dependencies for patch [3/4] to allow applying cleanly
>>> * Re-apply revert commit
>>>
>>> Mario Limonciello (2):
>>>    UBUNTU: SAUCE: drm/amd: Fix DP Tunneling with Thunderbolt monitors
>>>    Revert "drm/amd/display: Fix DPIA outbox timeout after S3/S4/reset"
>>>
>>> Meenakshikumar Somasundaram (1):
>>>    drm/amd/display: Fix for dmub outbox notification enable
>>>
>>> Nicholas Kazlauskas (2):
>>>    drm/amd/display: Reset link encoder assignments for GPU reset
>>>    drm/amd/display: Fix DPIA outbox timeout after S3/S4/reset
>>>
>>> Sanjay R Mehta (1):
>>>    UBUNTU: SAUCE: thunderbolt: Add DP out resource when DP tunnel is
>>>      discovered.
>>>
>>> Stylon Wang (1):
>>>    drm/amd/display: Fix new dmub notification enabling in DM
>>>
>>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 39 +++++++++--
>>>   drivers/gpu/drm/amd/display/dc/core/dc.c      | 66 +++++++++++++++++--
>>>   drivers/gpu/drm/amd/display/dc/dc.h           |  3 +
>>>   .../gpu/drm/amd/display/dc/dce/dmub_outbox.c  | 25 +++----
>>>   .../gpu/drm/amd/display/dc/dce/dmub_outbox.h  |  4 +-
>>>   .../amd/display/dc/dcn10/dcn10_hw_sequencer.c |  4 --
>>>   .../drm/amd/display/dc/dcn20/dcn20_hwseq.c    |  5 +-
>>>   .../drm/amd/display/dc/dcn31/dcn31_hwseq.c    |  2 +-
>>>   drivers/thunderbolt/tb.c                      | 15 +++++
>>>   drivers/thunderbolt/tb.h                      |  1 +
>>>   drivers/thunderbolt/tunnel.c                  |  2 +
>>>   11 files changed, 133 insertions(+), 33 deletions(-)
>>>
>>
>> I think this is a replacement for the previous set but lacking the topic. If 
>> that is correct, then the previous thread should be NACKed to make it clear.
> 
> Yes it's replacement for previous series to make everything apply cleanly.  I'll 
> drop a message to that thread to make it clearer.
> 
>>
>> The method chosen to work around the context change is somewhat worrying. 
>> Basically the patch reverted was upstream stable and had been adjusted in 
>> context to match the v5.15.y code base. Then you re-apply the unmodified 
>> upstream patch. So we are closer to what code looks like in upstream v5.16 or 
>> later but further away from linux-5.15.y.
> 
> Yes that's correct.  I guess it's up to you which way you prefer to take it.  I 
> thought it was more important to make things apply cleanly.
> 
> If you prefer the previous approach I'll resubmit the previous patches [2-4] and 
> the fixed [1/7] in this series as a v3.

In this case it probably is better to go with the revert and upstream version. 
It is just scary and makes me doubt how much I can help to keep the overall 
driver stable.

-Stefan

> 
>> I know we are already diverting. Just scary to see this example of how mind 
>> boggling things can become. This time it is just context but it might be a 
>> different flow of execution or locking in other cases. And this is near 
>> impossible to properly review.
> 
> Yeah I don't think this approach works every single time.
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20220803/924e644b/attachment.sig>