diff mbox series

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

Message ID 20260512-drm-mode-config-init-v4-18-591dfdcc1bf9@kernel.org (mailing list archive)
State New
Headers
Series drm/atomic: Rework initial state allocation |

Commit Message

Maxime Ripard May 12, 2026, 1:06 p.m. UTC 
Now that drm_mode_config_create_initial_state() exists to create the
initial state, use it instead of drm_mode_config_reset() during
driver probe.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/tidss/tidss_drv.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

sashiko-bot@kernel.org May 14, 2026, 1:25 a.m. UTC | #1 
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);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/tidss/tidss_drv.c b/drivers/gpu/drm/tidss/tidss_drv.c
index 1c8cc18bc53c..f5099d5d6e32 100644
--- a/drivers/gpu/drm/tidss/tidss_drv.c
+++ b/drivers/gpu/drm/tidss/tidss_drv.c
@@ -169,11 +169,15 @@  static int tidss_probe(struct platform_device *pdev)
 		goto err_runtime_suspend;
 	}
 
 	drm_kms_helper_poll_init(ddev);
 
-	drm_mode_config_reset(ddev);
+	ret = drm_mode_config_create_initial_state(ddev);
+	if (ret) {
+		dev_err(dev, "failed to create initial state: %d\n", ret);
+		goto err_irq_uninstall;
+	}
 
 	ret = drm_dev_register(ddev, 0);
 	if (ret) {
 		dev_err(dev, "failed to register DRM device\n");
 		goto err_irq_uninstall;