From mboxrd@z Thu Jan 1 00:00:00 1970 From: Felix Kuehling Subject: Flickering with page-flipping on Acer Iconia W500 (AMD C-50 APU) Date: Wed, 1 Feb 2012 17:43:26 -0500 Message-ID: <4F29C00E.4020300@amd.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------060806090007080707020004" Return-path: Received: from VA3EHSOBE007.bigfish.com (va3ehsobe002.messaging.microsoft.com [216.32.180.12]) by gabe.freedesktop.org (Postfix) with ESMTP id C3D3BA0DDE for ; Wed, 1 Feb 2012 14:58:37 -0800 (PST) List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: "Daenzer, Michel" , "Deucher, Alexander" Cc: mario.kleiner@tuebingen.mpg.de, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --------------060806090007080707020004 Content-Type: multipart/alternative; boundary="------------070003060903030703020503" --------------070003060903030703020503 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Following up on my message from Jan 19, now with a lot more hard data and a less intrusive modification. Still a prototype though. CC-ing DRI-devel and Mario Kleiner for a larger audience. To recap, I was seeing consistent flickering with Mesa/KMS on Android on my Iconia Tab W500 tablet with an AMD C-50 APU. I was also noticing occasional flickering under Ubuntu on the same system when maximizing/restoring windows and releasing windows after moving them. I believe that flickering is related to page flipping and the associated notifications to user space. A small modification to the page flipping code in the Radeon driver made the problem disappear on both Ubuntu and Android. As I understand it, the page flip notification logic works as follows: 1. Hardware issues vsync IRQ 2. IRQ handler calls radeon_crtc_handle_flip 3. radeon_crtc_handle_flip calls radeon_page_flip, which programs MMIO to flip pages and returns status whether the flip has been completed 4. if flip has not been completed, radeon_crtc_handle_flip uses current scanout position to predict whether flip will complete in the current frame or not 5. if flip is predicted to complete, signal user space, otherwise defer until next vsync IRQ The condition in step 4 needed a slight modification on my hardware. If the current scanout position is negative (inside vblank interval), the page flip will not complete until the next vsync on my hardware. I'm attaching two patches. radeon_flip_diag.diff adds some debug output to the kernel log that helped me understand the timing of VSync IRQs used for handling page-flips relative to the screen refresh in progress. It also measures the time it takes to program the page flip in MMIO in pixels scanned out (typically about 300 pixels, so relatively insignificant compared to the vertical refresh). On my system it turned out that the scanout position at the time radeon_crtc_handle_flip was called was somewhere between 798-800 and -2-0 (the LVDS screen having 800 visible rows). I used an awk script (also attached) to compute some statistics. With the original condition almost no page flips were detected as deferred to the next vsync IRQ. With the modified condition about 50% page flips were completed immediately according to radeon_page_flip and of the remaining ones about 50% were correctly predicted to complete based on the scanout position. In total about 25% were deferred until the next vsync. These 25% must have been causing flickering with the original condition. radeon_flip_fix.diff shows the minor modification to the condition used to decide whether a page-flip has been completed or will complete in the current screen refresh. My conclusion is that on this particular hardware the condition that predicts page flip completion must be modified in order to avoid notifying user space of completed page flips prematurely. Regards, Felix -- _____ Felix Kuehling \ _ | MTS Software Development Eng. /|_| | SW-Linux Base Gfx | AMD |__/ \| T 905.882.2600 x8928 --------------070003060903030703020503 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Following up on my message from Jan 19, now with a lot more hard data and a less intrusive modification. Still a prototype though. CC-ing DRI-devel and Mario Kleiner for a larger audience.

To recap, I was seeing consistent flickering with Mesa/KMS on Android on my Iconia Tab W500 tablet with an AMD C-50 APU. I was also noticing occasional flickering under Ubuntu on the same system when maximizing/restoring windows and releasing windows after moving them.

I believe that flickering is related to page flipping and the associated notifications to user space. A small modification to the page flipping code in the Radeon driver made the problem disappear on both Ubuntu and Android. As I understand it, the page flip notification logic works as follows:
  1. Hardware issues vsync IRQ
  2. IRQ handler calls radeon_crtc_handle_flip
  3. radeon_crtc_handle_flip calls radeon_page_flip, which programs MMIO to flip pages and returns status whether the flip has been completed
  4. if flip has not been completed, radeon_crtc_handle_flip uses current scanout position to predict whether flip will complete in the current frame or not
  5. if flip is predicted to complete, signal user space, otherwise defer until next vsync IRQ
The condition in step 4 needed a slight modification on my hardware. If the current scanout position is negative (inside vblank interval), the page flip will not complete until the next vsync on my hardware.

I'm attaching two patches. radeon_flip_diag.diff adds some debug output to the kernel log that helped me understand the timing of VSync IRQs used for handling page-flips relative to the screen refresh in progress. It also measures the time it takes to program the page flip in MMIO in pixels scanned out (typically about 300 pixels, so relatively insignificant compared to the vertical refresh).

On my system it turned out that the scanout position at the time radeon_crtc_handle_flip was called was somewhere between 798-800 and -2-0 (the LVDS screen having 800 visible rows). I used an awk script (also attached) to compute some statistics. With the original condition almost no page flips were detected as deferred to the next vsync IRQ. With the modified condition about 50% page flips were completed immediately according to radeon_page_flip and of the remaining ones about 50% were correctly predicted to complete based on the scanout position. In total about 25% were deferred until the next vsync. These 25% must have been causing flickering with the original condition.

