Skip to content

L
linux
Commit 07f18f0b authored by Mario Kleiner's avatar Mario Kleiner Committed by Alex Deucher
Browse files
Options

drm/radeon: Handle irqs only based on irq ring, not irq status regs. 

Trying to resolve issues with missed vblanks and impossible
values inside delivered kms pageflip completion events showed
that radeon's irq handling sometimes doesn't handle valid irqs,
but silently skips them. This was observed for vblank interrupts.

Although those irqs have corresponding events queued in the gpu's
irq ring at time of interrupt, and therefore the corresponding
handling code gets triggered by these events, the handling code
sometimes silently skipped processing the irq. The reason for those
skips is that the handling code double-checks for each irq event if
the corresponding irq status bits in the irq status registers
are set. Sometimes those bits are not set at time of check
for valid irqs, maybe due to some hardware race on some setups?

The problem only seems to happen on some machine + card combos
sometimes, e.g., never happened during my testing of different PC
cards of the DCE-2/3/4 generation a year ago, but happens consistently
now on two different Apple Mac cards (RV730, DCE-3, Apple iMac and
Evergreen JUNIPER, DCE-4 in a Apple MacPro). It also doesn't happen
at each interrupt but only occassionally every couple of
hundred or thousand vblank interrupts.

This results in XOrg warning messages like

"[  7084.472] (WW) RADEON(0): radeon_dri2_flip_event_handler:
Pageflip completion event has impossible msc 420120 < target_msc 420121"

as well as skipped frames and problems for applications that
use kms pageflip events or vblank events, e.g., users of DRI2 and
DRI3/Present, Waylands Weston compositor, etc. See also

https://bugs.freedesktop.org/show_bug.cgi?id=85203

After some talking to Alex and Michel, we decided to fix this
by turning the double-check for asserted irq status bits into a
warning. Whenever a irq event is queued in the IH ring, always
execute the corresponding interrupt handler. Still check the irq
status bits, but only to log a DRM_DEBUG message on a mismatch.

