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
