mbox series

[v7,00/10] drm/amd/display: Use drm_edid for more code

Message ID 20240918213845.158293-1-mario.limonciello@amd.com (mailing list archive)
Headers show
Series drm/amd/display: Use drm_edid for more code | expand

Message

Mario Limonciello Sept. 18, 2024, 9:38 p.m. UTC 
This is the successor of Melissa's v5 series that was posted [1] as well
as my series that was posted [2].

Melissa's patches are mostly unmodified from v5, but the series has been
rebase on the new 6.10 based amd-staging-drm-next.

As were both touching similar code for fetching the EDID, I've merged the
pertinent parts of my series into this one in allowing the connector to
fetch the EDID from _DDC if available for eDP as well.

There are still some remaining uses of drm_edid_raw() but they touch pure
DC code, so a wrapper or macro will probably be needed to convert them.
This can be for follow ups later on.

Link: https://lore.kernel.org/amd-gfx/20240807203207.2830-1-mwen@igalia.com/ [1]
Link: https://lore.kernel.org/dri-devel/20240214215756.6530-1-mario.limonciello@amd.com/ [2]

v7:
 * Rebase on amd-staging-drm-next which is now 6.10 based
 * Fix the two LKP robot reported issues

Mario Limonciello (1):
  drm/amd/display: Fetch the EDID from _DDC if available for eDP

Melissa Wen (9):
  drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid
  drm/amd/display: switch to setting physical address directly
  drm/amd/display: always call connector_update when parsing
    freesync_caps
  drm/amd/display: remove redundant freesync parser for DP
  drm/amd/display: use drm_edid_product_id for parsing EDID product info
  drm/amd/display: parse display name from drm_eld
  drm/amd/display: get SAD from drm_eld when parsing EDID caps
  drm/amd/display: get SADB from drm_eld when parsing EDID caps
  drm/amd/display: remove dc_edid handler from
    dm_helpers_parse_edid_caps

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 200 ++++++------------
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   4 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 172 +++++++++------
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  34 +--
 drivers/gpu/drm/amd/display/dc/dm_helpers.h   |   1 -
 .../drm/amd/display/dc/link/link_detection.c  |   6 +-
 drivers/gpu/drm/amd/include/amd_shared.h      |   5 +
 7 files changed, 200 insertions(+), 222 deletions(-)


base-commit: 0569603f945225067d963c8ba4fda31d967ab584

Comments

Alex Hung Sept. 27, 2024, 6:48 p.m. UTC | #1 
Hi Mario and Melissa,

There are three regressions identified during the test, and improvement 
is required before the patches can be merged. Please see details below.

1. null pointer when hot-plugging a dsc hub (+ three 4k60 monitors).

This may point to "drm/amd/display: use drm_edid_product_id for parsing 
EDID product info" since that's the only patch calling 
drm_edid_get_product_id.


[  227.797361] RIP: 0010:drm_edid_get_product_id+0x1d/0x50 [drm]
[  227.797388] Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00 
00 55 48 89 e5 48 85 ff 74 24 48 8b 47 08 48 85 c0 74 1b 48 83 3f 7f 76 
15 <48> 8b 50 08 5d 48 89 16 0f b7 40 10 66 89 46 08 e9 a9 47 ee ce 31
[  227.797391] RSP: 0018:ffffac58126f7930 EFLAGS: 00010216
[  227.797394] RAX: 000000000000372d RBX: ffff8eaeaf8ac808 RCX: 
ffff8eaeaf8ac107
[  227.797396] RDX: 0000000000000002 RSI: ffffac58126f7944 RDI: 
ffff8eaeeeaf9f80
[  227.797398] RBP: ffffac58126f7930 R08: ffff8eae8e500d00 R09: 
0000000000000001
[  227.797400] R10: ffffac58126f7978 R11: 000000000017f81b R12: 
ffff8eae84cb0000
[  227.797402] R13: ffff8eaeeeaf9f80 R14: 0000000000000000 R15: 
0000000000000100
[  227.797405] FS:  00007f031a616ac0(0000) GS:ffff8eb57cd80000(0000) 
knlGS:0000000000000000
[  227.797407] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  227.797409] CR2: 0000000000003735 CR3: 000000014decc000 CR4: 
0000000000750ef0
[  227.797411] PKRU: 55555554
[  227.797413] Call Trace:
[  227.797415]  <TASK>
[  227.797417]  ? show_regs+0x64/0x70
[  227.797423]  ? __die+0x24/0x70
[  227.797427]  ? page_fault_oops+0x160/0x520
[  227.797435]  ? do_user_addr_fault+0x2e9/0x6a0
[  227.797438]  ? dc_link_add_remote_sink+0x20/0x30 [amdgpu]
[  227.797654]  ? dm_dp_mst_get_modes+0xee/0x520 [amdgpu]
[  227.797882]  ? drm_helper_probe_single_connector_modes+0x1b5/0x670 
[drm_kms_helper]
[  227.797894]  ? exc_page_fault+0x7f/0x190
[  227.797899]  ? asm_exc_page_fault+0x27/0x30
[  227.797906]  ? drm_edid_get_product_id+0x1d/0x50 [drm]
[  227.797932]  dm_helpers_parse_edid_caps+0x69/0x260 [amdgpu]
[  227.798139] amdgpu 0000:0b:00.0: [drm:drm_dp_dpcd_write 
[drm_display_helper]] AMDGPU DM aux hw bus 2: 0x02003 AUX <- (ret=  1) 10
[  227.798158]  link_add_remote_sink+0xa8/0x1a0 [amdgpu]
[  227.798402]  dc_link_add_remote_sink+0x20/0x30 [amdgpu]
[  227.798615]  dm_dp_mst_get_modes+0xee/0x520 [amdgpu]
[  227.798843]  ? srso_alias_return_thunk+0x5/0xfbef5
[  227.798848]  drm_helper_probe_single_connector_modes+0x1b5/0x670 
[drm_kms_helper]


