[PATCH v7 00/10] drm/amd/display: Use drm_edid for more code
Melissa Wen
mwen at igalia.com
Fri Sep 27 20:45:13 UTC 2024
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
>
More information about the amd-gfx
mailing list