| Message ID | 1410363371-11282-3-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/10/2014 05:36 PM, Daniel Vetter wrote: > Imo u32 hints at a register value, but in reality all callers only > care whether the sampled timestamp is precise or not. So give them > just a bool. > > Also move the declaration out of drmP.h, it's only used in drm_irq.c. All good. Maybe then also remove EXPORT_SYMBOL(drm_get_last_vbltimestamp); in this patch if the method is now static to drm_irq.c ? Up to you. For all 4 patches... Reviewed-by: Mario Kleiner <mario.kleiner.de@gmail.com> -mario > Cc: Mario Kleiner <mario.kleiner.de@gmail.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/drm_irq.c | 24 +++++++++++++++--------- > include/drm/drmP.h | 2 -- > 2 files changed, 15 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index 922721ead29a..b16f0bcef959 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -70,6 +70,10 @@ module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600); > module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600); > module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 0600); > > +static bool > +drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, > + struct timeval *tvblank, unsigned flags); > + > /** > * drm_update_vblank_count - update the master vblank counter > * @dev: DRM device > @@ -89,7 +93,8 @@ module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 0600); > static void drm_update_vblank_count(struct drm_device *dev, int crtc) > { > struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; > - u32 cur_vblank, diff, tslot, rc; > + u32 cur_vblank, diff, tslot; > + bool rc; > struct timeval t_vblank; > > /* > @@ -147,7 +152,7 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) > unsigned long irqflags; > u32 vblcount; > s64 diff_ns; > - int vblrc; > + bool vblrc; > struct timeval tvblank; > int count = DRM_TIMESTAMP_MAXRETRIES; > > @@ -171,7 +176,7 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) > * vblank interrupt is disabled. > */ > if (!vblank->enabled && > - drm_get_last_vbltimestamp(dev, crtc, &tvblank, 0) > 0) { > + drm_get_last_vbltimestamp(dev, crtc, &tvblank, 0)) { > drm_update_vblank_count(dev, crtc); > spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); > return; > @@ -219,7 +224,7 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) > * available. In that case we can't account for this and just > * hope for the best. > */ > - if ((vblrc > 0) && (abs64(diff_ns) > 1000000)) { > + if (vblrc && (abs64(diff_ns) > 1000000)) { > /* Store new timestamp in ringbuffer. */ > vblanktimestamp(dev, crtc, vblcount + 1) = tvblank; > > @@ -786,10 +791,11 @@ static struct timeval get_drm_timestamp(void) > * call, i.e., it isn't very precisely locked to the true vblank. > * > * Returns: > - * Non-zero if timestamp is considered to be very precise, zero otherwise. > + * True if timestamp is considered to be very precise, false otherwise. > */ > -u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, > - struct timeval *tvblank, unsigned flags) > +static bool > +drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, > + struct timeval *tvblank, unsigned flags) > { > int ret; > > @@ -801,7 +807,7 @@ u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, > ret = dev->driver->get_vblank_timestamp(dev, crtc, &max_error, > tvblank, flags); > if (ret > 0) > - return (u32) ret; > + return true; > } > > /* GPU high precision timestamp query unsupported or failed. > @@ -809,7 +815,7 @@ u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, > */ > *tvblank = get_drm_timestamp(); > > - return 0; > + return false; > } > EXPORT_SYMBOL(drm_get_last_vbltimestamp); > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index ad952b08711e..2ccb0e715569 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -1004,8 +1004,6 @@ extern void drm_crtc_vblank_off(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_get_last_vbltimestamp(struct drm_device *dev, int crtc, > - struct timeval *tvblank, unsigned flags); > extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, > int crtc, int *max_error, > struct timeval *vblank_time,
On Thu, Sep 11, 2014 at 1:28 PM, Mario Kleiner <mario.kleiner.de@gmail.com> wrote: > On 09/10/2014 05:36 PM, Daniel Vetter wrote: >> >> Imo u32 hints at a register value, but in reality all callers only >> care whether the sampled timestamp is precise or not. So give them >> just a bool. >> >> Also move the declaration out of drmP.h, it's only used in drm_irq.c. > > > All good. Maybe then also remove > > EXPORT_SYMBOL(drm_get_last_vbltimestamp); Oh, I've missed that one when grepping. Will fix when applying. > in this patch if the method is now static to drm_irq.c ? Up to you. > > For all 4 patches... > > Reviewed-by: Mario Kleiner <mario.kleiner.de@gmail.com> Thanks a lot, I plan to send the pull request to Dave tomorrow. Presuming nothing else fails meanwhile ;-)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 922721ead29a..b16f0bcef959 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -70,6 +70,10 @@ module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600); module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600); module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 0600); +static bool +drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, + struct timeval *tvblank, unsigned flags); + /** * drm_update_vblank_count - update the master vblank counter * @dev: DRM device @@ -89,7 +93,8 @@ module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 0600); static void drm_update_vblank_count(struct drm_device *dev, int crtc) { struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; - u32 cur_vblank, diff, tslot, rc; + u32 cur_vblank, diff, tslot; + bool rc; struct timeval t_vblank; /* @@ -147,7 +152,7 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) unsigned long irqflags; u32 vblcount; s64 diff_ns; - int vblrc; + bool vblrc; struct timeval tvblank; int count = DRM_TIMESTAMP_MAXRETRIES; @@ -171,7 +176,7 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) * vblank interrupt is disabled. */ if (!vblank->enabled && - drm_get_last_vbltimestamp(dev, crtc, &tvblank, 0) > 0) { + drm_get_last_vbltimestamp(dev, crtc, &tvblank, 0)) { drm_update_vblank_count(dev, crtc); spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); return; @@ -219,7 +224,7 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) * available. In that case we can't account for this and just * hope for the best. */ - if ((vblrc > 0) && (abs64(diff_ns) > 1000000)) { + if (vblrc && (abs64(diff_ns) > 1000000)) { /* Store new timestamp in ringbuffer. */ vblanktimestamp(dev, crtc, vblcount + 1) = tvblank; @@ -786,10 +791,11 @@ static struct timeval get_drm_timestamp(void) * call, i.e., it isn't very precisely locked to the true vblank. * * Returns: - * Non-zero if timestamp is considered to be very precise, zero otherwise. + * True if timestamp is considered to be very precise, false otherwise. */ -u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, - struct timeval *tvblank, unsigned flags) +static bool +drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, + struct timeval *tvblank, unsigned flags) { int ret; @@ -801,7 +807,7 @@ u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, ret = dev->driver->get_vblank_timestamp(dev, crtc, &max_error, tvblank, flags); if (ret > 0) - return (u32) ret; + return true; } /* GPU high precision timestamp query unsupported or failed. @@ -809,7 +815,7 @@ u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, */ *tvblank = get_drm_timestamp(); - return 0; + return false; } EXPORT_SYMBOL(drm_get_last_vbltimestamp); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index ad952b08711e..2ccb0e715569 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1004,8 +1004,6 @@ extern void drm_crtc_vblank_off(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_get_last_vbltimestamp(struct drm_device *dev, int crtc, - struct timeval *tvblank, unsigned flags); extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc, int *max_error, struct timeval *vblank_time,
Imo u32 hints at a register value, but in reality all callers only care whether the sampled timestamp is precise or not. So give them just a bool. Also move the declaration out of drmP.h, it's only used in drm_irq.c. Cc: Mario Kleiner <mario.kleiner.de@gmail.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/drm_irq.c | 24 +++++++++++++++--------- include/drm/drmP.h | 2 -- 2 files changed, 15 insertions(+), 11 deletions(-)