2. DP2 Display is not listed as an audio device

Steps to reproduce:
- Attach display to system.
- Boot to Desktop (Wayland)
- Attempt to select display as an audio device.

This points to patch "drm/amd/display: get SAD from drm_eld when parsing 
EDID caps"


3. IGT amd_mem_leak'sconnector-suspend-resume fails on Navi 31.

This points to the patch "drm/amd/display: remove dc_edid handler from 
dm_helpers_parse_edid_caps".


Using IGT_SRANDOM=1727192429 for randomisation
Opened device: /dev/dri/card0
Starting subtest: connector-suspend-resume
[cmd] rtcwake: assuming RTC uses UTC ...
rtcwake: wakeup from "mem" using /dev/rtc0 at Tue Sep 24 15:40:50 2024
unreferenced object 0xffff95c683517b00 (size 256):
   comm "kworker/u64:30", pid 3636, jiffies 4295452761
   hex dump (first 32 bytes):
     00 ff ff ff ff ff ff 00 22 0e c2 36 00 00 00 00  ........"..6....
     33 1d 01 04 b5 3c 22 78 3b 28 91 a7 55 4e a3 26  3....<"x;(..UN.&
   backtrace (crc 5800a91d):
     [<ffffffffbb73502a>] kmemleak_alloc+0x4a/0x80
     [<ffffffffbabbde07>] kmalloc_node_track_caller_noprof+0x337/0x410
     [<ffffffffbab631f8>] krealloc_noprof+0x58/0xb0
     [<ffffffffc04e8e38>] _drm_do_get_edid+0x138/0x410 [drm]
     [<ffffffffc04e9155>] drm_edid_read_custom+0x35/0xd0 [drm]
     [<ffffffffc04e9244>] drm_edid_read_ddc+0x44/0x80 [drm]
     [<ffffffffc1061808>] dm_helpers_read_local_edid+0x1d8/0x340 [amdgpu]
     [<ffffffffc11b7446>] detect_link_and_local_sink+0x356/0x1230 [amdgpu]
     [<ffffffffc11b8366>] link_detect+0x36/0x4f0 [amdgpu]
     [<ffffffffc13667a0>] dc_link_detect+0x20/0x30 [amdgpu]
     [<ffffffffc10536f0>] dm_resume+0x1e0/0x7d0 [amdgpu]
     [<ffffffffc0d25908>] amdgpu_device_ip_resume_phase2+0x58/0xd0 [amdgpu]
     [<ffffffffc0d29cb7>] amdgpu_device_resume+0xa7/0x2e0 [amdgpu]
     [<ffffffffc0d2467c>] amdgpu_pmops_resume+0x4c/0x90 [amdgpu]
     [<ffffffffbb02e121>] pci_pm_resume+0x71/0x100
     [<ffffffffbb22c551>] dpm_run_callback+0x91/0x1e0
