Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On Wed, Feb 13, 2019 at 07:10:00PM +0100, Mario Kleiner wrote: > On Wed, Feb 13, 2019 at 5:03 PM Daniel Vetter wrote: > > > > On Wed, Feb 13, 2019 at 4:46 PM Kazlauskas, Nicholas > > wrote: > > > > > > On 2/13/19 10:14 AM, Daniel Vetter wrote: > > > > On Wed, Feb 13, 2019 at 3:33 PM Kazlauskas, Nicholas > > > > wrote: > > > >> > > > >> On 2/13/19 4:50 AM, Daniel Vetter wrote: > > > >>> On Tue, Feb 12, 2019 at 10:32:31PM +0100, Mario Kleiner wrote: > > > On Mon, Feb 11, 2019 at 6:04 PM Daniel Vetter > > > wrote: > > > > > > > > On Mon, Feb 11, 2019 at 4:01 PM Kazlauskas, Nicholas > > > > wrote: > > > >> > > > >> On 2/11/19 3:35 AM, Daniel Vetter wrote: > > > >>> On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote: > > > The pageflip completion timestamps transmitted to userspace > > > via pageflip completion events are supposed to describe the > > > time at which the first pixel of the new post-pageflip scanout > > > buffer leaves the video output of the gpu. This time is > > > identical to end of vblank, when active scanout starts. > > > > > > For a crtc in standard fixed refresh rate, the end of vblank > > > is identical to the vblank timestamps calculated by > > > drm_update_vblank_count() at each vblank interrupt, or each > > > vblank dis-/enable. Therefore pageflip events just carry > > > that vblank timestamp as their pageflip timestamp. > > > > > > For a crtc switched to variable refresh rate mode (vrr), the > > > pageflip completion timestamps are identical to the vblank > > > timestamps iff the pageflip was executed early in vblank, > > > before the minimum vblank duration elapsed. In this case > > > the time of display onset is identical to when the crtc > > > is running in fixed refresh rate. > > > > > > However, if a pageflip completes later in the vblank, inside > > > the "extended front porch" in vrr mode, then the vblank will > > > terminate at a fixed (back porch) duration after flip, so > > > the display onset time is delayed correspondingly. In this > > > case the vblank timestamp computed at vblank irq time would > > > be too early, and we need a way to calculate an estimated > > > pageflip timestamp that will be later than the vblank timestamp. > > > > > > How a driver determines such a "late flip" timestamp is hw > > > and driver specific, but this patch adds a new helper function > > > that allows the driver to propose such an alternate "late flip" > > > timestamp for use in pageflip events: > > > > > > drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > > > > > > When sending out pageflip events, we now compare that proposed > > > flip_timestamp against the vblank timestamp of the current > > > vblank of flip completion and choose to send out the greater/ > > > later timestamp as flip completion timestamp. > > > > > > The most simple way for a kms driver to supply a suitable > > > flip_timestamp in vrr mode would be to simply take a timestamp > > > at start of the pageflip completion handler, e.g., pageflip > > > irq handler: flip_timestamp = ktime_get(); and then set that > > > as proposed "late" alternative timestamp via ... > > > drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > > > > > > More clever approaches could try to add some corrective offset > > > for fixed back porch duration, or ideally use hardware features > > > like hw timestamps to calculate the exact end time of vblank. > > > > > > Signed-off-by: Mario Kleiner > > > Cc: Nicholas Kazlauskas > > > Cc: Harry Wentland > > > Cc: Alex Deucher > > > >>> > > > >>> Uh, this looks like a pretty bad hack. Can't we fix amdgpu to > > > >>> only give us > > > >>> the right timestampe, once? With this I guess if you do a vblank > > > >>> query in > > > >>> between the wrong and the right vblank you'll get the bogus > > > >>> value. Not > > > >>> really great for userspace. > > > >>> -Daniel > > > >> > > > >> I think we calculate the timestamp and send the vblank event both > > > >> within > > > >> the pageflip IRQ handler so calculating the right pageflip > > > >> timestamp > > > >> once could probably be done. I'm not sure if it's easier than > > > >> proposing > > > >> a later flip time with an API like this though. > > > >> > > > >> The actual scanout time should be known from the page-flip handler > > > >> so > > > >> the semantics for VRR on/off remain the same. This is because the > >
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On 2/13/19 1:10 PM, Mario Kleiner wrote: > On Wed, Feb 13, 2019 at 5:03 PM Daniel Vetter wrote: >> >> On Wed, Feb 13, 2019 at 4:46 PM Kazlauskas, Nicholas >> wrote: >>> >>> On 2/13/19 10:14 AM, Daniel Vetter wrote: On Wed, Feb 13, 2019 at 3:33 PM Kazlauskas, Nicholas wrote: > > On 2/13/19 4:50 AM, Daniel Vetter wrote: >> On Tue, Feb 12, 2019 at 10:32:31PM +0100, Mario Kleiner wrote: >>> On Mon, Feb 11, 2019 at 6:04 PM Daniel Vetter wrote: On Mon, Feb 11, 2019 at 4:01 PM Kazlauskas, Nicholas wrote: > > On 2/11/19 3:35 AM, Daniel Vetter wrote: >> On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote: >>> The pageflip completion timestamps transmitted to userspace >>> via pageflip completion events are supposed to describe the >>> time at which the first pixel of the new post-pageflip scanout >>> buffer leaves the video output of the gpu. This time is >>> identical to end of vblank, when active scanout starts. >>> >>> For a crtc in standard fixed refresh rate, the end of vblank >>> is identical to the vblank timestamps calculated by >>> drm_update_vblank_count() at each vblank interrupt, or each >>> vblank dis-/enable. Therefore pageflip events just carry >>> that vblank timestamp as their pageflip timestamp. >>> >>> For a crtc switched to variable refresh rate mode (vrr), the >>> pageflip completion timestamps are identical to the vblank >>> timestamps iff the pageflip was executed early in vblank, >>> before the minimum vblank duration elapsed. In this case >>> the time of display onset is identical to when the crtc >>> is running in fixed refresh rate. >>> >>> However, if a pageflip completes later in the vblank, inside >>> the "extended front porch" in vrr mode, then the vblank will >>> terminate at a fixed (back porch) duration after flip, so >>> the display onset time is delayed correspondingly. In this >>> case the vblank timestamp computed at vblank irq time would >>> be too early, and we need a way to calculate an estimated >>> pageflip timestamp that will be later than the vblank timestamp. >>> >>> How a driver determines such a "late flip" timestamp is hw >>> and driver specific, but this patch adds a new helper function >>> that allows the driver to propose such an alternate "late flip" >>> timestamp for use in pageflip events: >>> >>> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); >>> >>> When sending out pageflip events, we now compare that proposed >>> flip_timestamp against the vblank timestamp of the current >>> vblank of flip completion and choose to send out the greater/ >>> later timestamp as flip completion timestamp. >>> >>> The most simple way for a kms driver to supply a suitable >>> flip_timestamp in vrr mode would be to simply take a timestamp >>> at start of the pageflip completion handler, e.g., pageflip >>> irq handler: flip_timestamp = ktime_get(); and then set that >>> as proposed "late" alternative timestamp via ... >>> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); >>> >>> More clever approaches could try to add some corrective offset >>> for fixed back porch duration, or ideally use hardware features >>> like hw timestamps to calculate the exact end time of vblank. >>> >>> Signed-off-by: Mario Kleiner >>> Cc: Nicholas Kazlauskas >>> Cc: Harry Wentland >>> Cc: Alex Deucher >> >> Uh, this looks like a pretty bad hack. Can't we fix amdgpu to only >> give us >> the right timestampe, once? With this I guess if you do a vblank >> query in >> between the wrong and the right vblank you'll get the bogus value. >> Not >> really great for userspace. >> -Daniel > > I think we calculate the timestamp and send the vblank event both > within > the pageflip IRQ handler so calculating the right pageflip timestamp > once could probably be done. I'm not sure if it's easier than > proposing > a later flip time with an API like this though. > > The actual scanout time should be known from the page-flip handler so > the semantics for VRR on/off remain the same. This is because the > page-flip triggers entering the back porch if we're in the extended > front porch. > > But scanout time from vblank events for something like > DRM_IOCTL_WAIT_VBLANK are going to be wrong in most cases and are only > treated as estimates. If we're in the regular front porch
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On Wed, Feb 13, 2019 at 5:03 PM Daniel Vetter wrote: > > On Wed, Feb 13, 2019 at 4:46 PM Kazlauskas, Nicholas > wrote: > > > > On 2/13/19 10:14 AM, Daniel Vetter wrote: > > > On Wed, Feb 13, 2019 at 3:33 PM Kazlauskas, Nicholas > > > wrote: > > >> > > >> On 2/13/19 4:50 AM, Daniel Vetter wrote: > > >>> On Tue, Feb 12, 2019 at 10:32:31PM +0100, Mario Kleiner wrote: > > On Mon, Feb 11, 2019 at 6:04 PM Daniel Vetter wrote: > > > > > > On Mon, Feb 11, 2019 at 4:01 PM Kazlauskas, Nicholas > > > wrote: > > >> > > >> On 2/11/19 3:35 AM, Daniel Vetter wrote: > > >>> On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote: > > The pageflip completion timestamps transmitted to userspace > > via pageflip completion events are supposed to describe the > > time at which the first pixel of the new post-pageflip scanout > > buffer leaves the video output of the gpu. This time is > > identical to end of vblank, when active scanout starts. > > > > For a crtc in standard fixed refresh rate, the end of vblank > > is identical to the vblank timestamps calculated by > > drm_update_vblank_count() at each vblank interrupt, or each > > vblank dis-/enable. Therefore pageflip events just carry > > that vblank timestamp as their pageflip timestamp. > > > > For a crtc switched to variable refresh rate mode (vrr), the > > pageflip completion timestamps are identical to the vblank > > timestamps iff the pageflip was executed early in vblank, > > before the minimum vblank duration elapsed. In this case > > the time of display onset is identical to when the crtc > > is running in fixed refresh rate. > > > > However, if a pageflip completes later in the vblank, inside > > the "extended front porch" in vrr mode, then the vblank will > > terminate at a fixed (back porch) duration after flip, so > > the display onset time is delayed correspondingly. In this > > case the vblank timestamp computed at vblank irq time would > > be too early, and we need a way to calculate an estimated > > pageflip timestamp that will be later than the vblank timestamp. > > > > How a driver determines such a "late flip" timestamp is hw > > and driver specific, but this patch adds a new helper function > > that allows the driver to propose such an alternate "late flip" > > timestamp for use in pageflip events: > > > > drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > > > > When sending out pageflip events, we now compare that proposed > > flip_timestamp against the vblank timestamp of the current > > vblank of flip completion and choose to send out the greater/ > > later timestamp as flip completion timestamp. > > > > The most simple way for a kms driver to supply a suitable > > flip_timestamp in vrr mode would be to simply take a timestamp > > at start of the pageflip completion handler, e.g., pageflip > > irq handler: flip_timestamp = ktime_get(); and then set that > > as proposed "late" alternative timestamp via ... > > drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > > > > More clever approaches could try to add some corrective offset > > for fixed back porch duration, or ideally use hardware features > > like hw timestamps to calculate the exact end time of vblank. > > > > Signed-off-by: Mario Kleiner > > Cc: Nicholas Kazlauskas > > Cc: Harry Wentland > > Cc: Alex Deucher > > >>> > > >>> Uh, this looks like a pretty bad hack. Can't we fix amdgpu to only > > >>> give us > > >>> the right timestampe, once? With this I guess if you do a vblank > > >>> query in > > >>> between the wrong and the right vblank you'll get the bogus value. > > >>> Not > > >>> really great for userspace. > > >>> -Daniel > > >> > > >> I think we calculate the timestamp and send the vblank event both > > >> within > > >> the pageflip IRQ handler so calculating the right pageflip timestamp > > >> once could probably be done. I'm not sure if it's easier than > > >> proposing > > >> a later flip time with an API like this though. > > >> > > >> The actual scanout time should be known from the page-flip handler so > > >> the semantics for VRR on/off remain the same. This is because the > > >> page-flip triggers entering the back porch if we're in the extended > > >> front porch. > > >> > > >> But scanout time from vblank events for something like > > >> DRM_IOCTL_WAIT_VBLANK are going to be wrong in most cases and are > > >> only > > >>
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On Wed, Feb 13, 2019 at 4:46 PM Kazlauskas, Nicholas wrote: > > On 2/13/19 10:14 AM, Daniel Vetter wrote: > > On Wed, Feb 13, 2019 at 3:33 PM Kazlauskas, Nicholas > > wrote: > >> > >> On 2/13/19 4:50 AM, Daniel Vetter wrote: > >>> On Tue, Feb 12, 2019 at 10:32:31PM +0100, Mario Kleiner wrote: > On Mon, Feb 11, 2019 at 6:04 PM Daniel Vetter wrote: > > > > On Mon, Feb 11, 2019 at 4:01 PM Kazlauskas, Nicholas > > wrote: > >> > >> On 2/11/19 3:35 AM, Daniel Vetter wrote: > >>> On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote: > The pageflip completion timestamps transmitted to userspace > via pageflip completion events are supposed to describe the > time at which the first pixel of the new post-pageflip scanout > buffer leaves the video output of the gpu. This time is > identical to end of vblank, when active scanout starts. > > For a crtc in standard fixed refresh rate, the end of vblank > is identical to the vblank timestamps calculated by > drm_update_vblank_count() at each vblank interrupt, or each > vblank dis-/enable. Therefore pageflip events just carry > that vblank timestamp as their pageflip timestamp. > > For a crtc switched to variable refresh rate mode (vrr), the > pageflip completion timestamps are identical to the vblank > timestamps iff the pageflip was executed early in vblank, > before the minimum vblank duration elapsed. In this case > the time of display onset is identical to when the crtc > is running in fixed refresh rate. > > However, if a pageflip completes later in the vblank, inside > the "extended front porch" in vrr mode, then the vblank will > terminate at a fixed (back porch) duration after flip, so > the display onset time is delayed correspondingly. In this > case the vblank timestamp computed at vblank irq time would > be too early, and we need a way to calculate an estimated > pageflip timestamp that will be later than the vblank timestamp. > > How a driver determines such a "late flip" timestamp is hw > and driver specific, but this patch adds a new helper function > that allows the driver to propose such an alternate "late flip" > timestamp for use in pageflip events: > > drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > > When sending out pageflip events, we now compare that proposed > flip_timestamp against the vblank timestamp of the current > vblank of flip completion and choose to send out the greater/ > later timestamp as flip completion timestamp. > > The most simple way for a kms driver to supply a suitable > flip_timestamp in vrr mode would be to simply take a timestamp > at start of the pageflip completion handler, e.g., pageflip > irq handler: flip_timestamp = ktime_get(); and then set that > as proposed "late" alternative timestamp via ... > drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > > More clever approaches could try to add some corrective offset > for fixed back porch duration, or ideally use hardware features > like hw timestamps to calculate the exact end time of vblank. > > Signed-off-by: Mario Kleiner > Cc: Nicholas Kazlauskas > Cc: Harry Wentland > Cc: Alex Deucher > >>> > >>> Uh, this looks like a pretty bad hack. Can't we fix amdgpu to only > >>> give us > >>> the right timestampe, once? With this I guess if you do a vblank > >>> query in > >>> between the wrong and the right vblank you'll get the bogus value. Not > >>> really great for userspace. > >>> -Daniel > >> > >> I think we calculate the timestamp and send the vblank event both > >> within > >> the pageflip IRQ handler so calculating the right pageflip timestamp > >> once could probably be done. I'm not sure if it's easier than proposing > >> a later flip time with an API like this though. > >> > >> The actual scanout time should be known from the page-flip handler so > >> the semantics for VRR on/off remain the same. This is because the > >> page-flip triggers entering the back porch if we're in the extended > >> front porch. > >> > >> But scanout time from vblank events for something like > >> DRM_IOCTL_WAIT_VBLANK are going to be wrong in most cases and are only > >> treated as estimates. If we're in the regular front porch then the > >> timing to scanout is based on the fixed duration front porch for the > >> current mode. If we're in the extended back porch then it's technically > >> driver defined but the most reasonable guess is
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On 2/13/19 10:14 AM, Daniel Vetter wrote: > On Wed, Feb 13, 2019 at 3:33 PM Kazlauskas, Nicholas > wrote: >> >> On 2/13/19 4:50 AM, Daniel Vetter wrote: >>> On Tue, Feb 12, 2019 at 10:32:31PM +0100, Mario Kleiner wrote: On Mon, Feb 11, 2019 at 6:04 PM Daniel Vetter wrote: > > On Mon, Feb 11, 2019 at 4:01 PM Kazlauskas, Nicholas > wrote: >> >> On 2/11/19 3:35 AM, Daniel Vetter wrote: >>> On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote: The pageflip completion timestamps transmitted to userspace via pageflip completion events are supposed to describe the time at which the first pixel of the new post-pageflip scanout buffer leaves the video output of the gpu. This time is identical to end of vblank, when active scanout starts. For a crtc in standard fixed refresh rate, the end of vblank is identical to the vblank timestamps calculated by drm_update_vblank_count() at each vblank interrupt, or each vblank dis-/enable. Therefore pageflip events just carry that vblank timestamp as their pageflip timestamp. For a crtc switched to variable refresh rate mode (vrr), the pageflip completion timestamps are identical to the vblank timestamps iff the pageflip was executed early in vblank, before the minimum vblank duration elapsed. In this case the time of display onset is identical to when the crtc is running in fixed refresh rate. However, if a pageflip completes later in the vblank, inside the "extended front porch" in vrr mode, then the vblank will terminate at a fixed (back porch) duration after flip, so the display onset time is delayed correspondingly. In this case the vblank timestamp computed at vblank irq time would be too early, and we need a way to calculate an estimated pageflip timestamp that will be later than the vblank timestamp. How a driver determines such a "late flip" timestamp is hw and driver specific, but this patch adds a new helper function that allows the driver to propose such an alternate "late flip" timestamp for use in pageflip events: drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); When sending out pageflip events, we now compare that proposed flip_timestamp against the vblank timestamp of the current vblank of flip completion and choose to send out the greater/ later timestamp as flip completion timestamp. The most simple way for a kms driver to supply a suitable flip_timestamp in vrr mode would be to simply take a timestamp at start of the pageflip completion handler, e.g., pageflip irq handler: flip_timestamp = ktime_get(); and then set that as proposed "late" alternative timestamp via ... drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); More clever approaches could try to add some corrective offset for fixed back porch duration, or ideally use hardware features like hw timestamps to calculate the exact end time of vblank. Signed-off-by: Mario Kleiner Cc: Nicholas Kazlauskas Cc: Harry Wentland Cc: Alex Deucher >>> >>> Uh, this looks like a pretty bad hack. Can't we fix amdgpu to only give >>> us >>> the right timestampe, once? With this I guess if you do a vblank query >>> in >>> between the wrong and the right vblank you'll get the bogus value. Not >>> really great for userspace. >>> -Daniel >> >> I think we calculate the timestamp and send the vblank event both within >> the pageflip IRQ handler so calculating the right pageflip timestamp >> once could probably be done. I'm not sure if it's easier than proposing >> a later flip time with an API like this though. >> >> The actual scanout time should be known from the page-flip handler so >> the semantics for VRR on/off remain the same. This is because the >> page-flip triggers entering the back porch if we're in the extended >> front porch. >> >> But scanout time from vblank events for something like >> DRM_IOCTL_WAIT_VBLANK are going to be wrong in most cases and are only >> treated as estimates. If we're in the regular front porch then the >> timing to scanout is based on the fixed duration front porch for the >> current mode. If we're in the extended back porch then it's technically >> driver defined but the most reasonable guess is to assume that the front >> porch is going to end at any moment, so just return the length of the >> back porch for getting the scanout time. >> >> Proposing the late timestamp shouldn't affect vblank event in the >>
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On Wed, Feb 13, 2019 at 3:33 PM Kazlauskas, Nicholas wrote: > > On 2/13/19 4:50 AM, Daniel Vetter wrote: > > On Tue, Feb 12, 2019 at 10:32:31PM +0100, Mario Kleiner wrote: > >> On Mon, Feb 11, 2019 at 6:04 PM Daniel Vetter wrote: > >>> > >>> On Mon, Feb 11, 2019 at 4:01 PM Kazlauskas, Nicholas > >>> wrote: > > On 2/11/19 3:35 AM, Daniel Vetter wrote: > > On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote: > >> The pageflip completion timestamps transmitted to userspace > >> via pageflip completion events are supposed to describe the > >> time at which the first pixel of the new post-pageflip scanout > >> buffer leaves the video output of the gpu. This time is > >> identical to end of vblank, when active scanout starts. > >> > >> For a crtc in standard fixed refresh rate, the end of vblank > >> is identical to the vblank timestamps calculated by > >> drm_update_vblank_count() at each vblank interrupt, or each > >> vblank dis-/enable. Therefore pageflip events just carry > >> that vblank timestamp as their pageflip timestamp. > >> > >> For a crtc switched to variable refresh rate mode (vrr), the > >> pageflip completion timestamps are identical to the vblank > >> timestamps iff the pageflip was executed early in vblank, > >> before the minimum vblank duration elapsed. In this case > >> the time of display onset is identical to when the crtc > >> is running in fixed refresh rate. > >> > >> However, if a pageflip completes later in the vblank, inside > >> the "extended front porch" in vrr mode, then the vblank will > >> terminate at a fixed (back porch) duration after flip, so > >> the display onset time is delayed correspondingly. In this > >> case the vblank timestamp computed at vblank irq time would > >> be too early, and we need a way to calculate an estimated > >> pageflip timestamp that will be later than the vblank timestamp. > >> > >> How a driver determines such a "late flip" timestamp is hw > >> and driver specific, but this patch adds a new helper function > >> that allows the driver to propose such an alternate "late flip" > >> timestamp for use in pageflip events: > >> > >> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > >> > >> When sending out pageflip events, we now compare that proposed > >> flip_timestamp against the vblank timestamp of the current > >> vblank of flip completion and choose to send out the greater/ > >> later timestamp as flip completion timestamp. > >> > >> The most simple way for a kms driver to supply a suitable > >> flip_timestamp in vrr mode would be to simply take a timestamp > >> at start of the pageflip completion handler, e.g., pageflip > >> irq handler: flip_timestamp = ktime_get(); and then set that > >> as proposed "late" alternative timestamp via ... > >> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > >> > >> More clever approaches could try to add some corrective offset > >> for fixed back porch duration, or ideally use hardware features > >> like hw timestamps to calculate the exact end time of vblank. > >> > >> Signed-off-by: Mario Kleiner > >> Cc: Nicholas Kazlauskas > >> Cc: Harry Wentland > >> Cc: Alex Deucher > > > > Uh, this looks like a pretty bad hack. Can't we fix amdgpu to only give > > us > > the right timestampe, once? With this I guess if you do a vblank query > > in > > between the wrong and the right vblank you'll get the bogus value. Not > > really great for userspace. > > -Daniel > > I think we calculate the timestamp and send the vblank event both within > the pageflip IRQ handler so calculating the right pageflip timestamp > once could probably be done. I'm not sure if it's easier than proposing > a later flip time with an API like this though. > > The actual scanout time should be known from the page-flip handler so > the semantics for VRR on/off remain the same. This is because the > page-flip triggers entering the back porch if we're in the extended > front porch. > > But scanout time from vblank events for something like > DRM_IOCTL_WAIT_VBLANK are going to be wrong in most cases and are only > treated as estimates. If we're in the regular front porch then the > timing to scanout is based on the fixed duration front porch for the > current mode. If we're in the extended back porch then it's technically > driver defined but the most reasonable guess is to assume that the front > porch is going to end at any moment, so just return the length of the > back porch for getting the scanout time. > > Proposing the late timestamp shouldn't affect vblank event in the > DRM_IOCTL_WAIT_VBLANK case and should only be used in the page-flip
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On 2/13/19 4:50 AM, Daniel Vetter wrote: > On Tue, Feb 12, 2019 at 10:32:31PM +0100, Mario Kleiner wrote: >> On Mon, Feb 11, 2019 at 6:04 PM Daniel Vetter wrote: >>> >>> On Mon, Feb 11, 2019 at 4:01 PM Kazlauskas, Nicholas >>> wrote: On 2/11/19 3:35 AM, Daniel Vetter wrote: > On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote: >> The pageflip completion timestamps transmitted to userspace >> via pageflip completion events are supposed to describe the >> time at which the first pixel of the new post-pageflip scanout >> buffer leaves the video output of the gpu. This time is >> identical to end of vblank, when active scanout starts. >> >> For a crtc in standard fixed refresh rate, the end of vblank >> is identical to the vblank timestamps calculated by >> drm_update_vblank_count() at each vblank interrupt, or each >> vblank dis-/enable. Therefore pageflip events just carry >> that vblank timestamp as their pageflip timestamp. >> >> For a crtc switched to variable refresh rate mode (vrr), the >> pageflip completion timestamps are identical to the vblank >> timestamps iff the pageflip was executed early in vblank, >> before the minimum vblank duration elapsed. In this case >> the time of display onset is identical to when the crtc >> is running in fixed refresh rate. >> >> However, if a pageflip completes later in the vblank, inside >> the "extended front porch" in vrr mode, then the vblank will >> terminate at a fixed (back porch) duration after flip, so >> the display onset time is delayed correspondingly. In this >> case the vblank timestamp computed at vblank irq time would >> be too early, and we need a way to calculate an estimated >> pageflip timestamp that will be later than the vblank timestamp. >> >> How a driver determines such a "late flip" timestamp is hw >> and driver specific, but this patch adds a new helper function >> that allows the driver to propose such an alternate "late flip" >> timestamp for use in pageflip events: >> >> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); >> >> When sending out pageflip events, we now compare that proposed >> flip_timestamp against the vblank timestamp of the current >> vblank of flip completion and choose to send out the greater/ >> later timestamp as flip completion timestamp. >> >> The most simple way for a kms driver to supply a suitable >> flip_timestamp in vrr mode would be to simply take a timestamp >> at start of the pageflip completion handler, e.g., pageflip >> irq handler: flip_timestamp = ktime_get(); and then set that >> as proposed "late" alternative timestamp via ... >> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); >> >> More clever approaches could try to add some corrective offset >> for fixed back porch duration, or ideally use hardware features >> like hw timestamps to calculate the exact end time of vblank. >> >> Signed-off-by: Mario Kleiner >> Cc: Nicholas Kazlauskas >> Cc: Harry Wentland >> Cc: Alex Deucher > > Uh, this looks like a pretty bad hack. Can't we fix amdgpu to only give us > the right timestampe, once? With this I guess if you do a vblank query in > between the wrong and the right vblank you'll get the bogus value. Not > really great for userspace. > -Daniel I think we calculate the timestamp and send the vblank event both within the pageflip IRQ handler so calculating the right pageflip timestamp once could probably be done. I'm not sure if it's easier than proposing a later flip time with an API like this though. The actual scanout time should be known from the page-flip handler so the semantics for VRR on/off remain the same. This is because the page-flip triggers entering the back porch if we're in the extended front porch. But scanout time from vblank events for something like DRM_IOCTL_WAIT_VBLANK are going to be wrong in most cases and are only treated as estimates. If we're in the regular front porch then the timing to scanout is based on the fixed duration front porch for the current mode. If we're in the extended back porch then it's technically driver defined but the most reasonable guess is to assume that the front porch is going to end at any moment, so just return the length of the back porch for getting the scanout time. Proposing the late timestamp shouldn't affect vblank event in the DRM_IOCTL_WAIT_VBLANK case and should only be used in the page-flip event case. I'm not sure if that's what's guaranteed to happen with this patch though. There doesn't seem to be any locking on either dev->vblank_time_lock or the vblank->seqlock so while it's likely to get the same vblank event back as the
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On Wed, Feb 13, 2019 at 12:05 PM Mario Kleiner wrote: > > On Wed, Feb 13, 2019 at 10:56 AM Chris Wilson > wrote: > > > > Quoting Daniel Vetter (2019-02-13 09:50:55) > > > On Tue, Feb 12, 2019 at 10:32:31PM +0100, Mario Kleiner wrote: > > > > I think all kms drivers try to call drm_crtc_handle_vblank() at start > > > > of vblank to give Mesa the most time for frontbuffer rendering for > > > > classic X. But vblank events are also used for scheduling bufferswaps > > > > or other stuff for redirected windowed rendering, or via api's like > > > > OML_sync_controls glXWaitForMscOML, so there might be other things > > > > affected by a more delayed vblank handling. > > > > > > The frontbuffer rendering is very much X driver specific, and I think > > > -amdgpu/radeon is the only one that requires this. No i915 driver ever > > > used the vblank interrupt to schedule frontbuffer blits, we use some > > > CS-side stalls. > > > > Fwiw, the Present midlayer does use vblank scheduling for inplace copy > > updates. Not that I wish to encourage anyone to use frontbuffer > > rendering. > > -Chris > > Yes, that's what i meant. Under DRI2 at least AMD, Intel and nouveau > have throttling based on CS stalls to avoid tearing and do throttling. > DRI3/Present last time i checked just waited for a vblank event and > then triggered the blit - something that causes tearing even when > triggered at start of vblank. Hm, might be good to document all that stuff somewhere. But in the end I'd say the answer is "don't do frontbuffer rendering". I'm not sure we managed to document all the rules for existing vblank vs. flip interactions anywhere. At least I didn't find anything with a look at our docs. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On Wed, Feb 13, 2019 at 10:50 AM Daniel Vetter wrote: > > On Tue, Feb 12, 2019 at 10:32:31PM +0100, Mario Kleiner wrote: > > On Mon, Feb 11, 2019 at 6:04 PM Daniel Vetter wrote: > > > > > > On Mon, Feb 11, 2019 at 4:01 PM Kazlauskas, Nicholas > > > wrote: > > > > > > > > On 2/11/19 3:35 AM, Daniel Vetter wrote: > > > > > On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote: > > > > >> The pageflip completion timestamps transmitted to userspace ... > > > > > > > > > > Uh, this looks like a pretty bad hack. Can't we fix amdgpu to only > > > > > give us > > > > > the right timestampe, once? With this I guess if you do a vblank > > > > > query in > > > > > between the wrong and the right vblank you'll get the bogus value. Not > > > > > really great for userspace. > > > > > -Daniel > > > > > > > > I think we calculate the timestamp and send the vblank event both within > > > > the pageflip IRQ handler so calculating the right pageflip timestamp > > > > once could probably be done. I'm not sure if it's easier than proposing > > > > a later flip time with an API like this though. > > > > > > > > The actual scanout time should be known from the page-flip handler so > > > > the semantics for VRR on/off remain the same. This is because the > > > > page-flip triggers entering the back porch if we're in the extended > > > > front porch. > > > > > > > > But scanout time from vblank events for something like > > > > DRM_IOCTL_WAIT_VBLANK are going to be wrong in most cases and are only > > > > treated as estimates. If we're in the regular front porch then the > > > > timing to scanout is based on the fixed duration front porch for the > > > > current mode. If we're in the extended back porch then it's technically > > > > driver defined but the most reasonable guess is to assume that the front > > > > porch is going to end at any moment, so just return the length of the > > > > back porch for getting the scanout time. > > > > > > > > Proposing the late timestamp shouldn't affect vblank event in the > > > > DRM_IOCTL_WAIT_VBLANK case and should only be used in the page-flip > > > > event case. I'm not sure if that's what's guaranteed to happen with this > > > > patch though. There doesn't seem to be any locking on either > > > > dev->vblank_time_lock or the vblank->seqlock so while it's likely to get > > > > the same vblank event back as the one just stored I don't think it's > > > > guaranteed. > > > > > > That's the inconsistency I mean to highlight - the timestamp for the > > > same frame as observed through flip complete and through the > > > wait_vblank ioctl can differ. Which they really shouldn't. > > > > > > > Ideally they shouldn't differ. The kernel docs for drm_crtc_state say > > that vblank and pageflip timestamps should always match. But then the > > kernel docs for "Variable refresh properties" in drm_connector.c for > > vblank timestamps were changed for the VRR implementation in Linux > > 5.0-rc to redefine them when in VRR mode. They are defined, but > > probably rather useless for any practical purpose, like this: > > > > "The semantics for the vertical blank timestamp differ when > > variable refresh rate is active. The vertical blank timestamp > > is defined to be an estimate using the current mode's fixed > > refresh rate timings. The semantics for the page-flip event > > timestamp remain the same." > > Uh I missed that. That sounds like nonsense tbh. > > > So our docs contradict each other as of Linux 5.0-rc. Certainly having > > useful vblank timetamps would be useful. > > Yup, imo vblank should still match page_flip. Otherwise I expect a lot of > hilarity will ensue. > > > > Now added complication is that amdgpu sends out vblank events really > > > early, which is used by userspace to do frontbuffer rendering in the > > > vblank time. But I don't think anyone wants to do both VRR and > > > > I think all kms drivers try to call drm_crtc_handle_vblank() at start > > of vblank to give Mesa the most time for frontbuffer rendering for > > classic X. But vblank events are also used for scheduling bufferswaps > > or other stuff for redirected windowed rendering, or via api's like > > OML_sync_controls glXWaitForMscOML, so there might be other things > > affected by a more delayed vblank handling. > > The frontbuffer rendering is very much X driver specific, and I think > -amdgpu/radeon is the only one that requires this. No i915 driver ever > used the vblank interrupt to schedule frontbuffer blits, we use some > CS-side stalls. > > Wrt scheduling pageflips: The rule is that right after you've received the > vblank for frame X, then an immediately schedule pageflip should hit X+1, > but not X. amdgpu had this broken, but it's fixed since a while. > > > > frontbuffer rendering, hence I think it should be possible to create > > > correct vblank timestamps for the VRR case, while leaving the current > > > logic in place for everything else. But that means moving the entire > > > vblank
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On Wed, Feb 13, 2019 at 10:56 AM Chris Wilson wrote: > > Quoting Daniel Vetter (2019-02-13 09:50:55) > > On Tue, Feb 12, 2019 at 10:32:31PM +0100, Mario Kleiner wrote: > > > I think all kms drivers try to call drm_crtc_handle_vblank() at start > > > of vblank to give Mesa the most time for frontbuffer rendering for > > > classic X. But vblank events are also used for scheduling bufferswaps > > > or other stuff for redirected windowed rendering, or via api's like > > > OML_sync_controls glXWaitForMscOML, so there might be other things > > > affected by a more delayed vblank handling. > > > > The frontbuffer rendering is very much X driver specific, and I think > > -amdgpu/radeon is the only one that requires this. No i915 driver ever > > used the vblank interrupt to schedule frontbuffer blits, we use some > > CS-side stalls. > > Fwiw, the Present midlayer does use vblank scheduling for inplace copy > updates. Not that I wish to encourage anyone to use frontbuffer > rendering. > -Chris Yes, that's what i meant. Under DRI2 at least AMD, Intel and nouveau have throttling based on CS stalls to avoid tearing and do throttling. DRI3/Present last time i checked just waited for a vblank event and then triggered the blit - something that causes tearing even when triggered at start of vblank. -mario ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
Quoting Daniel Vetter (2019-02-13 09:50:55) > On Tue, Feb 12, 2019 at 10:32:31PM +0100, Mario Kleiner wrote: > > I think all kms drivers try to call drm_crtc_handle_vblank() at start > > of vblank to give Mesa the most time for frontbuffer rendering for > > classic X. But vblank events are also used for scheduling bufferswaps > > or other stuff for redirected windowed rendering, or via api's like > > OML_sync_controls glXWaitForMscOML, so there might be other things > > affected by a more delayed vblank handling. > > The frontbuffer rendering is very much X driver specific, and I think > -amdgpu/radeon is the only one that requires this. No i915 driver ever > used the vblank interrupt to schedule frontbuffer blits, we use some > CS-side stalls. Fwiw, the Present midlayer does use vblank scheduling for inplace copy updates. Not that I wish to encourage anyone to use frontbuffer rendering. -Chris ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On Tue, Feb 12, 2019 at 10:32:31PM +0100, Mario Kleiner wrote: > On Mon, Feb 11, 2019 at 6:04 PM Daniel Vetter wrote: > > > > On Mon, Feb 11, 2019 at 4:01 PM Kazlauskas, Nicholas > > wrote: > > > > > > On 2/11/19 3:35 AM, Daniel Vetter wrote: > > > > On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote: > > > >> The pageflip completion timestamps transmitted to userspace > > > >> via pageflip completion events are supposed to describe the > > > >> time at which the first pixel of the new post-pageflip scanout > > > >> buffer leaves the video output of the gpu. This time is > > > >> identical to end of vblank, when active scanout starts. > > > >> > > > >> For a crtc in standard fixed refresh rate, the end of vblank > > > >> is identical to the vblank timestamps calculated by > > > >> drm_update_vblank_count() at each vblank interrupt, or each > > > >> vblank dis-/enable. Therefore pageflip events just carry > > > >> that vblank timestamp as their pageflip timestamp. > > > >> > > > >> For a crtc switched to variable refresh rate mode (vrr), the > > > >> pageflip completion timestamps are identical to the vblank > > > >> timestamps iff the pageflip was executed early in vblank, > > > >> before the minimum vblank duration elapsed. In this case > > > >> the time of display onset is identical to when the crtc > > > >> is running in fixed refresh rate. > > > >> > > > >> However, if a pageflip completes later in the vblank, inside > > > >> the "extended front porch" in vrr mode, then the vblank will > > > >> terminate at a fixed (back porch) duration after flip, so > > > >> the display onset time is delayed correspondingly. In this > > > >> case the vblank timestamp computed at vblank irq time would > > > >> be too early, and we need a way to calculate an estimated > > > >> pageflip timestamp that will be later than the vblank timestamp. > > > >> > > > >> How a driver determines such a "late flip" timestamp is hw > > > >> and driver specific, but this patch adds a new helper function > > > >> that allows the driver to propose such an alternate "late flip" > > > >> timestamp for use in pageflip events: > > > >> > > > >> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > > > >> > > > >> When sending out pageflip events, we now compare that proposed > > > >> flip_timestamp against the vblank timestamp of the current > > > >> vblank of flip completion and choose to send out the greater/ > > > >> later timestamp as flip completion timestamp. > > > >> > > > >> The most simple way for a kms driver to supply a suitable > > > >> flip_timestamp in vrr mode would be to simply take a timestamp > > > >> at start of the pageflip completion handler, e.g., pageflip > > > >> irq handler: flip_timestamp = ktime_get(); and then set that > > > >> as proposed "late" alternative timestamp via ... > > > >> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > > > >> > > > >> More clever approaches could try to add some corrective offset > > > >> for fixed back porch duration, or ideally use hardware features > > > >> like hw timestamps to calculate the exact end time of vblank. > > > >> > > > >> Signed-off-by: Mario Kleiner > > > >> Cc: Nicholas Kazlauskas > > > >> Cc: Harry Wentland > > > >> Cc: Alex Deucher > > > > > > > > Uh, this looks like a pretty bad hack. Can't we fix amdgpu to only give > > > > us > > > > the right timestampe, once? With this I guess if you do a vblank query > > > > in > > > > between the wrong and the right vblank you'll get the bogus value. Not > > > > really great for userspace. > > > > -Daniel > > > > > > I think we calculate the timestamp and send the vblank event both within > > > the pageflip IRQ handler so calculating the right pageflip timestamp > > > once could probably be done. I'm not sure if it's easier than proposing > > > a later flip time with an API like this though. > > > > > > The actual scanout time should be known from the page-flip handler so > > > the semantics for VRR on/off remain the same. This is because the > > > page-flip triggers entering the back porch if we're in the extended > > > front porch. > > > > > > But scanout time from vblank events for something like > > > DRM_IOCTL_WAIT_VBLANK are going to be wrong in most cases and are only > > > treated as estimates. If we're in the regular front porch then the > > > timing to scanout is based on the fixed duration front porch for the > > > current mode. If we're in the extended back porch then it's technically > > > driver defined but the most reasonable guess is to assume that the front > > > porch is going to end at any moment, so just return the length of the > > > back porch for getting the scanout time. > > > > > > Proposing the late timestamp shouldn't affect vblank event in the > > > DRM_IOCTL_WAIT_VBLANK case and should only be used in the page-flip > > > event case. I'm not sure if that's what's guaranteed to happen with this > > > patch though. There doesn't seem to be any locking on
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On Mon, Feb 11, 2019 at 6:04 PM Daniel Vetter wrote: > > On Mon, Feb 11, 2019 at 4:01 PM Kazlauskas, Nicholas > wrote: > > > > On 2/11/19 3:35 AM, Daniel Vetter wrote: > > > On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote: > > >> The pageflip completion timestamps transmitted to userspace > > >> via pageflip completion events are supposed to describe the > > >> time at which the first pixel of the new post-pageflip scanout > > >> buffer leaves the video output of the gpu. This time is > > >> identical to end of vblank, when active scanout starts. > > >> > > >> For a crtc in standard fixed refresh rate, the end of vblank > > >> is identical to the vblank timestamps calculated by > > >> drm_update_vblank_count() at each vblank interrupt, or each > > >> vblank dis-/enable. Therefore pageflip events just carry > > >> that vblank timestamp as their pageflip timestamp. > > >> > > >> For a crtc switched to variable refresh rate mode (vrr), the > > >> pageflip completion timestamps are identical to the vblank > > >> timestamps iff the pageflip was executed early in vblank, > > >> before the minimum vblank duration elapsed. In this case > > >> the time of display onset is identical to when the crtc > > >> is running in fixed refresh rate. > > >> > > >> However, if a pageflip completes later in the vblank, inside > > >> the "extended front porch" in vrr mode, then the vblank will > > >> terminate at a fixed (back porch) duration after flip, so > > >> the display onset time is delayed correspondingly. In this > > >> case the vblank timestamp computed at vblank irq time would > > >> be too early, and we need a way to calculate an estimated > > >> pageflip timestamp that will be later than the vblank timestamp. > > >> > > >> How a driver determines such a "late flip" timestamp is hw > > >> and driver specific, but this patch adds a new helper function > > >> that allows the driver to propose such an alternate "late flip" > > >> timestamp for use in pageflip events: > > >> > > >> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > > >> > > >> When sending out pageflip events, we now compare that proposed > > >> flip_timestamp against the vblank timestamp of the current > > >> vblank of flip completion and choose to send out the greater/ > > >> later timestamp as flip completion timestamp. > > >> > > >> The most simple way for a kms driver to supply a suitable > > >> flip_timestamp in vrr mode would be to simply take a timestamp > > >> at start of the pageflip completion handler, e.g., pageflip > > >> irq handler: flip_timestamp = ktime_get(); and then set that > > >> as proposed "late" alternative timestamp via ... > > >> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > > >> > > >> More clever approaches could try to add some corrective offset > > >> for fixed back porch duration, or ideally use hardware features > > >> like hw timestamps to calculate the exact end time of vblank. > > >> > > >> Signed-off-by: Mario Kleiner > > >> Cc: Nicholas Kazlauskas > > >> Cc: Harry Wentland > > >> Cc: Alex Deucher > > > > > > Uh, this looks like a pretty bad hack. Can't we fix amdgpu to only give us > > > the right timestampe, once? With this I guess if you do a vblank query in > > > between the wrong and the right vblank you'll get the bogus value. Not > > > really great for userspace. > > > -Daniel > > > > I think we calculate the timestamp and send the vblank event both within > > the pageflip IRQ handler so calculating the right pageflip timestamp > > once could probably be done. I'm not sure if it's easier than proposing > > a later flip time with an API like this though. > > > > The actual scanout time should be known from the page-flip handler so > > the semantics for VRR on/off remain the same. This is because the > > page-flip triggers entering the back porch if we're in the extended > > front porch. > > > > But scanout time from vblank events for something like > > DRM_IOCTL_WAIT_VBLANK are going to be wrong in most cases and are only > > treated as estimates. If we're in the regular front porch then the > > timing to scanout is based on the fixed duration front porch for the > > current mode. If we're in the extended back porch then it's technically > > driver defined but the most reasonable guess is to assume that the front > > porch is going to end at any moment, so just return the length of the > > back porch for getting the scanout time. > > > > Proposing the late timestamp shouldn't affect vblank event in the > > DRM_IOCTL_WAIT_VBLANK case and should only be used in the page-flip > > event case. I'm not sure if that's what's guaranteed to happen with this > > patch though. There doesn't seem to be any locking on either > > dev->vblank_time_lock or the vblank->seqlock so while it's likely to get > > the same vblank event back as the one just stored I don't think it's > > guaranteed. > > That's the inconsistency I mean to highlight - the timestamp for the > same frame as
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On Mon, Feb 11, 2019 at 4:01 PM Kazlauskas, Nicholas wrote: > > On 2/11/19 3:35 AM, Daniel Vetter wrote: > > On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote: > >> The pageflip completion timestamps transmitted to userspace > >> via pageflip completion events are supposed to describe the > >> time at which the first pixel of the new post-pageflip scanout > >> buffer leaves the video output of the gpu. This time is > >> identical to end of vblank, when active scanout starts. > >> > >> For a crtc in standard fixed refresh rate, the end of vblank > >> is identical to the vblank timestamps calculated by > >> drm_update_vblank_count() at each vblank interrupt, or each > >> vblank dis-/enable. Therefore pageflip events just carry > >> that vblank timestamp as their pageflip timestamp. > >> > >> For a crtc switched to variable refresh rate mode (vrr), the > >> pageflip completion timestamps are identical to the vblank > >> timestamps iff the pageflip was executed early in vblank, > >> before the minimum vblank duration elapsed. In this case > >> the time of display onset is identical to when the crtc > >> is running in fixed refresh rate. > >> > >> However, if a pageflip completes later in the vblank, inside > >> the "extended front porch" in vrr mode, then the vblank will > >> terminate at a fixed (back porch) duration after flip, so > >> the display onset time is delayed correspondingly. In this > >> case the vblank timestamp computed at vblank irq time would > >> be too early, and we need a way to calculate an estimated > >> pageflip timestamp that will be later than the vblank timestamp. > >> > >> How a driver determines such a "late flip" timestamp is hw > >> and driver specific, but this patch adds a new helper function > >> that allows the driver to propose such an alternate "late flip" > >> timestamp for use in pageflip events: > >> > >> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > >> > >> When sending out pageflip events, we now compare that proposed > >> flip_timestamp against the vblank timestamp of the current > >> vblank of flip completion and choose to send out the greater/ > >> later timestamp as flip completion timestamp. > >> > >> The most simple way for a kms driver to supply a suitable > >> flip_timestamp in vrr mode would be to simply take a timestamp > >> at start of the pageflip completion handler, e.g., pageflip > >> irq handler: flip_timestamp = ktime_get(); and then set that > >> as proposed "late" alternative timestamp via ... > >> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp); > >> > >> More clever approaches could try to add some corrective offset > >> for fixed back porch duration, or ideally use hardware features > >> like hw timestamps to calculate the exact end time of vblank. > >> > >> Signed-off-by: Mario Kleiner > >> Cc: Nicholas Kazlauskas > >> Cc: Harry Wentland > >> Cc: Alex Deucher > > > > Uh, this looks like a pretty bad hack. Can't we fix amdgpu to only give us > > the right timestampe, once? With this I guess if you do a vblank query in > > between the wrong and the right vblank you'll get the bogus value. Not > > really great for userspace. > > -Daniel > > I think we calculate the timestamp and send the vblank event both within > the pageflip IRQ handler so calculating the right pageflip timestamp > once could probably be done. I'm not sure if it's easier than proposing > a later flip time with an API like this though. > > The actual scanout time should be known from the page-flip handler so > the semantics for VRR on/off remain the same. This is because the > page-flip triggers entering the back porch if we're in the extended > front porch. > > But scanout time from vblank events for something like > DRM_IOCTL_WAIT_VBLANK are going to be wrong in most cases and are only > treated as estimates. If we're in the regular front porch then the > timing to scanout is based on the fixed duration front porch for the > current mode. If we're in the extended back porch then it's technically > driver defined but the most reasonable guess is to assume that the front > porch is going to end at any moment, so just return the length of the > back porch for getting the scanout time. > > Proposing the late timestamp shouldn't affect vblank event in the > DRM_IOCTL_WAIT_VBLANK case and should only be used in the page-flip > event case. I'm not sure if that's what's guaranteed to happen with this > patch though. There doesn't seem to be any locking on either > dev->vblank_time_lock or the vblank->seqlock so while it's likely to get > the same vblank event back as the one just stored I don't think it's > guaranteed. That's the inconsistency I mean to highlight - the timestamp for the same frame as observed through flip complete and through the wait_vblank ioctl can differ. Which they really shouldn't. Now added complication is that amdgpu sends out vblank events really early, which is used by userspace to do frontbuffer rendering in the
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On 2/11/19 3:35 AM, Daniel Vetter wrote:
> On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote:
>> The pageflip completion timestamps transmitted to userspace
>> via pageflip completion events are supposed to describe the
>> time at which the first pixel of the new post-pageflip scanout
>> buffer leaves the video output of the gpu. This time is
>> identical to end of vblank, when active scanout starts.
>>
>> For a crtc in standard fixed refresh rate, the end of vblank
>> is identical to the vblank timestamps calculated by
>> drm_update_vblank_count() at each vblank interrupt, or each
>> vblank dis-/enable. Therefore pageflip events just carry
>> that vblank timestamp as their pageflip timestamp.
>>
>> For a crtc switched to variable refresh rate mode (vrr), the
>> pageflip completion timestamps are identical to the vblank
>> timestamps iff the pageflip was executed early in vblank,
>> before the minimum vblank duration elapsed. In this case
>> the time of display onset is identical to when the crtc
>> is running in fixed refresh rate.
>>
>> However, if a pageflip completes later in the vblank, inside
>> the "extended front porch" in vrr mode, then the vblank will
>> terminate at a fixed (back porch) duration after flip, so
>> the display onset time is delayed correspondingly. In this
>> case the vblank timestamp computed at vblank irq time would
>> be too early, and we need a way to calculate an estimated
>> pageflip timestamp that will be later than the vblank timestamp.
>>
>> How a driver determines such a "late flip" timestamp is hw
>> and driver specific, but this patch adds a new helper function
>> that allows the driver to propose such an alternate "late flip"
>> timestamp for use in pageflip events:
>>
>> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp);
>>
>> When sending out pageflip events, we now compare that proposed
>> flip_timestamp against the vblank timestamp of the current
>> vblank of flip completion and choose to send out the greater/
>> later timestamp as flip completion timestamp.
>>
>> The most simple way for a kms driver to supply a suitable
>> flip_timestamp in vrr mode would be to simply take a timestamp
>> at start of the pageflip completion handler, e.g., pageflip
>> irq handler: flip_timestamp = ktime_get(); and then set that
>> as proposed "late" alternative timestamp via ...
>> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp);
>>
>> More clever approaches could try to add some corrective offset
>> for fixed back porch duration, or ideally use hardware features
>> like hw timestamps to calculate the exact end time of vblank.
>>
>> Signed-off-by: Mario Kleiner
>> Cc: Nicholas Kazlauskas
>> Cc: Harry Wentland
>> Cc: Alex Deucher
>
> Uh, this looks like a pretty bad hack. Can't we fix amdgpu to only give us
> the right timestampe, once? With this I guess if you do a vblank query in
> between the wrong and the right vblank you'll get the bogus value. Not
> really great for userspace.
> -Daniel
I think we calculate the timestamp and send the vblank event both within
the pageflip IRQ handler so calculating the right pageflip timestamp
once could probably be done. I'm not sure if it's easier than proposing
a later flip time with an API like this though.
The actual scanout time should be known from the page-flip handler so
the semantics for VRR on/off remain the same. This is because the
page-flip triggers entering the back porch if we're in the extended
front porch.
But scanout time from vblank events for something like
DRM_IOCTL_WAIT_VBLANK are going to be wrong in most cases and are only
treated as estimates. If we're in the regular front porch then the
timing to scanout is based on the fixed duration front porch for the
current mode. If we're in the extended back porch then it's technically
driver defined but the most reasonable guess is to assume that the front
porch is going to end at any moment, so just return the length of the
back porch for getting the scanout time.
Proposing the late timestamp shouldn't affect vblank event in the
DRM_IOCTL_WAIT_VBLANK case and should only be used in the page-flip
event case. I'm not sure if that's what's guaranteed to happen with this
patch though. There doesn't seem to be any locking on either
dev->vblank_time_lock or the vblank->seqlock so while it's likely to get
the same vblank event back as the one just stored I don't think it's
guaranteed.
Nicholas Kazlauskas
>
>> ---
>> drivers/gpu/drm/drm_vblank.c | 49 +++-
>> include/drm/drm_vblank.h | 8 ++
>> 2 files changed, 56 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
>> index 98e091175921..4b3a4c38fabe 100644
>> --- a/drivers/gpu/drm/drm_vblank.c
>> +++ b/drivers/gpu/drm/drm_vblank.c
>> @@ -814,10 +814,21 @@ static void send_vblank_event(struct drm_device *dev,
>> u64 seq, ktime_t now)
>> {
>> struct
Re: [PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
On Mon, Feb 11, 2019 at 04:22:24AM +0100, Mario Kleiner wrote:
> The pageflip completion timestamps transmitted to userspace
> via pageflip completion events are supposed to describe the
> time at which the first pixel of the new post-pageflip scanout
> buffer leaves the video output of the gpu. This time is
> identical to end of vblank, when active scanout starts.
>
> For a crtc in standard fixed refresh rate, the end of vblank
> is identical to the vblank timestamps calculated by
> drm_update_vblank_count() at each vblank interrupt, or each
> vblank dis-/enable. Therefore pageflip events just carry
> that vblank timestamp as their pageflip timestamp.
>
> For a crtc switched to variable refresh rate mode (vrr), the
> pageflip completion timestamps are identical to the vblank
> timestamps iff the pageflip was executed early in vblank,
> before the minimum vblank duration elapsed. In this case
> the time of display onset is identical to when the crtc
> is running in fixed refresh rate.
>
> However, if a pageflip completes later in the vblank, inside
> the "extended front porch" in vrr mode, then the vblank will
> terminate at a fixed (back porch) duration after flip, so
> the display onset time is delayed correspondingly. In this
> case the vblank timestamp computed at vblank irq time would
> be too early, and we need a way to calculate an estimated
> pageflip timestamp that will be later than the vblank timestamp.
>
> How a driver determines such a "late flip" timestamp is hw
> and driver specific, but this patch adds a new helper function
> that allows the driver to propose such an alternate "late flip"
> timestamp for use in pageflip events:
>
> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp);
>
> When sending out pageflip events, we now compare that proposed
> flip_timestamp against the vblank timestamp of the current
> vblank of flip completion and choose to send out the greater/
> later timestamp as flip completion timestamp.
>
> The most simple way for a kms driver to supply a suitable
> flip_timestamp in vrr mode would be to simply take a timestamp
> at start of the pageflip completion handler, e.g., pageflip
> irq handler: flip_timestamp = ktime_get(); and then set that
> as proposed "late" alternative timestamp via ...
> drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp);
>
> More clever approaches could try to add some corrective offset
> for fixed back porch duration, or ideally use hardware features
> like hw timestamps to calculate the exact end time of vblank.
>
> Signed-off-by: Mario Kleiner
> Cc: Nicholas Kazlauskas
> Cc: Harry Wentland
> Cc: Alex Deucher
Uh, this looks like a pretty bad hack. Can't we fix amdgpu to only give us
the right timestampe, once? With this I guess if you do a vblank query in
between the wrong and the right vblank you'll get the bogus value. Not
really great for userspace.
-Daniel
> ---
> drivers/gpu/drm/drm_vblank.c | 49 +++-
> include/drm/drm_vblank.h | 8 ++
> 2 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 98e091175921..4b3a4c38fabe 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -814,10 +814,21 @@ static void send_vblank_event(struct drm_device *dev,
> u64 seq, ktime_t now)
> {
> struct timespec64 tv;
> + ktime_t alt_flip_time;
>
> switch (e->event.base.type) {
> - case DRM_EVENT_VBLANK:
> case DRM_EVENT_FLIP_COMPLETE:
> + /*
> + * For flip completion events, override "now" time
> + * with alt_flip_time provided by the driver via
> + * drm_crtc_set_vrr_pageflip_timestamp() in VRR mode
> + * if that time is later than given "now" vblank time.
> + */
> + alt_flip_time = dev->vblank[e->pipe].alt_flip_time;
> + if (alt_flip_time > now)
> + now = alt_flip_time;
> + /* Fallthrough */
> + case DRM_EVENT_VBLANK:
> tv = ktime_to_timespec64(now);
> e->event.vbl.sequence = seq;
> /*
> @@ -916,11 +927,47 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
>
> now = ktime_get();
> }
> +
> e->pipe = pipe;
> send_vblank_event(dev, e, seq, now);
> }
> EXPORT_SYMBOL(drm_crtc_send_vblank_event);
>
> +/**
> + * drm_crtc_set_vrr_pageflip_timestamp - helper to set alternate pageflip
> time
> + * @crtc: the source CRTC of the pageflip completion event
> + * @flip_time: The alternate pageflip completion timestamp in VRR mode
> + *
> + * In variable refresh rate mode (VRR), a pageflip completion timestamp
> carried
> + * by the pageflip event can never be earlier than the vblank timestamp of
> the
> + * vblank of flip completion, as that vblank timestamp defines the end of the
> + * shortest possible vblank duration. In
[PATCH 2/3] drm: Add basic helper to allow precise pageflip timestamps in vrr.
The pageflip completion timestamps transmitted to userspace
via pageflip completion events are supposed to describe the
time at which the first pixel of the new post-pageflip scanout
buffer leaves the video output of the gpu. This time is
identical to end of vblank, when active scanout starts.
For a crtc in standard fixed refresh rate, the end of vblank
is identical to the vblank timestamps calculated by
drm_update_vblank_count() at each vblank interrupt, or each
vblank dis-/enable. Therefore pageflip events just carry
that vblank timestamp as their pageflip timestamp.
For a crtc switched to variable refresh rate mode (vrr), the
pageflip completion timestamps are identical to the vblank
timestamps iff the pageflip was executed early in vblank,
before the minimum vblank duration elapsed. In this case
the time of display onset is identical to when the crtc
is running in fixed refresh rate.
However, if a pageflip completes later in the vblank, inside
the "extended front porch" in vrr mode, then the vblank will
terminate at a fixed (back porch) duration after flip, so
the display onset time is delayed correspondingly. In this
case the vblank timestamp computed at vblank irq time would
be too early, and we need a way to calculate an estimated
pageflip timestamp that will be later than the vblank timestamp.
How a driver determines such a "late flip" timestamp is hw
and driver specific, but this patch adds a new helper function
that allows the driver to propose such an alternate "late flip"
timestamp for use in pageflip events:
drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp);
When sending out pageflip events, we now compare that proposed
flip_timestamp against the vblank timestamp of the current
vblank of flip completion and choose to send out the greater/
later timestamp as flip completion timestamp.
The most simple way for a kms driver to supply a suitable
flip_timestamp in vrr mode would be to simply take a timestamp
at start of the pageflip completion handler, e.g., pageflip
irq handler: flip_timestamp = ktime_get(); and then set that
as proposed "late" alternative timestamp via ...
drm_crtc_set_vrr_pageflip_timestamp(crtc, flip_timestamp);
More clever approaches could try to add some corrective offset
for fixed back porch duration, or ideally use hardware features
like hw timestamps to calculate the exact end time of vblank.
Signed-off-by: Mario Kleiner
Cc: Nicholas Kazlauskas
Cc: Harry Wentland
Cc: Alex Deucher
---
drivers/gpu/drm/drm_vblank.c | 49 +++-
include/drm/drm_vblank.h | 8 ++
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 98e091175921..4b3a4c38fabe 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -814,10 +814,21 @@ static void send_vblank_event(struct drm_device *dev,
u64 seq, ktime_t now)
{
struct timespec64 tv;
+ ktime_t alt_flip_time;
switch (e->event.base.type) {
- case DRM_EVENT_VBLANK:
case DRM_EVENT_FLIP_COMPLETE:
+ /*
+* For flip completion events, override "now" time
+* with alt_flip_time provided by the driver via
+* drm_crtc_set_vrr_pageflip_timestamp() in VRR mode
+* if that time is later than given "now" vblank time.
+*/
+ alt_flip_time = dev->vblank[e->pipe].alt_flip_time;
+ if (alt_flip_time > now)
+ now = alt_flip_time;
+ /* Fallthrough */
+ case DRM_EVENT_VBLANK:
tv = ktime_to_timespec64(now);
e->event.vbl.sequence = seq;
/*
@@ -916,11 +927,47 @@ void drm_crtc_send_vblank_event(struct drm_crtc *crtc,
now = ktime_get();
}
+
e->pipe = pipe;
send_vblank_event(dev, e, seq, now);
}
EXPORT_SYMBOL(drm_crtc_send_vblank_event);
+/**
+ * drm_crtc_set_vrr_pageflip_timestamp - helper to set alternate pageflip time
+ * @crtc: the source CRTC of the pageflip completion event
+ * @flip_time: The alternate pageflip completion timestamp in VRR mode
+ *
+ * In variable refresh rate mode (VRR), a pageflip completion timestamp carried
+ * by the pageflip event can never be earlier than the vblank timestamp of the
+ * vblank of flip completion, as that vblank timestamp defines the end of the
+ * shortest possible vblank duration. In case of a delayed flip completion
+ * inside the extended VRR front porch however, the end of vblank can be much
+ * later, so the driver must assign an estimated timestamp of that later end of
+ * vblank. For a CRTC in VRR mode, the driver should use this helper function
to
+ * set an alternate flip completion timestamp in case of late flip completions
+ * in extended vblank. In the most simple case, this @flip_time timestamp could
+ * simply be a ktime_get() timestamp taken at the start
