On 2025-12-01 18:18, [email protected] wrote:
> From: Leo Li <[email protected]>
>
> 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 <[email protected]>
Reviewed-by: Harry Wentland <[email protected]>
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);
