Re: [PATCH v4 18/20] drm/tidss: Switch to drm_mode_config_create_initial_state()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Uninitialized atomic state for CRTCs and planes due to missing `.atomic_create_state` callbacks.
- [High] Connector reference and lockdep leak in `drm_mode_config_create_initial_state` error path.
--

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

drm/tidss: Switch to drm_mode_config_create_initial_state()

This patch updates the tidss driver probe to use
drm_mode_config_create_initial_state() instead of drm_mode_config_reset()
for creating the initial DRM state.

> diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
> index 1c8cc18bc53c3..f5099d5d6e32a 100644
> --- a/drivers/gpu/drm/tidss/tidss_drv.c
> +++ b/drivers/gpu/drm/tidss/tidss_drv.c
> @@ -171,7 +171,11 @@ static int tidss_probe(struct platform_device *pdev)
>  
>  	drm_kms_helper_poll_init(ddev);
>  
> -	drm_mode_config_reset(ddev);
> +	ret = drm_mode_config_create_initial_state(ddev);

Since drm_mode_config_create_initial_state() allocates software states using
the .atomic_create_state hooks for CRTCs, planes, and connectors, does this
leave plane->state and crtc->state as NULL? 

It appears the tidss driver does not implement .atomic_create_state yet and
still relies on the legacy .reset hooks at this point in the series. If these
states remain NULL, could it trigger a WARN_ON(!plane->state) and a NULL
pointer dereference during the initial atomic commit?

I understand this is fixed later in the series by the commit titled
"drm/tidss: Convert to atomic_create_state", but could this break git bisect?

Also, looking at the implementation of drm_mode_config_create_initial_state()
in drm_mode_config.c that is being called here:

drivers/gpu/drm/drm_mode_config.c:drm_mode_config_create_initial_state() {
	...
	drm_connector_list_iter_begin(dev, &conn_iter);
	drm_for_each_connector_iter(connector, &conn_iter) {
		if (connector->state)
			continue;

		ret = drm_mode_config_connector_create_state(connector);
		if (ret)
			return ret;
	}
	drm_connector_list_iter_end(&conn_iter);
	...
}

If drm_mode_config_connector_create_state() fails, this returns directly
without calling drm_connector_list_iter_end(). Does this leak the connector
reference and leave the lockdep context acquired indefinitely?

> +	if (ret) {
> +		dev_err(dev, "failed to create initial state: %d\n", ret);
> +		goto err_irq_uninstall;
> +	}
>  
>  	ret = drm_dev_register(ddev, 0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-drm-mode-config-init-v4-0-591dfdcc1bf9@kernel.org?part=18