On 22/04/17 01:23 AM, Mario Kleiner wrote: > Since DC now uses CRTC_VERTICAL_INTERRUPT0 as VBLANK irq trigger > and vblank interrupts actually happen earliest at start of vblank, > instead of a bit before vblank, we no longer need some of the > fudging logic to deal with too early vblank irq handling (grep for > lb_vblank_lead_lines). This itself fixes a pageflip scheduling > bug in DC, caused by uninitialized use of lb_vblank_lead_lines, > with a wrong startup value of 0. Thanks to the new vblank irq > trigger this value of zero is now actually correct for DC :). > > A new problem is that vblank irq's race against pflip irq's, > and as both can fire at first line of vblank, it is no longer > guaranteed that vblank irq handling (therefore -> drm_handle_vblank() > -> drm_update_vblank_count()) executes before pflip irq handling > for a given vblank interval when a pageflip completes. Therefore > the vblank count and timestamps emitted to user-space as part of > the pageflip completion event will be often stale and cause new > timestamping and swap scheduling errors in user-space. > > This was observed with large frequency on R9 380 Tonga Pro. > > Fix this by enforcing a vblank count+timestamp update right > before emitting the pageflip completion event from the pflip > irq handler. The logic in core drm_update_vblank_count() makes > sure that no redundant or conflicting updates happen, iow. the > call turns into a no-op if it wasn't needed for that vblank, > burning a few microseconds of cpu time though. > > Successfully tested on AMD R9 380 "Tonga Pro" (VI/DCE 10) > with DC enabled on the current DC staging branch. Independent > measurement of pageflip completion timing with special hardware > measurement equipment now confirms correct pageflip timestamps > and counts in the pageflip completion events. > > Note that there is another unresolved pageflip bug present in current > dc staging, which causes pageflips to complete one vblank too early > when the pageflip ioctl gets called while in vblank. Something seems > to be amiss in the way amdgpu_dm_do_flip() handles 'target_vblank', > or how amdgpu_dm_atomic_commit_tail() computes 'target' for calling > amdgpu_dm_do_flip(). If the last paragraph refers to the problem fixed by patch 2, I'd drop that paragraph. With that, Reviewed-by: Michel Dänzer <michel.daenzer at amd.com> -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
