mbox series

[v4,00/22] drm: Clean up VBLANK callbacks in struct drm_driver

Message ID 20200123135943.24140-1-tzimmermann@suse.de
Headers show
Series drm: Clean up VBLANK callbacks in struct drm_driver | expand

Message

Thomas Zimmermann Jan. 23, 2020, 1:59 p.m. UTC
VBLANK handlers in struct drm_driver are deprecated. Only legacy,
non-KMS drivers are supposed to used them. DRM drivers with kernel
modesetting are supposed to use VBLANK callbacks of the CRTC
infrastructure.

This patchset converts all DRM drivers to CRTC VBLANK callbacks and
cleans up struct drm_driver. The remaining VBLANK callbacks in struct
drm_driver are only used by legacy drivers.

Patch 1 removes an additional setup step of vblank_disable_immediate
in struct drm_device. This simplifies the integration of CRTC VBLANK
callbacks in patch 3. If necessary, a future patch could move
vblank_disable_immedate to struct drm_crtc, so that high-precision
VBLANKs could be enabled on a per-CRTC basis.

Patches 2 and 3 prepare the DRM infrastructure. These patches add
get_scanout_position() to struct drm_crtc_helper_funcs,
get_vblank_timestamp() to struct drm_crtc_funcs, and add helpers for
the new interfaces.

Patches 4 to 20 convert drivers over.

In patch 21, all VBLANK callbacks are removed from struct drm_driver,
except for get_vblank_counter(), enable_vblank(), and disable_vblank().
These interfaces are moved to the legacy section at the end of the
structure. Old helper code is now unused and being removed as well.
Finally, patch 22 removes an older version of get_scanout_position()
from the VBLANK interface.

To cover all affected drivers, I build the patchset in x86, x86-64,
arm and aarch64. I smoke-tested amdgpu, gma500, i915, radeon and vc4 on
respective hardware.

v4:
	* fixed warnings and improved code readability (Ville, Jani)
v3:
	* refactor drm_calc_vbltimestamp_from_scanout_pos to share code
	  with new helper (Villa, Jani)
	* do more checks for crtc != NULL to cover non-KMS drivers (Ville)
	* add function typedefs for readability (Ville)
v2:
	* reorder patches so the i915 can be converted without duplicating
	  helper code.
	* merged cleanup patches
	* changed VBLANK function signatures in amdgpu (Alex)

