Le 27/09/24 - 16:39, Lyude Paul a écrit :
> Currently, there's nothing actually stopping a driver from only registering
> vblank support for some of it's CRTCs and not for others. As far as I can
> tell, this isn't really defined behavior on the C side of things - as the
> documentation explicitly mentions to not use drm_vblank_init() if you don't
> have vblank support - since DRM then steps in and adds its own vblank
> emulation implementation.
>
> So, let's fix this edge case and check to make sure it's all or none.
>
> Signed-off-by: Lyude Paul <ly...@redhat.com>
> Fixes: 3ed4351a83ca ("drm: Extract drm_vblank.[hc]")
> Cc: Stefan Agner <ste...@agner.ch>
> Cc: Daniel Vetter <daniel.vet...@intel.com>
> Cc: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Cc: Maxime Ripard <mrip...@kernel.org>
> Cc: Thomas Zimmermann <tzimmerm...@suse.de>
> Cc: David Airlie <airl...@gmail.com>
> Cc: Simona Vetter <sim...@ffwll.ch>
> Cc: dri-devel@lists.freedesktop.org
> Cc: <sta...@vger.kernel.org> # v4.13+
> ---
> drivers/gpu/drm/drm_vblank.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 94e45ed6869d0..4d00937e8ca2e 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -525,9 +525,19 @@ static void drm_vblank_init_release(struct drm_device
> *dev, void *ptr)
> */
> int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs)
> {
> + struct drm_crtc *crtc;
> int ret;
> unsigned int i;
>
> + // Confirm that the required vblank functions have been filled out for
> all CRTCS
> + drm_for_each_crtc(crtc, dev) {
> + if (!crtc->funcs->enable_vblank ||
> !crtc->funcs->disable_vblank) {
> + drm_err(dev, "CRTC vblank functions not initialized for
> %s, abort\n",
> + crtc->name);
> + return -EINVAL;
> + }
> + }
> +
Hi,
I noticed that the kernel bot reported an issue with VKMS and this patch.
I did not take the time to reproduce the issue, but it may come from the
fact that VKMS call drm_vblank_init before calling
drmm_crtc_init_with_planes [1]. I don't see anything in the documentation
that requires the CRTC to be initialized before the vblank, is it a change
of the API or an issue in VKMS since 2018 [2]?
Anyway, if this is a requirement, can you explain it in the
drm_vblank_init documentation?
Thanks,
Louis Chauvet
[1]:https://elixir.bootlin.com/linux/v6.11.2/source/drivers/gpu/drm/vkms/vkms_drv.c#L209
[2]:https://lore.kernel.org/all/5d9ca7b3884c1995bd4a983b1d2ff1b840eb7f1a.1531402095.git.rodrigosiqueiram...@gmail.com/
> spin_lock_init(&dev->vbl_lock);
> spin_lock_init(&dev->vblank_time_lock);
>
>
> base-commit: 22512c3ee0f47faab5def71c4453638923c62522
> --
> 2.46.1
>
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
