wait_event_timeout() usage in decon_wait_for_vblank()

wait_event_timeout() usage in decon_wait_for_vblank()

[Bjorn Helgaas]

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?

Bjorn

Re: wait_event_timeout() usage in decon_wait_for_vblank()

[Inki Dae]

Hi,

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.

Thanks,
Inki Dae

> Bjorn
>

Re: wait_event_timeout() usage in decon_wait_for_vblank()

[Bjorn Helgaas]

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