Re: [PATCH 3/5] drm/vblank: Remove num_crtcs argument from drm_vblank_init()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] drm_vblank_init() returns -ENODEV causing probe failures in multiple drivers
--

--- Patch [3]: [PATCH 3/5] drm/vblank: Remove num_crtcs argument from drm_vblank_init() ---
commit 049e1e142facf2bbdd92cd8a8847a53f6b18b3aa
Author: Lyude Paul <[email protected]>

drm/vblank: Remove num_crtcs argument from drm_vblank_init()

This commit removes the num_crtcs argument from drm_vblank_init() and
updates callers across the tree to rely on the internal DRM count of CRTCs
instead. The change was applied mechanically using a Coccinelle patch.

> diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> index d4577663a1f0e..ba78695d1015c 100644
> --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c
> @@ -205,7 +205,7 @@ static int aspeed_gfx_load(struct drm_device *drm)
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = drm_vblank_init(drm, 1);
> +	ret = drm_vblank_init(drm);

[Severity: Critical]
Will this cause a probe regression for this driver?

By removing the explicit CRTC count, drm_vblank_init() now relies on the
DRM device's internal CRTC list being populated. However, aspeed_gfx_load()
calls drm_vblank_init() before calling aspeed_gfx_create_pipe(), which is
where the CRTC is actually created.

Because the CRTC list is empty at this point, drm_vblank_init() will
return -ENODEV and fail the probe.

>  	if (ret < 0) {
>  		dev_err(drm->dev, "Failed to initialise vblank\n");
>  		return ret;

> [ ... ]

> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> index 9228a7ad0dfa4..c9e29da53d056 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> @@ -888,7 +888,7 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>  	 * Initialize vertical blanking interrupts handling. Start with vblank
>  	 * disabled for all CRTCs.
>  	 */
> -	ret = drm_vblank_init(dev, rcdu->num_crtcs);
> +	ret = drm_vblank_init(dev);

[Severity: Critical]
Does this introduce a similar regression for rcar-du?

The driver calls drm_vblank_init() here, but the CRTCs are not created
until the rcar_du_crtc_create() loop further down. This means
drm_vblank_init() will see an empty CRTC list and fail with -ENODEV.

>  	if (ret < 0)
>  		return ret;

> [ ... ]

> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 5a640b531d88f..5a830cee7fe9b 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -192,8 +192,7 @@ int vkms_create(struct vkms_config *config)
>  		goto out_devres;
>  	}
>  
> -	ret = drm_vblank_init(&vkms_device->drm,
> -			      vkms_config_get_num_crtcs(config));
> +	ret = drm_vblank_init(&vkms_device->drm);

[Severity: Critical]
Can this lead to a complete initialization failure for vkms?

The call to drm_vblank_init() occurs before vkms_modeset_init(), meaning
the pipeline components have not been created yet and the internal CRTC
count is zero.

>  	if (ret) {
>  		DRM_ERROR("Failed to vblank\n");
>  		goto out_devres;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3