Thomas Zimmermann (22):
  drm: Remove internal setup of struct
    drm_device.vblank_disable_immediate
  drm: Add get_scanout_position() to struct drm_crtc_helper_funcs
  drm: Add get_vblank_timestamp() to struct drm_crtc_funcs
  drm/amdgpu: Convert to struct
    drm_crtc_helper_funcs.get_scanout_position()
  drm/amdgpu: Convert to CRTC VBLANK callbacks
  drm/gma500: Convert to CRTC VBLANK callbacks
  drm/i915: Convert to CRTC VBLANK callbacks
  drm/nouveau: Convert to struct
    drm_crtc_helper_funcs.get_scanout_position()
  drm/nouveau: Convert to CRTC VBLANK callbacks
  drm/radeon: Convert to struct
    drm_crtc_helper_funcs.get_scanout_position()
  drm/radeon: Convert to CRTC VBLANK callbacks
  drm/msm: Convert to struct
    drm_crtc_helper_funcs.get_scanout_position()
  drm/msm: Convert to CRTC VBLANK callbacks
  drm/stm: Convert to struct
    drm_crtc_helper_funcs.get_scanout_position()
  drm/stm: Convert to CRTC VBLANK callbacks
  drm/sti: Convert to CRTC VBLANK callbacks
  drm/vc4: Convert to struct
    drm_crtc_helper_funcs.get_scanout_position()
  drm/vc4: Convert to CRTC VBLANK callbacks
  drm/vkms: Convert to CRTC VBLANK callbacks
  drm/vmwgfx: Convert to CRTC VBLANK callbacks
  drm: Clean-up VBLANK-related callbacks in struct drm_driver
  drm: Remove legacy version of get_scanout_position()

 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |   6 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  16 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  15 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c       |  21 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h      |   5 +
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c        |   5 +
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c        |   5 +
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c         |   5 +
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c         |   5 +
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c      |   5 +
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  13 +-
 drivers/gpu/drm/drm_vblank.c                  | 146 +++++++++-------
 drivers/gpu/drm/gma500/cdv_intel_display.c    |   3 +
 drivers/gpu/drm/gma500/psb_drv.c              |   4 -
 drivers/gpu/drm/gma500/psb_drv.h              |   6 +-
 drivers/gpu/drm/gma500/psb_intel_display.c    |   3 +
 drivers/gpu/drm/gma500/psb_irq.c              |  12 +-
 drivers/gpu/drm/gma500/psb_irq.h              |   7 +-
 drivers/gpu/drm/i915/display/intel_display.c  |   7 +
 drivers/gpu/drm/i915/i915_drv.c               |   3 -
 drivers/gpu/drm/i915/i915_irq.c               |  20 ++-
 drivers/gpu/drm/i915/i915_irq.h               |   6 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      |   2 +
 drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c     |   2 +
 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c     |  82 +++++++++
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c      |  95 -----------
 drivers/gpu/drm/msm/msm_drv.c                 |  10 +-
 drivers/gpu/drm/msm/msm_drv.h                 |   3 +
 drivers/gpu/drm/nouveau/dispnv04/crtc.c       |   4 +
 drivers/gpu/drm/nouveau/dispnv50/head.c       |   5 +
 drivers/gpu/drm/nouveau/nouveau_display.c     |  28 +---
 drivers/gpu/drm/nouveau/nouveau_display.h     |  11 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c         |   5 -
 drivers/gpu/drm/radeon/atombios_crtc.c        |   1 +
 drivers/gpu/drm/radeon/radeon_display.c       |  25 ++-
 drivers/gpu/drm/radeon/radeon_drv.c           |  18 --
 drivers/gpu/drm/radeon/radeon_kms.c           |  29 ++--
 drivers/gpu/drm/radeon/radeon_legacy_crtc.c   |   3 +-
 drivers/gpu/drm/radeon/radeon_mode.h          |   6 +
 drivers/gpu/drm/sti/sti_crtc.c                |  11 +-
 drivers/gpu/drm/sti/sti_crtc.h                |   2 -
 drivers/gpu/drm/sti/sti_drv.c                 |   4 -
 drivers/gpu/drm/stm/drv.c                     |   2 -
 drivers/gpu/drm/stm/ltdc.c                    |  66 ++++----
 drivers/gpu/drm/stm/ltdc.h                    |   5 -
 drivers/gpu/drm/vc4/vc4_crtc.c                |  13 +-
 drivers/gpu/drm/vc4/vc4_drv.c                 |   3 -
 drivers/gpu/drm/vc4/vc4_drv.h                 |   4 -
 drivers/gpu/drm/vkms/vkms_crtc.c              |   9 +-
 drivers/gpu/drm/vkms/vkms_drv.c               |   1 -
 drivers/gpu/drm/vkms/vkms_drv.h               |   4 -
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c           |   3 -
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h           |   6 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c           |   6 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c           |   3 +
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c          |   3 +
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c          |   3 +
 include/drm/drm_crtc.h                        |  46 +++++-
 include/drm/drm_drv.h                         | 156 +-----------------
 include/drm/drm_modeset_helper_vtables.h      |  47 ++++++
 include/drm/drm_vblank.h                      |  35 +++-
 61 files changed, 559 insertions(+), 520 deletions(-)

--
2.24.1

Comments

Ben Skeggs Jan. 29, 2020, 8:25 a.m. UTC | #1
On Fri, 24 Jan 2020 at 00:00, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> The callback struct drm_driver.get_scanout_position() is deprecated in
> favor of struct drm_crtc_helper_funcs.get_scanout_position(). Convert
> nouveau over.
>
> v4:
>         * add argument names in function declaration
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Ben Skeggs <bskeggs@redhat.com>

