Re: Flickering with page-flipping on Acer Iconia W500 (AMD C-50 APU)

[Felix Kuehling <felix.kuehling@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 2317 bytes --]

On 12-02-22 11:20 AM, Felix Kuehling wrote:
> On 12-02-21 07:49 PM, Mario Kleiner wrote:
>> On 02/21/2012 09:07 PM, Alex Deucher wrote:
> [snip]
>>> The fix looks ok to me.  Mario any thoughts?
>>>
>>> Reviewed-by: Alex Deucher<alexdeucher@gmail.com>
>>>
>> Hi,
>>
>> the fix looks ok to me for that device, but could we make it
>> conditional on the AMD C-50 APU and similar pieces? It is the right
>> thing to do for that gpu, but for regular desktop gpus it is too
>> pessimistic if it defers the pageflip timestamping and completion
>> event for an already completed flip:
>>
>> 1. Makes the timestamps 1 refresh too late, causing timing sensitive
>> software like mine to detect false positives -- reporting skipped
>> frames were there weren't any. Not as bad as missing a really skipped
>> frame, but still not great.
> Agreed. I was going to perform some more experiments on other hardware
> to determine what the right threshold is for different hardware
> generations. I hope I'll get to that this week.

I have a final version of my patch including an explanation of the
observations it's based on (attached). It's against current drm-next
from git://people.freedesktop.org/~airlied/linux.

>
>> 2. Can reduce the framerate due to throttling the client, especially
>> on systems that are already challenged wrt. to their irq timing.
>>
>> Is the vblank period very short on these kind of devices? From Felix
>> description is sounds as if it is only 2 scanlines?
> It looks like that.

Turns out that that's not correct. Smaller negative values of vpos never
showed up in my log output because I didn't print it in case
update_pending was 0. The actual vblank period is 8 scan lines on this
device. Still not much compared to the ~40 I was seeing with an external
monitor. Anything > -4 would result in flickering in my experiments, so
only 5 scan lines worth of time are available for submitting the page
flip in time for the next frame. If I miss that time window, the flip is
deferred by an extra frame. In practice that seems to occur in about 25%
of cases on this particular device.

Regards,
  Felix

> Thanks for the feedback,
>   Felix
>
>> thanks,
>> -mario
>>

-- 
 _____    Felix Kuehling
 \ _  |   MTS Software Development Eng.
 /|_| |   SW-Linux Base Gfx | AMD
|__/ \|   T 905.882.2600 x8928


[-- Attachment #2: 0001-Fix-deferred-page-flip-detection-logic-on-Avivo-base.patch --]
[-- Type: text/x-patch, Size: 2438 bytes --]

>From ce90d636215554042809f8cc0a27dab03ba3ba8a Mon Sep 17 00:00:00 2001
From: Felix Kuehling <Felix.Kuehling@amd.com>
Date: Thu, 23 Feb 2012 19:16:12 -0500
Subject: [PATCH] Fix deferred page-flip detection logic on Avivo-based ASICs

This fixes page-flip-related flickering observed on Iconia Tab W500.

The update_pending status returned by radeon_page_flip is very accurate on
Avivo-based ASICs when vpos is negative.

Experiments were conducted on several ASIC generations ranging from RS690
to Cayman where the page flip was artificially timed to occur at a specific
vpos. With negative vpos, overriding update_pending always lead to
flickering.

The same experiment on RV380 and RV410 showed that update_pending is not
accurate with negative vpos. In most cases update_pending == 1 is returned
although the flip would complete before the start of the next frame.
Therefore I left the behaviour unchanged for pre-AVIVO ASICs for
performance reasons, although this may result in flickering in rare cases.

This change also makes the logic a little easier to understand.

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/radeon/radeon_display.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 7cb062d..1f98e5f 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -303,8 +303,17 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id)
 	if (update_pending &&
 	    (DRM_SCANOUTPOS_VALID & radeon_get_crtc_scanoutpos(rdev->ddev, crtc_id,
 							       &vpos, &hpos)) &&
-	    (vpos >=0) &&
-	    (vpos < (99 * rdev->mode_info.crtcs[crtc_id]->base.hwmode.crtc_vdisplay)/100)) {
+	    ((vpos >= (99 * rdev->mode_info.crtcs[crtc_id]->base.hwmode.crtc_vdisplay)/100) ||
+	     (vpos < 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
+		 * (nearly) complete and the flip will (likely)
+		 * complete before the start of the next frame.
+		 */
+		update_pending = 0;
+	}
+	if (update_pending) {
 		/* crtc didn't flip in this target vblank interval,
 		 * but flip is pending in crtc. It will complete it
 		 * in next vblank interval, so complete the flip at
-- 
1.7.5.4


[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel