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?
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
>
