| Message ID | 1459258735-21871-1-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 29, 2016 at 03:38:55PM +0200, Maarten Lankhorst wrote: > This function is useful for gen2 intel devices which have no frame > counter, but need a way to determine the current vblank count without > racing with the vblank interrupt handler. > > intel_pipe_update_start checks if no vblank interrupt will occur > during vblank evasion, but cannot check whether the vblank handler has > run to completion. This function uses the timestamps to determine > when the last vblank has happened, and interpolates from there. Didn't really read it in detail, but on a glance it seems too complicated to me. All we should really need is something like this: drm_vblank_get(); drm_update_vblank_count(); accurate = drm_vblank_count(); > > Cc: Mario Kleiner <mario.kleiner.de@gmail.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > Unfortunately only compile time tested, I don't have a gen2 to test with. > > drivers/gpu/drm/drm_irq.c | 90 +++++++++++++++++++++++++++++++++++------------ > include/drm/drmP.h | 2 ++ > 2 files changed, 69 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index 881c5a6c180c..44d8f9607ccd 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -155,24 +155,10 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe > spin_unlock(&dev->vblank_time_lock); > } > > -/** > - * drm_update_vblank_count - update the master vblank counter > - * @dev: DRM device > - * @pipe: counter to update > - * > - * Call back into the driver to update the appropriate vblank counter > - * (specified by @pipe). Deal with wraparound, if it occurred, and > - * update the last read value so we can deal with wraparound on the next > - * call if necessary. > - * > - * Only necessary when going from off->on, to account for frames we > - * didn't get an interrupt for. > - * > - * Note: caller must hold dev->vbl_lock since this reads & writes > - * device vblank fields. > - */ > -static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, > - unsigned long flags) > +static u32 __drm_accurate_vblank_count_and_time(struct drm_device *dev, unsigned int pipe, > + const struct timeval *t_old, > + unsigned long flags, u32 *pdiff, > + struct timeval *tv_ret) > { > struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; > u32 cur_vblank, diff; > @@ -202,10 +188,8 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, > /* trust the hw counter when it's around */ > diff = (cur_vblank - vblank->last) & dev->max_vblank_count; > } else if (rc && framedur_ns) { > - const struct timeval *t_old; > u64 diff_ns; > > - t_old = &vblanktimestamp(dev, pipe, vblank->count); > diff_ns = timeval_to_ns(&t_vblank) - timeval_to_ns(t_old); > > /* > @@ -286,9 +270,13 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, > " current=%u, diff=%u, hw=%u hw_last=%u\n", > pipe, vblank->count, diff, cur_vblank, vblank->last); > > + *pdiff = diff; > + *tv_ret = t_vblank; > + > if (diff == 0) { > WARN_ON_ONCE(cur_vblank != vblank->last); > - return; > + > + return cur_vblank; > } > > /* > @@ -298,9 +286,65 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, > * for now, to mark the vblanktimestamp as invalid. > */ > if (!rc && (flags & DRM_CALLED_FROM_VBLIRQ) == 0) > - t_vblank = (struct timeval) {0, 0}; > + *tv_ret = (struct timeval) {0, 0}; > + > + return cur_vblank; > +} > + > +/** > + * drm_accurate_vblank_count_and_time - retrieve the master vblank counter > + * @crtc: which counter to retrieve > + * @tv_ret: last time counter was updated > + * > + * This function is similar to @drm_update_vblank_count_and_time but > + * this function interpolates to handle a race with vblank irq's, and > + * is only useful for crtc's that have no hw vblank counter. > + */ > + > +u32 drm_accurate_vblank_count_and_time(struct drm_crtc *crtc, > + struct timeval *tv_ret) > +{ > + struct drm_device *dev = crtc->dev; > + u32 pdiff, old_vblank; > + struct timeval tv_old = {}; > + > + WARN(dev->max_vblank_count, "This function is only useful when a hw counter is unavailable."); > + > + old_vblank = drm_crtc_vblank_count_and_time(crtc, &tv_old); > + > + __drm_accurate_vblank_count_and_time(dev, drm_crtc_index(crtc), > + &tv_old, 0, &pdiff, tv_ret); > + > + return old_vblank + pdiff; > +} > +EXPORT_SYMBOL(drm_accurate_vblank_count_and_time); > + > +/** > + * drm_update_vblank_count - update the master vblank counter > + * @dev: DRM device > + * @pipe: counter to update > + * > + * Call back into the driver to update the appropriate vblank counter > + * (specified by @pipe). Deal with wraparound, if it occurred, and > + * update the last read value so we can deal with wraparound on the next > + * call if necessary. > + * > + * Only necessary when going from off->on, to account for frames we > + * didn't get an interrupt for. > + * > + * Note: caller must hold dev->vbl_lock since this reads & writes > + * device vblank fields. > + */ > +static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, > + unsigned long flags) > +{ > + u32 diff, cur_vblank, old_vblank = dev->vblank[pipe].count; > + struct timeval t_vblank; > + const struct timeval *tv_old = &vblanktimestamp(dev, pipe, old_vblank); > > - store_vblank(dev, pipe, diff, &t_vblank, cur_vblank); > + cur_vblank = __drm_accurate_vblank_count_and_time(dev, pipe, tv_old, flags, &diff, &t_vblank); > + if (diff) > + store_vblank(dev, pipe, diff, &t_vblank, cur_vblank); > } > > /* > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 31483c2fef51..1df65922c7c6 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -995,6 +995,8 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc); > extern void drm_crtc_vblank_reset(struct drm_crtc *crtc); > extern void drm_crtc_vblank_on(struct drm_crtc *crtc); > extern void drm_vblank_cleanup(struct drm_device *dev); > +extern u32 drm_accurate_vblank_count_and_time(struct drm_crtc *crtc, > + struct timeval *tv_ret); > extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe); > > extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, > -- > 2.1.0
Op 30-03-16 om 15:13 schreef Ville Syrjälä: > On Tue, Mar 29, 2016 at 03:38:55PM +0200, Maarten Lankhorst wrote: >> This function is useful for gen2 intel devices which have no frame >> counter, but need a way to determine the current vblank count without >> racing with the vblank interrupt handler. >> >> intel_pipe_update_start checks if no vblank interrupt will occur >> during vblank evasion, but cannot check whether the vblank handler has >> run to completion. This function uses the timestamps to determine >> when the last vblank has happened, and interpolates from there. > Didn't really read it in detail, but on a glance it seems too > complicated to me. All we should really need is something like this: > > drm_vblank_get(); > drm_update_vblank_count(); > accurate = drm_vblank_count(); > Updating from non vblank irq context might cause the vblank clock to drift, or at least contend the vblank lock. This is why I call drm_vblank_count_and_time, and then interpolate as if update_vblank_count was called on the result. ~Maarten
On Wed, Mar 30, 2016 at 03:41:26PM +0200, Maarten Lankhorst wrote: > Op 30-03-16 om 15:13 schreef Ville Syrjälä: > > On Tue, Mar 29, 2016 at 03:38:55PM +0200, Maarten Lankhorst wrote: > >> This function is useful for gen2 intel devices which have no frame > >> counter, but need a way to determine the current vblank count without > >> racing with the vblank interrupt handler. > >> > >> intel_pipe_update_start checks if no vblank interrupt will occur > >> during vblank evasion, but cannot check whether the vblank handler has > >> run to completion. This function uses the timestamps to determine > >> when the last vblank has happened, and interpolates from there. > > Didn't really read it in detail, but on a glance it seems too > > complicated to me. All we should really need is something like this: > > > > drm_vblank_get(); > > drm_update_vblank_count(); > > accurate = drm_vblank_count(); > > > Updating from non vblank irq context might cause the vblank clock to drift, or at least contend the vblank lock. I don't see a problem. The scanout position is anyway used to fix up the timestamp. > > This is why I call drm_vblank_count_and_time, and then interpolate as if update_vblank_count was called on the result. > > ~Maarten
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 881c5a6c180c..44d8f9607ccd 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -155,24 +155,10 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe spin_unlock(&dev->vblank_time_lock); } -/** - * drm_update_vblank_count - update the master vblank counter - * @dev: DRM device - * @pipe: counter to update - * - * Call back into the driver to update the appropriate vblank counter - * (specified by @pipe). Deal with wraparound, if it occurred, and - * update the last read value so we can deal with wraparound on the next - * call if necessary. - * - * Only necessary when going from off->on, to account for frames we - * didn't get an interrupt for. - * - * Note: caller must hold dev->vbl_lock since this reads & writes - * device vblank fields. - */ -static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, - unsigned long flags) +static u32 __drm_accurate_vblank_count_and_time(struct drm_device *dev, unsigned int pipe, + const struct timeval *t_old, + unsigned long flags, u32 *pdiff, + struct timeval *tv_ret) { struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; u32 cur_vblank, diff; @@ -202,10 +188,8 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, /* trust the hw counter when it's around */ diff = (cur_vblank - vblank->last) & dev->max_vblank_count; } else if (rc && framedur_ns) { - const struct timeval *t_old; u64 diff_ns; - t_old = &vblanktimestamp(dev, pipe, vblank->count); diff_ns = timeval_to_ns(&t_vblank) - timeval_to_ns(t_old); /* @@ -286,9 +270,13 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, " current=%u, diff=%u, hw=%u hw_last=%u\n", pipe, vblank->count, diff, cur_vblank, vblank->last); + *pdiff = diff; + *tv_ret = t_vblank; + if (diff == 0) { WARN_ON_ONCE(cur_vblank != vblank->last); - return; + + return cur_vblank; } /* @@ -298,9 +286,65 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, * for now, to mark the vblanktimestamp as invalid. */ if (!rc && (flags & DRM_CALLED_FROM_VBLIRQ) == 0) - t_vblank = (struct timeval) {0, 0}; + *tv_ret = (struct timeval) {0, 0}; + + return cur_vblank; +} + +/** + * drm_accurate_vblank_count_and_time - retrieve the master vblank counter + * @crtc: which counter to retrieve + * @tv_ret: last time counter was updated + * + * This function is similar to @drm_update_vblank_count_and_time but + * this function interpolates to handle a race with vblank irq's, and + * is only useful for crtc's that have no hw vblank counter. + */ + +u32 drm_accurate_vblank_count_and_time(struct drm_crtc *crtc, + struct timeval *tv_ret) +{ + struct drm_device *dev = crtc->dev; + u32 pdiff, old_vblank; + struct timeval tv_old = {}; + + WARN(dev->max_vblank_count, "This function is only useful when a hw counter is unavailable."); + + old_vblank = drm_crtc_vblank_count_and_time(crtc, &tv_old); + + __drm_accurate_vblank_count_and_time(dev, drm_crtc_index(crtc), + &tv_old, 0, &pdiff, tv_ret); + + return old_vblank + pdiff; +} +EXPORT_SYMBOL(drm_accurate_vblank_count_and_time); + +/** + * drm_update_vblank_count - update the master vblank counter + * @dev: DRM device + * @pipe: counter to update + * + * Call back into the driver to update the appropriate vblank counter + * (specified by @pipe). Deal with wraparound, if it occurred, and + * update the last read value so we can deal with wraparound on the next + * call if necessary. + * + * Only necessary when going from off->on, to account for frames we + * didn't get an interrupt for. + * + * Note: caller must hold dev->vbl_lock since this reads & writes + * device vblank fields. + */ +static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, + unsigned long flags) +{ + u32 diff, cur_vblank, old_vblank = dev->vblank[pipe].count; + struct timeval t_vblank; + const struct timeval *tv_old = &vblanktimestamp(dev, pipe, old_vblank); - store_vblank(dev, pipe, diff, &t_vblank, cur_vblank); + cur_vblank = __drm_accurate_vblank_count_and_time(dev, pipe, tv_old, flags, &diff, &t_vblank); + if (diff) + store_vblank(dev, pipe, diff, &t_vblank, cur_vblank); } /* diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 31483c2fef51..1df65922c7c6 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -995,6 +995,8 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc); extern void drm_crtc_vblank_reset(struct drm_crtc *crtc); extern void drm_crtc_vblank_on(struct drm_crtc *crtc); extern void drm_vblank_cleanup(struct drm_device *dev); +extern u32 drm_accurate_vblank_count_and_time(struct drm_crtc *crtc, + struct timeval *tv_ret); extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe); extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
This function is useful for gen2 intel devices which have no frame counter, but need a way to determine the current vblank count without racing with the vblank interrupt handler. intel_pipe_update_start checks if no vblank interrupt will occur during vblank evasion, but cannot check whether the vblank handler has run to completion. This function uses the timestamps to determine when the last vblank has happened, and interpolates from there. Cc: Mario Kleiner <mario.kleiner.de@gmail.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- Unfortunately only compile time tested, I don't have a gen2 to test with. drivers/gpu/drm/drm_irq.c | 90 +++++++++++++++++++++++++++++++++++------------ include/drm/drmP.h | 2 ++ 2 files changed, 69 insertions(+), 23 deletions(-)