unreferenced object 0xffff95c6c58dd0c0 (size 16):
   comm "kworker/u64:30", pid 3636, jiffies 4295452764
   hex dump (first 16 bytes):
     00 01 00 00 00 00 00 00 00 7b 51 83 c6 95 ff ff  .........{Q.....
   backtrace (crc 70d89717):
     [<ffffffffbb73502a>] kmemleak_alloc+0x4a/0x80
     [<ffffffffbabbbf3e>] kmalloc_trace_noprof+0x28e/0x300
     [<ffffffffc04e6845>] _drm_edid_alloc+0x35/0x60 [drm]
     [<ffffffffc04e9175>] drm_edid_read_custom+0x55/0xd0 [drm]
     [<ffffffffc04e9244>] drm_edid_read_ddc+0x44/0x80 [drm]
     [<ffffffffc1061808>] dm_helpers_read_local_edid+0x1d8/0x340 [amdgpu]
     [<ffffffffc11b7446>] detect_link_and_local_sink+0x356/0x1230 [amdgpu]
     [<ffffffffc11b8366>] link_detect+0x36/0x4f0 [amdgpu]
     [<ffffffffc13667a0>] dc_link_detect+0x20/0x30 [amdgpu]
     [<ffffffffc10536f0>] dm_resume+0x1e0/0x7d0 [amdgpu]
     [<ffffffffc0d25908>] amdgpu_device_ip_resume_phase2+0x58/0xd0 [amdgpu]
     [<ffffffffc0d29cb7>] amdgpu_device_resume+0xa7/0x2e0 [amdgpu]
     [<ffffffffc0d2467c>] amdgpu_pmops_resume+0x4c/0x90 [amdgpu]
     [<ffffffffbb02e121>] pci_pm_resume+0x71/0x100
     [<ffffffffbb22c551>] dpm_run_callback+0x91/0x1e0
     [<ffffffffbb22ca76>] device_resume+0xb6/0x2c0
Stack trace:
   #0 ../lib/igt_core.c:2051 __igt_fail_assert()
   #1 ../tests/amdgpu/amd_mem_leak.c:202 __igt_unique____real_main208()
   #2 ../tests/amdgpu/amd_mem_leak.c:208 main()
   #3 ../sysdeps/nptl/libc_start_call_main.h:74 __libc_start_call_main()
   #4 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
   #5 [_start+0x25]
Subtest connector-suspend-resume: FAIL (9.535s)

On 9/18/24 15:38, Mario Limonciello wrote:
> This is the successor of Melissa's v5 series that was posted [1] as well
> as my series that was posted [2].
> 
> Melissa's patches are mostly unmodified from v5, but the series has been
> rebase on the new 6.10 based amd-staging-drm-next.
> 
> As were both touching similar code for fetching the EDID, I've merged the
> pertinent parts of my series into this one in allowing the connector to
> fetch the EDID from _DDC if available for eDP as well.
> 
> There are still some remaining uses of drm_edid_raw() but they touch pure
> DC code, so a wrapper or macro will probably be needed to convert them.
> This can be for follow ups later on.
> 
> Link: https://lore.kernel.org/amd-gfx/20240807203207.2830-1-mwen@igalia.com/ [1]
> Link: https://lore.kernel.org/dri-devel/20240214215756.6530-1-mario.limonciello@amd.com/ [2]
> 
> v7:
>   * Rebase on amd-staging-drm-next which is now 6.10 based
>   * Fix the two LKP robot reported issues
> 
> Mario Limonciello (1):
>    drm/amd/display: Fetch the EDID from _DDC if available for eDP
> 
> Melissa Wen (9):
>    drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid
>    drm/amd/display: switch to setting physical address directly
>    drm/amd/display: always call connector_update when parsing
>      freesync_caps
>    drm/amd/display: remove redundant freesync parser for DP
>    drm/amd/display: use drm_edid_product_id for parsing EDID product info
>    drm/amd/display: parse display name from drm_eld
>    drm/amd/display: get SAD from drm_eld when parsing EDID caps
>    drm/amd/display: get SADB from drm_eld when parsing EDID caps
>    drm/amd/display: remove dc_edid handler from
>      dm_helpers_parse_edid_caps
> 
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 200 ++++++------------
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   4 +-
>   .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 172 +++++++++------
>   .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  34 +--
>   drivers/gpu/drm/amd/display/dc/dm_helpers.h   |   1 -
>   .../drm/amd/display/dc/link/link_detection.c  |   6 +-
>   drivers/gpu/drm/amd/include/amd_shared.h      |   5 +
>   7 files changed, 200 insertions(+), 222 deletions(-)
> 
> 
> base-commit: 0569603f945225067d963c8ba4fda31d967ab584
Melissa Wen Sept. 27, 2024, 8:45 p.m. UTC | #2 
Hi Alex,

Thanks for the intensive testing.

I'll need some time to reproduce and debug these regressions.

So, we can divide this series into four steps:
1-2 are the basis for drm_edid migration
3-4 are code cleanups
5-9 are drm_edid_product_id migration
10 is for ACPI EDID feature.

Bearing this and the reported regressions around the product_id part in 
mind, I wonder if we can start by applying the drm_edid migration and 
the ACPI EDID feature, which means applying patches 1-4 and 10. And then 
let the second part of product_id migration for the next iteration.
IMHO it seems a smoother transition.

WDYT?

Melissa


On 27/09/2024 15:48, Alex Hung wrote:
> Hi Mario and Melissa,
>
> There are three regressions identified during the test, and 
> improvement is required before the patches can be merged. Please see 
> details below.
>
> 1. null pointer when hot-plugging a dsc hub (+ three 4k60 monitors).
>
> This may point to "drm/amd/display: use drm_edid_product_id for 
> parsing EDID product info" since that's the only patch calling 
> drm_edid_get_product_id.
>
>
> [  227.797361] RIP: 0010:drm_edid_get_product_id+0x1d/0x50 [drm]
> [  227.797388] Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 
> 00 00 55 48 89 e5 48 85 ff 74 24 48 8b 47 08 48 85 c0 74 1b 48 83 3f 
> 7f 76 15 <48> 8b 50 08 5d 48 89 16 0f b7 40 10 66 89 46 08 e9 a9 47 ee 
> ce 31
> [  227.797391] RSP: 0018:ffffac58126f7930 EFLAGS: 00010216
> [  227.797394] RAX: 000000000000372d RBX: ffff8eaeaf8ac808 RCX: 
> ffff8eaeaf8ac107
> [  227.797396] RDX: 0000000000000002 RSI: ffffac58126f7944 RDI: 
> ffff8eaeeeaf9f80
> [  227.797398] RBP: ffffac58126f7930 R08: ffff8eae8e500d00 R09: 
> 0000000000000001
> [  227.797400] R10: ffffac58126f7978 R11: 000000000017f81b R12: 
> ffff8eae84cb0000
> [  227.797402] R13: ffff8eaeeeaf9f80 R14: 0000000000000000 R15: 
> 0000000000000100
> [  227.797405] FS:  00007f031a616ac0(0000) GS:ffff8eb57cd80000(0000) 
> knlGS:0000000000000000
> [  227.797407] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  227.797409] CR2: 0000000000003735 CR3: 000000014decc000 CR4: 
> 0000000000750ef0
> [  227.797411] PKRU: 55555554
> [  227.797413] Call Trace:
> [  227.797415]  <TASK>
> [  227.797417]  ? show_regs+0x64/0x70
> [  227.797423]  ? __die+0x24/0x70
> [  227.797427]  ? page_fault_oops+0x160/0x520
> [  227.797435]  ? do_user_addr_fault+0x2e9/0x6a0
> [  227.797438]  ? dc_link_add_remote_sink+0x20/0x30 [amdgpu]
> [  227.797654]  ? dm_dp_mst_get_modes+0xee/0x520 [amdgpu]
> [  227.797882]  ? drm_helper_probe_single_connector_modes+0x1b5/0x670 
> [drm_kms_helper]
> [  227.797894]  ? exc_page_fault+0x7f/0x190
> [  227.797899]  ? asm_exc_page_fault+0x27/0x30
> [  227.797906]  ? drm_edid_get_product_id+0x1d/0x50 [drm]
> [  227.797932]  dm_helpers_parse_edid_caps+0x69/0x260 [amdgpu]
> [  227.798139] amdgpu 0000:0b:00.0: [drm:drm_dp_dpcd_write 
> [drm_display_helper]] AMDGPU DM aux hw bus 2: 0x02003 AUX <- (ret=  1) 10
> [  227.798158]  link_add_remote_sink+0xa8/0x1a0 [amdgpu]
> [  227.798402]  dc_link_add_remote_sink+0x20/0x30 [amdgpu]
> [  227.798615]  dm_dp_mst_get_modes+0xee/0x520 [amdgpu]
> [  227.798843]  ? srso_alias_return_thunk+0x5/0xfbef5
> [  227.798848] drm_helper_probe_single_connector_modes+0x1b5/0x670 
> [drm_kms_helper]
>
>
> 2. DP2 Display is not listed as an audio device
>
> Steps to reproduce:
> - Attach display to system.
> - Boot to Desktop (Wayland)
> - Attempt to select display as an audio device.
>
> This points to patch "drm/amd/display: get SAD from drm_eld when 
> parsing EDID caps"
>
>
> 3. IGT amd_mem_leak'sconnector-suspend-resume fails on Navi 31.
>
> This points to the patch "drm/amd/display: remove dc_edid handler from 
> dm_helpers_parse_edid_caps".
>
>
> Using IGT_SRANDOM=1727192429 for randomisation
> Opened device: /dev/dri/card0
> Starting subtest: connector-suspend-resume
> [cmd] rtcwake: assuming RTC uses UTC ...
> rtcwake: wakeup from "mem" using /dev/rtc0 at Tue Sep 24 15:40:50 2024
> unreferenced object 0xffff95c683517b00 (size 256):
>   comm "kworker/u64:30", pid 3636, jiffies 4295452761
>   hex dump (first 32 bytes):
>     00 ff ff ff ff ff ff 00 22 0e c2 36 00 00 00 00 ........"..6....
>     33 1d 01 04 b5 3c 22 78 3b 28 91 a7 55 4e a3 26 3....<"x;(..UN.&
>   backtrace (crc 5800a91d):
>     [<ffffffffbb73502a>] kmemleak_alloc+0x4a/0x80
>     [<ffffffffbabbde07>] kmalloc_node_track_caller_noprof+0x337/0x410
>     [<ffffffffbab631f8>] krealloc_noprof+0x58/0xb0
>     [<ffffffffc04e8e38>] _drm_do_get_edid+0x138/0x410 [drm]
>     [<ffffffffc04e9155>] drm_edid_read_custom+0x35/0xd0 [drm]
>     [<ffffffffc04e9244>] drm_edid_read_ddc+0x44/0x80 [drm]
>     [<ffffffffc1061808>] dm_helpers_read_local_edid+0x1d8/0x340 [amdgpu]
>     [<ffffffffc11b7446>] detect_link_and_local_sink+0x356/0x1230 [amdgpu]
>     [<ffffffffc11b8366>] link_detect+0x36/0x4f0 [amdgpu]
>     [<ffffffffc13667a0>] dc_link_detect+0x20/0x30 [amdgpu]
>     [<ffffffffc10536f0>] dm_resume+0x1e0/0x7d0 [amdgpu]
>     [<ffffffffc0d25908>] amdgpu_device_ip_resume_phase2+0x58/0xd0 
> [amdgpu]
>     [<ffffffffc0d29cb7>] amdgpu_device_resume+0xa7/0x2e0 [amdgpu]
>     [<ffffffffc0d2467c>] amdgpu_pmops_resume+0x4c/0x90 [amdgpu]
>     [<ffffffffbb02e121>] pci_pm_resume+0x71/0x100
>     [<ffffffffbb22c551>] dpm_run_callback+0x91/0x1e0
> unreferenced object 0xffff95c6c58dd0c0 (size 16):
>   comm "kworker/u64:30", pid 3636, jiffies 4295452764
>   hex dump (first 16 bytes):
>     00 01 00 00 00 00 00 00 00 7b 51 83 c6 95 ff ff .........{Q.....
>   backtrace (crc 70d89717):
>     [<ffffffffbb73502a>] kmemleak_alloc+0x4a/0x80
>     [<ffffffffbabbbf3e>] kmalloc_trace_noprof+0x28e/0x300
>     [<ffffffffc04e6845>] _drm_edid_alloc+0x35/0x60 [drm]
>     [<ffffffffc04e9175>] drm_edid_read_custom+0x55/0xd0 [drm]
>     [<ffffffffc04e9244>] drm_edid_read_ddc+0x44/0x80 [drm]
>     [<ffffffffc1061808>] dm_helpers_read_local_edid+0x1d8/0x340 [amdgpu]
>     [<ffffffffc11b7446>] detect_link_and_local_sink+0x356/0x1230 [amdgpu]
>     [<ffffffffc11b8366>] link_detect+0x36/0x4f0 [amdgpu]
>     [<ffffffffc13667a0>] dc_link_detect+0x20/0x30 [amdgpu]
>     [<ffffffffc10536f0>] dm_resume+0x1e0/0x7d0 [amdgpu]
>     [<ffffffffc0d25908>] amdgpu_device_ip_resume_phase2+0x58/0xd0 
> [amdgpu]
>     [<ffffffffc0d29cb7>] amdgpu_device_resume+0xa7/0x2e0 [amdgpu]
>     [<ffffffffc0d2467c>] amdgpu_pmops_resume+0x4c/0x90 [amdgpu]
>     [<ffffffffbb02e121>] pci_pm_resume+0x71/0x100
>     [<ffffffffbb22c551>] dpm_run_callback+0x91/0x1e0
>     [<ffffffffbb22ca76>] device_resume+0xb6/0x2c0
> Stack trace:
>   #0 ../lib/igt_core.c:2051 __igt_fail_assert()
>   #1 ../tests/amdgpu/amd_mem_leak.c:202 __igt_unique____real_main208()
>   #2 ../tests/amdgpu/amd_mem_leak.c:208 main()
>   #3 ../sysdeps/nptl/libc_start_call_main.h:74 __libc_start_call_main()
>   #4 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
>   #5 [_start+0x25]
> Subtest connector-suspend-resume: FAIL (9.535s)
>
> On 9/18/24 15:38, Mario Limonciello wrote:
>> This is the successor of Melissa's v5 series that was posted [1] as well
>> as my series that was posted [2].
>>
>> Melissa's patches are mostly unmodified from v5, but the series has been
>> rebase on the new 6.10 based amd-staging-drm-next.
>>
>> As were both touching similar code for fetching the EDID, I've merged 
>> the
>> pertinent parts of my series into this one in allowing the connector to
>> fetch the EDID from _DDC if available for eDP as well.
>>
>> There are still some remaining uses of drm_edid_raw() but they touch 
>> pure
>> DC code, so a wrapper or macro will probably be needed to convert them.
>> This can be for follow ups later on.
>>
>> Link: 
>> https://lore.kernel.org/amd-gfx/20240807203207.2830-1-mwen@igalia.com/ 
>> [1]
>> Link: 
>> https://lore.kernel.org/dri-devel/20240214215756.6530-1-mario.limonciello@amd.com/ 
>> [2]
>>
>> v7:
>>   * Rebase on amd-staging-drm-next which is now 6.10 based
>>   * Fix the two LKP robot reported issues
>>
>> Mario Limonciello (1):
>>    drm/amd/display: Fetch the EDID from _DDC if available for eDP
>>
>> Melissa Wen (9):
>>    drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid
>>    drm/amd/display: switch to setting physical address directly
>>    drm/amd/display: always call connector_update when parsing
>>      freesync_caps
>>    drm/amd/display: remove redundant freesync parser for DP
>>    drm/amd/display: use drm_edid_product_id for parsing EDID product 
>> info
>>    drm/amd/display: parse display name from drm_eld
>>    drm/amd/display: get SAD from drm_eld when parsing EDID caps
>>    drm/amd/display: get SADB from drm_eld when parsing EDID caps
>>    drm/amd/display: remove dc_edid handler from
>>      dm_helpers_parse_edid_caps
>>
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 200 ++++++------------
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   4 +-
>>   .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 172 +++++++++------
>>   .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  34 +--
>>   drivers/gpu/drm/amd/display/dc/dm_helpers.h   |   1 -
>>   .../drm/amd/display/dc/link/link_detection.c  |   6 +-
>>   drivers/gpu/drm/amd/include/amd_shared.h      |   5 +
>>   7 files changed, 200 insertions(+), 222 deletions(-)
>>
>>
>> base-commit: 0569603f945225067d963c8ba4fda31d967ab584
>
Alex Hung Sept. 27, 2024, 9:34 p.m. UTC | #3 
On 9/27/24 14:45, Melissa Wen wrote:
> Hi Alex,
> 
> Thanks for the intensive testing.
> 
> I'll need some time to reproduce and debug these regressions.
> 
> So, we can divide this series into four steps:
> 1-2 are the basis for drm_edid migration
> 3-4 are code cleanups
> 5-9 are drm_edid_product_id migration
> 10 is for ACPI EDID feature.
> 
> Bearing this and the reported regressions around the product_id part in 
> mind, I wonder if we can start by applying the drm_edid migration and 
> the ACPI EDID feature, which means applying patches 1-4 and 10. And then 
> let the second part of product_id migration for the next iteration.
> IMHO it seems a smoother transition.
> 
> WDYT?

This sounds like a good plan.

Not all tests were completed on 1-4 + 10 as the series were dropped due 
to the regressions. I can send the 5 patches for tests next week again; 
however, 10 cannot be applied cleanly on top of 1-4, and help from Mario 
to rebase is probably needed.

As for the rest, let me know if you cannot reproduce these issues since 
you may or may not have the same hardware configs.

> 
> Melissa
> 
> 
> On 27/09/2024 15:48, Alex Hung wrote:
>> Hi Mario and Melissa,
>>
>> There are three regressions identified during the test, and 
>> improvement is required before the patches can be merged. Please see 
>> details below.
>>
>> 1. null pointer when hot-plugging a dsc hub (+ three 4k60 monitors).
>>
>> This may point to "drm/amd/display: use drm_edid_product_id for 
>> parsing EDID product info" since that's the only patch calling 
>> drm_edid_get_product_id.
>>
>>
>> [  227.797361] RIP: 0010:drm_edid_get_product_id+0x1d/0x50 [drm]
>> [  227.797388] Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 
>> 00 00 55 48 89 e5 48 85 ff 74 24 48 8b 47 08 48 85 c0 74 1b 48 83 3f 
>> 7f 76 15 <48> 8b 50 08 5d 48 89 16 0f b7 40 10 66 89 46 08 e9 a9 47 ee 
>> ce 31
>> [  227.797391] RSP: 0018:ffffac58126f7930 EFLAGS: 00010216
>> [  227.797394] RAX: 000000000000372d RBX: ffff8eaeaf8ac808 RCX: 
>> ffff8eaeaf8ac107
>> [  227.797396] RDX: 0000000000000002 RSI: ffffac58126f7944 RDI: 
>> ffff8eaeeeaf9f80
>> [  227.797398] RBP: ffffac58126f7930 R08: ffff8eae8e500d00 R09: 
>> 0000000000000001
>> [  227.797400] R10: ffffac58126f7978 R11: 000000000017f81b R12: 
>> ffff8eae84cb0000
>> [  227.797402] R13: ffff8eaeeeaf9f80 R14: 0000000000000000 R15: 
>> 0000000000000100
>> [  227.797405] FS:  00007f031a616ac0(0000) GS:ffff8eb57cd80000(0000) 
>> knlGS:0000000000000000
>> [  227.797407] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [  227.797409] CR2: 0000000000003735 CR3: 000000014decc000 CR4: 
>> 0000000000750ef0
>> [  227.797411] PKRU: 55555554
>> [  227.797413] Call Trace:
>> [  227.797415]  <TASK>
>> [  227.797417]  ? show_regs+0x64/0x70
>> [  227.797423]  ? __die+0x24/0x70
>> [  227.797427]  ? page_fault_oops+0x160/0x520
>> [  227.797435]  ? do_user_addr_fault+0x2e9/0x6a0
>> [  227.797438]  ? dc_link_add_remote_sink+0x20/0x30 [amdgpu]
>> [  227.797654]  ? dm_dp_mst_get_modes+0xee/0x520 [amdgpu]
>> [  227.797882]  ? drm_helper_probe_single_connector_modes+0x1b5/0x670 
>> [drm_kms_helper]
>> [  227.797894]  ? exc_page_fault+0x7f/0x190
>> [  227.797899]  ? asm_exc_page_fault+0x27/0x30
>> [  227.797906]  ? drm_edid_get_product_id+0x1d/0x50 [drm]
>> [  227.797932]  dm_helpers_parse_edid_caps+0x69/0x260 [amdgpu]
>> [  227.798139] amdgpu 0000:0b:00.0: [drm:drm_dp_dpcd_write 
>> [drm_display_helper]] AMDGPU DM aux hw bus 2: 0x02003 AUX <- (ret=  1) 10
>> [  227.798158]  link_add_remote_sink+0xa8/0x1a0 [amdgpu]
>> [  227.798402]  dc_link_add_remote_sink+0x20/0x30 [amdgpu]
>> [  227.798615]  dm_dp_mst_get_modes+0xee/0x520 [amdgpu]
>> [  227.798843]  ? srso_alias_return_thunk+0x5/0xfbef5
>> [  227.798848] drm_helper_probe_single_connector_modes+0x1b5/0x670 
>> [drm_kms_helper]
>>
>>
>> 2. DP2 Display is not listed as an audio device
>>
>> Steps to reproduce:
>> - Attach display to system.
>> - Boot to Desktop (Wayland)
>> - Attempt to select display as an audio device.
>>
>> This points to patch "drm/amd/display: get SAD from drm_eld when 
>> parsing EDID caps"
>>
>>
>> 3. IGT amd_mem_leak'sconnector-suspend-resume fails on Navi 31.
>>
>> This points to the patch "drm/amd/display: remove dc_edid handler from 
>> dm_helpers_parse_edid_caps".
>>
>>
>> Using IGT_SRANDOM=1727192429 for randomisation
>> Opened device: /dev/dri/card0
>> Starting subtest: connector-suspend-resume
>> [cmd] rtcwake: assuming RTC uses UTC ...
>> rtcwake: wakeup from "mem" using /dev/rtc0 at Tue Sep 24 15:40:50 2024
>> unreferenced object 0xffff95c683517b00 (size 256):
>>   comm "kworker/u64:30", pid 3636, jiffies 4295452761
>>   hex dump (first 32 bytes):
>>     00 ff ff ff ff ff ff 00 22 0e c2 36 00 00 00 00 ........"..6....
>>     33 1d 01 04 b5 3c 22 78 3b 28 91 a7 55 4e a3 26 3....<"x;(..UN.&
>>   backtrace (crc 5800a91d):
>>     [<ffffffffbb73502a>] kmemleak_alloc+0x4a/0x80
>>     [<ffffffffbabbde07>] kmalloc_node_track_caller_noprof+0x337/0x410
>>     [<ffffffffbab631f8>] krealloc_noprof+0x58/0xb0
>>     [<ffffffffc04e8e38>] _drm_do_get_edid+0x138/0x410 [drm]
>>     [<ffffffffc04e9155>] drm_edid_read_custom+0x35/0xd0 [drm]
>>     [<ffffffffc04e9244>] drm_edid_read_ddc+0x44/0x80 [drm]
>>     [<ffffffffc1061808>] dm_helpers_read_local_edid+0x1d8/0x340 [amdgpu]
>>     [<ffffffffc11b7446>] detect_link_and_local_sink+0x356/0x1230 [amdgpu]
>>     [<ffffffffc11b8366>] link_detect+0x36/0x4f0 [amdgpu]
>>     [<ffffffffc13667a0>] dc_link_detect+0x20/0x30 [amdgpu]
>>     [<ffffffffc10536f0>] dm_resume+0x1e0/0x7d0 [amdgpu]
>>     [<ffffffffc0d25908>] amdgpu_device_ip_resume_phase2+0x58/0xd0 
>> [amdgpu]
>>     [<ffffffffc0d29cb7>] amdgpu_device_resume+0xa7/0x2e0 [amdgpu]
>>     [<ffffffffc0d2467c>] amdgpu_pmops_resume+0x4c/0x90 [amdgpu]
>>     [<ffffffffbb02e121>] pci_pm_resume+0x71/0x100
>>     [<ffffffffbb22c551>] dpm_run_callback+0x91/0x1e0
>> unreferenced object 0xffff95c6c58dd0c0 (size 16):
>>   comm "kworker/u64:30", pid 3636, jiffies 4295452764
>>   hex dump (first 16 bytes):
>>     00 01 00 00 00 00 00 00 00 7b 51 83 c6 95 ff ff .........{Q.....
>>   backtrace (crc 70d89717):
>>     [<ffffffffbb73502a>] kmemleak_alloc+0x4a/0x80
>>     [<ffffffffbabbbf3e>] kmalloc_trace_noprof+0x28e/0x300
>>     [<ffffffffc04e6845>] _drm_edid_alloc+0x35/0x60 [drm]
>>     [<ffffffffc04e9175>] drm_edid_read_custom+0x55/0xd0 [drm]
>>     [<ffffffffc04e9244>] drm_edid_read_ddc+0x44/0x80 [drm]
>>     [<ffffffffc1061808>] dm_helpers_read_local_edid+0x1d8/0x340 [amdgpu]
>>     [<ffffffffc11b7446>] detect_link_and_local_sink+0x356/0x1230 [amdgpu]
>>     [<ffffffffc11b8366>] link_detect+0x36/0x4f0 [amdgpu]
>>     [<ffffffffc13667a0>] dc_link_detect+0x20/0x30 [amdgpu]
>>     [<ffffffffc10536f0>] dm_resume+0x1e0/0x7d0 [amdgpu]
>>     [<ffffffffc0d25908>] amdgpu_device_ip_resume_phase2+0x58/0xd0 
>> [amdgpu]
>>     [<ffffffffc0d29cb7>] amdgpu_device_resume+0xa7/0x2e0 [amdgpu]
>>     [<ffffffffc0d2467c>] amdgpu_pmops_resume+0x4c/0x90 [amdgpu]
>>     [<ffffffffbb02e121>] pci_pm_resume+0x71/0x100
>>     [<ffffffffbb22c551>] dpm_run_callback+0x91/0x1e0
>>     [<ffffffffbb22ca76>] device_resume+0xb6/0x2c0
>> Stack trace:
>>   #0 ../lib/igt_core.c:2051 __igt_fail_assert()
>>   #1 ../tests/amdgpu/amd_mem_leak.c:202 __igt_unique____real_main208()
>>   #2 ../tests/amdgpu/amd_mem_leak.c:208 main()
>>   #3 ../sysdeps/nptl/libc_start_call_main.h:74 __libc_start_call_main()
>>   #4 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
>>   #5 [_start+0x25]
>> Subtest connector-suspend-resume: FAIL (9.535s)
>>
>> On 9/18/24 15:38, Mario Limonciello wrote:
>>> This is the successor of Melissa's v5 series that was posted [1] as well
>>> as my series that was posted [2].
>>>
>>> Melissa's patches are mostly unmodified from v5, but the series has been
>>> rebase on the new 6.10 based amd-staging-drm-next.
>>>
>>> As were both touching similar code for fetching the EDID, I've merged 
>>> the
>>> pertinent parts of my series into this one in allowing the connector to
>>> fetch the EDID from _DDC if available for eDP as well.
>>>
>>> There are still some remaining uses of drm_edid_raw() but they touch 
>>> pure
>>> DC code, so a wrapper or macro will probably be needed to convert them.
>>> This can be for follow ups later on.
>>>
>>> Link: https://lore.kernel.org/amd-gfx/20240807203207.2830-1- 
>>> mwen@igalia.com/ [1]
>>> Link: https://lore.kernel.org/dri-devel/20240214215756.6530-1- 
>>> mario.limonciello@amd.com/ [2]
>>>
>>> v7:
>>>   * Rebase on amd-staging-drm-next which is now 6.10 based
>>>   * Fix the two LKP robot reported issues
>>>
>>> Mario Limonciello (1):
>>>    drm/amd/display: Fetch the EDID from _DDC if available for eDP
>>>
>>> Melissa Wen (9):
>>>    drm/amd/display: switch amdgpu_dm_connector to use struct drm_edid
>>>    drm/amd/display: switch to setting physical address directly
>>>    drm/amd/display: always call connector_update when parsing
>>>      freesync_caps
>>>    drm/amd/display: remove redundant freesync parser for DP
>>>    drm/amd/display: use drm_edid_product_id for parsing EDID product 
>>> info
>>>    drm/amd/display: parse display name from drm_eld
>>>    drm/amd/display: get SAD from drm_eld when parsing EDID caps
>>>    drm/amd/display: get SADB from drm_eld when parsing EDID caps
>>>    drm/amd/display: remove dc_edid handler from
>>>      dm_helpers_parse_edid_caps
>>>
>>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 200 ++++++------------
>>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   4 +-
>>>   .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 172 +++++++++------
>>>   .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |  34 +--
>>>   drivers/gpu/drm/amd/display/dc/dm_helpers.h   |   1 -
>>>   .../drm/amd/display/dc/link/link_detection.c  |   6 +-
>>>   drivers/gpu/drm/amd/include/amd_shared.h      |   5 +
>>>   7 files changed, 200 insertions(+), 222 deletions(-)
>>>
>>>
>>> base-commit: 0569603f945225067d963c8ba4fda31d967ab584
>>
>