diff mbox

[1/2] drm/radeon: Slightly more robust flip completion handling for < DCE-4

Message ID 1474115139-25201-2-git-send-email-mario.kleiner.de@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mario Kleiner Sept. 17, 2016, 12:25 p.m. UTC
Pre DCE4 hardware doesn't have (reliable) pageflip completion
irqs, therefore we have to use the old polling method for flip
completion handling in vblank irq.

As vblank irqs fire a bit before start of vblank (when the
linebuffer fifo read position reaches end of scanout), we
have some fudge for flip completion handling in the last
lines of active scanout. Old code assumed the threshold to
be 99% of active scanout height, a ballpark estimate which
worked ok. Since we know since a while how to calculate the
actual threshold from linebuffer size, lets make use of it
to get a more accurate threshold.

This completion path is still prone to some races in corner
cases, especially on pre-AVIVO hardware, so document them
a bit better in the code comments.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 drivers/gpu/drm/radeon/radeon_display.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

Comments

Michel Dänzer Sept. 20, 2016, 2:58 a.m. UTC | #1
On 17/09/16 09:25 PM, Mario Kleiner wrote:
> Pre DCE4 hardware doesn't have (reliable) pageflip completion
> irqs, therefore we have to use the old polling method for flip
> completion handling in vblank irq.
> 
> As vblank irqs fire a bit before start of vblank (when the
> linebuffer fifo read position reaches end of scanout), we
> have some fudge for flip completion handling in the last
> lines of active scanout. Old code assumed the threshold to
> be 99% of active scanout height, a ballpark estimate which
> worked ok. Since we know since a while how to calculate the
> actual threshold from linebuffer size, lets make use of it
> to get a more accurate threshold.
> 
> This completion path is still prone to some races in corner
> cases, especially on pre-AVIVO hardware, so document them
> a bit better in the code comments.
> 
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>

[...]

> +	 * Note that this method of completion handling is still not 100% race
> +	 * free, as we could execute before the radeon_flip_work_func managed
> +	 * to run and set the RADEON_FLIP_SUBMITTED status, thereby we no-op,
> +	 * but the flip still gets programmed into hw and completed during
> +	 * vblank, leading to a delayed emission of the flip completion event.
> +	 * This applies at least to pre-AVIVO hardware, where flips are always
> +	 * completing inside vblank, not only at leading edge of vblank.

Does this part of the comment still apply with patch 2?

Anyway,

Acked-by: Michel Dänzer <michel.daenzer@amd.com>
diff mbox

Patch

diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 890171f..5070646 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -321,16 +321,30 @@  void radeon_crtc_handle_vblank(struct radeon_device *rdev, int crtc_id)
 	update_pending = radeon_page_flip_pending(rdev, crtc_id);
 
 	/* Has the pageflip already completed in crtc, or is it certain
-	 * to complete in this vblank?
+	 * to complete in this vblank? GET_DISTANCE_TO_VBLANKSTART provides
+	 * distance to start of "fudged earlier" vblank in vpos, distance to
+	 * start of real vblank in hpos. vpos >= 0 && hpos < 0 means we are in
+	 * the last few scanlines before start of real vblank, where the vblank
+	 * irq can fire, so we have sampled update_pending a bit too early and
+	 * know the flip will complete at leading edge of the upcoming real
+	 * vblank. On pre-AVIVO hardware, flips also complete inside the real
+	 * vblank, not only at leading edge, so if update_pending for hpos >= 0
+	 *  == inside real vblank, the flip will complete almost immediately.
+	 * Note that this method of completion handling is still not 100% race
+	 * free, as we could execute before the radeon_flip_work_func managed
+	 * to run and set the RADEON_FLIP_SUBMITTED status, thereby we no-op,
+	 * but the flip still gets programmed into hw and completed during
+	 * vblank, leading to a delayed emission of the flip completion event.
+	 * This applies at least to pre-AVIVO hardware, where flips are always
+	 * completing inside vblank, not only at leading edge of vblank.
 	 */
 	if (update_pending &&
-	    (DRM_SCANOUTPOS_VALID & radeon_get_crtc_scanoutpos(rdev->ddev,
-							       crtc_id,
-							       USE_REAL_VBLANKSTART,
-							       &vpos, &hpos, NULL, NULL,
-							       &rdev->mode_info.crtcs[crtc_id]->base.hwmode)) &&
-	    ((vpos >= (99 * rdev->mode_info.crtcs[crtc_id]->base.hwmode.crtc_vdisplay)/100) ||
-	     (vpos < 0 && !ASIC_IS_AVIVO(rdev)))) {
+	    (DRM_SCANOUTPOS_VALID &
+	     radeon_get_crtc_scanoutpos(rdev->ddev, crtc_id,
+					GET_DISTANCE_TO_VBLANKSTART,
+					&vpos, &hpos, NULL, NULL,
+					&rdev->mode_info.crtcs[crtc_id]->base.hwmode)) &&
+	    ((vpos >= 0 && hpos < 0) || (hpos >= 0 && !ASIC_IS_AVIVO(rdev)))) {
 		/* crtc didn't flip in this target vblank interval,
 		 * but flip is pending in crtc. Based on the current
 		 * scanout position we know that the current frame is