[Intel-gfx] [PATCH 12/16] drm/vblank: Switch to bool in_vblank_irq in get_vblank_timestamp

Ville Syrjälä ville.syrjala at linux.intel.com
Wed Mar 22 18:23:54 UTC 2017


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 at linux.intel.com>

> 
> Cc: Mario Kleiner <mario.kleiner at tuebingen.mpg.de>
> Cc: Eric Anholt <eric at anholt.net>
> Cc: Rob Clark <robdclark at gmail.com>
> Cc: linux-arm-msm at vger.kernel.org
> Cc: freedreno at lists.freedesktop.org
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Cc: Christian König <christian.koenig at amd.com>
> Cc: Ben Skeggs <bskeggs at redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>  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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC


More information about the Intel-gfx mailing list