Re: [PATCH 08/11] drm/vblank: drop the mode argument from drm_calc_vbltimestamp_from_scanoutpos

[Neil Armstrong <narmstrong@baylibre.com>]

On 04/04/2017 11:53 AM, Daniel Vetter wrote:
> If we restrict this helper to only kms drivers (which is the case) we
> can look up the correct mode easily ourselves. But it's a bit tricky:
> 
> - All legacy drivers look at crtc->hwmode. But that is update already
is updated ?
>   at the beginning of the modeset helper, which means when we disable
>   a pipe. Hence the final timestamps might be a bit off. But since
>   this is an existing bug I'm not going to change it, but just try to
>   be bug-for-bug compatible with the current code. This only applies
>   to radeon&amdgpu.
> 
> - i915 tries to get it perfect by updating crtc->hwmode when the pipe
>   is off (i.e. vblank->enabled = false).
> 
> - All other atomic drivers look at crtc->state->adjusted_mode. Those
>   that look at state->requested_mode simply don't adjust their mode,
>   so it's the same. That has two problems: Accessing crtc->state from
>   interrupt handling code is unsafe, and it's updated before we shut
>   down the pipe. For nonblocking modesets it's even worse.
> 
> For atomic drivers try to implement what i915 does. To do that we add
> a new hwmode field to the vblank structure, and update it from
> drm_calc_timestamping_constants(). For atomic drivers that's called
> from the right spot by the helper library already, so all fine. But
> for safety let's enforce that.
> 
> For legacy driver this function is only called at the end (oh the
> fun), which is broken, so again let's not bother and just stay
> bug-for-bug compatible.
> 
> The  benefit is that we can use drm_calc_vbltimestamp_from_scanoutpos
> directly to implement ->get_vblank_timestamp in every driver, deleting
> a lot of code.
> 
> v2: Completely new approach, trying to mimick the i915 solution.
> 
> v3: Fixup kerneldoc.
> 
> v4: Drop the WARN_ON to check that the vblank is off, atomic helpers
> currently unconditionally call this. Recomputing the same stuff should
> be harmless.
> 
> Cc: Mario Kleiner <mario.kleiner@tuebingen.mpg.de>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  4 ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   | 41 -------------------------------
>  drivers/gpu/drm/drm_irq.c                 | 27 +++++++++++++++++---
>  drivers/gpu/drm/i915/i915_irq.c           | 33 +------------------------
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   | 26 +-------------------
>  drivers/gpu/drm/nouveau/nouveau_display.c | 22 -----------------
>  drivers/gpu/drm/nouveau/nouveau_display.h |  2 --
>  drivers/gpu/drm/nouveau/nouveau_drm.c     |  2 +-
>  drivers/gpu/drm/radeon/radeon_drv.c       |  6 +----
>  drivers/gpu/drm/radeon/radeon_kms.c       | 37 ----------------------------
>  drivers/gpu/drm/vc4/vc4_crtc.c            | 13 ----------
>  drivers/gpu/drm/vc4/vc4_drv.c             |  2 +-
>  drivers/gpu/drm/vc4/vc4_drv.h             |  3 ---
>  include/drm/drm_irq.h                     | 15 +++++++++--
>  15 files changed, 42 insertions(+), 193 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 310b20ef794a..11ebe861ec78 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1913,10 +1913,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool resume, bool fbcon);
>  u32 amdgpu_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe);
>  int amdgpu_enable_vblank_kms(struct drm_device *dev, unsigned int pipe);
>  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,
> -				     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_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 400917fd7486..40dd5d19f835 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -725,7 +725,7 @@ static struct drm_driver kms_driver = {
>  	.get_vblank_counter = amdgpu_get_vblank_counter_kms,
>  	.enable_vblank = amdgpu_enable_vblank_kms,
>  	.disable_vblank = amdgpu_disable_vblank_kms,
> -	.get_vblank_timestamp = amdgpu_get_vblank_timestamp_kms,
> +	.get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
>  	.get_scanout_position = amdgpu_get_crtc_scanoutpos,
>  #if defined(CONFIG_DEBUG_FS)
>  	.debugfs_init = amdgpu_debugfs_init,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 109a77ffc692..991ae253171b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -940,47 +940,6 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe)
>  	amdgpu_irq_put(adev, &adev->crtc_irq, idx);
>  }
>  
> -/**
> - * amdgpu_get_vblank_timestamp_kms - get vblank timestamp
> - *
> - * @dev: drm dev pointer
> - * @crtc: crtc to get the timestamp for
> - * @max_error: max error
> - * @vblank_time: time value
> - * @in_vblank_irq: called from drm_handle_vblank()
> - *
> - * Gets the timestamp on the requested crtc based on the
> - * scanout position.  (all asics).
> - * Returns true on success, false on failure.
> - */
> -bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
> -				     int *max_error,
> -				     struct timeval *vblank_time,
> -				     bool in_vblank_irq)
> -{
> -	struct drm_crtc *crtc;
> -	struct amdgpu_device *adev = dev->dev_private;
> -
> -	if (pipe >= dev->num_crtcs) {
> -		DRM_ERROR("Invalid crtc %u\n", pipe);
> -		return false;
> -	}
> -
> -	/* Get associated drm_crtc: */
> -	crtc = &adev->mode_info.crtcs[pipe]->base;
> -	if (!crtc) {
> -		/* This can occur on driver load if some component fails to
> -		 * initialize completely and driver is unloaded */
> -		DRM_ERROR("Uninitialized crtc %d\n", pipe);
> -		return false;
> -	}
> -
> -	/* Helper routine in DRM core does all the work: */
> -	return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
> -						     vblank_time, in_vblank_irq,
> -						     &crtc->hwmode);
> -}
> -
>  const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
>  	DRM_IOCTL_DEF_DRV(AMDGPU_GEM_CREATE, amdgpu_gem_create_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(AMDGPU_CTX, amdgpu_ctx_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 82b6e0521fe1..992eb3719c3e 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -684,6 +684,7 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,
>  
>  	vblank->linedur_ns  = linedur_ns;
>  	vblank->framedur_ns = framedur_ns;
> +	vblank->hwmode = *mode;
>  
>  	DRM_DEBUG("crtc %u: hwmode: htotal %d, vtotal %d, vdisplay %d\n",
>  		  crtc->base.id, mode->crtc_htotal,
> @@ -704,7 +705,6 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants);
>   *     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
>   * timings and current video scanout position of a CRTC. This can be called from
> @@ -724,6 +724,13 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants);
>   * returns as no operation if a doublescan or interlaced video mode is
>   * active. Higher level code is expected to handle this.
>   *
> + * This function can be used to implement the &drm_driver.get_vblank_timestamp
> + * directly, if the driver implements the &drm_driver.get_scanout_position hook.
> + *
> + * Note that atomic drivers must call drm_calc_timestamping_constants() before
> + * enabling a CRTC. The atomic helpers already take care of that in
> + * drm_atomic_helper_update_legacy_modeset_state().
> + *
>   * Returns:
>   *
>   * Returns true on success, and false on failure, i.e. when no accurate
> @@ -733,17 +740,24 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>  					   unsigned int pipe,
>  					   int *max_error,
>  					   struct timeval *vblank_time,
> -					   bool in_vblank_irq,
> -					   const struct drm_display_mode *mode)
> +					   bool in_vblank_irq)
>  {
>  	struct timeval tv_etime;
>  	ktime_t stime, etime;
>  	unsigned int vbl_status;
> +	struct drm_crtc *crtc;
> +	const struct drm_display_mode *mode;
> +	struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
>  	int vpos, hpos, i;
>  	int delta_ns, duration_ns;
>  	unsigned flags = in_vblank_irq ? DRM_CALLED_FROM_VBLIRQ : 0;
>  
> -	if (pipe >= dev->num_crtcs) {
> +	if (!drm_core_check_feature(dev, DRIVER_MODESET))
> +		return false;
> +
> +	crtc = drm_crtc_from_index(dev, pipe);
> +
> +	if (pipe >= dev->num_crtcs || !crtc) {
>  		DRM_ERROR("Invalid crtc %u\n", pipe);
>  		return false;
>  	}
> @@ -754,6 +768,11 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>  		return false;
>  	}
>  
> +	if (drm_drv_uses_atomic_modeset(dev))
> +		mode = &vblank->hwmode;
> +	else
> +		mode = &crtc->hwmode;
> +
>  	/* If mode timing undefined, just return as no-op:
>  	 * Happens during initial modesetting of a crtc.
>  	 */
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 6e1063d3629e..fdb1162427a9 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -964,37 +964,6 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
>  	return position;
>  }
>  
> -static bool i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
> -			      int *max_error,
> -			      struct timeval *vblank_time,
> -			      bool in_vblank_irq)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> -	struct intel_crtc *crtc;
> -
> -	if (pipe >= INTEL_INFO(dev_priv)->num_pipes) {
> -		DRM_ERROR("Invalid crtc %u\n", pipe);
> -		return false;
> -	}
> -
> -	/* Get drm_crtc to timestamp: */
> -	crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> -	if (crtc == NULL) {
> -		DRM_ERROR("Invalid crtc %u\n", pipe);
> -		return false;
> -	}
> -
> -	if (!crtc->base.hwmode.crtc_clock) {
> -		DRM_DEBUG_KMS("crtc %u is disabled\n", pipe);
> -		return false;
> -	}
> -
> -	/* Helper routine in DRM core does all the work: */
> -	return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
> -						     vblank_time, in_vblank_irq,
> -						     &crtc->base.hwmode);
> -}
> -
>  static void ironlake_rps_change_irq_handler(struct drm_i915_private *dev_priv)
>  {
>  	u32 busy_up, busy_down, max_avg, min_avg;
> @@ -4329,7 +4298,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>  
>  	dev_priv->hotplug.hpd_storm_threshold = HPD_STORM_DEFAULT_THRESHOLD;
>  
> -	dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp;
> +	dev->driver->get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos;
>  	dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
>  
>  	if (IS_CHERRYVIEW(dev_priv)) {
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> index 16184ccbdd3b..6ba216b8bba9 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> @@ -595,30 +595,6 @@ static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe,
>  	return ret;
>  }
>  
> -static bool mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
> -				      int *max_error,
> -				      struct timeval *vblank_time,
> -				      bool in_vblank_irq)
> -{
> -	struct msm_drm_private *priv = dev->dev_private;
> -	struct drm_crtc *crtc;
> -
> -	if (pipe < 0 || pipe >= priv->num_crtcs) {
> -		DRM_ERROR("Invalid crtc %d\n", pipe);
> -		return false;
> -	}
> -
> -	crtc = priv->crtcs[pipe];
> -	if (!crtc) {
> -		DRM_ERROR("Invalid crtc %d\n", pipe);
> -		return false;
> -	}
> -
> -	return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
> -						     vblank_time, in_vblank_irq,
> -						     &crtc->mode);
> -}
> -
>  static u32 mdp5_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
>  {
>  	struct msm_drm_private *priv = dev->dev_private;
> @@ -728,7 +704,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
>  	dev->mode_config.max_width = 0xffff;
>  	dev->mode_config.max_height = 0xffff;
>  
> -	dev->driver->get_vblank_timestamp = mdp5_get_vblank_timestamp;
> +	dev->driver->get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos;
>  	dev->driver->get_scanout_position = mdp5_get_scanoutpos;
>  	dev->driver->get_vblank_counter = mdp5_get_vblank_counter;
>  	dev->max_vblank_count = 0xffffffff;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index c7caa0674c54..33d0a6175d44 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -156,28 +156,6 @@ nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe,
>  	return 0;
>  }
>  
> -bool
> -nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe,
> -			 int *max_error, struct timeval *time, bool in_vblank_irq)
> -{
> -	struct drm_crtc *crtc;
> -
> -	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> -		if (nouveau_crtc(crtc)->index == pipe) {
> -			struct drm_display_mode *mode;
> -			if (drm_drv_uses_atomic_modeset(dev))
> -				mode = &crtc->state->adjusted_mode;
> -			else
> -				mode = &crtc->hwmode;
> -			return drm_calc_vbltimestamp_from_scanoutpos(dev,
> -					pipe, max_error, time, in_vblank_irq,
> -					mode);
> -		}
> -	}
> -
> -	return false;
> -}
> -
>  static void
>  nouveau_display_vblank_fini(struct drm_device *dev)
>  {
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
> index 8ec86259c5ac..a8285c2747a9 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.h
> @@ -71,8 +71,6 @@ void nouveau_display_vblank_disable(struct drm_device *, unsigned int);
>  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 *, 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/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index ec719df619a6..1f751a3f570c 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -978,7 +978,7 @@ driver_stub = {
>  	.enable_vblank = nouveau_display_vblank_enable,
>  	.disable_vblank = nouveau_display_vblank_disable,
>  	.get_scanout_position = nouveau_display_scanoutpos,
> -	.get_vblank_timestamp = nouveau_display_vblstamp,
> +	.get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
>  
>  	.ioctls = nouveau_ioctls,
>  	.num_ioctls = ARRAY_SIZE(nouveau_ioctls),
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 88fc791ec8fb..50d97f12e7bf 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -115,10 +115,6 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon);
>  u32 radeon_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe);
>  int radeon_enable_vblank_kms(struct drm_device *dev, unsigned int pipe);
>  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,
> -				     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);
> @@ -544,7 +540,7 @@ static struct drm_driver kms_driver = {
>  	.get_vblank_counter = radeon_get_vblank_counter_kms,
>  	.enable_vblank = radeon_enable_vblank_kms,
>  	.disable_vblank = radeon_disable_vblank_kms,
> -	.get_vblank_timestamp = radeon_get_vblank_timestamp_kms,
> +	.get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
>  	.get_scanout_position = radeon_get_crtc_scanoutpos,
>  	.irq_preinstall = radeon_driver_irq_preinstall_kms,
>  	.irq_postinstall = radeon_driver_irq_postinstall_kms,
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
> index 5bccdeae0773..6a68d440bc44 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -858,43 +858,6 @@ void radeon_disable_vblank_kms(struct drm_device *dev, int crtc)
>  	spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
>  }
>  
> -/**
> - * radeon_get_vblank_timestamp_kms - get vblank timestamp
> - *
> - * @dev: drm dev pointer
> - * @crtc: crtc to get the timestamp for
> - * @max_error: max error
> - * @vblank_time: time value
> - * @flags: flags passed to the driver
> - *
> - * Gets the timestamp on the requested crtc based on the
> - * scanout position.  (all asics).
> - * Returns true on success, false on failure.
> - */
> -bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc,
> -				     int *max_error,
> -				     struct timeval *vblank_time,
> -				     bool in_vblank_irq)
> -{
> -	struct drm_crtc *drmcrtc;
> -	struct radeon_device *rdev = dev->dev_private;
> -
> -	if (crtc < 0 || crtc >= dev->num_crtcs) {
> -		DRM_ERROR("Invalid crtc %d\n", crtc);
> -		return false;
> -	}
> -
> -	/* Get associated drm_crtc: */
> -	drmcrtc = &rdev->mode_info.crtcs[crtc]->base;
> -	if (!drmcrtc)
> -		return false;
> -
> -	/* Helper routine in DRM core does all the work: */
> -	return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc, max_error,
> -						     vblank_time, in_vblank_irq,
> -						     &drmcrtc->hwmode);
> -}
> -
>  const struct drm_ioctl_desc radeon_ioctls_kms[] = {
>  	DRM_IOCTL_DEF_DRV(RADEON_CP_INIT, drm_invalid_op, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>  	DRM_IOCTL_DEF_DRV(RADEON_CP_START, drm_invalid_op, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 38f5cc740cd1..2acfecc5b811 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -270,19 +270,6 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
>  	return ret;
>  }
>  
> -bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id,
> -				  int *max_error, struct timeval *vblank_time,
> -				  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, in_vblank_irq,
> -						     &state->adjusted_mode);
> -}
> -
>  static void vc4_crtc_destroy(struct drm_crtc *crtc)
>  {
>  	drm_crtc_cleanup(crtc);
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index 61e674baf3a6..e864256e12e5 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -154,7 +154,7 @@ static struct drm_driver vc4_drm_driver = {
>  	.irq_uninstall = vc4_irq_uninstall,
>  
>  	.get_scanout_position = vc4_crtc_get_scanoutpos,
> -	.get_vblank_timestamp = vc4_crtc_get_vblank_timestamp,
> +	.get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
>  
>  #if defined(CONFIG_DEBUG_FS)
>  	.debugfs_init = vc4_debugfs_init,
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 815cdeb54971..64c92e0eb8f7 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -450,9 +450,6 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
>  			    unsigned int flags, int *vpos, int *hpos,
>  			    ktime_t *stime, ktime_t *etime,
>  			    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,
> -				  bool in_vblank_irq);
>  
>  /* vc4_debugfs.c */
>  int vc4_debugfs_init(struct drm_minor *minor);
> diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h
> index 445406efb8dc..b489cc856e7a 100644
> --- a/include/drm/drm_irq.h
> +++ b/include/drm/drm_irq.h
> @@ -121,6 +121,18 @@ struct drm_vblank_crtc {
>  	 * drm_calc_timestamping_constants().
>  	 */
>  	int linedur_ns;
> +
> +	/**
> +	 * @hwmode:
> +	 *
> +	 * Cache of the current hardware display mode. Only valide when @enabled
s/valide/valid/
> +	 * is set. This is used by helpers like
> +	 * drm_calc_vbltimestamp_from_scanoutpos(). We can't just access the
> +	 * hardware mode by e.g. looking at &drm_crtc_state.adjusted_mode,
> +	 * because that one is really hard to get at from interrupt context.
s/at from/from/
> +	 */
> +	struct drm_display_mode hwmode;
> +
>  	/**
>  	 * @enabled: Tracks the enabling state of the corresponding &drm_crtc to
>  	 * avoid double-disabling and hence corrupting saved state. Needed by
> @@ -156,8 +168,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,
> -					   bool in_vblank_irq,
> -					   const struct drm_display_mode *mode);
> +					   bool in_vblank_irq);
>  void drm_calc_timestamping_constants(struct drm_crtc *crtc,
>  				     const struct drm_display_mode *mode);
>  
> 

With the typo fixes,
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx