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

[Mario Kleiner]

On Feb 27, 2012, at 4:47 PM, Felix Kuehling wrote:

> On 12-02-24 11:38 PM, Mario Kleiner wrote:
>> On Feb 24, 2012, at 10:20 PM, Felix Kuehling wrote:
>>
>>> 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 at 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.
>>>
>>
>> The idea of the current logic was that it happened that the
>> update_pending bit isn't read back as zero at the end of the  
>> page_flip
>> programming function, immediately after clearing the graphics  
>> hardware
>> update_lock, e.g., maybe because there is some delay between clearing
>> that register and the double buffering taking place. But according to
>> the specs, if update_pending is high, ie., the hw has "accepted" the
>> request for a page flip, it will complete as long as that request
>> still happened within the vblank. So on a return from the page_flip
>> function with update_pending == 1 we check if we are still inside the
>> vblank area, ie., if the hw will certainly complete the double
>> buffering within the current vblank, because the request was made in
>> time.
>>
>> I did all my original testing of these bits with avivo hw (rv530,
>> r600) and with one radeon hd 5850, so i'm a bit surprised if the  
>> avivo
>> path would unconditionally need this adjustment. Could it be that the
>> observed accuracy of update_pending depends on the rest of the hw,
>> e.g., bus or processor speed, or bus activity? My test machines  
>> were a
>> MacBookPro with Core2Duo 2.3 Ghz for rv530 and a rather old AMD
>> Athlon-64 PC for r600.
>
> I used a few different systems:
>
> Brazos reference board for Brazos (E-350)
> Iconia Tab W500 (C-50)
> PhenomII X4 for most of the discrete cards
> Athlon64 X2 for RS690
>
> The results I got were consistent across all those systems. The only
> differences I saw between different generations of Avivo-based  
> hardware
> were about the exact timing of the threshold when I would start seeing
> flickering. On R5xx and R6xx it would start flickering at -3. On newer
> hardware at -4 (I didn't test on R7xx).
>
>>
>> I'm worried that your observed reliability of update_pending on >=
>> AVIVO asics could be an artifact of the specific computer hw you used
>> and that this doesn't generalize on older or different hw.
>
> Given the number of different ASICs and systems, I think that's
> unlikely. It's more likely that this depends on the exact mode  
> timings.
> I was running most of my experiments with a 19" DFP 1280x1024 at 60Hz,
> connected by DVI if available, or VGA on some of the IGPs.
>
> It's possible that different mode timings would result in a slightly
> different threshold.
>
>> If it doesn't generalize then the patch could defer a lot of  
>> perfectly
>> good pageflips by 1 frame, screwing the timestamps and reducing
>> framerate.
>
> I understand your concern. But given my observations, the current
> implementation potentially produces much more annoying artefacts.
>
>>
>> Here is what the rv630 register programming guide says about the
>> double-buffering of the surface base address registers:
>>
>> D1GRPH_SURFACE_UPDATE_PENDING: "the double buffering occurs in
>> vertical retrace when D1GRPH_SURFACE_UPDATE_PENDING = 1 and
>> D1GRPH_UPDATE_LOCK = 0 and V_UPDATE = 1."
>
> As far as I can tell, D1GRPH_SURFACE_UPDATE_PENDING will only be 1, if
> the page flip is initiated while outside the vertical retrace. The the
> actual page flip will occur when V_UPDATE changes to 1, that is, when
> the next vertical retrace occurs. So if we read
> D1GRPH_SURFACE_UPDATE_PENDING after initiating a page flip, it implies
> that we're currently not in a vertical retrace.
>
>>
>> D1CRTC_V_UPDATE: "Current vertical position ... 1 =  within the
>> V_UPDATE region (between end of vertical active display and  
>> start_line)"
>>
>> For us that would mean the threshold for deferred flip completion
>> would need to be whatever that mentioned "start_line" is, and for the
>> tested avivo hw, start_line used to be == start of active scanout, so
>> the threshold of zero made sense.
>
> In my experiments the start_line seemed to be between -4 and -2. On no
> ASIC did I observe a start_line == 0.
>
>>
>> Alex: I don't know where start_line is stored. Could it be that this
>> value changed due to hw changes or changes in the kms driver or
>> atombios? Or would it be possible to read that value back from some
>> register and use it as threshold?
>>
>> If you look at radeon_get_crtc_scanoutpos() you can also see that the
>> returned vpos is corrected for some offsets. Could it be that
>> something changed there for the DCE4.1 or DCE5 display engine or
>> whatever the C-50 APU uses? That could also explain why suddenly such
>> a weird threshold as vpos > -4 is needed on your tablet, because the
>> returned vpos is offset wrongly by a few scanlines.
>
> I believe the reason I observe flickering on my tablet is due to the
> short vsync period. Otherwise there is nothing special about my  
> tablet.
> I was able to induce the same flickering by artificially delaying the
> page flip on other Avivo hardware. My proposed fix resolves this
> potential problem on all Avivo hardware. I am very confident that it
> does that without deferring page flip notifications to user space
> unnecessarily.
>

Ok, you had a larger test set of machines and more specific test  
cases for this problem, i'm convinced. Thanks for all the testing.
You have my...

Reviewed-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de>

Wrt. to that new CRTC_ADVANCED_START_LINE_POSITION register on >=  
DCE4.1 that Alex found. Seems its default setting of 0x4 is the  
reason for the flicker threshold at vpos -4. Maybe it would by worth  
trying for the W500 tablet or devices/modes with similar short vblank  
intervals to reprogram that register with 0? That would give more  
headroom and maybe reduce your 25% number of deferred flips? Or is  
this likely to cause some hw trouble or artifacts because some line  
buffer can't fetch pixel data fast enough?

best,
-mario