On 10/31/18 12:20 PM, Michel Dänzer wrote: > On 2018-10-31 3:41 p.m., Kazlauskas, Nicholas wrote: >> On 10/31/18 10:12 AM, Michel Dänzer wrote: >>> On 2018-10-31 2:38 p.m., Kazlauskas, Nicholas wrote: >>>> On 10/30/18 11:34 AM, Kazlauskas, Nicholas wrote: >>>>> >>>>> I understand the issue you're describing now. The timestamp is supposed >>>>> to signify the end of the current vblank. The call to get scanout >>>>> position is supposed to return the number of lines until scanout (a >>>>> negative value) or the number of lines since the next scanout began (a >>>>> positive value). >>>>> >>>>> The amdgpu driver calculates the number of lines based on a hardware >>>>> register status position which returns an increasing value from 0 that >>>>> indicates the current vpos/hpos for the display. >>>>> >>>>> For any vpos below vbl_start we know the value is correct since the next >>>>> vblank hasn't begun yet. But for anythign about vbl_start it's probably >>>>> wrong since it applies a corrective offset based on the fixed value of >>>>> crtc_vtotal. It's even worse when the value is above crtc_vtotal since >>>>> it'll be calculating the number of lines since the last scanout since >>>>> it'll be a positive value. >>>>> >>>>> So the issue becomes determining when the vfront porch will end. >>>>> >>>>> When the flip address gets written the vfront porch will end at the >>>>> start of the next line leaving only the back porch plus part of the >>>>> line. But we don't know when the flip will occur, if at all. It hasn't >>>>> occurred yet in this case. >>>>> >>>>> Waiting for the wrap around to 0 might be the best choice here since >>>>> there's no guarantee the flip will occur. >>>> >>>> I put some more thought into this and I don't think this is as bad as I >>>> had originally thought. >>>> >>>> I think the vblank timestamp is supposed to be for the first active >>>> pixel of the next scanout. The usage of which is for clients to time >>>> their content/animation/etc. >>>> >>>> The client likely doesn't care when they issue their flip, just that >>>> their content is matched for that vblank timestamp. The fixed refresh >>>> model works really well for that kind of application. >>>> >>>> Utilizing variable refresh rate would be a mistake in that case since >>>> the client would then have to time based on when they flip which is a >>>> lot harder to "predict" precisely. >>> >>> It's only a "mistake" as long as the timestamps are misleading. :) As >>> discussed before, accurate presentation timestamps are one requirement >>> for achieving perfectly smooth animation. >> >> For most 3D games the game world/rendering can advance by an arbitrary >> timestep - and faster will be smoother, which is what the native mode >> would give you. >> >> For fixed interval content/animation where you can't do that variable >> refresh rate could vastly improve the output. But like discussed before >> that would need a way to specify the interval/presentation timestamp to >> control that front porch duration. The timestamp will be misleading in >> any other case. > > I don't agree that an accurate timestamp can ever be more "misleading" > than an inaccurate one. But yeah, accurate timestamps and time-based > presentation are two sides of the same coin which can buy the holy grail > of perfectly smooth animation. :) > > >>>> I did some more investigation into when amdgpu gets the scanout position >>>> and what values we get back out of it. The timestamp is updated shortly >>>> after the crtc irq vblank which is typically within a few lines of >>>> vbl_start. This makes sense, since we can provide the prediction >>>> timestamp early. Waiting for the vblank to wrap around to 0 doesn't >>>> really make sense here since that would mean we already hit timeout or >>>> the flip occurred >>> >>> Sounds like you're mixing up the two cases of "actual" vblank events >>> (triggered by the "vblank" interrupt => drm_(crtc_)handle_vblank) and >>> flip completion events (triggered by the PFLIP interrupt with our >>> hardware => drm_crtc_send_vblank_event). >>> >>> Actual vblank events need to be delivered to userspace at the start of >>> vblank, so we indeed can't wait until the timestamp is accurate for >>> them. We just need to document the exact semantics of their timestamp >>> with VRR. >>> >>> For page flip completion events though, the timestamp needs to be >>> accurate and correspond to when the flipped frame starts being scanned >>> out, otherwise we'll surely break at least some userspace relying on >>> this information. >>> >> Yeah, I was. I guess what's sent is the estimated vblank timestamp >> calculated at the start of the interrupt. > > s/interrupt/vblank/, yeah. > > >> And since that's just a guess rather than what's actually going to >> happen it's going to be wrong in a lot of cases. >> >> I could see the wrap-around method working if the vblank timestamp was >> somehow updated in amdgpu or in drm_crtc_send_vblank_event. > > DC already calls drm_crtc_accurate_vblank_count before > drm_crtc_send_vblank_event, we "just" need to make sure that results in > an accurate timestamp. > > >> This would be a relatively simple fix but would break anything in >> userspace that relied on the timestamp for vblank interrupt and the >> flip completion being the same value. > > Hmm, that's a good point. So while VRR is enabled, maybe it is safer to > defer delivery of vblank events until the accurate timestamp is known as > well, at least by default. If there is userspace which needs the events > earlier even with VRR but can live with the guessed timestamp, a flag or > something could be added for that. > > I was under the impression that the vblank timestamp was reused but it's already going to differ since we call drm_crtc_accurate_vblank_count before drm_crtc_send_vblank_event. Thanks for pointing that out. Since that works for updating timestamp what's left is making sure that it waits for the wrap around if it's above crtc_vtotal. It might make sense to add a new flag for this that's only used within amdgpu_get_crtc_scanout_position so the other call sites aren't affected. There isn't a way to get an accurate timestamp with VRR enabled until after the flip happens. So deferring it kind of defeats the purpose of a client using it to make predictions before the flip for displaying their content. Nicholas Kazlauskas _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
