| Message ID | 20180112215707.3084-2-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 12, 2018 at 09:57:04PM +0000, Dhinakaran Pandiyan wrote: > Now that drm_vblank_count() returns all bits of the vblank count, update > drm_crtc_arm_vblank_event() so that it queues the correct sequence. > Otherwise, this leads to prolonged waits for a vblank sequence when the > current count is >=2^32. This could be probably squashed to the previous patch... Specially if you apply the Fixes tag. Anyways, in case you decide to go with 2 patches: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Cc: Keith Packard <keithp@keithp.com> > Cc: Michel Dänzer <michel@daenzer.net> > Cc: Daniel Vetter <daniel@ffwll.ch> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > --- > drivers/gpu/drm/drm_vblank.c | 4 ++-- > include/drm/drm_vblank.h | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c > index 768a8e44d99b..f2bf1f5dbaa5 100644 > --- a/drivers/gpu/drm/drm_vblank.c > +++ b/drivers/gpu/drm/drm_vblank.c > @@ -292,11 +292,11 @@ static u64 drm_vblank_count(struct drm_device *dev, unsigned int pipe) > * This is mostly useful for hardware that can obtain the scanout position, but > * doesn't have a hardware frame counter. > */ > -u32 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc) > +u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > unsigned int pipe = drm_crtc_index(crtc); > - u32 vblank; > + u64 vblank; > unsigned long flags; > > WARN_ONCE(drm_debug & DRM_UT_VBL && !dev->driver->get_vblank_timestamp, > diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h > index 848b463a0af5..a4c3b0a0a197 100644 > --- a/include/drm/drm_vblank.h > +++ b/include/drm/drm_vblank.h > @@ -179,7 +179,7 @@ void drm_crtc_wait_one_vblank(struct drm_crtc *crtc); > void drm_crtc_vblank_off(struct drm_crtc *crtc); > void drm_crtc_vblank_reset(struct drm_crtc *crtc); > void drm_crtc_vblank_on(struct drm_crtc *crtc); > -u32 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc); > +u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc); > > bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, > unsigned int pipe, int *max_error, > -- > 2.11.0 >
Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> writes: > Now that drm_vblank_count() returns all bits of the vblank count, update > drm_crtc_arm_vblank_event() so that it queues the correct sequence. > Otherwise, this leads to prolonged waits for a vblank sequence when the > current count is >=2^32. The summary for this patch is incorrect; the function being fixed is drm_crtc_accurate_vblank_event. And, I'm afraid you've uncovered a rabbit hole here -- there are a couple of users of this function outside of the core, including i915 in a couple of places and nouveau. We should at least review the whole call graph starting here and make sure it does what we think it should. Thanks for finding these bugs!
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c index 768a8e44d99b..f2bf1f5dbaa5 100644 --- a/drivers/gpu/drm/drm_vblank.c +++ b/drivers/gpu/drm/drm_vblank.c @@ -292,11 +292,11 @@ static u64 drm_vblank_count(struct drm_device *dev, unsigned int pipe) * This is mostly useful for hardware that can obtain the scanout position, but * doesn't have a hardware frame counter. */ -u32 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc) +u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; unsigned int pipe = drm_crtc_index(crtc); - u32 vblank; + u64 vblank; unsigned long flags; WARN_ONCE(drm_debug & DRM_UT_VBL && !dev->driver->get_vblank_timestamp, diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h index 848b463a0af5..a4c3b0a0a197 100644 --- a/include/drm/drm_vblank.h +++ b/include/drm/drm_vblank.h @@ -179,7 +179,7 @@ void drm_crtc_wait_one_vblank(struct drm_crtc *crtc); void drm_crtc_vblank_off(struct drm_crtc *crtc); void drm_crtc_vblank_reset(struct drm_crtc *crtc); void drm_crtc_vblank_on(struct drm_crtc *crtc); -u32 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc); +u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc); bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, unsigned int pipe, int *max_error,
Now that drm_vblank_count() returns all bits of the vblank count, update drm_crtc_arm_vblank_event() so that it queues the correct sequence. Otherwise, this leads to prolonged waits for a vblank sequence when the current count is >=2^32. Cc: Keith Packard <keithp@keithp.com> Cc: Michel Dänzer <michel@daenzer.net> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> --- drivers/gpu/drm/drm_vblank.c | 4 ++-- include/drm/drm_vblank.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)