On Mon, 10 Nov 2025, Thomas Zimmermann <[email protected]> wrote: > Hi > > Am 07.11.25 um 12:04 schrieb Jani Nikula: >> We have drm_crtc_vblank_waitqueue() to get the wait_queue_head_t pointer >> for a vblank. Use it instead of poking at dev->vblank[] directly. >> >> Due to the macro maze of wait_event_timeout() that uses the address-of >> operator on the argument, we have to pass it in with the indirection >> operator. >> >> Signed-off-by: Jani Nikula <[email protected]> > > Reviewed-by Thomas Zimmermann <[email protected]> > > But... drm_crtc_vblank_waitqueue() is a terrible interface IMHO, as it > exports internal details of the vblank implementation. > > I wonder if the existing users at [1] and [2] couldn't be replaced with > a common vblank helper. > > And there's drm_wait_one_vblank() [3] and the waiting that's being fixed > here [4]. The latter looks like [3] but with multiple CRTC waiting for > their next vblank. I'd say this could be a single implementation within > the vblank code.
I don't disagree, but getting that done is a bit more involved than what I have time for right now. Need to think. In the mean time, pushed the drm_crtc_vblank_crtc() related patches in the series, and left the drm_crtc_vblank_waitqueue() ones to simmer. Thanks for the reviews. BR, Jani. > > [1] > https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/i915/display/intel_display_rps.c#L73 > [2] > https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/i915/display/intel_vblank.c#L715 > [3] > https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/drm_vblank.c#L1304 > [4] > https://elixir.bootlin.com/linux/v6.18-rc4/source/drivers/gpu/drm/drm_atomic_helper.c#L1837 > > Best regards > Thomas > >> --- >> drivers/gpu/drm/drm_atomic_helper.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c >> b/drivers/gpu/drm/drm_atomic_helper.c >> index 5a473a274ff0..e641fcf8c568 100644 >> --- a/drivers/gpu/drm/drm_atomic_helper.c >> +++ b/drivers/gpu/drm/drm_atomic_helper.c >> @@ -1831,10 +1831,12 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device >> *dev, >> } >> >> for_each_old_crtc_in_state(state, crtc, old_crtc_state, i) { >> + wait_queue_head_t *queue = drm_crtc_vblank_waitqueue(crtc); >> + >> if (!(crtc_mask & drm_crtc_mask(crtc))) >> continue; >> >> - ret = wait_event_timeout(dev->vblank[i].queue, >> + ret = wait_event_timeout(*queue, >> state->crtcs[i].last_vblank_count != >> drm_crtc_vblank_count(crtc), >> msecs_to_jiffies(100)); -- Jani Nikula, Intel
