On Wed, Mar 22, 2017 at 09:36:13AM +0100, Daniel Vetter wrote: > It's overkill to have a flag parameter which is essentially used just > as a boolean. This takes care of core + adjusting drivers. > > Adjusting the scanout position callback is a bit harder, since radeon > also supplies it's own driver-private flags in there. This part worried me, but indeed radeon only passes the custom flag to the scanout position hook. Patch lgtm Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Cc: Mario Kleiner <mario.kleiner@xxxxxxxxxxxxxxxx> > Cc: Eric Anholt <eric@xxxxxxxxxx> > Cc: Rob Clark <robdclark@xxxxxxxxx> > Cc: linux-arm-msm@xxxxxxxxxxxxxxx > Cc: freedreno@xxxxxxxxxxxxxxxxxxxxx > Cc: Alex Deucher <alexander.deucher@xxxxxxx> > Cc: Christian König <christian.koenig@xxxxxxx> > Cc: Ben Skeggs <bskeggs@xxxxxxxxxx> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 ++--- > drivers/gpu/drm/drm_irq.c | 41 +++++++++++++++++-------------- > drivers/gpu/drm/i915/i915_irq.c | 4 +-- > drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 4 +-- > drivers/gpu/drm/nouveau/nouveau_display.c | 5 ++-- > drivers/gpu/drm/nouveau/nouveau_display.h | 2 +- > drivers/gpu/drm/radeon/radeon_drv.c | 2 +- > drivers/gpu/drm/radeon/radeon_kms.c | 4 +-- > drivers/gpu/drm/vc4/vc4_crtc.c | 4 +-- > drivers/gpu/drm/vc4/vc4_drv.h | 2 +- > include/drm/drm_drv.h | 11 ++++----- > include/drm/drm_irq.h | 2 +- > 13 files changed, 46 insertions(+), 43 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index acd8631d8024..edb3bb83e1a9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -1771,7 +1771,7 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe); > bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, > int *max_error, > struct timeval *vblank_time, > - unsigned flags); > + bool in_vblank_irq); > long amdgpu_kms_compat_ioctl(struct file *filp, unsigned int cmd, > unsigned long arg); > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > index ac42f707c046..ad295e822d45 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c > @@ -834,7 +834,7 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe) > * @crtc: crtc to get the timestamp for > * @max_error: max error > * @vblank_time: time value > - * @flags: flags passed to the driver > + * @in_vblank_irq: called from drm_handle_vblank() > * > * Gets the timestamp on the requested crtc based on the > * scanout position. (all asics). > @@ -843,7 +843,7 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe) > bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, > int *max_error, > struct timeval *vblank_time, > - unsigned flags) > + bool in_vblank_irq) > { > struct drm_crtc *crtc; > struct amdgpu_device *adev = dev->dev_private; > @@ -864,7 +864,7 @@ bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, > > /* Helper routine in DRM core does all the work: */ > return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error, > - vblank_time, flags, > + vblank_time, in_vblank_irq, > &crtc->hwmode); > } > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index 2121ea29e1b2..059c3346db68 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -54,7 +54,7 @@ > > static bool > drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, > - struct timeval *tvblank, unsigned flags); > + struct timeval *tvblank, bool in_vblank_irq); > > static unsigned int drm_timestamp_precision = 20; /* Default to 20 usecs. */ > > @@ -138,7 +138,7 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe > */ > do { > cur_vblank = __get_vblank_counter(dev, pipe); > - rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, 0); > + rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, false); > } while (cur_vblank != __get_vblank_counter(dev, pipe) && --count > 0); > > /* > @@ -171,7 +171,7 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe > * device vblank fields. > */ > static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, > - unsigned long flags) > + bool in_vblank_irq) > { > struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; > u32 cur_vblank, diff; > @@ -194,7 +194,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, > */ > do { > cur_vblank = __get_vblank_counter(dev, pipe); > - rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, flags); > + rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, in_vblank_irq); > } while (cur_vblank != __get_vblank_counter(dev, pipe) && --count > 0); > > if (dev->max_vblank_count != 0) { > @@ -214,13 +214,13 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, > */ > diff = DIV_ROUND_CLOSEST_ULL(diff_ns, framedur_ns); > > - if (diff == 0 && flags & DRM_CALLED_FROM_VBLIRQ) > + if (diff == 0 && in_vblank_irq) > DRM_DEBUG_VBL("crtc %u: Redundant vblirq ignored." > " diff_ns = %lld, framedur_ns = %d)\n", > pipe, (long long) diff_ns, framedur_ns); > } else { > /* some kind of default for drivers w/o accurate vbl timestamping */ > - diff = (flags & DRM_CALLED_FROM_VBLIRQ) != 0; > + diff = in_vblank_irq ? 1 : 0; > } > > /* > @@ -253,7 +253,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, > * Otherwise reinitialize delayed at next vblank interrupt and assign 0 > * for now, to mark the vblanktimestamp as invalid. > */ > - if (!rc && (flags & DRM_CALLED_FROM_VBLIRQ) == 0) > + if (!rc && in_vblank_irq) > t_vblank = (struct timeval) {0, 0}; > > store_vblank(dev, pipe, diff, &t_vblank, cur_vblank); > @@ -291,7 +291,7 @@ u32 drm_accurate_vblank_count(struct drm_crtc *crtc) > > spin_lock_irqsave(&dev->vblank_time_lock, flags); > > - drm_update_vblank_count(dev, pipe, 0); > + drm_update_vblank_count(dev, pipe, false); > vblank = drm_vblank_count(dev, pipe); > > spin_unlock_irqrestore(&dev->vblank_time_lock, flags); > @@ -347,7 +347,7 @@ static void vblank_disable_and_save(struct drm_device *dev, unsigned int pipe) > * this time. This makes the count account for the entire time > * between drm_crtc_vblank_on() and drm_crtc_vblank_off(). > */ > - drm_update_vblank_count(dev, pipe, 0); > + drm_update_vblank_count(dev, pipe, false); > > spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); > } > @@ -698,9 +698,10 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants); > * @max_error: Desired maximum allowable error in timestamps (nanosecs) > * On return contains true maximum error of timestamp > * @vblank_time: Pointer to struct timeval which should receive the timestamp > - * @flags: Flags to pass to driver: > - * 0 = Default, > - * DRM_CALLED_FROM_VBLIRQ = If function is called from vbl IRQ handler > + * @in_vblank_irq: > + * True when called from drm_crtc_handle_vblank(). Some drivers > + * need to apply some workarounds for gpu-specific vblank irq quirks > + * if flag is set. > * @mode: mode which defines the scanout timings > * > * Implements calculation of exact vblank timestamps from given drm_display_mode > @@ -730,7 +731,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, > unsigned int pipe, > int *max_error, > struct timeval *vblank_time, > - unsigned flags, > + bool in_vblank_irq, > const struct drm_display_mode *mode) > { > struct timeval tv_etime; > @@ -738,6 +739,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, > unsigned int vbl_status; > int vpos, hpos, i; > int delta_ns, duration_ns; > + unsigned flags = in_vblank_irq ? DRM_CALLED_FROM_VBLIRQ : 0; > > if (pipe >= dev->num_crtcs) { > DRM_ERROR("Invalid crtc %u\n", pipe); > @@ -841,9 +843,10 @@ static struct timeval get_drm_timestamp(void) > * @dev: DRM device > * @pipe: index of CRTC whose vblank timestamp to retrieve > * @tvblank: Pointer to target struct timeval which should receive the timestamp > - * @flags: Flags to pass to driver: > - * 0 = Default, > - * DRM_CALLED_FROM_VBLIRQ = If function is called from vbl IRQ handler > + * @in_vblank_irq: > + * True when called from drm_crtc_handle_vblank(). Some drivers > + * need to apply some workarounds for gpu-specific vblank irq quirks > + * if flag is set. > * > * Fetches the system timestamp corresponding to the time of the most recent > * vblank interval on specified CRTC. May call into kms-driver to > @@ -857,7 +860,7 @@ static struct timeval get_drm_timestamp(void) > */ > static bool > drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, > - struct timeval *tvblank, unsigned flags) > + struct timeval *tvblank, bool in_vblank_irq) > { > int ret; > > @@ -867,7 +870,7 @@ drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, > /* Query driver if possible and precision timestamping enabled. */ > if (dev->driver->get_vblank_timestamp && (max_error > 0)) { > ret = dev->driver->get_vblank_timestamp(dev, pipe, &max_error, > - tvblank, flags); > + tvblank, in_vblank_irq); > if (ret > 0) > return true; > } > @@ -1710,7 +1713,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe) > return false; > } > > - drm_update_vblank_count(dev, pipe, DRM_CALLED_FROM_VBLIRQ); > + drm_update_vblank_count(dev, pipe, true); > > spin_unlock(&dev->vblank_time_lock); > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index df149d159ce7..6c8a7e1284c3 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -967,7 +967,7 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc) > static bool i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, > int *max_error, > struct timeval *vblank_time, > - unsigned flags) > + bool in_vblank_irq) > { > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *crtc; > @@ -991,7 +991,7 @@ static bool i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, > > /* Helper routine in DRM core does all the work: */ > return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error, > - vblank_time, flags, > + vblank_time, in_vblank_irq, > &crtc->base.hwmode); > } > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c > index 655700eb42ba..16184ccbdd3b 100644 > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c > @@ -598,7 +598,7 @@ static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe, > static bool mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, > int *max_error, > struct timeval *vblank_time, > - unsigned flags) > + bool in_vblank_irq) > { > struct msm_drm_private *priv = dev->dev_private; > struct drm_crtc *crtc; > @@ -615,7 +615,7 @@ static bool mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, > } > > return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error, > - vblank_time, flags, > + vblank_time, in_vblank_irq, > &crtc->mode); > } > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c > index 42f18b0b9c43..be8ec18ba126 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_display.c > +++ b/drivers/gpu/drm/nouveau/nouveau_display.c > @@ -158,7 +158,7 @@ nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe, > > bool > nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe, > - int *max_error, struct timeval *time, unsigned flags) > + int *max_error, struct timeval *time, bool in_vblank_irq) > { > struct drm_crtc *crtc; > > @@ -170,7 +170,8 @@ nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe, > else > mode = &crtc->hwmode; > return drm_calc_vbltimestamp_from_scanoutpos(dev, > - pipe, max_error, time, flags, mode); > + pipe, max_error, time, in_vblank_irq, > + mode); > } > } > > diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h > index bc9d1e7b0117..f821fc9e2de3 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_display.h > +++ b/drivers/gpu/drm/nouveau/nouveau_display.h > @@ -72,7 +72,7 @@ int nouveau_display_scanoutpos(struct drm_device *, unsigned int, > unsigned int, int *, int *, ktime_t *, > ktime_t *, const struct drm_display_mode *); > bool nouveau_display_vblstamp(struct drm_device *, unsigned int, int *, > - struct timeval *, unsigned); > + struct timeval *, bool); > > int nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, > struct drm_pending_vblank_event *event, > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c > index 6d79b5c2805b..5fbbc6ac165d 100644 > --- a/drivers/gpu/drm/radeon/radeon_drv.c > +++ b/drivers/gpu/drm/radeon/radeon_drv.c > @@ -117,7 +117,7 @@ void radeon_disable_vblank_kms(struct drm_device *dev, unsigned int pipe); > bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, > int *max_error, > struct timeval *vblank_time, > - unsigned flags); > + bool in_vblank_irq); > void radeon_driver_irq_preinstall_kms(struct drm_device *dev); > int radeon_driver_irq_postinstall_kms(struct drm_device *dev); > void radeon_driver_irq_uninstall_kms(struct drm_device *dev); > diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c > index 9afe72010c64..41765d18f863 100644 > --- a/drivers/gpu/drm/radeon/radeon_kms.c > +++ b/drivers/gpu/drm/radeon/radeon_kms.c > @@ -873,7 +873,7 @@ void radeon_disable_vblank_kms(struct drm_device *dev, int crtc) > bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc, > int *max_error, > struct timeval *vblank_time, > - unsigned flags) > + bool in_vblank_irq) > { > struct drm_crtc *drmcrtc; > struct radeon_device *rdev = dev->dev_private; > @@ -890,7 +890,7 @@ bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc, > > /* Helper routine in DRM core does all the work: */ > return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc, max_error, > - vblank_time, flags, > + vblank_time, in_vblank_irq, > &drmcrtc->hwmode); > } > > diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c > index 74260990b6bf..18bd0d816fe3 100644 > --- a/drivers/gpu/drm/vc4/vc4_crtc.c > +++ b/drivers/gpu/drm/vc4/vc4_crtc.c > @@ -272,14 +272,14 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, > > bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id, > int *max_error, struct timeval *vblank_time, > - unsigned flags) > + bool in_vblank_irq) > { > struct drm_crtc *crtc = drm_crtc_from_index(dev, crtc_id); > struct drm_crtc_state *state = crtc->state; > > /* Helper routine in DRM core does all the work: */ > return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc_id, max_error, > - vblank_time, flags, > + vblank_time, in_vblank_irq, > &state->adjusted_mode); > } > > diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h > index 8a5d0e12ee02..815cdeb54971 100644 > --- a/drivers/gpu/drm/vc4/vc4_drv.h > +++ b/drivers/gpu/drm/vc4/vc4_drv.h > @@ -452,7 +452,7 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, > const struct drm_display_mode *mode); > bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id, > int *max_error, struct timeval *vblank_time, > - unsigned flags); > + bool in_vblank_irq); > > /* vc4_debugfs.c */ > int vc4_debugfs_init(struct drm_minor *minor); > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h > index da78e248d9d8..9fe6301edd6a 100644 > --- a/include/drm/drm_drv.h > +++ b/include/drm/drm_drv.h > @@ -308,11 +308,10 @@ struct drm_driver { > * Returns true upper bound on error for timestamp. > * vblank_time: > * Target location for returned vblank timestamp. > - * flags: > - * 0 = Defaults, no special treatment needed. > - * DRM_CALLED_FROM_VBLIRQ = Function is called from vblank > - * irq handler. Some drivers need to apply some workarounds > - * for gpu-specific vblank irq quirks if flag is set. > + * in_vblank_irq: > + * True when called from drm_crtc_handle_vblank(). Some drivers > + * need to apply some workarounds for gpu-specific vblank irq quirks > + * if flag is set. > * > * Returns: > * > @@ -322,7 +321,7 @@ struct drm_driver { > bool (*get_vblank_timestamp) (struct drm_device *dev, unsigned int pipe, > int *max_error, > struct timeval *vblank_time, > - unsigned flags); > + bool in_vblank_irq); > > /* these have to be filled in */ > > diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h > index f0d5ccf9b282..445406efb8dc 100644 > --- a/include/drm/drm_irq.h > +++ b/include/drm/drm_irq.h > @@ -156,7 +156,7 @@ u32 drm_accurate_vblank_count(struct drm_crtc *crtc); > bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, > unsigned int pipe, int *max_error, > struct timeval *vblank_time, > - unsigned flags, > + bool in_vblank_irq, > const struct drm_display_mode *mode); > void drm_calc_timestamping_constants(struct drm_crtc *crtc, > const struct drm_display_mode *mode); > -- > 2.11.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html