Re: [PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()

Re: [PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20251201231807.287414-1-sunpeng.li@amd.com>
References: <20251201231807.287414-1-sunpeng.li@amd.com>
TO: sunpeng.li@amd.com
TO: amd-gfx@lists.freedesktop.org
TO: dri-devel@lists.freedesktop.org
CC: Harry.Wentland@amd.com
CC: Nicholas.Kazlauskas@amd.com
CC: simona@ffwll.ch
CC: airlied@gmail.com
CC: Leo Li <sunpeng.li@amd.com>

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-i915/for-linux-next-fixes]
[also build test WARNING on daeinki-drm-exynos/exynos-drm-next v6.18]
[cannot apply to drm-misc/drm-misc-next drm/drm-next drm-i915/for-linux-next drm-tip/drm-tip linus/master next-20251204]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/sunpeng-li-amd-com/drm-amd-display-Implement-prepare_vblank_enable-callback/20251202-072501
base:   https://gitlab.freedesktop.org/drm/i915/kernel.git for-linux-next-fixes
patch link:    https://lore.kernel.org/r/20251201231807.287414-1-sunpeng.li%40amd.com
patch subject: [PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
config: nios2-randconfig-r071-20251204 (https://download.01.org/0day-ci/archive/20251205/202512051120.3HWHmkDI-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 8.5.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202512051120.3HWHmkDI-lkp@intel.com/

smatch warnings:
drivers/gpu/drm/drm_vblank.c:1527 drm_crtc_vblank_on_config() warn: variable dereferenced before check 'crtc' (see line 1519)

vim +/crtc +1527 drivers/gpu/drm/drm_vblank.c

ed20151a7699bb Ville Syrjälä 2018-11-27  1502  
3ed4351a83ca05 Simona Vetter 2017-05-31  1503  /**
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1504   * drm_crtc_vblank_on_config - enable vblank events on a CRTC with custom
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1505   *     configuration options
3ed4351a83ca05 Simona Vetter 2017-05-31  1506   * @crtc: CRTC in question
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1507   * @config: Vblank configuration value
3ed4351a83ca05 Simona Vetter 2017-05-31  1508   *
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1509   * See drm_crtc_vblank_on(). In addition, this function allows you to provide a
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1510   * custom vblank configuration for a given CRTC.
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1511   *
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1512   * Note that @config is copied, the pointer does not need to stay valid beyond
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1513   * this function call. For details of the parameters see
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1514   * struct drm_vblank_crtc_config.
3ed4351a83ca05 Simona Vetter 2017-05-31  1515   */
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1516  void drm_crtc_vblank_on_config(struct drm_crtc *crtc,
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1517  			       const struct drm_vblank_crtc_config *config)
3ed4351a83ca05 Simona Vetter 2017-05-31  1518  {
3ed4351a83ca05 Simona Vetter 2017-05-31 @1519  	struct drm_device *dev = crtc->dev;
3ed4351a83ca05 Simona Vetter 2017-05-31  1520  	unsigned int pipe = drm_crtc_index(crtc);
d12e36494dc2bf Ville Syrjälä 2024-04-08  1521  	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
38bd1e412d0aa4 Leo Li        2025-12-01  1522  	int ret;
3ed4351a83ca05 Simona Vetter 2017-05-31  1523  
5a4784f49b2dcf Sam Ravnborg  2020-05-23  1524  	if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
3ed4351a83ca05 Simona Vetter 2017-05-31  1525  		return;
3ed4351a83ca05 Simona Vetter 2017-05-31  1526  
38bd1e412d0aa4 Leo Li        2025-12-01 @1527  	if (crtc) {
38bd1e412d0aa4 Leo Li        2025-12-01  1528  		ret = drm_crtc_vblank_prepare(crtc);
38bd1e412d0aa4 Leo Li        2025-12-01  1529  		drm_WARN_ON(dev, ret);
38bd1e412d0aa4 Leo Li        2025-12-01  1530  		if (ret)
38bd1e412d0aa4 Leo Li        2025-12-01  1531  			return;
38bd1e412d0aa4 Leo Li        2025-12-01  1532  	}
38bd1e412d0aa4 Leo Li        2025-12-01  1533  
92cc68e35863c1 Lyude Paul    2020-07-20  1534  	spin_lock_irq(&dev->vbl_lock);
02149a76d32bd8 Sam Ravnborg  2020-05-23  1535  	drm_dbg_vbl(dev, "crtc %d, vblank enabled %d, inmodeset %d\n",
3ed4351a83ca05 Simona Vetter 2017-05-31  1536  		    pipe, vblank->enabled, vblank->inmodeset);
3ed4351a83ca05 Simona Vetter 2017-05-31  1537  
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1538  	vblank->config = *config;
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1539  
3ed4351a83ca05 Simona Vetter 2017-05-31  1540  	/* Drop our private "prevent drm_vblank_get" refcount */
3ed4351a83ca05 Simona Vetter 2017-05-31  1541  	if (vblank->inmodeset) {
3ed4351a83ca05 Simona Vetter 2017-05-31  1542  		atomic_dec(&vblank->refcount);
3ed4351a83ca05 Simona Vetter 2017-05-31  1543  		vblank->inmodeset = 0;
3ed4351a83ca05 Simona Vetter 2017-05-31  1544  	}
3ed4351a83ca05 Simona Vetter 2017-05-31  1545  
3ed4351a83ca05 Simona Vetter 2017-05-31  1546  	drm_reset_vblank_timestamp(dev, pipe);
3ed4351a83ca05 Simona Vetter 2017-05-31  1547  
3ed4351a83ca05 Simona Vetter 2017-05-31  1548  	/*
3ed4351a83ca05 Simona Vetter 2017-05-31  1549  	 * re-enable interrupts if there are users left, or the
3ed4351a83ca05 Simona Vetter 2017-05-31  1550  	 * user wishes vblank interrupts to be enabled all the time.
3ed4351a83ca05 Simona Vetter 2017-05-31  1551  	 */
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1552  	if (atomic_read(&vblank->refcount) != 0 || !vblank->config.offdelay_ms)
5a4784f49b2dcf Sam Ravnborg  2020-05-23  1553  		drm_WARN_ON(dev, drm_vblank_enable(dev, pipe));
92cc68e35863c1 Lyude Paul    2020-07-20  1554  	spin_unlock_irq(&dev->vbl_lock);
3ed4351a83ca05 Simona Vetter 2017-05-31  1555  }
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1556  EXPORT_SYMBOL(drm_crtc_vblank_on_config);
0d5040e406d2c4 Hamza Mahfooz 2024-07-25  1557  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20251201231807.287414-1-sunpeng.li@amd.com>
References: <20251201231807.287414-1-sunpeng.li@amd.com>
TO: sunpeng.li@amd.com
TO: amd-gfx@lists.freedesktop.org
TO: dri-devel@lists.freedesktop.org
CC: Harry.Wentland@amd.com
CC: Nicholas.Kazlauskas@amd.com
CC: simona@ffwll.ch
CC: airlied@gmail.com
CC: Leo Li <sunpeng.li@amd.com>

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-i915/for-linux-next-fixes]
[also build test WARNING on daeinki-drm-exynos/exynos-drm-next linus/master v6.18]
[cannot apply to drm-misc/drm-misc-next drm/drm-next drm-i915/for-linux-next drm-tip/drm-tip next-20251203]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/sunpeng-li-amd-com/drm-amd-display-Implement-prepare_vblank_enable-callback/20251202-072501
base:   https://gitlab.freedesktop.org/drm/i915/kernel.git for-linux-next-fixes
patch link:    https://lore.kernel.org/r/20251201231807.287414-1-sunpeng.li%40amd.com
patch subject: [PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago
config: nios2-randconfig-r071-20251204 (https://download.01.org/0day-ci/archive/20251204/202512041153.nYks4oYu-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 8.5.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202512041153.nYks4oYu-lkp@intel.com/

smatch warnings:
drivers/gpu/drm/drm_vblank.c:1527 drm_crtc_vblank_on_config() warn: variable dereferenced before check 'crtc' (see line 1519)

vim +/crtc +1527 drivers/gpu/drm/drm_vblank.c

ed20151a7699bb2 Ville Syrjälä 2018-11-27  1502  
3ed4351a83ca05d Simona Vetter 2017-05-31  1503  /**
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1504   * drm_crtc_vblank_on_config - enable vblank events on a CRTC with custom
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1505   *     configuration options
3ed4351a83ca05d Simona Vetter 2017-05-31  1506   * @crtc: CRTC in question
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1507   * @config: Vblank configuration value
3ed4351a83ca05d Simona Vetter 2017-05-31  1508   *
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1509   * See drm_crtc_vblank_on(). In addition, this function allows you to provide a
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1510   * custom vblank configuration for a given CRTC.
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1511   *
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1512   * Note that @config is copied, the pointer does not need to stay valid beyond
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1513   * this function call. For details of the parameters see
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1514   * struct drm_vblank_crtc_config.
3ed4351a83ca05d Simona Vetter 2017-05-31  1515   */
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1516  void drm_crtc_vblank_on_config(struct drm_crtc *crtc,
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1517  			       const struct drm_vblank_crtc_config *config)
3ed4351a83ca05d Simona Vetter 2017-05-31  1518  {
3ed4351a83ca05d Simona Vetter 2017-05-31 @1519  	struct drm_device *dev = crtc->dev;
3ed4351a83ca05d Simona Vetter 2017-05-31  1520  	unsigned int pipe = drm_crtc_index(crtc);
d12e36494dc2bf2 Ville Syrjälä 2024-04-08  1521  	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
38bd1e412d0aa4b Leo Li        2025-12-01  1522  	int ret;
3ed4351a83ca05d Simona Vetter 2017-05-31  1523  
5a4784f49b2dcff Sam Ravnborg  2020-05-23  1524  	if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
3ed4351a83ca05d Simona Vetter 2017-05-31  1525  		return;
3ed4351a83ca05d Simona Vetter 2017-05-31  1526  
38bd1e412d0aa4b Leo Li        2025-12-01 @1527  	if (crtc) {
38bd1e412d0aa4b Leo Li        2025-12-01  1528  		ret = drm_crtc_vblank_prepare(crtc);
38bd1e412d0aa4b Leo Li        2025-12-01  1529  		drm_WARN_ON(dev, ret);
38bd1e412d0aa4b Leo Li        2025-12-01  1530  		if (ret)
38bd1e412d0aa4b Leo Li        2025-12-01  1531  			return;
38bd1e412d0aa4b Leo Li        2025-12-01  1532  	}
38bd1e412d0aa4b Leo Li        2025-12-01  1533  
92cc68e35863c1c Lyude Paul    2020-07-20  1534  	spin_lock_irq(&dev->vbl_lock);
02149a76d32bd8f Sam Ravnborg  2020-05-23  1535  	drm_dbg_vbl(dev, "crtc %d, vblank enabled %d, inmodeset %d\n",
3ed4351a83ca05d Simona Vetter 2017-05-31  1536  		    pipe, vblank->enabled, vblank->inmodeset);
3ed4351a83ca05d Simona Vetter 2017-05-31  1537  
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1538  	vblank->config = *config;
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1539  
3ed4351a83ca05d Simona Vetter 2017-05-31  1540  	/* Drop our private "prevent drm_vblank_get" refcount */
3ed4351a83ca05d Simona Vetter 2017-05-31  1541  	if (vblank->inmodeset) {
3ed4351a83ca05d Simona Vetter 2017-05-31  1542  		atomic_dec(&vblank->refcount);
3ed4351a83ca05d Simona Vetter 2017-05-31  1543  		vblank->inmodeset = 0;
3ed4351a83ca05d Simona Vetter 2017-05-31  1544  	}
3ed4351a83ca05d Simona Vetter 2017-05-31  1545  
3ed4351a83ca05d Simona Vetter 2017-05-31  1546  	drm_reset_vblank_timestamp(dev, pipe);
3ed4351a83ca05d Simona Vetter 2017-05-31  1547  
3ed4351a83ca05d Simona Vetter 2017-05-31  1548  	/*
3ed4351a83ca05d Simona Vetter 2017-05-31  1549  	 * re-enable interrupts if there are users left, or the
3ed4351a83ca05d Simona Vetter 2017-05-31  1550  	 * user wishes vblank interrupts to be enabled all the time.
3ed4351a83ca05d Simona Vetter 2017-05-31  1551  	 */
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1552  	if (atomic_read(&vblank->refcount) != 0 || !vblank->config.offdelay_ms)
5a4784f49b2dcff Sam Ravnborg  2020-05-23  1553  		drm_WARN_ON(dev, drm_vblank_enable(dev, pipe));
92cc68e35863c1c Lyude Paul    2020-07-20  1554  	spin_unlock_irq(&dev->vbl_lock);
3ed4351a83ca05d Simona Vetter 2017-05-31  1555  }
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1556  EXPORT_SYMBOL(drm_crtc_vblank_on_config);
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1557  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()

[Dan Carpenter]

Hi,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/sunpeng-li-amd-com/drm-amd-display-Implement-prepare_vblank_enable-callback/20251202-072501
base:   https://gitlab.freedesktop.org/drm/i915/kernel.git for-linux-next-fixes
patch link:    https://lore.kernel.org/r/20251201231807.287414-1-sunpeng.li%40amd.com
patch subject: [PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()
config: nios2-randconfig-r071-20251204 (https://download.01.org/0day-ci/archive/20251204/202512041153.nYks4oYu-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 8.5.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202512041153.nYks4oYu-lkp@intel.com/

smatch warnings:
drivers/gpu/drm/drm_vblank.c:1527 drm_crtc_vblank_on_config() warn: variable dereferenced before check 'crtc' (see line 1519)

vim +/crtc +1527 drivers/gpu/drm/drm_vblank.c

0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1516  void drm_crtc_vblank_on_config(struct drm_crtc *crtc,
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1517  			       const struct drm_vblank_crtc_config *config)
3ed4351a83ca05d Simona Vetter 2017-05-31  1518  {
3ed4351a83ca05d Simona Vetter 2017-05-31 @1519  	struct drm_device *dev = crtc->dev;
3ed4351a83ca05d Simona Vetter 2017-05-31  1520  	unsigned int pipe = drm_crtc_index(crtc);
                                                                                           ^^^^
Unchecked dereference.  I'm pretty certain crtc can't be NULL.

d12e36494dc2bf2 Ville Syrjälä 2024-04-08  1521  	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
38bd1e412d0aa4b Leo Li        2025-12-01  1522  	int ret;
3ed4351a83ca05d Simona Vetter 2017-05-31  1523  
5a4784f49b2dcff Sam Ravnborg  2020-05-23  1524  	if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
3ed4351a83ca05d Simona Vetter 2017-05-31  1525  		return;
3ed4351a83ca05d Simona Vetter 2017-05-31  1526  
38bd1e412d0aa4b Leo Li        2025-12-01 @1527  	if (crtc) {
                                                        ^^^^^^^^^^^
So this NULL check is too late, and can be removed.

38bd1e412d0aa4b Leo Li        2025-12-01  1528  		ret = drm_crtc_vblank_prepare(crtc);
38bd1e412d0aa4b Leo Li        2025-12-01  1529  		drm_WARN_ON(dev, ret);
38bd1e412d0aa4b Leo Li        2025-12-01  1530  		if (ret)
38bd1e412d0aa4b Leo Li        2025-12-01  1531  			return;
38bd1e412d0aa4b Leo Li        2025-12-01  1532  	}
38bd1e412d0aa4b Leo Li        2025-12-01  1533  
92cc68e35863c1c Lyude Paul    2020-07-20  1534  	spin_lock_irq(&dev->vbl_lock);
02149a76d32bd8f Sam Ravnborg  2020-05-23  1535  	drm_dbg_vbl(dev, "crtc %d, vblank enabled %d, inmodeset %d\n",
3ed4351a83ca05d Simona Vetter 2017-05-31  1536  		    pipe, vblank->enabled, vblank->inmodeset);
3ed4351a83ca05d Simona Vetter 2017-05-31  1537  
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1538  	vblank->config = *config;
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1539  
3ed4351a83ca05d Simona Vetter 2017-05-31  1540  	/* Drop our private "prevent drm_vblank_get" refcount */
3ed4351a83ca05d Simona Vetter 2017-05-31  1541  	if (vblank->inmodeset) {
3ed4351a83ca05d Simona Vetter 2017-05-31  1542  		atomic_dec(&vblank->refcount);
3ed4351a83ca05d Simona Vetter 2017-05-31  1543  		vblank->inmodeset = 0;
3ed4351a83ca05d Simona Vetter 2017-05-31  1544  	}
3ed4351a83ca05d Simona Vetter 2017-05-31  1545  
3ed4351a83ca05d Simona Vetter 2017-05-31  1546  	drm_reset_vblank_timestamp(dev, pipe);
3ed4351a83ca05d Simona Vetter 2017-05-31  1547  
3ed4351a83ca05d Simona Vetter 2017-05-31  1548  	/*
3ed4351a83ca05d Simona Vetter 2017-05-31  1549  	 * re-enable interrupts if there are users left, or the
3ed4351a83ca05d Simona Vetter 2017-05-31  1550  	 * user wishes vblank interrupts to be enabled all the time.
3ed4351a83ca05d Simona Vetter 2017-05-31  1551  	 */
0d5040e406d2c44 Hamza Mahfooz 2024-07-25  1552  	if (atomic_read(&vblank->refcount) != 0 || !vblank->config.offdelay_ms)
5a4784f49b2dcff Sam Ravnborg  2020-05-23  1553  		drm_WARN_ON(dev, drm_vblank_enable(dev, pipe));
92cc68e35863c1c Lyude Paul    2020-07-20  1554  	spin_unlock_irq(&dev->vbl_lock);
3ed4351a83ca05d Simona Vetter 2017-05-31  1555  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()

[sunpeng.li]

From: Leo Li <sunpeng.li@amd.com>

Some drivers need to perform blocking operations prior to enabling
vblank interrupts. A display hardware spin-up from a low-power state
that requires synchronization with the rest of the driver via a mutex,
for example.

To support this, introduce a new drm_crtc_vblank_prepare() helper that
calls back into the driver -- if implemented -- for the driver to do
such preparation work.

In DRM core, call this helper before drm_vblank_get(). Drivers can
choose to call this if they implement the callback in the future.

Signed-off-by: Leo Li <sunpeng.li@amd.com>
---
 drivers/gpu/drm/drm_atomic_helper.c |  8 ++++
 drivers/gpu/drm/drm_fb_helper.c     |  4 ++
 drivers/gpu/drm/drm_plane.c         |  4 ++
 drivers/gpu/drm/drm_vblank.c        | 69 +++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_vblank_work.c   |  8 ++++
 include/drm/drm_crtc.h              | 27 +++++++++++
 include/drm/drm_vblank.h            |  1 +
 7 files changed, 121 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index ef56b474acf59..e52dd41f83117 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1264,6 +1264,10 @@ crtc_disable(struct drm_device *dev, struct drm_atomic_state *state)
 		if (!drm_dev_has_vblank(dev))
 			continue;
 
+		ret = drm_crtc_vblank_prepare(crtc);
+		if (ret)
+			continue;
+
 		ret = drm_crtc_vblank_get(crtc);
 		/*
 		 * Self-refresh is not a true "disable"; ensure vblank remains
@@ -1815,6 +1819,10 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
 		if (!new_crtc_state->active)
 			continue;
 
+		ret = drm_crtc_vblank_prepare(crtc);
+		if (ret != 0)
+			continue;
+
 		ret = drm_crtc_vblank_get(crtc);
 		if (ret != 0)
 			continue;
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 11a5b60cb9ce4..7400942fd7d1d 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1103,6 +1103,10 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 		 * enabled, otherwise just don't do anythintg,
 		 * not even report an error.
 		 */
+		ret = drm_crtc_vblank_prepare(crtc);
+		if (ret)
+			break;
+
 		ret = drm_crtc_vblank_get(crtc);
 		if (!ret) {
 			drm_crtc_wait_one_vblank(crtc);
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 38f82391bfda5..f2e40eaa385e6 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -1421,6 +1421,10 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 		u32 current_vblank;
 		int r;
 
+		r = drm_crtc_vblank_prepare(crtc);
+		if (r)
+			return r;
+
 		r = drm_crtc_vblank_get(crtc);
 		if (r)
 			return r;
diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 46f59883183d9..4dac3228c021f 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1194,6 +1194,30 @@ static int drm_vblank_enable(struct drm_device *dev, unsigned int pipe)
 	return ret;
 }
 
+/**
+ * drm_crtc_vblank_prepare - prepare to enable vblank interrupts
+ *
+ * @crtc: which CRTC to prepare
+ *
+ * Some drivers may need to run blocking operations to prepare for enabling
+ * vblank interrupts. This function calls the prepare_enable_vblank callback, if
+ * available, to allow drivers to do that.
+ *
+ * The spin-up may call blocking functions, such as mutex_lock(). Therefore,
+ * this must be called from process context, where sleeping is allowed.
+ *
+ * Also see &drm_crtc_funcs.prepare_enable_vblank.
+ *
+ * Returns: Zero on success or a negative error code on failure.
+ */
+int drm_crtc_vblank_prepare(struct drm_crtc *crtc)
+{
+	if (crtc->funcs->prepare_enable_vblank)
+		return crtc->funcs->prepare_enable_vblank(crtc);
+	return 0;
+}
+EXPORT_SYMBOL(drm_crtc_vblank_prepare);
+
 int drm_vblank_get(struct drm_device *dev, unsigned int pipe)
 {
 	struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
@@ -1288,12 +1312,22 @@ EXPORT_SYMBOL(drm_crtc_vblank_put);
 void drm_wait_one_vblank(struct drm_device *dev, unsigned int pipe)
 {
 	struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
+	struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
 	int ret;
 	u64 last;
 
 	if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
 		return;
 
+	crtc = drm_crtc_from_index(dev, pipe);
+	if (crtc) {
+		ret = drm_crtc_vblank_prepare(crtc);
+		if (drm_WARN(dev, ret,
+			     "prepare vblank failed on crtc %i, ret=%i\n",
+			     pipe, ret))
+			return;
+	}
+
 	ret = drm_vblank_get(dev, pipe);
 	if (drm_WARN(dev, ret, "vblank not available on crtc %i, ret=%i\n",
 		     pipe, ret))
@@ -1485,10 +1519,18 @@ void drm_crtc_vblank_on_config(struct drm_crtc *crtc,
 	struct drm_device *dev = crtc->dev;
 	unsigned int pipe = drm_crtc_index(crtc);
 	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
+	int ret;
 
 	if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
 		return;
 
+	if (crtc) {
+		ret = drm_crtc_vblank_prepare(crtc);
+		drm_WARN_ON(dev, ret);
+		if (ret)
+			return;
+	}
+
 	spin_lock_irq(&dev->vbl_lock);
 	drm_dbg_vbl(dev, "crtc %d, vblank enabled %d, inmodeset %d\n",
 		    pipe, vblank->enabled, vblank->inmodeset);
@@ -1796,6 +1838,17 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
 		return 0;
 	}
 
+	crtc = drm_crtc_from_index(dev, vblank->pipe);
+	if (crtc) {
+		ret = drm_crtc_vblank_prepare(crtc);
+		if (ret) {
+			drm_dbg_core(dev,
+				     "prepare vblank failed on crtc %i, ret=%i\n",
+				     pipe, ret);
+			return ret;
+		}
+	}
+
 	ret = drm_vblank_get(dev, pipe);
 	if (ret) {
 		drm_dbg_core(dev,
@@ -2031,6 +2084,14 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data,
 		READ_ONCE(vblank->enabled);
 
 	if (!vblank_enabled) {
+		ret = drm_crtc_vblank_prepare(crtc);
+		if (ret) {
+			drm_dbg_core(dev,
+				     "prepare vblank failed on crtc %i, ret=%i\n",
+				     pipe, ret);
+			return ret;
+		}
+
 		ret = drm_crtc_vblank_get(crtc);
 		if (ret) {
 			drm_dbg_core(dev,
@@ -2098,6 +2159,14 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
 	if (e == NULL)
 		return -ENOMEM;
 
+	ret = drm_crtc_vblank_prepare(crtc);
+	if (ret) {
+		drm_dbg_core(dev,
+			     "prepare vblank failed on crtc %i, ret=%i\n",
+			     pipe, ret);
+		return ret;
+	}
+
 	ret = drm_crtc_vblank_get(crtc);
 	if (ret) {
 		drm_dbg_core(dev,
diff --git a/drivers/gpu/drm/drm_vblank_work.c b/drivers/gpu/drm/drm_vblank_work.c
index e4e1873f0e1e1..582ee7fd94adf 100644
--- a/drivers/gpu/drm/drm_vblank_work.c
+++ b/drivers/gpu/drm/drm_vblank_work.c
@@ -113,11 +113,19 @@ int drm_vblank_work_schedule(struct drm_vblank_work *work,
 {
 	struct drm_vblank_crtc *vblank = work->vblank;
 	struct drm_device *dev = vblank->dev;
+	struct drm_crtc *crtc;
 	u64 cur_vbl;
 	unsigned long irqflags;
 	bool passed, inmodeset, rescheduling = false, wake = false;
 	int ret = 0;
 
+	crtc = drm_crtc_from_index(dev, vblank->pipe);
+	if (crtc) {
+		ret = drm_crtc_vblank_prepare(crtc);
+		if (ret)
+			return ret;
+	}
+
 	spin_lock_irqsave(&dev->event_lock, irqflags);
 	if (work->cancelling)
 		goto out;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index caa56e039da2a..456cf9ba0143a 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -860,6 +860,33 @@ struct drm_crtc_funcs {
 	 */
 	u32 (*get_vblank_counter)(struct drm_crtc *crtc);
 
+	/**
+	 * @prepare_enable_vblank:
+	 *
+	 * An optional callback for preparing to enable vblank interrupts. It
+	 * allows drivers to perform blocking operations, and thus is called
+	 * without any vblank spinlocks. Consequently, this callback is not
+	 * synchronized with the rest of drm_vblank management; drivers are
+	 * responsible for ensuring it won't race with drm_vblank and it's other
+	 * driver callbacks.
+	 *
+	 * For example, drivers may use this to spin-up hardware from a low
+	 * power state -- which may require blocking operations -- such that
+	 * hardware registers are available to read/write. However, the driver
+	 * must be careful as to when to reenter low-power state, such that it
+	 * won't race with enable_vblank.
+	 *
+	 * It is called unconditionally, regardless of whether vblank interrupts
+	 * are already enabled or not.
+	 *
+	 * This callback is optional. If not set, no preparation is performed.
+	 *
+	 * Returns:
+	 *
+	 * Zero on success, negative errno on failure.
+	 */
+	int (*prepare_enable_vblank)(struct drm_crtc *crtc);
+
 	/**
 	 * @enable_vblank:
 	 *
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index 151ab1e85b1b7..5abc367aa4376 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -272,6 +272,7 @@ void drm_vblank_set_event(struct drm_pending_vblank_event *e,
 			  ktime_t *now);
 bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
 bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
+int drm_crtc_vblank_prepare(struct drm_crtc *crtc);
 int drm_crtc_vblank_get(struct drm_crtc *crtc);
 void drm_crtc_vblank_put(struct drm_crtc *crtc);
 void drm_wait_one_vblank(struct drm_device *dev, unsigned int pipe);
-- 
2.51.0

Re: [PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()

[Harry Wentland]

On 2025-12-01 18:18, sunpeng.li@amd.com wrote:
> From: Leo Li <sunpeng.li@amd.com>
> 
> Some drivers need to perform blocking operations prior to enabling
> vblank interrupts. A display hardware spin-up from a low-power state
> that requires synchronization with the rest of the driver via a mutex,
> for example.
> 
> To support this, introduce a new drm_crtc_vblank_prepare() helper that
> calls back into the driver -- if implemented -- for the driver to do
> such preparation work.
> 
> In DRM core, call this helper before drm_vblank_get(). Drivers can
> choose to call this if they implement the callback in the future.
> 
> Signed-off-by: Leo Li <sunpeng.li@amd.com>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  drivers/gpu/drm/drm_atomic_helper.c |  8 ++++
>  drivers/gpu/drm/drm_fb_helper.c     |  4 ++
>  drivers/gpu/drm/drm_plane.c         |  4 ++
>  drivers/gpu/drm/drm_vblank.c        | 69 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_vblank_work.c   |  8 ++++
>  include/drm/drm_crtc.h              | 27 +++++++++++
>  include/drm/drm_vblank.h            |  1 +
>  7 files changed, 121 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ef56b474acf59..e52dd41f83117 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1264,6 +1264,10 @@ crtc_disable(struct drm_device *dev, struct drm_atomic_state *state)
>  		if (!drm_dev_has_vblank(dev))
>  			continue;
>  
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (ret)
> +			continue;
> +
>  		ret = drm_crtc_vblank_get(crtc);
>  		/*
>  		 * Self-refresh is not a true "disable"; ensure vblank remains
> @@ -1815,6 +1819,10 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
>  		if (!new_crtc_state->active)
>  			continue;
>  
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (ret != 0)
> +			continue;
> +
>  		ret = drm_crtc_vblank_get(crtc);
>  		if (ret != 0)
>  			continue;
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 11a5b60cb9ce4..7400942fd7d1d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1103,6 +1103,10 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
>  		 * enabled, otherwise just don't do anythintg,
>  		 * not even report an error.
>  		 */
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (ret)
> +			break;
> +
>  		ret = drm_crtc_vblank_get(crtc);
>  		if (!ret) {
>  			drm_crtc_wait_one_vblank(crtc);
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 38f82391bfda5..f2e40eaa385e6 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -1421,6 +1421,10 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>  		u32 current_vblank;
>  		int r;
>  
> +		r = drm_crtc_vblank_prepare(crtc);
> +		if (r)
> +			return r;
> +
>  		r = drm_crtc_vblank_get(crtc);
>  		if (r)
>  			return r;
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 46f59883183d9..4dac3228c021f 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1194,6 +1194,30 @@ static int drm_vblank_enable(struct drm_device *dev, unsigned int pipe)
>  	return ret;
>  }
>  
> +/**
> + * drm_crtc_vblank_prepare - prepare to enable vblank interrupts
> + *
> + * @crtc: which CRTC to prepare
> + *
> + * Some drivers may need to run blocking operations to prepare for enabling
> + * vblank interrupts. This function calls the prepare_enable_vblank callback, if
> + * available, to allow drivers to do that.
> + *
> + * The spin-up may call blocking functions, such as mutex_lock(). Therefore,
> + * this must be called from process context, where sleeping is allowed.
> + *
> + * Also see &drm_crtc_funcs.prepare_enable_vblank.
> + *
> + * Returns: Zero on success or a negative error code on failure.
> + */
> +int drm_crtc_vblank_prepare(struct drm_crtc *crtc)
> +{
> +	if (crtc->funcs->prepare_enable_vblank)
> +		return crtc->funcs->prepare_enable_vblank(crtc);
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_crtc_vblank_prepare);
> +
>  int drm_vblank_get(struct drm_device *dev, unsigned int pipe)
>  {
>  	struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
> @@ -1288,12 +1312,22 @@ EXPORT_SYMBOL(drm_crtc_vblank_put);
>  void drm_wait_one_vblank(struct drm_device *dev, unsigned int pipe)
>  {
>  	struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
> +	struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
>  	int ret;
>  	u64 last;
>  
>  	if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
>  		return;
>  
> +	crtc = drm_crtc_from_index(dev, pipe);
> +	if (crtc) {
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (drm_WARN(dev, ret,
> +			     "prepare vblank failed on crtc %i, ret=%i\n",
> +			     pipe, ret))
> +			return;
> +	}
> +
>  	ret = drm_vblank_get(dev, pipe);
>  	if (drm_WARN(dev, ret, "vblank not available on crtc %i, ret=%i\n",
>  		     pipe, ret))
> @@ -1485,10 +1519,18 @@ void drm_crtc_vblank_on_config(struct drm_crtc *crtc,
>  	struct drm_device *dev = crtc->dev;
>  	unsigned int pipe = drm_crtc_index(crtc);
>  	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
> +	int ret;
>  
>  	if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
>  		return;
>  
> +	if (crtc) {
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		drm_WARN_ON(dev, ret);
> +		if (ret)
> +			return;
> +	}
> +
>  	spin_lock_irq(&dev->vbl_lock);
>  	drm_dbg_vbl(dev, "crtc %d, vblank enabled %d, inmodeset %d\n",
>  		    pipe, vblank->enabled, vblank->inmodeset);
> @@ -1796,6 +1838,17 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
>  		return 0;
>  	}
>  
> +	crtc = drm_crtc_from_index(dev, vblank->pipe);
> +	if (crtc) {
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (ret) {
> +			drm_dbg_core(dev,
> +				     "prepare vblank failed on crtc %i, ret=%i\n",
> +				     pipe, ret);
> +			return ret;
> +		}
> +	}
> +
>  	ret = drm_vblank_get(dev, pipe);
>  	if (ret) {
>  		drm_dbg_core(dev,
> @@ -2031,6 +2084,14 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data,
>  		READ_ONCE(vblank->enabled);
>  
>  	if (!vblank_enabled) {
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (ret) {
> +			drm_dbg_core(dev,
> +				     "prepare vblank failed on crtc %i, ret=%i\n",
> +				     pipe, ret);
> +			return ret;
> +		}
> +
>  		ret = drm_crtc_vblank_get(crtc);
>  		if (ret) {
>  			drm_dbg_core(dev,
> @@ -2098,6 +2159,14 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
>  	if (e == NULL)
>  		return -ENOMEM;
>  
> +	ret = drm_crtc_vblank_prepare(crtc);
> +	if (ret) {
> +		drm_dbg_core(dev,
> +			     "prepare vblank failed on crtc %i, ret=%i\n",
> +			     pipe, ret);
> +		return ret;
> +	}
> +
>  	ret = drm_crtc_vblank_get(crtc);
>  	if (ret) {
>  		drm_dbg_core(dev,
> diff --git a/drivers/gpu/drm/drm_vblank_work.c b/drivers/gpu/drm/drm_vblank_work.c
> index e4e1873f0e1e1..582ee7fd94adf 100644
> --- a/drivers/gpu/drm/drm_vblank_work.c
> +++ b/drivers/gpu/drm/drm_vblank_work.c
> @@ -113,11 +113,19 @@ int drm_vblank_work_schedule(struct drm_vblank_work *work,
>  {
>  	struct drm_vblank_crtc *vblank = work->vblank;
>  	struct drm_device *dev = vblank->dev;
> +	struct drm_crtc *crtc;
>  	u64 cur_vbl;
>  	unsigned long irqflags;
>  	bool passed, inmodeset, rescheduling = false, wake = false;
>  	int ret = 0;
>  
> +	crtc = drm_crtc_from_index(dev, vblank->pipe);
> +	if (crtc) {
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	spin_lock_irqsave(&dev->event_lock, irqflags);
>  	if (work->cancelling)
>  		goto out;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index caa56e039da2a..456cf9ba0143a 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -860,6 +860,33 @@ struct drm_crtc_funcs {
>  	 */
>  	u32 (*get_vblank_counter)(struct drm_crtc *crtc);
>  
> +	/**
> +	 * @prepare_enable_vblank:
> +	 *
> +	 * An optional callback for preparing to enable vblank interrupts. It
> +	 * allows drivers to perform blocking operations, and thus is called
> +	 * without any vblank spinlocks. Consequently, this callback is not
> +	 * synchronized with the rest of drm_vblank management; drivers are
> +	 * responsible for ensuring it won't race with drm_vblank and it's other
> +	 * driver callbacks.
> +	 *
> +	 * For example, drivers may use this to spin-up hardware from a low
> +	 * power state -- which may require blocking operations -- such that
> +	 * hardware registers are available to read/write. However, the driver
> +	 * must be careful as to when to reenter low-power state, such that it
> +	 * won't race with enable_vblank.
> +	 *
> +	 * It is called unconditionally, regardless of whether vblank interrupts
> +	 * are already enabled or not.
> +	 *
> +	 * This callback is optional. If not set, no preparation is performed.
> +	 *
> +	 * Returns:
> +	 *
> +	 * Zero on success, negative errno on failure.
> +	 */
> +	int (*prepare_enable_vblank)(struct drm_crtc *crtc);
> +
>  	/**
>  	 * @enable_vblank:
>  	 *
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index 151ab1e85b1b7..5abc367aa4376 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -272,6 +272,7 @@ void drm_vblank_set_event(struct drm_pending_vblank_event *e,
>  			  ktime_t *now);
>  bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
>  bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
> +int drm_crtc_vblank_prepare(struct drm_crtc *crtc);
>  int drm_crtc_vblank_get(struct drm_crtc *crtc);
>  void drm_crtc_vblank_put(struct drm_crtc *crtc);
>  void drm_wait_one_vblank(struct drm_device *dev, unsigned int pipe);

Re: [PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()

[Jani Nikula]

On Mon, 01 Dec 2025, <sunpeng.li@amd.com> wrote:
> From: Leo Li <sunpeng.li@amd.com>
>
> Some drivers need to perform blocking operations prior to enabling
> vblank interrupts. A display hardware spin-up from a low-power state
> that requires synchronization with the rest of the driver via a mutex,
> for example.
>
> To support this, introduce a new drm_crtc_vblank_prepare() helper that
> calls back into the driver -- if implemented -- for the driver to do
> such preparation work.
>
> In DRM core, call this helper before drm_vblank_get(). Drivers can
> choose to call this if they implement the callback in the future.

Have you considered hiding all of this inside drm_vblank.c? Call prepare
in drm_crtc_vblank_get() and a couple of other places? And actually
don't call it on !drm_dev_has_vblank(dev)?

There's just so much littering all over the place with the prepare, and
it seems brittle. Especially when you expect not only the drm core but
also the relevant drivers to call drm_crtc_vblank_prepare() when needed.

There does seem to be a few places in amdgpu that wrap the
drm_crtc_vblank_get() inside dev->event_lock, but is there really any
need to do so? Do the get first, and grab the event_lock after?

Some random comments inline.

Cc: Ville


BR,
Jani.


>
> Signed-off-by: Leo Li <sunpeng.li@amd.com>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c |  8 ++++
>  drivers/gpu/drm/drm_fb_helper.c     |  4 ++
>  drivers/gpu/drm/drm_plane.c         |  4 ++
>  drivers/gpu/drm/drm_vblank.c        | 69 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_vblank_work.c   |  8 ++++
>  include/drm/drm_crtc.h              | 27 +++++++++++
>  include/drm/drm_vblank.h            |  1 +
>  7 files changed, 121 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ef56b474acf59..e52dd41f83117 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1264,6 +1264,10 @@ crtc_disable(struct drm_device *dev, struct drm_atomic_state *state)
>  		if (!drm_dev_has_vblank(dev))
>  			continue;
>  
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (ret)
> +			continue;
> +
>  		ret = drm_crtc_vblank_get(crtc);
>  		/*
>  		 * Self-refresh is not a true "disable"; ensure vblank remains
> @@ -1815,6 +1819,10 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
>  		if (!new_crtc_state->active)
>  			continue;
>  
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (ret != 0)
> +			continue;
> +
>  		ret = drm_crtc_vblank_get(crtc);
>  		if (ret != 0)
>  			continue;
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 11a5b60cb9ce4..7400942fd7d1d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1103,6 +1103,10 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
>  		 * enabled, otherwise just don't do anythintg,
>  		 * not even report an error.
>  		 */
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (ret)
> +			break;
> +
>  		ret = drm_crtc_vblank_get(crtc);
>  		if (!ret) {
>  			drm_crtc_wait_one_vblank(crtc);
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 38f82391bfda5..f2e40eaa385e6 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -1421,6 +1421,10 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>  		u32 current_vblank;
>  		int r;
>  
> +		r = drm_crtc_vblank_prepare(crtc);
> +		if (r)
> +			return r;
> +
>  		r = drm_crtc_vblank_get(crtc);
>  		if (r)
>  			return r;
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 46f59883183d9..4dac3228c021f 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1194,6 +1194,30 @@ static int drm_vblank_enable(struct drm_device *dev, unsigned int pipe)
>  	return ret;
>  }
>  
> +/**
> + * drm_crtc_vblank_prepare - prepare to enable vblank interrupts
> + *
> + * @crtc: which CRTC to prepare
> + *
> + * Some drivers may need to run blocking operations to prepare for enabling
> + * vblank interrupts. This function calls the prepare_enable_vblank callback, if
> + * available, to allow drivers to do that.
> + *
> + * The spin-up may call blocking functions, such as mutex_lock(). Therefore,
> + * this must be called from process context, where sleeping is allowed.
> + *
> + * Also see &drm_crtc_funcs.prepare_enable_vblank.
> + *
> + * Returns: Zero on success or a negative error code on failure.
> + */
> +int drm_crtc_vblank_prepare(struct drm_crtc *crtc)
> +{
> +	if (crtc->funcs->prepare_enable_vblank)
> +		return crtc->funcs->prepare_enable_vblank(crtc);
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_crtc_vblank_prepare);
> +
>  int drm_vblank_get(struct drm_device *dev, unsigned int pipe)
>  {
>  	struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
> @@ -1288,12 +1312,22 @@ EXPORT_SYMBOL(drm_crtc_vblank_put);
>  void drm_wait_one_vblank(struct drm_device *dev, unsigned int pipe)
>  {
>  	struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
> +	struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);

Initialization...

>  	int ret;
>  	u64 last;
>  
>  	if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
>  		return;
>  
> +	crtc = drm_crtc_from_index(dev, pipe);

...and another initialization.

But really, this function needs to die, and you'll have the crtc without
looking it up [1]. I'd really love to land that first to not make that
*and* this series harder for absolutely no reason.

[1] https://lore.kernel.org/r/2a17538a24f1d12c3c82d9cde03363195b64d0cf.1764933891.git.jani.nikula@intel.com

> +	if (crtc) {
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (drm_WARN(dev, ret,
> +			     "prepare vblank failed on crtc %i, ret=%i\n",
> +			     pipe, ret))

Do we really need the warning backtraces or debug logs for every call?
There's one driver that needs the call, and it always returns 0. And
there's like a hundred lines of debug logging in this patch.

If you insist, please at least use the [CRTC:%d:%s] style logging, and
make it all somehow consistent.

> +			return;
> +	}
> +
>  	ret = drm_vblank_get(dev, pipe);
>  	if (drm_WARN(dev, ret, "vblank not available on crtc %i, ret=%i\n",
>  		     pipe, ret))
> @@ -1485,10 +1519,18 @@ void drm_crtc_vblank_on_config(struct drm_crtc *crtc,
>  	struct drm_device *dev = crtc->dev;
>  	unsigned int pipe = drm_crtc_index(crtc);
>  	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
> +	int ret;
>  
>  	if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
>  		return;
>  
> +	if (crtc) {
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		drm_WARN_ON(dev, ret);
> +		if (ret)
> +			return;
> +	}
> +
>  	spin_lock_irq(&dev->vbl_lock);
>  	drm_dbg_vbl(dev, "crtc %d, vblank enabled %d, inmodeset %d\n",
>  		    pipe, vblank->enabled, vblank->inmodeset);
> @@ -1796,6 +1838,17 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
>  		return 0;
>  	}
>  
> +	crtc = drm_crtc_from_index(dev, vblank->pipe);
> +	if (crtc) {
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (ret) {
> +			drm_dbg_core(dev,
> +				     "prepare vblank failed on crtc %i, ret=%i\n",
> +				     pipe, ret);
> +			return ret;
> +		}
> +	}
> +
>  	ret = drm_vblank_get(dev, pipe);
>  	if (ret) {
>  		drm_dbg_core(dev,
> @@ -2031,6 +2084,14 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data,
>  		READ_ONCE(vblank->enabled);
>  
>  	if (!vblank_enabled) {
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (ret) {
> +			drm_dbg_core(dev,
> +				     "prepare vblank failed on crtc %i, ret=%i\n",
> +				     pipe, ret);
> +			return ret;
> +		}
> +
>  		ret = drm_crtc_vblank_get(crtc);
>  		if (ret) {
>  			drm_dbg_core(dev,
> @@ -2098,6 +2159,14 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
>  	if (e == NULL)
>  		return -ENOMEM;
>  
> +	ret = drm_crtc_vblank_prepare(crtc);
> +	if (ret) {
> +		drm_dbg_core(dev,
> +			     "prepare vblank failed on crtc %i, ret=%i\n",
> +			     pipe, ret);
> +		return ret;
> +	}
> +
>  	ret = drm_crtc_vblank_get(crtc);
>  	if (ret) {
>  		drm_dbg_core(dev,
> diff --git a/drivers/gpu/drm/drm_vblank_work.c b/drivers/gpu/drm/drm_vblank_work.c
> index e4e1873f0e1e1..582ee7fd94adf 100644
> --- a/drivers/gpu/drm/drm_vblank_work.c
> +++ b/drivers/gpu/drm/drm_vblank_work.c
> @@ -113,11 +113,19 @@ int drm_vblank_work_schedule(struct drm_vblank_work *work,
>  {
>  	struct drm_vblank_crtc *vblank = work->vblank;
>  	struct drm_device *dev = vblank->dev;
> +	struct drm_crtc *crtc;
>  	u64 cur_vbl;
>  	unsigned long irqflags;
>  	bool passed, inmodeset, rescheduling = false, wake = false;
>  	int ret = 0;
>  
> +	crtc = drm_crtc_from_index(dev, vblank->pipe);
> +	if (crtc) {
> +		ret = drm_crtc_vblank_prepare(crtc);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	spin_lock_irqsave(&dev->event_lock, irqflags);
>  	if (work->cancelling)
>  		goto out;
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index caa56e039da2a..456cf9ba0143a 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -860,6 +860,33 @@ struct drm_crtc_funcs {
>  	 */
>  	u32 (*get_vblank_counter)(struct drm_crtc *crtc);
>  
> +	/**
> +	 * @prepare_enable_vblank:
> +	 *
> +	 * An optional callback for preparing to enable vblank interrupts. It
> +	 * allows drivers to perform blocking operations, and thus is called
> +	 * without any vblank spinlocks. Consequently, this callback is not
> +	 * synchronized with the rest of drm_vblank management; drivers are
> +	 * responsible for ensuring it won't race with drm_vblank and it's other
> +	 * driver callbacks.
> +	 *
> +	 * For example, drivers may use this to spin-up hardware from a low
> +	 * power state -- which may require blocking operations -- such that
> +	 * hardware registers are available to read/write. However, the driver
> +	 * must be careful as to when to reenter low-power state, such that it
> +	 * won't race with enable_vblank.
> +	 *
> +	 * It is called unconditionally, regardless of whether vblank interrupts
> +	 * are already enabled or not.
> +	 *
> +	 * This callback is optional. If not set, no preparation is performed.
> +	 *
> +	 * Returns:
> +	 *
> +	 * Zero on success, negative errno on failure.
> +	 */
> +	int (*prepare_enable_vblank)(struct drm_crtc *crtc);
> +
>  	/**
>  	 * @enable_vblank:
>  	 *
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index 151ab1e85b1b7..5abc367aa4376 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -272,6 +272,7 @@ void drm_vblank_set_event(struct drm_pending_vblank_event *e,
>  			  ktime_t *now);
>  bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
>  bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
> +int drm_crtc_vblank_prepare(struct drm_crtc *crtc);
>  int drm_crtc_vblank_get(struct drm_crtc *crtc);
>  void drm_crtc_vblank_put(struct drm_crtc *crtc);
>  void drm_wait_one_vblank(struct drm_device *dev, unsigned int pipe);

-- 
Jani Nikula, Intel

Re: [PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()

[Ville Syrjälä]

On Tue, Dec 09, 2025 at 12:05:31PM +0200, Jani Nikula wrote:
> On Mon, 01 Dec 2025, <sunpeng.li@amd.com> wrote:
> > From: Leo Li <sunpeng.li@amd.com>
> >
> > Some drivers need to perform blocking operations prior to enabling
> > vblank interrupts. A display hardware spin-up from a low-power state
> > that requires synchronization with the rest of the driver via a mutex,
> > for example.
> >
> > To support this, introduce a new drm_crtc_vblank_prepare() helper that
> > calls back into the driver -- if implemented -- for the driver to do
> > such preparation work.
> >
> > In DRM core, call this helper before drm_vblank_get(). Drivers can
> > choose to call this if they implement the callback in the future.
> 
> Have you considered hiding all of this inside drm_vblank.c? Call prepare
> in drm_crtc_vblank_get() and a couple of other places? And actually
> don't call it on !drm_dev_has_vblank(dev)?
> 
> There's just so much littering all over the place with the prepare, and
> it seems brittle. Especially when you expect not only the drm core but
> also the relevant drivers to call drm_crtc_vblank_prepare() when needed.
> 
> There does seem to be a few places in amdgpu that wrap the
> drm_crtc_vblank_get() inside dev->event_lock, but is there really any
> need to do so? Do the get first, and grab the event_lock after?
> 
> Some random comments inline.
> 
> Cc: Ville

drm_vblank_get() can get called from any kind of context.
The only workable solution might be the schedule a work from
.vblank_enable() and do whatever is needed from there.

-- 
Ville Syrjälä
Intel

Re: [PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()

[Leo Li]

On 2025-12-09 05:47, Ville Syrjälä wrote:
> On Tue, Dec 09, 2025 at 12:05:31PM +0200, Jani Nikula wrote:
>> On Mon, 01 Dec 2025, <sunpeng.li@amd.com> wrote:
>>> From: Leo Li <sunpeng.li@amd.com>
>>>
>>> Some drivers need to perform blocking operations prior to enabling
>>> vblank interrupts. A display hardware spin-up from a low-power state
>>> that requires synchronization with the rest of the driver via a mutex,
>>> for example.
>>>
>>> To support this, introduce a new drm_crtc_vblank_prepare() helper that
>>> calls back into the driver -- if implemented -- for the driver to do
>>> such preparation work.
>>>
>>> In DRM core, call this helper before drm_vblank_get(). Drivers can
>>> choose to call this if they implement the callback in the future.
>>
>> Have you considered hiding all of this inside drm_vblank.c? Call prepare
>> in drm_crtc_vblank_get() and a couple of other places? And actually
>> don't call it on !drm_dev_has_vblank(dev)?
>>
>> There's just so much littering all over the place with the prepare, and
>> it seems brittle. Especially when you expect not only the drm core but
>> also the relevant drivers to call drm_crtc_vblank_prepare() when needed.
>>
>> There does seem to be a few places in amdgpu that wrap the
>> drm_crtc_vblank_get() inside dev->event_lock, but is there really any
>> need to do so? Do the get first, and grab the event_lock after?
>>
>> Some random comments inline.
>>
>> Cc: Ville
> 
> drm_vblank_get() can get called from any kind of context.
> The only workable solution might be the schedule a work from
> .vblank_enable() and do whatever is needed from there.
> 
Yeah, drm_vblank_get() can be called from interrupt context, so the sleeping
work has to happen outside of it.

The issue with deferring work from .enable_vblank() is drm_vblank_enable()
follows up immediately with a drm_update_vblank_count(); it expects HW to be
online for reading the vblank counter. Meanwhile, the prepare work can be
pending, and waiting for HW to be online from drm_update_vblank_count() wouldn't
be ideal.

I've thought about implementing a sw vblank counter while HW is waking up, but
that sounds overly complex. It would be simpler to somehow signal prepare work
before we enter non-sleeping context.

Would a function like drm_crtc_get_vblank_with_prepare() help? vblank_prepare()
can be wrapped in it before calling get_vblank(). drm_crtc_get_vblank()
call-sites outside of drm_vblank.c -- called from process context -- can be
replaced with it.

The only case it's not called from process context (within drm core) is in
drm_crtc_vblank_on_config(), which is in drm_vblank.c itself. I think
vblank_prepare() can be open coded there. Drivers can choose to call
get_vblank_with_prepare() if they actually have blocking prepare work.

Thanks,
Leo

Re: [PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()

[Leo Li]

On 2025-12-10 12:55, Leo Li wrote:
> 
> 
> On 2025-12-09 05:47, Ville Syrjälä wrote:
>> On Tue, Dec 09, 2025 at 12:05:31PM +0200, Jani Nikula wrote:
>>> On Mon, 01 Dec 2025, <sunpeng.li@amd.com> wrote:
>>>> From: Leo Li <sunpeng.li@amd.com>
>>>>
>>>> Some drivers need to perform blocking operations prior to enabling
>>>> vblank interrupts. A display hardware spin-up from a low-power state
>>>> that requires synchronization with the rest of the driver via a mutex,
>>>> for example.
>>>>
>>>> To support this, introduce a new drm_crtc_vblank_prepare() helper that
>>>> calls back into the driver -- if implemented -- for the driver to do
>>>> such preparation work.
>>>>
>>>> In DRM core, call this helper before drm_vblank_get(). Drivers can
>>>> choose to call this if they implement the callback in the future.
>>>
>>> Have you considered hiding all of this inside drm_vblank.c? Call prepare
>>> in drm_crtc_vblank_get() and a couple of other places? And actually
>>> don't call it on !drm_dev_has_vblank(dev)?
>>>
>>> There's just so much littering all over the place with the prepare, and
>>> it seems brittle. Especially when you expect not only the drm core but
>>> also the relevant drivers to call drm_crtc_vblank_prepare() when needed.
>>>
>>> There does seem to be a few places in amdgpu that wrap the
>>> drm_crtc_vblank_get() inside dev->event_lock, but is there really any
>>> need to do so? Do the get first, and grab the event_lock after?
>>>
>>> Some random comments inline.
>>>
>>> Cc: Ville
>>
>> drm_vblank_get() can get called from any kind of context.
>> The only workable solution might be the schedule a work from
>> .vblank_enable() and do whatever is needed from there.
>>
> Yeah, drm_vblank_get() can be called from interrupt context, so the sleeping
> work has to happen outside of it.
> 
> The issue with deferring work from .enable_vblank() is drm_vblank_enable()
> follows up immediately with a drm_update_vblank_count(); it expects HW to be
> online for reading the vblank counter. Meanwhile, the prepare work can be
> pending, and waiting for HW to be online from drm_update_vblank_count() wouldn't
> be ideal.
> 
> I've thought about implementing a sw vblank counter while HW is waking up, but
> that sounds overly complex. It would be simpler to somehow signal prepare work
> before we enter non-sleeping context.
> 
> Would a function like drm_crtc_get_vblank_with_prepare() help? vblank_prepare()
> can be wrapped in it before calling get_vblank(). drm_crtc_get_vblank()
> call-sites outside of drm_vblank.c -- called from process context -- can be
> replaced with it.
> 
> The only case it's not called from process context (within drm core) is in
> drm_crtc_vblank_on_config(), which is in drm_vblank.c itself. I think
> vblank_prepare() can be open coded there. Drivers can choose to call
> get_vblank_with_prepare() if they actually have blocking prepare work.

Sorry for the delay, just had some time to look at this again.

It seems I missed a place in my last comment about combining prepare() and get()
into one function. drm_vblank_work_schedule() in drm_vblank_work.c calls
vblank_get() with the event_lock, so an option to vblank_prepare() separately is
still needed.

I don't think we actually need to export drm_crtc_vblank_prepare() to drivers.
It's purpose is for DRM to run blocking driver work to prepare for vblank
programming, not the other way around. Drivers can sequence prepare work
themselves, if they need to, before doing any programming.

I think vblank_prepare() can be hidden in drm_internal.h, to limit it's use to
DRM core. I'll spin up a v2 rebased on Jani's series with this and some other
comments addressed.

Thanks,
Leo

> 
> Thanks,
> Leo
> 

Re: [PATCH v2 1/2] drm: Introduce drm_crtc_vblank_prepare()

[Leo Li]

On 2025-12-09 05:05, Jani Nikula wrote:
> On Mon, 01 Dec 2025, <sunpeng.li@amd.com> wrote:
>> From: Leo Li <sunpeng.li@amd.com>
>>
>> Some drivers need to perform blocking operations prior to enabling
>> vblank interrupts. A display hardware spin-up from a low-power state
>> that requires synchronization with the rest of the driver via a mutex,
>> for example.
>>
>> To support this, introduce a new drm_crtc_vblank_prepare() helper that
>> calls back into the driver -- if implemented -- for the driver to do
>> such preparation work.
>>
>> In DRM core, call this helper before drm_vblank_get(). Drivers can
>> choose to call this if they implement the callback in the future.
> 
> Have you considered hiding all of this inside drm_vblank.c? Call prepare
> in drm_crtc_vblank_get() and a couple of other places? And actually
> don't call it on !drm_dev_has_vblank(dev)?
> 
> There's just so much littering all over the place with the prepare, and
> it seems brittle. Especially when you expect not only the drm core but
> also the relevant drivers to call drm_crtc_vblank_prepare() when needed.
> 
> There does seem to be a few places in amdgpu that wrap the
> drm_crtc_vblank_get() inside dev->event_lock, but is there really any
> need to do so? Do the get first, and grab the event_lock after?
> 
> Some random comments inline.
> 
> Cc: Ville
> 
> 
> BR,
> Jani.

Hi Jani,
Thanks for the feedback. I'll reply to the above in the other thread with
Ville, but addressing the in-lines below.
- Leo
> 
> 
>>
>> Signed-off-by: Leo Li <sunpeng.li@amd.com>
>> ---
>>  drivers/gpu/drm/drm_atomic_helper.c |  8 ++++
>>  drivers/gpu/drm/drm_fb_helper.c     |  4 ++
>>  drivers/gpu/drm/drm_plane.c         |  4 ++
>>  drivers/gpu/drm/drm_vblank.c        | 69 +++++++++++++++++++++++++++++
>>  drivers/gpu/drm/drm_vblank_work.c   |  8 ++++
>>  include/drm/drm_crtc.h              | 27 +++++++++++
>>  include/drm/drm_vblank.h            |  1 +
>>  7 files changed, 121 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index ef56b474acf59..e52dd41f83117 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -1264,6 +1264,10 @@ crtc_disable(struct drm_device *dev, struct drm_atomic_state *state)
>>  		if (!drm_dev_has_vblank(dev))
>>  			continue;
>>  
>> +		ret = drm_crtc_vblank_prepare(crtc);
>> +		if (ret)
>> +			continue;
>> +
>>  		ret = drm_crtc_vblank_get(crtc);
>>  		/*
>>  		 * Self-refresh is not a true "disable"; ensure vblank remains
>> @@ -1815,6 +1819,10 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device *dev,
>>  		if (!new_crtc_state->active)
>>  			continue;
>>  
>> +		ret = drm_crtc_vblank_prepare(crtc);
>> +		if (ret != 0)
>> +			continue;
>> +
>>  		ret = drm_crtc_vblank_get(crtc);
>>  		if (ret != 0)
>>  			continue;
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index 11a5b60cb9ce4..7400942fd7d1d 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -1103,6 +1103,10 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
>>  		 * enabled, otherwise just don't do anythintg,
>>  		 * not even report an error.
>>  		 */
>> +		ret = drm_crtc_vblank_prepare(crtc);
>> +		if (ret)
>> +			break;
>> +
>>  		ret = drm_crtc_vblank_get(crtc);
>>  		if (!ret) {
>>  			drm_crtc_wait_one_vblank(crtc);
>> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
>> index 38f82391bfda5..f2e40eaa385e6 100644
>> --- a/drivers/gpu/drm/drm_plane.c
>> +++ b/drivers/gpu/drm/drm_plane.c
>> @@ -1421,6 +1421,10 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>>  		u32 current_vblank;
>>  		int r;
>>  
>> +		r = drm_crtc_vblank_prepare(crtc);
>> +		if (r)
>> +			return r;
>> +
>>  		r = drm_crtc_vblank_get(crtc);
>>  		if (r)
>>  			return r;
>> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
>> index 46f59883183d9..4dac3228c021f 100644
>> --- a/drivers/gpu/drm/drm_vblank.c
>> +++ b/drivers/gpu/drm/drm_vblank.c
>> @@ -1194,6 +1194,30 @@ static int drm_vblank_enable(struct drm_device *dev, unsigned int pipe)
>>  	return ret;
>>  }
>>  
>> +/**
>> + * drm_crtc_vblank_prepare - prepare to enable vblank interrupts
>> + *
>> + * @crtc: which CRTC to prepare
>> + *
>> + * Some drivers may need to run blocking operations to prepare for enabling
>> + * vblank interrupts. This function calls the prepare_enable_vblank callback, if
>> + * available, to allow drivers to do that.
>> + *
>> + * The spin-up may call blocking functions, such as mutex_lock(). Therefore,
>> + * this must be called from process context, where sleeping is allowed.
>> + *
>> + * Also see &drm_crtc_funcs.prepare_enable_vblank.
>> + *
>> + * Returns: Zero on success or a negative error code on failure.
>> + */
>> +int drm_crtc_vblank_prepare(struct drm_crtc *crtc)
>> +{
>> +	if (crtc->funcs->prepare_enable_vblank)
>> +		return crtc->funcs->prepare_enable_vblank(crtc);
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(drm_crtc_vblank_prepare);
>> +
>>  int drm_vblank_get(struct drm_device *dev, unsigned int pipe)
>>  {
>>  	struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
>> @@ -1288,12 +1312,22 @@ EXPORT_SYMBOL(drm_crtc_vblank_put);
>>  void drm_wait_one_vblank(struct drm_device *dev, unsigned int pipe)
>>  {
>>  	struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
>> +	struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
> 
> Initialization...
> 
>>  	int ret;
>>  	u64 last;
>>  
>>  	if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
>>  		return;
>>  
>> +	crtc = drm_crtc_from_index(dev, pipe);
> 
> ...and another initialization.
> 
> But really, this function needs to die, and you'll have the crtc without
> looking it up [1]. I'd really love to land that first to not make that
> *and* this series harder for absolutely no reason.
> 
> [1] https://lore.kernel.org/r/2a17538a24f1d12c3c82d9cde03363195b64d0cf.1764933891.git.jani.nikula@intel.com
>

The series looks sensible. I can definitely rebase on them. 
>> +	if (crtc) {
>> +		ret = drm_crtc_vblank_prepare(crtc);
>> +		if (drm_WARN(dev, ret,
>> +			     "prepare vblank failed on crtc %i, ret=%i\n",
>> +			     pipe, ret))
> 
> Do we really need the warning backtraces or debug logs for every call?
> There's one driver that needs the call, and it always returns 0. And
> there's like a hundred lines of debug logging in this patch.
> 
> If you insist, please at least use the [CRTC:%d:%s] style logging, and
> make it all somehow consistent.
> 

I'm OK with dropping these. It may be better for drivers to print warnings
themselves since preparation work is vendor specific.
>> +			return;
>> +	}
>> +
>>  	ret = drm_vblank_get(dev, pipe);
>>  	if (drm_WARN(dev, ret, "vblank not available on crtc %i, ret=%i\n",
>>  		     pipe, ret))
>> @@ -1485,10 +1519,18 @@ void drm_crtc_vblank_on_config(struct drm_crtc *crtc,
>>  	struct drm_device *dev = crtc->dev;
>>  	unsigned int pipe = drm_crtc_index(crtc);
>>  	struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
>> +	int ret;
>>  
>>  	if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
>>  		return;
>>  
>> +	if (crtc) {
>> +		ret = drm_crtc_vblank_prepare(crtc);
>> +		drm_WARN_ON(dev, ret);
>> +		if (ret)
>> +			return;
>> +	}
>> +
>>  	spin_lock_irq(&dev->vbl_lock);
>>  	drm_dbg_vbl(dev, "crtc %d, vblank enabled %d, inmodeset %d\n",
>>  		    pipe, vblank->enabled, vblank->inmodeset);
>> @@ -1796,6 +1838,17 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
>>  		return 0;
>>  	}
>>  
>> +	crtc = drm_crtc_from_index(dev, vblank->pipe);
>> +	if (crtc) {
>> +		ret = drm_crtc_vblank_prepare(crtc);
>> +		if (ret) {
>> +			drm_dbg_core(dev,
>> +				     "prepare vblank failed on crtc %i, ret=%i\n",
>> +				     pipe, ret);
>> +			return ret;
>> +		}
>> +	}
>> +
>>  	ret = drm_vblank_get(dev, pipe);
>>  	if (ret) {
>>  		drm_dbg_core(dev,
>> @@ -2031,6 +2084,14 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data,
>>  		READ_ONCE(vblank->enabled);
>>  
>>  	if (!vblank_enabled) {
>> +		ret = drm_crtc_vblank_prepare(crtc);
>> +		if (ret) {
>> +			drm_dbg_core(dev,
>> +				     "prepare vblank failed on crtc %i, ret=%i\n",
>> +				     pipe, ret);
>> +			return ret;
>> +		}
>> +
>>  		ret = drm_crtc_vblank_get(crtc);
>>  		if (ret) {
>>  			drm_dbg_core(dev,
>> @@ -2098,6 +2159,14 @@ int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data,
>>  	if (e == NULL)
>>  		return -ENOMEM;
>>  
>> +	ret = drm_crtc_vblank_prepare(crtc);
>> +	if (ret) {
>> +		drm_dbg_core(dev,
>> +			     "prepare vblank failed on crtc %i, ret=%i\n",
>> +			     pipe, ret);
>> +		return ret;
>> +	}
>> +
>>  	ret = drm_crtc_vblank_get(crtc);
>>  	if (ret) {
>>  		drm_dbg_core(dev,
>> diff --git a/drivers/gpu/drm/drm_vblank_work.c b/drivers/gpu/drm/drm_vblank_work.c
>> index e4e1873f0e1e1..582ee7fd94adf 100644
>> --- a/drivers/gpu/drm/drm_vblank_work.c
>> +++ b/drivers/gpu/drm/drm_vblank_work.c
>> @@ -113,11 +113,19 @@ int drm_vblank_work_schedule(struct drm_vblank_work *work,
>>  {
>>  	struct drm_vblank_crtc *vblank = work->vblank;
>>  	struct drm_device *dev = vblank->dev;
>> +	struct drm_crtc *crtc;
>>  	u64 cur_vbl;
>>  	unsigned long irqflags;
>>  	bool passed, inmodeset, rescheduling = false, wake = false;
>>  	int ret = 0;
>>  
>> +	crtc = drm_crtc_from_index(dev, vblank->pipe);
>> +	if (crtc) {
>> +		ret = drm_crtc_vblank_prepare(crtc);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>>  	spin_lock_irqsave(&dev->event_lock, irqflags);
>>  	if (work->cancelling)
>>  		goto out;
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index caa56e039da2a..456cf9ba0143a 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -860,6 +860,33 @@ struct drm_crtc_funcs {
>>  	 */
>>  	u32 (*get_vblank_counter)(struct drm_crtc *crtc);
>>  
>> +	/**
>> +	 * @prepare_enable_vblank:
>> +	 *
>> +	 * An optional callback for preparing to enable vblank interrupts. It
>> +	 * allows drivers to perform blocking operations, and thus is called
>> +	 * without any vblank spinlocks. Consequently, this callback is not
>> +	 * synchronized with the rest of drm_vblank management; drivers are
>> +	 * responsible for ensuring it won't race with drm_vblank and it's other
>> +	 * driver callbacks.
>> +	 *
>> +	 * For example, drivers may use this to spin-up hardware from a low
>> +	 * power state -- which may require blocking operations -- such that
>> +	 * hardware registers are available to read/write. However, the driver
>> +	 * must be careful as to when to reenter low-power state, such that it
>> +	 * won't race with enable_vblank.
>> +	 *
>> +	 * It is called unconditionally, regardless of whether vblank interrupts
>> +	 * are already enabled or not.
>> +	 *
>> +	 * This callback is optional. If not set, no preparation is performed.
>> +	 *
>> +	 * Returns:
>> +	 *
>> +	 * Zero on success, negative errno on failure.
>> +	 */
>> +	int (*prepare_enable_vblank)(struct drm_crtc *crtc);
>> +
>>  	/**
>>  	 * @enable_vblank:
>>  	 *
>> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
>> index 151ab1e85b1b7..5abc367aa4376 100644
>> --- a/include/drm/drm_vblank.h
>> +++ b/include/drm/drm_vblank.h
>> @@ -272,6 +272,7 @@ void drm_vblank_set_event(struct drm_pending_vblank_event *e,
>>  			  ktime_t *now);
>>  bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe);
>>  bool drm_crtc_handle_vblank(struct drm_crtc *crtc);
>> +int drm_crtc_vblank_prepare(struct drm_crtc *crtc);
>>  int drm_crtc_vblank_get(struct drm_crtc *crtc);
>>  void drm_crtc_vblank_put(struct drm_crtc *crtc);
>>  void drm_wait_one_vblank(struct drm_device *dev, unsigned int pipe);
>