On 10/23/26 00:48, Albert Esteve wrote:
Hi John,
On Thu, Oct 22, 2026 at 10:31:54AM -0700, John Harrison wrote:
A bunch of the KUNIT tests (dm_test_hpd_init_*) expect that the init
function can be called with adev->dm.dc set to null. Indeed, the
dm_test_hpd_init_fini_with_connectors test has a comment explicitly
saying that the null is intentional. Unfortunately, since the patch
mentioned below, the first thing that the init function does is access
adev->dm.dc->link_count. Thus it is a null pointer dereference and a
boom (at least if running with KASAN).
I'm not sure if a null DC can ever occur outside of the KUNIT tests,
but there is presumably a reason for the KUNITs explicitly testing it.
So add a null check around the link_count loop.
This feels backwards reasoning. The tests assume in comments that
"a NULL dc that fallback is a safe no-op", which isn't true after
commit e38159437a91. Also, in amdgpu_dm_init() the DC is created and
checked, and if it is NULL errors out. Thus, DC is never NULL at
amdgpu_dm_hpd_init() in production.
The fix should be in the tests, maybe allocating a stub with
link_count 0?
I was meaning the later comment "DC is absent (NULL), so the HW writes
are NULL-safe no-ops.". To me that reads as the test is explicitly
checking that a NULL DC is safe. Also, the
dm_test_hpd_init_empty_connectors test says "init must complete without
touching the DC". Which also seems to be saying that a null DC is a
valid thing to happen and must be allowed.
If you are certain that there is never a possibility nor a reason for
having a null DC in reality, then sure, the tests can be updated
instead. This is not code I am especially familiar with - I'm just
trying to run the tests to ensure that other changes don't break
existing drivers. I just don't want to go removing test coverage simply
because a driver change breaks a test - that is generally the backwards
direction!
John.
BR,
Albert
Fixes: e38159437a91 ("drm/amd/display: only clear the hotplug sources the ASIC
has")
Cc: Mattia Tadini <[email protected]>
Cc: Roman Li <[email protected]>
Cc: Alex Deucher <[email protected]>
Cc: Alex Hung <[email protected]>
Cc: George Zhang <[email protected]>
Cc: Bhawanpreet Lakha <[email protected]>
Cc: "Mario Limonciello (AMD)" <[email protected]>
Cc: Leo Li <[email protected]>
Cc: "Timur Kristóf" <[email protected]>
Cc: Albert Esteve <[email protected]>
Cc: Geoffrey McRae <[email protected]>
Cc: Harry Wentland <[email protected]>
Signed-off-by: John Harrison <[email protected]>
---
.../drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c | 26 ++++++++++---------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
index 5b4ff079cd55..427f7bba343d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c
@@ -989,18 +989,20 @@ void amdgpu_dm_hpd_init(struct amdgpu_device *adev)
* has rather than over the whole enum: the unused slots of the DC
* interrupt table hold dummy handlers that assert when touched.
*/
- for (i = 0; i < adev->dm.dc->link_count; i++) {
- const struct dc_link *link = adev->dm.dc->links[i];
-
- if (link->irq_source_hpd != DC_IRQ_SOURCE_INVALID &&
- !amdgpu_dm_irq_set(adev, link->irq_source_hpd, false))
- drm_err(dev, "Failed to clear hpd source=%d on init\n",
- link->irq_source_hpd);
-
- if (link->irq_source_hpd_rx != DC_IRQ_SOURCE_INVALID &&
- !amdgpu_dm_irq_set(adev, link->irq_source_hpd_rx, false))
- drm_err(dev, "Failed to clear hpdrx source=%d on
init\n",
- link->irq_source_hpd_rx);
+ if (adev->dm.dc) {
+ for (i = 0; i < adev->dm.dc->link_count; i++) {
+ const struct dc_link *link = adev->dm.dc->links[i];
+
+ if (link->irq_source_hpd != DC_IRQ_SOURCE_INVALID &&
+ !amdgpu_dm_irq_set(adev, link->irq_source_hpd,
false))
+ drm_err(dev, "Failed to clear hpd source=%d on
init\n",
+ link->irq_source_hpd);
+
+ if (link->irq_source_hpd_rx != DC_IRQ_SOURCE_INVALID &&
+ !amdgpu_dm_irq_set(adev, link->irq_source_hpd_rx,
false))
+ drm_err(dev, "Failed to clear hpdrx source=%d on
init\n",
+ link->irq_source_hpd_rx);
+ }
}
drm_connector_list_iter_begin(dev, &iter);
--
2.43.0
