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 <[email protected]> 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/[email protected]?part=81