radeon_flip_fix.diff shows the minor modification to the condition used to decide whether a page-flip has been completed or will complete in the current screen refresh.

My conclusion is that on this particular hardware the condition that  predicts page flip completion must be modified in order to avoid notifying user space of completed page flips prematurely.

Regards,
  Felix
-- 
 _____    Felix Kuehling
 \ _  |   MTS Software Development Eng.
 /|_| |   SW-Linux Base Gfx | AMD
|__/ \|   T 905.882.2600 x8928
--------------070003060903030703020503-- --------------060806090007080707020004 Content-Type: application/x-awk; name="parse_flip.awk" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="parse_flip.awk" Content-Description: parse_flip.awk QkVHSU4gICAgICAgICAgICAgICAgICAge2RlZmVycmVkPTA7IG5vdF9kZWZlcnJlZD0wOyBj b21wbGV0ZWQ9MH0KL1BlbmRpbmcgZGVmZXJyZWQvICAgICAge2RlZmVycmVkKyt9Ci9QZW5k aW5nIG5vdCBkZWZlcnJlZC8gIHtub3RfZGVmZXJyZWQrK30KL05vdCBwZW5kaW5nLyAgICAg ICAgICAge2NvbXBsZXRlZCsrfQpFTkQgewogICAgcHJpbnRmICJwZW5kaW5nIChkZWZlcnJl ZCwgbm90IGRlZmVycmVkKSA9ICglZCwgJWQpLCBpbW1lZGlhdGUgPSAlZFxuIiwgXAogICAg ICAgIGRlZmVycmVkLCBub3RfZGVmZXJyZWQsIGNvbXBsZXRlZC1kZWZlcnJyZWQKfQoKCiMg LTI6IChubyBmbGlja2VyKQojIHBlbmRpbmcgKGRlZmVycmVkLCBub3QgZGVmZXJyZWQpID0g KDE3MiwgMTg5KSwgaW1tZWRpYXRlID0gMzEzCgojIC0xOiAoZmxpY2tlcikKIyBwZW5kaW5n IChkZWZlcnJlZCwgbm90IGRlZmVycmVkKSA9ICg4LCA1NjQpLCBpbW1lZGlhdGUgPSAyMTkK --------------060806090007080707020004 Content-Type: text/x-patch; name="radeon_flip_diag.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="radeon_flip_diag.diff" Content-Description: radeon_flip_diag.diff diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index 292f73f..b050e11 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -277,7 +277,7 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id) struct timeval now; unsigned long flags; u32 update_pending; - int vpos, hpos; + int vpos, hpos, valid; spin_lock_irqsave(&rdev->ddev->event_lock, flags); work = radeon_crtc->unpin_work; @@ -288,8 +288,17 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id) } /* New pageflip, or just completion of a previous one? */ if (!radeon_crtc->deferred_flip_completion) { + int vpos0, hpos0, pixels; + valid = radeon_get_crtc_scanoutpos(rdev->ddev, crtc_id, + &vpos0, &hpos0); /* do the flip (mmio) */ update_pending = radeon_page_flip(rdev, crtc_id, work->new_crtc_base); + valid = radeon_get_crtc_scanoutpos(rdev->ddev, crtc_id, + &vpos, &hpos); + if (vpos < vpos0) + vpos0 -= rdev->mode_info.crtcs[crtc_id]->base.hwmode.crtc_vtotal; + pixels = hpos - hpos0 + (vpos - vpos0) * rdev->mode_info.crtcs[crtc_id]->base.hwmode.crtc_htotal; + DRM_ERROR ("radeon_page_flip took %d pixels\n", pixels); } else { /* This is just a completion of a flip queued in crtc * at last invocation. Make sure we go directly to @@ -302,19 +311,24 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id) /* Has the pageflip already completed in crtc, or is it certain * to complete in this vblank? */ - 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)) { - /* 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 - * next vblank irq. - */ - radeon_crtc->deferred_flip_completion = 1; - spin_unlock_irqrestore(&rdev->ddev->event_lock, flags); - return; + if (update_pending) { + if ((valid & DRM_SCANOUTPOS_VALID) && + (vpos >=0) && + (vpos < (99 * rdev->mode_info.crtcs[crtc_id]->base.hwmode.crtc_vdisplay)/100)) { + /* 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 + * next vblank irq. + */ + radeon_crtc->deferred_flip_completion = 1; + spin_unlock_irqrestore(&rdev->ddev->event_lock, flags); + DRM_ERROR ("Pending deferred: %d\n", vpos); + return; + } else { + DRM_ERROR ("Pending not deferred: %d\n", vpos); + } + } else { + DRM_ERROR ("Not pending\n"); } /* Pageflip (will be) certainly completed in this vblank. Clean up. */ --------------060806090007080707020004 Content-Type: text/x-patch; name="radeon_flip_fix.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="radeon_flip_fix.diff" Content-Description: radeon_flip_fix.diff diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index b050e11..66bcfcc 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -313,7 +313,7 @@ void radeon_crtc_handle_flip(struct radeon_device *rdev, int crtc_id) */ if (update_pending) { if ((valid & DRM_SCANOUTPOS_VALID) && - (vpos >=0) && + (vpos >=-2) && (vpos < (99 * rdev->mode_info.crtcs[crtc_id]->base.hwmode.crtc_vdisplay)/100)) { /* crtc didn't flip in this target vblank interval, * but flip is pending in crtc. It will complete it --------------060806090007080707020004 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --------------060806090007080707020004--