Skip to content

Commit 40b8c14

Browse files
leo-sunli1alexdeucher
authored andcommitted
drm/amd/display: Disable unneeded hpd interrupts during dm_init
[Why] It seems HPD interrupts are enabled by default for all connectors, even if the hpd source isn't valid. An eDP for example, does not have a valid hpd source (but does have a valid hpdrx source; see construct_phy()). Thus, eDPs should have their hpd interrupt disabled. In the past, this wasn't really an issue. Although the driver gets interrupted, then acks by writing to hw registers, there weren't any subscribed handlers that did anything meaningful (see register_hpd_handlers()). But things changed with the introduction of IPS. s2idle requires that the driver allows IPS for DMUB fw to put hw to sleep. Since register access requires hw to be awake, the driver will block IPS entry to do so. And no IPS means no hw sleep during s2idle. This was the observation on DCN35 systems with an eDP. During suspend, the eDP toggled its hpd pin as part of the panel power down sequence. The driver was then interrupted, and acked by writing to registers, blocking IPS entry. [How] Since DC marks eDP connections as having invalid hpd sources (see construct_phy()), DM should disable them at the hw level. Do so in amdgpu_dm_hpd_init() by disabling all hpd ints first, then selectively enabling ones for connectors that have valid hpd sources. Cc: Mario Limonciello <mario.limonciello@amd.com> Cc: Alex Deucher <alexander.deucher@amd.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Signed-off-by: Leo Li <sunpeng.li@amd.com> Signed-off-by: Tom Chung <chiahsuan.chung@amd.com> Tested-by: Daniel Wheeler <daniel.wheeler@amd.com> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> (cherry picked from commit 7b1ba19) Cc: stable@vger.kernel.org
1 parent 4afacc9 commit 40b8c14

File tree

1 file changed

+45
-19
lines changed

1 file changed

+45
-19
lines changed

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c

Lines changed: 45 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -894,8 +894,16 @@ void amdgpu_dm_hpd_init(struct amdgpu_device *adev)
894894
struct drm_device *dev = adev_to_drm(adev);
895895
struct drm_connector *connector;
896896
struct drm_connector_list_iter iter;
897+
int irq_type;
897898
int i;
898899

900+
/* First, clear all hpd and hpdrx interrupts */
901+
for (i = DC_IRQ_SOURCE_HPD1; i <= DC_IRQ_SOURCE_HPD6RX; i++) {
902+
if (!dc_interrupt_set(adev->dm.dc, i, false))
903+
drm_err(dev, "Failed to clear hpd(rx) source=%d on init\n",
904+
i);
905+
}
906+
899907
drm_connector_list_iter_begin(dev, &iter);
900908
drm_for_each_connector_iter(connector, &iter) {
901909
struct amdgpu_dm_connector *amdgpu_dm_connector;
@@ -908,10 +916,31 @@ void amdgpu_dm_hpd_init(struct amdgpu_device *adev)
908916

909917
dc_link = amdgpu_dm_connector->dc_link;
910918

919+
/*
920+
* Get a base driver irq reference for hpd ints for the lifetime
921+
* of dm. Note that only hpd interrupt types are registered with
922+
* base driver; hpd_rx types aren't. IOW, amdgpu_irq_get/put on
923+
* hpd_rx isn't available. DM currently controls hpd_rx
924+
* explicitly with dc_interrupt_set()
925+
*/
911926
if (dc_link->irq_source_hpd != DC_IRQ_SOURCE_INVALID) {
912-
dc_interrupt_set(adev->dm.dc,
913-
dc_link->irq_source_hpd,
914-
true);
927+
irq_type = dc_link->irq_source_hpd - DC_IRQ_SOURCE_HPD1;
928+
/*
929+
* TODO: There's a mismatch between mode_info.num_hpd
930+
* and what bios reports as the # of connectors with hpd
931+
* sources. Since the # of hpd source types registered
932+
* with base driver == mode_info.num_hpd, we have to
933+
* fallback to dc_interrupt_set for the remaining types.
934+
*/
935+
if (irq_type < adev->mode_info.num_hpd) {
936+
if (amdgpu_irq_get(adev, &adev->hpd_irq, irq_type))
937+
drm_err(dev, "DM_IRQ: Failed get HPD for source=%d)!\n",
938+
dc_link->irq_source_hpd);
939+
} else {
940+
dc_interrupt_set(adev->dm.dc,
941+
dc_link->irq_source_hpd,
942+
true);
943+
}
915944
}
916945

917946
if (dc_link->irq_source_hpd_rx != DC_IRQ_SOURCE_INVALID) {
@@ -921,12 +950,6 @@ void amdgpu_dm_hpd_init(struct amdgpu_device *adev)
921950
}
922951
}
923952
drm_connector_list_iter_end(&iter);
924-
925-
/* Update reference counts for HPDs */
926-
for (i = DC_IRQ_SOURCE_HPD1; i <= adev->mode_info.num_hpd; i++) {
927-
if (amdgpu_irq_get(adev, &adev->hpd_irq, i - DC_IRQ_SOURCE_HPD1))
928-
drm_err(dev, "DM_IRQ: Failed get HPD for source=%d)!\n", i);
929-
}
930953
}
931954

932955
/**
@@ -942,7 +965,7 @@ void amdgpu_dm_hpd_fini(struct amdgpu_device *adev)
942965
struct drm_device *dev = adev_to_drm(adev);
943966
struct drm_connector *connector;
944967
struct drm_connector_list_iter iter;
945-
int i;
968+
int irq_type;
946969

947970
drm_connector_list_iter_begin(dev, &iter);
948971
drm_for_each_connector_iter(connector, &iter) {
@@ -956,9 +979,18 @@ void amdgpu_dm_hpd_fini(struct amdgpu_device *adev)
956979
dc_link = amdgpu_dm_connector->dc_link;
957980

958981
if (dc_link->irq_source_hpd != DC_IRQ_SOURCE_INVALID) {
959-
dc_interrupt_set(adev->dm.dc,
960-
dc_link->irq_source_hpd,
961-
false);
982+
irq_type = dc_link->irq_source_hpd - DC_IRQ_SOURCE_HPD1;
983+
984+
/* TODO: See same TODO in amdgpu_dm_hpd_init() */
985+
if (irq_type < adev->mode_info.num_hpd) {
986+
if (amdgpu_irq_put(adev, &adev->hpd_irq, irq_type))
987+
drm_err(dev, "DM_IRQ: Failed put HPD for source=%d!\n",
988+
dc_link->irq_source_hpd);
989+
} else {
990+
dc_interrupt_set(adev->dm.dc,
991+
dc_link->irq_source_hpd,
992+
false);
993+
}
962994
}
963995

964996
if (dc_link->irq_source_hpd_rx != DC_IRQ_SOURCE_INVALID) {
@@ -968,10 +1000,4 @@ void amdgpu_dm_hpd_fini(struct amdgpu_device *adev)
9681000
}
9691001
}
9701002
drm_connector_list_iter_end(&iter);
971-
972-
/* Update reference counts for HPDs */
973-
for (i = DC_IRQ_SOURCE_HPD1; i <= adev->mode_info.num_hpd; i++) {
974-
if (amdgpu_irq_put(adev, &adev->hpd_irq, i - DC_IRQ_SOURCE_HPD1))
975-
drm_err(dev, "DM_IRQ: Failed put HPD for source=%d!\n", i);
976-
}
9771003
}

0 commit comments

Comments
 (0)