> ---
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c   |  1 +
>  drivers/gpu/drm/nouveau/dispnv50/head.c   |  1 +
>  drivers/gpu/drm/nouveau/nouveau_display.c | 14 +++-----------
>  drivers/gpu/drm/nouveau/nouveau_display.h |  7 ++++---
>  drivers/gpu/drm/nouveau/nouveau_drm.c     |  1 -
>  5 files changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index 37c50ea8f847..17e9d1c078a0 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> @@ -1258,6 +1258,7 @@ static const struct drm_crtc_helper_funcs nv04_crtc_helper_funcs = {
>         .mode_set_base = nv04_crtc_mode_set_base,
>         .mode_set_base_atomic = nv04_crtc_mode_set_base_atomic,
>         .disable = nv_crtc_disable,
> +       .get_scanout_position = nouveau_display_scanoutpos,
>  };
>
>  static const uint32_t modeset_formats[] = {
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index d9d64602947d..41852dd8fdbd 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -413,6 +413,7 @@ nv50_head_atomic_check(struct drm_crtc *crtc, struct drm_crtc_state *state)
>  static const struct drm_crtc_helper_funcs
>  nv50_head_help = {
>         .atomic_check = nv50_head_atomic_check,
> +       .get_scanout_position = nouveau_display_scanoutpos,
>  };
>
>  static void
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index 53f9bceaf17a..86f99dc8fcef 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -136,21 +136,13 @@ nouveau_display_scanoutpos_head(struct drm_crtc *crtc, int *vpos, int *hpos,
>  }
>
>  bool
> -nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe,
> +nouveau_display_scanoutpos(struct drm_crtc *crtc,
>                            bool in_vblank_irq, int *vpos, int *hpos,
>                            ktime_t *stime, ktime_t *etime,
>                            const struct drm_display_mode *mode)
>  {
> -       struct drm_crtc *crtc;
> -
> -       list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> -               if (nouveau_crtc(crtc)->index == pipe) {
> -                       return nouveau_display_scanoutpos_head(crtc, vpos, hpos,
> -                                                              stime, etime);
> -               }
> -       }
> -
> -       return false;
> +       return nouveau_display_scanoutpos_head(crtc, vpos, hpos,
> +                                              stime, etime);
>  }
>
>  static void
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
> index 6e8e66882e45..26d34f1a77da 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.h
> @@ -63,9 +63,10 @@ int  nouveau_display_suspend(struct drm_device *dev, bool runtime);
>  void nouveau_display_resume(struct drm_device *dev, bool runtime);
>  int  nouveau_display_vblank_enable(struct drm_device *, unsigned int);
>  void nouveau_display_vblank_disable(struct drm_device *, unsigned int);
> -bool  nouveau_display_scanoutpos(struct drm_device *, unsigned int,
> -                                bool, int *, int *, ktime_t *,
> -                                ktime_t *, const struct drm_display_mode *);
> +bool nouveau_display_scanoutpos(struct drm_crtc *crtc,
> +                               bool in_vblank_irq, int *vpos, int *hpos,
> +                               ktime_t *stime, ktime_t *etime,
> +                               const struct drm_display_mode *mode);
>
>  int  nouveau_display_dumb_create(struct drm_file *, struct drm_device *,
>                                  struct drm_mode_create_dumb *args);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index b65ae817eabf..fcc036a08965 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -1122,7 +1122,6 @@ driver_stub = {
>
>         .enable_vblank = nouveau_display_vblank_enable,
>         .disable_vblank = nouveau_display_vblank_disable,
> -       .get_scanout_position = nouveau_display_scanoutpos,
>         .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
>
>         .ioctls = nouveau_ioctls,
> --
> 2.24.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Daniel Vetter March 2, 2020, 9:55 p.m. UTC | #2
On Thu, Jan 23, 2020 at 2:59 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> The new callback get_scanout_position() reads the current location
> of the scanout process. The operation is currently located in struct
> drm_driver, but really belongs to the CRTC. Drivers will be converted
> in separate patches.
>
> To help with the conversion, the timestamp calculation has been
> moved from drm_calc_vbltimestamp_from_scanoutpos() to
> drm_crtc_vblank_helper_get_vblank_timestamp_internal(). The helper
> function supports the new and old interface of get_scanout_position().
> drm_calc_vbltimestamp_from_scanoutpos() remains as a wrapper around
> the new function.
>
> Callback functions return the scanout position from the CRTC. The
> legacy version of the interface receives the device and pipe index,
> the modern version receives a pointer to the CRTC. We keep the
> legacy version until all drivers have been converted.
>
> v4:
>         * 80-character line fixes
> v3:
>         * refactor drm_calc_vbltimestamp_from_scanoutpos() to minimize
>           code duplication
>         * define types for get_scanout_position() callbacks
> v2:
>         * fix logical op in drm_calc_vbltimestamp_from_scanoutpos()
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Tested-by: Yannick Fertré <yannick.fertre@st.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

This patch causes new kerneldoc build warnings:

./drivers/gpu/drm/drm_vblank.c:623: warning: Excess function parameter
'dev' description in
'drm_crtc_vblank_helper_get_vblank_timestamp_internal'
./drivers/gpu/drm/drm_vblank.c:623: warning: Excess function parameter
'pipe' description in
'drm_crtc_vblank_helper_get_vblank_timestamp_internal'
./drivers/gpu/drm/drm_vblank.c:624: warning: Function parameter or
member 'crtc' not described in
'drm_crtc_vblank_helper_get_vblank_timestamp_internal'
./drivers/gpu/drm/drm_vblank.c:624: warning: Excess function parameter
'dev' description in
'drm_crtc_vblank_helper_get_vblank_timestamp_internal'
./drivers/gpu/drm/drm_vblank.c:624: warning: Excess function parameter
'pipe' description in
'drm_crtc_vblank_helper_get_vblank_timestamp_internal'


Please fix.
-Daniel

> ---
>  drivers/gpu/drm/drm_vblank.c             | 101 +++++++++++++++++++----
>  include/drm/drm_drv.h                    |   7 +-
>  include/drm/drm_modeset_helper_vtables.h |  47 +++++++++++
>  include/drm/drm_vblank.h                 |  25 ++++++
>  4 files changed, 157 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 326db52f2ad8..7e962c29780c 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -30,6 +30,7 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_framebuffer.h>
> +#include <drm/drm_modeset_helper_vtables.h>
>  #include <drm/drm_print.h>
>  #include <drm/drm_vblank.h>
>
> @@ -577,7 +578,7 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants);
>   * Implements calculation of exact vblank timestamps from given drm_display_mode
>   * timings and current video scanout position of a CRTC. This can be directly
>   * used as the &drm_driver.get_vblank_timestamp implementation of a kms driver
> - * if &drm_driver.get_scanout_position is implemented.
> + * if &drm_crtc_helper_funcs.get_scanout_position is implemented.
>   *
>   * The current implementation only handles standard video modes. For double scan
>   * and interlaced modes the driver is supposed to adjust the hardware mode
> @@ -599,28 +600,85 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>                                            ktime_t *vblank_time,
>                                            bool in_vblank_irq)
>  {
> -       struct timespec64 ts_etime, ts_vblank_time;
> -       ktime_t stime, etime;
> -       bool 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;
>
>         if (!drm_core_check_feature(dev, DRIVER_MODESET))
>                 return false;
>
>         crtc = drm_crtc_from_index(dev, pipe);
> +       if (!crtc)
> +               return false;
>
> -       if (pipe >= dev->num_crtcs || !crtc) {
> +       return drm_crtc_vblank_helper_get_vblank_timestamp_internal(crtc,
> +                                                                   max_error,
> +                                                                   vblank_time,
> +                                                                   in_vblank_irq,
> +                                                                   crtc->helper_private->get_scanout_position,
> +                                                                   dev->driver->get_scanout_position);
> +}
> +EXPORT_SYMBOL(drm_calc_vbltimestamp_from_scanoutpos);
> +
> +/**
> + * drm_crtc_vblank_helper_get_vblank_timestamp_internal - precise vblank
> + *                                                        timestamp helper
> + * @dev: DRM device
> + * @pipe: index of CRTC whose vblank timestamp to retrieve
> + * @max_error: Desired maximum allowable error in timestamps (nanosecs)
> + *             On return contains true maximum error of timestamp
> + * @vblank_time: Pointer to time which should receive the timestamp
> + * @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.
> + * @get_scanout_position:
> + *     Callback function to retrieve the scanout position. See
> + *     @struct drm_crtc_helper_funcs.get_scanout_position.
> + * @get_scanout_position_legacy:
> + *     Callback function to retrieve the scanout position. See
> + *     @struct drm_driver.get_scanout_position.
> + *
> + * Implements calculation of exact vblank timestamps from given drm_display_mode
> + * timings and current video scanout position of a CRTC.
> + *
> + * The current implementation only handles standard video modes. For double scan
> + * and interlaced modes the driver is supposed to adjust the hardware mode
> + * (taken from &drm_crtc_state.adjusted mode for atomic modeset drivers) to
> + * match the scanout position reported.
> + *
> + * 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
> + * timestamp could be acquired.
> + */
> +bool
> +drm_crtc_vblank_helper_get_vblank_timestamp_internal(
> +       struct drm_crtc *crtc, int *max_error, ktime_t *vblank_time,
> +       bool in_vblank_irq,
> +       drm_vblank_get_scanout_position_func get_scanout_position,
> +       drm_vblank_get_scanout_position_legacy_func get_scanout_position_legacy)
> +{
> +       struct drm_device *dev = crtc->dev;
> +       unsigned int pipe = crtc->index;
> +       struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
> +       struct timespec64 ts_etime, ts_vblank_time;
> +       ktime_t stime, etime;
> +       bool vbl_status;
> +       const struct drm_display_mode *mode;
> +       int vpos, hpos, i;
> +       int delta_ns, duration_ns;
> +
> +       if (pipe >= dev->num_crtcs) {
>                 DRM_ERROR("Invalid crtc %u\n", pipe);
>                 return false;
>         }
>
>         /* Scanout position query not supported? Should not happen. */
> -       if (!dev->driver->get_scanout_position) {
> -               DRM_ERROR("Called from driver w/o get_scanout_position()!?\n");
> +       if (!get_scanout_position && !get_scanout_position_legacy) {
> +               DRM_ERROR("Called from CRTC w/o get_scanout_position()!?\n");
>                 return false;
>         }
>
> @@ -635,7 +693,6 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>         if (mode->crtc_clock == 0) {
>                 DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n", pipe);
>                 WARN_ON_ONCE(drm_drv_uses_atomic_modeset(dev));
> -
>                 return false;
>         }
>
> @@ -651,11 +708,19 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>                  * Get vertical and horizontal scanout position vpos, hpos,
>                  * and bounding timestamps stime, etime, pre/post query.
>                  */
> -               vbl_status = dev->driver->get_scanout_position(dev, pipe,
> -                                                              in_vblank_irq,
> -                                                              &vpos, &hpos,
> -                                                              &stime, &etime,
> -                                                              mode);
> +               if (get_scanout_position) {
> +                       vbl_status = get_scanout_position(crtc,
> +                                                         in_vblank_irq,
> +                                                         &vpos, &hpos,
> +                                                         &stime, &etime,
> +                                                         mode);
> +               } else {
> +                       vbl_status = get_scanout_position_legacy(dev, pipe,
> +                                                                in_vblank_irq,
> +                                                                &vpos, &hpos,
> +                                                                &stime, &etime,
> +                                                                mode);
> +               }
>
>                 /* Return as no-op if scanout query unsupported or failed. */
>                 if (!vbl_status) {
> @@ -707,7 +772,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>
>         return true;
>  }
> -EXPORT_SYMBOL(drm_calc_vbltimestamp_from_scanoutpos);
> +EXPORT_SYMBOL(drm_crtc_vblank_helper_get_vblank_timestamp_internal);
>
>  /**
>   * drm_get_last_vbltimestamp - retrieve raw timestamp for the most recent
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index cf13470810a5..d0049e5786fc 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -362,11 +362,8 @@ struct drm_driver {
>          * True on success, false if a reliable scanout position counter could
>          * not be read out.
>          *
> -        * FIXME:
> -        *
> -        * Since this is a helper to implement @get_vblank_timestamp, we should
> -        * move it to &struct drm_crtc_helper_funcs, like all the other
> -        * helper-internal hooks.
> +        * This is deprecated and should not be used by new drivers.
> +        * Use &drm_crtc_helper_funcs.get_scanout_position instead.
>          */
>         bool (*get_scanout_position) (struct drm_device *dev, unsigned int pipe,
>                                       bool in_vblank_irq, int *vpos, int *hpos,
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index 5a87f1bd7a3f..e398512bfd5f 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -450,6 +450,53 @@ struct drm_crtc_helper_funcs {
>          */
>         void (*atomic_disable)(struct drm_crtc *crtc,
>                                struct drm_crtc_state *old_crtc_state);
> +
> +       /**
> +        * @get_scanout_position:
> +        *
> +        * Called by vblank timestamping code.
> +        *
> +        * Returns the current display scanout position from a CRTC and an
> +        * optional accurate ktime_get() timestamp of when the position was
> +        * measured. Note that this is a helper callback which is only used
> +        * if a driver uses drm_calc_vbltimestamp_from_scanoutpos() for the
> +        * @drm_driver.get_vblank_timestamp callback.
> +        *
> +        * Parameters:
> +        *
> +        * crtc:
> +        *     The CRTC.
> +        * 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 the flag is set.
> +        * vpos:
> +        *     Target location for current vertical scanout position.
> +        * hpos:
> +        *     Target location for current horizontal scanout position.
> +        * stime:
> +        *     Target location for timestamp taken immediately before
> +        *     scanout position query. Can be NULL to skip timestamp.
> +        * etime:
> +        *     Target location for timestamp taken immediately after
> +        *     scanout position query. Can be NULL to skip timestamp.
> +        * mode:
> +        *     Current display timings.
> +        *
> +        * Returns vpos as a positive number while in active scanout area.
> +        * Returns vpos as a negative number inside vblank, counting the number
> +        * of scanlines to go until end of vblank, e.g., -1 means "one scanline
> +        * until start of active scanout / end of vblank."
> +        *
> +        * Returns:
> +        *
> +        * True on success, false if a reliable scanout position counter could
> +        * not be read out.
> +        */
> +       bool (*get_scanout_position)(struct drm_crtc *crtc,
> +                                    bool in_vblank_irq, int *vpos, int *hpos,
> +                                    ktime_t *stime, ktime_t *etime,
> +                                    const struct drm_display_mode *mode);
>  };
>
>  /**
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index c16c44052b3d..66d1fb376600 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -238,4 +238,29 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,
>  wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc);
>  void drm_crtc_set_max_vblank_count(struct drm_crtc *crtc,
>                                    u32 max_vblank_count);
> +
> +typedef bool (*drm_vblank_get_scanout_position_func)(struct drm_crtc *crtc,
> +                                                    bool in_vblank_irq,
> +                                                    int *vpos, int *hpos,
> +                                                    ktime_t *stime,
> +                                                    ktime_t *etime,
> +                                                    const struct drm_display_mode *mode);
> +
> +typedef bool (*drm_vblank_get_scanout_position_legacy_func)(struct drm_device *dev,
> +                                                           unsigned int pipe,
> +                                                           bool in_vblank_irq,
> +                                                           int *vpos,
> +                                                           int *hpos,
> +                                                           ktime_t *stime,
> +                                                           ktime_t *etime,
> +                                                           const struct drm_display_mode *mode);
> +
> +bool
> +drm_crtc_vblank_helper_get_vblank_timestamp_internal(struct drm_crtc *crtc,
> +                                                    int *max_error,
> +                                                    ktime_t *vblank_time,
> +                                                    bool in_vblank_irq,
> +                                                    drm_vblank_get_scanout_position_func get_scanout_position,
> +                                                    drm_vblank_get_scanout_position_legacy_func get_scanout_position_legacy);
> +
>  #endif
> --
> 2.24.1
>