On Mon, Nov 10, 2025 at 06:17:41PM +0200, Jani Nikula wrote:
> Now that drm_vblank_crtc() is the only place that indexes dev->vblank[],
> and its usage has reduced considerably, add the primary pipe
> out-of-bounds check there, and return NULL. Expect callers to check it
> and act accordingly.
>
> In drm_crtc_vblank_crtc(), warn and return NULL, and let it go boom. If
> the crtc->pipe is out of bounds, it's a driver error that needs to be
> fixed.
>
> Remove superfluous pipe checks all around.
>
> Signed-off-by: Jani Nikula <[email protected]>
> ---
> drivers/gpu/drm/drm_vblank.c | 36 +++++++++++++++---------------------
> 1 file changed, 15 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 44fb8d225485..7829e64e42b4 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -177,13 +177,22 @@ MODULE_PARM_DESC(timestamp_precision_usec, "Max. error
> on timestamps [usecs]");
> static struct drm_vblank_crtc *
> drm_vblank_crtc(struct drm_device *dev, unsigned int pipe)
> {
> + if (pipe >= dev->num_crtcs)
> + return NULL;
> +
> return &dev->vblank[pipe];
> }
>
> struct drm_vblank_crtc *
> drm_crtc_vblank_crtc(struct drm_crtc *crtc)
> {
> - return drm_vblank_crtc(crtc->dev, drm_crtc_index(crtc));
> + struct drm_vblank_crtc *vblank;
> +
> + vblank = drm_vblank_crtc(crtc->dev, drm_crtc_index(crtc));
> + if (drm_WARN_ON(crtc->dev, !vblank))
> + return NULL;
> +
> + return vblank;
> }
> EXPORT_SYMBOL(drm_crtc_vblank_crtc);
>
> @@ -631,7 +640,6 @@ void drm_calc_timestamping_constants(struct drm_crtc
> *crtc,
> const struct drm_display_mode *mode)
> {
> struct drm_device *dev = crtc->dev;
> - unsigned int pipe = drm_crtc_index(crtc);
> struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
> int linedur_ns = 0, framedur_ns = 0;
> int dotclock = mode->crtc_clock;
> @@ -639,9 +647,6 @@ void drm_calc_timestamping_constants(struct drm_crtc
> *crtc,
> if (!drm_dev_has_vblank(dev))
> return;
I belive this at least gets called from the atomic helpers even
for drivers that don't have vblank support. In which case the
drm_crtc_vblank_crtc() call would have to be done after the
drm_dev_has_vblank() check or else you'll get spurious WARNs.
I don't remember if there are other cases like this as well.
>
> - if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
> - return;
> -
> /* Valid dotclock? */
> if (dotclock > 0) {
> int frame_size = mode->crtc_htotal * mode->crtc_vtotal;
> @@ -724,11 +729,6 @@ drm_crtc_vblank_helper_get_vblank_timestamp_internal(
> int vpos, hpos, i;
> int delta_ns, duration_ns;
>
> - if (pipe >= dev->num_crtcs) {
> - drm_err(dev, "Invalid crtc %u\n", pipe);
> - return false;
> - }
> -
> /* Scanout position query not supported? Should not happen. */
> if (!get_scanout_position) {
> drm_err(dev, "Called from CRTC w/o get_scanout_position()!?\n");
> @@ -1339,9 +1339,6 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
> ktime_t now;
> u64 seq;
>
> - if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
> - return;
> -
> /*
> * Grab event_lock early to prevent vblank work from being scheduled
> * while we're in the middle of shutting down vblank interrupts
> @@ -1480,9 +1477,6 @@ void drm_crtc_vblank_on_config(struct drm_crtc *crtc,
> unsigned int pipe = drm_crtc_index(crtc);
> struct drm_vblank_crtc *vblank = drm_crtc_vblank_crtc(crtc);
>
> - if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
> - return;
> -
> spin_lock_irq(&dev->vbl_lock);
> drm_dbg_vbl(dev, "crtc %d, vblank enabled %d, inmodeset %d\n",
> pipe, vblank->enabled, vblank->inmodeset);
> @@ -1764,10 +1758,9 @@ int drm_wait_vblank_ioctl(struct drm_device *dev, void
> *data,
> pipe = pipe_index;
> }
>
> - if (pipe >= dev->num_crtcs)
> - return -EINVAL;
> -
> vblank = drm_vblank_crtc(dev, pipe);
> + if (!vblank)
> + return -EINVAL;
>
> /* If the counter is currently enabled and accurate, short-circuit
> * queries to return the cached timestamp of the last vblank.
> @@ -1902,14 +1895,15 @@ static void drm_handle_vblank_events(struct
> drm_vblank_crtc *vblank)
> */
> bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe)
> {
> - struct drm_vblank_crtc *vblank = drm_vblank_crtc(dev, pipe);
> + struct drm_vblank_crtc *vblank;
> unsigned long irqflags;
> bool disable_irq;
>
> if (drm_WARN_ON_ONCE(dev, !drm_dev_has_vblank(dev)))
> return false;
>
> - if (drm_WARN_ON(dev, pipe >= dev->num_crtcs))
> + vblank = drm_vblank_crtc(dev, pipe);
> + if (drm_WARN_ON(dev, !vblank))
> return false;
>
> spin_lock_irqsave(&dev->event_lock, irqflags);
> --
> 2.47.3
--
Ville Syrjälä
Intel