This fixed the problems reliably on both previously failing
cards, RV-730 dual-head tested on both crtcs (pipes D1 and D2)
and a triple-output Juniper HD-5770 card tested on all three
available crtcs (D1/D2/D3). The r600 and evergreen irq handling
is therefore tested, but the cik an si handling is only compile
tested due to lack of hw.
Reviewed-by: default avatarChristian König <christian.koenig@amd.com>
Signed-off-by: default avatarMario Kleiner <mario.kleiner.de@gmail.com>
CC: Michel Dänzer <michel.daenzer@amd.com>
CC: Alex Deucher <alexander.deucher@amd.com>
CC: <stable@vger.kernel.org> # v3.16+
Signed-off-by: default avatarAlex Deucher <alexander.deucher@amd.com>
parent d6ac4ffc
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 688 additions and 531 deletions
drivers/gpu/drm/radeon/cik.c
View file @ 07f18f0b
This diff is collapsed.
drivers/gpu/drm/radeon/evergreen.c
View file @ 07f18f0b
This diff is collapsed.
drivers/gpu/drm/radeon/r600.c
View file @ 07f18f0b
......@@ -4086,23 +4086,27 @@ int r600_irq_process(struct radeon_device *rdev)
case 1: /* D1 vblank/vline */
switch (src_data) {
case 0: /* D1 vblank */
if (rdev->irq.stat_regs.r600.disp_int & LB_D1_VBLANK_INTERRUPT) {
if (rdev->irq.crtc_vblank_int[0]) {
drm_handle_vblank(rdev->ddev, 0);
rdev->pm.vblank_sync = true;
wake_up(&rdev->irq.vblank_queue);
}
if (atomic_read(&rdev->irq.pflip[0]))
radeon_crtc_handle_vblank(rdev, 0);
rdev->irq.stat_regs.r600.disp_int &= ~LB_D1_VBLANK_INTERRUPT;
DRM_DEBUG("IH: D1 vblank\n");
if (!(rdev->irq.stat_regs.r600.disp_int & LB_D1_VBLANK_INTERRUPT))
DRM_DEBUG("IH: D1 vblank - IH event w/o asserted irq bit?\n");
if (rdev->irq.crtc_vblank_int[0]) {
drm_handle_vblank(rdev->ddev, 0);
rdev->pm.vblank_sync = true;
wake_up(&rdev->irq.vblank_queue);
}
if (atomic_read(&rdev->irq.pflip[0]))
radeon_crtc_handle_vblank(rdev, 0);
rdev->irq.stat_regs.r600.disp_int &= ~LB_D1_VBLANK_INTERRUPT;
DRM_DEBUG("IH: D1 vblank\n");
break;
case 1: /* D1 vline */
if (rdev->irq.stat_regs.r600.disp_int & LB_D1_VLINE_INTERRUPT) {
rdev->irq.stat_regs.r600.disp_int &= ~LB_D1_VLINE_INTERRUPT;
DRM_DEBUG("IH: D1 vline\n");
}
if (!(rdev->irq.stat_regs.r600.disp_int & LB_D1_VLINE_INTERRUPT))
DRM_DEBUG("IH: D1 vline - IH event w/o asserted irq bit?\n");
rdev->irq.stat_regs.r600.disp_int &= ~LB_D1_VLINE_INTERRUPT;
DRM_DEBUG("IH: D1 vline\n");
break;
default:
DRM_DEBUG("Unhandled interrupt: %d %d\n", src_id, src_data);
......@@ -4112,23 +4116,27 @@ int r600_irq_process(struct radeon_device *rdev)
case 5: /* D2 vblank/vline */
switch (src_data) {
case 0: /* D2 vblank */
if (rdev->irq.stat_regs.r600.disp_int & LB_D2_VBLANK_INTERRUPT) {
if (rdev->irq.crtc_vblank_int[1]) {
drm_handle_vblank(rdev->ddev, 1);
rdev->pm.vblank_sync = true;
wake_up(&rdev->irq.vblank_queue);
}
if (atomic_read(&rdev->irq.pflip[1]))
radeon_crtc_handle_vblank(rdev, 1);
rdev->irq.stat_regs.r600.disp_int &= ~LB_D2_VBLANK_INTERRUPT;
DRM_DEBUG("IH: D2 vblank\n");
if (!(rdev->irq.stat_regs.r600.disp_int & LB_D2_VBLANK_INTERRUPT))
DRM_DEBUG("IH: D2 vblank - IH event w/o asserted irq bit?\n");
if (rdev->irq.crtc_vblank_int[1]) {
drm_handle_vblank(rdev->ddev, 1);
rdev->pm.vblank_sync = true;
wake_up(&rdev->irq.vblank_queue);
}
if (atomic_read(&rdev->irq.pflip[1]))
radeon_crtc_handle_vblank(rdev, 1);
rdev->irq.stat_regs.r600.disp_int &= ~LB_D2_VBLANK_INTERRUPT;
DRM_DEBUG("IH: D2 vblank\n");
break;
case 1: /* D1 vline */
if (rdev->irq.stat_regs.r600.disp_int & LB_D2_VLINE_INTERRUPT) {
rdev->irq.stat_regs.r600.disp_int &= ~LB_D2_VLINE_INTERRUPT;
DRM_DEBUG("IH: D2 vline\n");
}
if (!(rdev->irq.stat_regs.r600.disp_int & LB_D2_VLINE_INTERRUPT))
DRM_DEBUG("IH: D2 vline - IH event w/o asserted irq bit?\n");
rdev->irq.stat_regs.r600.disp_int &= ~LB_D2_VLINE_INTERRUPT;
DRM_DEBUG("IH: D2 vline\n");
break;
default:
DRM_DEBUG("Unhandled interrupt: %d %d\n", src_id, src_data);
......@@ -4148,46 +4156,53 @@ int r600_irq_process(struct radeon_device *rdev)
case 19: /* HPD/DAC hotplug */
switch (src_data) {
case 0:
if (rdev->irq.stat_regs.r600.disp_int & DC_HPD1_INTERRUPT) {
rdev->irq.stat_regs.r600.disp_int &= ~DC_HPD1_INTERRUPT;
queue_hotplug = true;
DRM_DEBUG("IH: HPD1\n");
}
if (!(rdev->irq.stat_regs.r600.disp_int & DC_HPD1_INTERRUPT))
DRM_DEBUG("IH: HPD1 - IH event w/o asserted irq bit?\n");
rdev->irq.stat_regs.r600.disp_int &= ~DC_HPD1_INTERRUPT;
queue_hotplug = true;
DRM_DEBUG("IH: HPD1\n");
break;
case 1:
if (rdev->irq.stat_regs.r600.disp_int & DC_HPD2_INTERRUPT) {
rdev->irq.stat_regs.r600.disp_int &= ~DC_HPD2_INTERRUPT;
queue_hotplug = true;
DRM_DEBUG("IH: HPD2\n");
}
if (!(rdev->irq.stat_regs.r600.disp_int & DC_HPD2_INTERRUPT))
DRM_DEBUG("IH: HPD2 - IH event w/o asserted irq bit?\n");
rdev->irq.stat_regs.r600.disp_int &= ~DC_HPD2_INTERRUPT;
queue_hotplug = true;
DRM_DEBUG("IH: HPD2\n");
break;
case 4:
if (rdev->irq.stat_regs.r600.disp_int_cont & DC_HPD3_INTERRUPT) {
rdev->irq.stat_regs.r600.disp_int_cont &= ~DC_HPD3_INTERRUPT;
queue_hotplug = true;
DRM_DEBUG("IH: HPD3\n");
}
if (!(rdev->irq.stat_regs.r600.disp_int_cont & DC_HPD3_INTERRUPT))
DRM_DEBUG("IH: HPD3 - IH event w/o asserted irq bit?\n");
rdev->irq.stat_regs.r600.disp_int_cont &= ~DC_HPD3_INTERRUPT;
queue_hotplug = true;
DRM_DEBUG("IH: HPD3\n");
break;
case 5:
if (rdev->irq.stat_regs.r600.disp_int_cont & DC_HPD4_INTERRUPT) {
rdev->irq.stat_regs.r600.disp_int_cont &= ~DC_HPD4_INTERRUPT;
queue_hotplug = true;
DRM_DEBUG("IH: HPD4\n");
}
if (!(rdev->irq.stat_regs.r600.disp_int_cont & DC_HPD4_INTERRUPT))
DRM_DEBUG("IH: HPD4 - IH event w/o asserted irq bit?\n");
rdev->irq.stat_regs.r600.disp_int_cont &= ~DC_HPD4_INTERRUPT;
queue_hotplug = true;
DRM_DEBUG("IH: HPD4\n");
break;
case 10:
if (rdev->irq.stat_regs.r600.disp_int_cont2 & DC_HPD5_INTERRUPT) {
rdev->irq.stat_regs.r600.disp_int_cont2 &= ~DC_HPD5_INTERRUPT;
queue_hotplug = true;
DRM_DEBUG("IH: HPD5\n");
}
if (!(rdev->irq.stat_regs.r600.disp_int_cont2 & DC_HPD5_INTERRUPT))
DRM_DEBUG("IH: HPD5 - IH event w/o asserted irq bit?\n");
rdev->irq.stat_regs.r600.disp_int_cont2 &= ~DC_HPD5_INTERRUPT;
queue_hotplug = true;
DRM_DEBUG("IH: HPD5\n");
break;
case 12:
if (rdev->irq.stat_regs.r600.disp_int_cont2 & DC_HPD6_INTERRUPT) {
rdev->irq.stat_regs.r600.disp_int_cont2 &= ~DC_HPD6_INTERRUPT;
queue_hotplug = true;
DRM_DEBUG("IH: HPD6\n");
}
if (!(rdev->irq.stat_regs.r600.disp_int_cont2 & DC_HPD6_INTERRUPT))
DRM_DEBUG("IH: HPD6 - IH event w/o asserted irq bit?\n");
rdev->irq.stat_regs.r600.disp_int_cont2 &= ~DC_HPD6_INTERRUPT;
queue_hotplug = true;
DRM_DEBUG("IH: HPD6\n");
break;
default:
DRM_DEBUG("Unhandled interrupt: %d %d\n", src_id, src_data);
......@@ -4197,18 +4212,22 @@ int r600_irq_process(struct radeon_device *rdev)
case 21: /* hdmi */
switch (src_data) {
case 4:
if (rdev->irq.stat_regs.r600.hdmi0_status & HDMI0_AZ_FORMAT_WTRIG) {
rdev->irq.stat_regs.r600.hdmi0_status &= ~HDMI0_AZ_FORMAT_WTRIG;
queue_hdmi = true;
DRM_DEBUG("IH: HDMI0\n");
}
if (!(rdev->irq.stat_regs.r600.hdmi0_status & HDMI0_AZ_FORMAT_WTRIG))
DRM_DEBUG("IH: HDMI0 - IH event w/o asserted irq bit?\n");
rdev->irq.stat_regs.r600.hdmi0_status &= ~HDMI0_AZ_FORMAT_WTRIG;
queue_hdmi = true;
DRM_DEBUG("IH: HDMI0\n");
break;
case 5:
if (rdev->irq.stat_regs.r600.hdmi1_status & HDMI0_AZ_FORMAT_WTRIG) {
rdev->irq.stat_regs.r600.hdmi1_status &= ~HDMI0_AZ_FORMAT_WTRIG;
queue_hdmi = true;
DRM_DEBUG("IH: HDMI1\n");
}
if (!(rdev->irq.stat_regs.r600.hdmi1_status & HDMI0_AZ_FORMAT_WTRIG))
DRM_DEBUG("IH: HDMI1 - IH event w/o asserted irq bit?\n");
rdev->irq.stat_regs.r600.hdmi1_status &= ~HDMI0_AZ_FORMAT_WTRIG;
queue_hdmi = true;
DRM_DEBUG("IH: HDMI1\n");
break;
default:
DRM_ERROR("Unhandled interrupt: %d %d\n", src_id, src_data);
......
drivers/gpu/drm/radeon/si.c
View file @ 07f18f0b
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7