Re: [PATCH RESEND 81/83] drm/vmwgfx: Switch to drm_mode_config_create_initial_state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/vmwgfx: state creation error in display init triggers broken fallback
--

commit 4961d66508852dc07ab5238728474ad461cb6c65
Author: Maxime Ripard <mripard@kernel.org>

drm/vmwgfx: Switch to drm_mode_config_create_initial_state

This commit replaces calls to drm_mode_config_reset() with
drm_mode_config_create_initial_state() during KMS display unit
initialization in vmwgfx_ldu.c, vmwgfx_scrn.c, and vmwgfx_stdu.c.

> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> index 39d58fc392bf4..82fe7db266335 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> @@ -939,9 +939,7 @@ int vmw_kms_sou_init_display(struct vmw_private *dev_priv)
>  
>  	dev_priv->active_display_unit = vmw_du_screen_object;
>  
> -	drm_mode_config_reset(dev);
> -
> -	return 0;
> +	return drm_mode_config_create_initial_state(dev);
>  }
>  
>  static int do_bo_define_gmrfb(struct vmw_private *dev_priv,
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index b505f8ba3e834..80defd7358135 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -1708,7 +1708,5 @@ int vmw_kms_stdu_init_display(struct vmw_private *dev_priv)
>  		}
>  	}
>  
> -	drm_mode_config_reset(dev);
> -
> -	return 0;
> +	return drm_mode_config_create_initial_state(dev);
>  }

[Severity: Medium]
What happens in vmw_kms_init() if drm_mode_config_create_initial_state()
fails at the end of vmw_kms_stdu_init_display() or
vmw_kms_sou_init_display()?

Previously, drm_mode_config_reset() returned void, so neither function
returned an error after registering all VMWGFX_NUM_DISPLAY_UNITS (8)
display units on dev->mode_config (and vmw_kms_sou_init_display() only ever
returned -ENOSYS before initializing any display units).

In drivers/gpu/drm/vmwgfx/vmwgfx_kms.c, vmw_kms_init() treats any non-zero
return value as a fallback trigger:

	ret = vmw_kms_stdu_init_display(dev_priv);
	if (ret) {
		ret = vmw_kms_sou_init_display(dev_priv);
		if (ret) /* Fallback */
			ret = vmw_kms_ldu_init_display(dev_priv);
	}

If drm_mode_config_create_initial_state() fails after the display units are
initialized, those 8 CRTCs, planes, encoders, and connectors remain on
dev->mode_config while vmw_kms_init() proceeds to initialize the next
display backend on the same drm_device.

When the fallback backend registers another set of CRTCs,
dev->mode_config.num_crtc grows past VMWGFX_NUM_DISPLAY_UNITS (8). If
dev_priv->vkms_enabled is set, vmw_vkms_init() only allocated dev->vblank
for VMWGFX_NUM_DISPLAY_UNITS CRTCs via drm_vblank_init(), so the subsequent
drm_mode_config_create_initial_state() -> drm_mode_config_crtc_create_state()
-> drm_crtc_vblank_reset() call will index dev->vblank out of bounds. Even
when vkms is disabled, if the fallback succeeds, dev->mode_config is left
with duplicate mixed-backend KMS objects and more than
VMWGFX_NUM_DISPLAY_UNITS CRTCs (which can also overflow fixed-size stack
arrays such as units[VMWGFX_NUM_DISPLAY_UNITS] in vmw_kms_helper_dirty()).

Could drm_mode_config_create_initial_state(dev) be called once in
vmw_kms_init() after the display unit fallback chain succeeds instead of
inside each backend initializer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-drm-no-more-config-reset-v1-0-1f58d1e33380@kernel.org?part=81