Re: wait_event_timeout() usage in decon_wait_for_vblank()

[Bjorn Helgaas <helgaas@kernel.org>]

On Sun, Feb 02, 2025 at 01:02:47PM +0900, Inki Dae wrote:
> 2025년 2월 1일 (토) 오전 1:56, Bjorn Helgaas <helgaas@kernel.org>님이 작성:
> >
> > I don't know this code at all, so this is likely just noise, but the
> > wait_event_timeout() usage in decon_wait_for_vblank() looks funny to
> > me.
> >
> > decon_wait_for_vblank() waits on wait_vsync_queue for wait_vsync_event
> > to be cleared.
> >
> > But decon_irq_handler() only clears wait_vsync_event and wakes up
> > wait_vsync_queue when !ctx->i80_if.
> >
> > I don't see any i80_if connection in decon_wait_for_vblank().  Does
> > the wait always time out if i80_if is set?  Is there some reason we
> > don't use decon_wait_for_vblank() at all if i80_if is set?
> 
> For the decon series, the implementation for the i80 interface has not
> yet been completed. If you would like to see how the i80 interface is
> handled in Exynos, please refer to the code for the legacy display
> controller module, exynos_drm_fimd.c, available at the following link:
> https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git/tree/drivers/gpu/drm/exynos/exynos_drm_fimd.c?h=exynos-drm-fixes#n1053
> 
> Furthermore, please note that within the decon driver, prior to
> creating the CRTC, it is necessary to implement the TE handler for the
> i80 interface in the te_handler of exynos_drm_crtc_ops and register it
> as a callback.

I'll take your word for it that this all works as intended.  For many
wait_event_timeout() callers, the connection between kicking off some
operation, waiting for completion, and doing the wakeup is a little
bit obscure and hard to analyze without deep knowledge of the driver.
This was just one instance of those.

Bjorn