Re: [PATCH 04/75] drm/amdgpu: dm: Switch to drm_atomic_helper_connector_create_state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] drm/amdgpu: tests store ERR_PTR in connector->state leading to cleanup crash
- [Low] drm: WARN_ON triggered during test connector init due to missing atomic hooks
--

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

drm/amdgpu: dm: Switch to drm_atomic_helper_connector_create_state

The amdgpu: dm connector implementation uses the deprecated
drm_atomic_helper_connector_reset() as its reset hook. Switch to
drm_atomic_helper_connector_create_state() instead.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_crc_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_crc_test.c
> index 29a513dab9cd8..e47b4dd210b8d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_crc_test.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_crc_test.c

[ ... ]

> @@ -654,8 +654,9 @@ static void dm_test_crtc_set_crc_source_dprx_no_connector(struct kunit *test)
>  						 DRM_MODE_CONNECTOR_WRITEBACK), 0);
>  	KUNIT_ASSERT_EQ(test, kunit_add_action_or_reset(test,
>  			dm_test_crc_connector_cleanup, wb_conn), 0);
> -	drm_atomic_helper_connector_reset(wb_conn);
> -	KUNIT_ASSERT_NOT_NULL(test, wb_conn->state);
> +
> +	wb_conn->state = drm_atomic_helper_connector_create_state(wb_conn);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, wb_conn->state);

[Severity: Medium]
Can this assignment introduce a regression by causing a crash in the deferred
KUnit cleanup path if memory allocation fails?

In dm_test_crtc_set_crc_source_dprx_no_connector(), if
drm_atomic_helper_connector_create_state() fails, it returns ERR_PTR(-ENOMEM).
Because it's directly assigned to wb_conn->state, the assertion
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, wb_conn->state) will fail, aborting the
test.

This test abortion jumps to KUnit's deferred cleanup routines. The previously
registered cleanup action dm_test_crc_connector_cleanup() invokes
drm_connector_cleanup(wb_conn).

Since wb_conn->state contains ERR_PTR(-ENOMEM), which is not NULL,
drm_connector_cleanup() will consider it a valid pointer and pass it to
drm_atomic_helper_connector_destroy_state(). This will blindly dereference
the error pointer, leading to an invalid memory access.

Should the return value be validated in a local variable before being assigned
to wb_conn->state to ensure it remains NULL on failure?

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_helpers_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_helpers_test.c
> index 51c3995de76d2..a369a3cbd4afa 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_helpers_test.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_helpers_test.c

[ ... ]

> @@ -4106,7 +4106,7 @@ static void dm_test_is_dp_sink_present_queries_link_service(struct kunit *test)
>   * device pointer, so build a real one here.
>   */
>  static const struct drm_connector_funcs dm_test_connector_funcs = {
> -	.reset = drm_atomic_helper_connector_reset,
> +	.atomic_create_state = drm_atomic_helper_connector_create_state,
>  };

[Severity: Low]
Will this missing hook introduce a regression by triggering a kernel warning
during test connector initialization?

The KUnit test allocates a DRM device with the DRIVER_ATOMIC feature flag.
When dm_test_alloc_real_connector() initializes the connector using
drmm_connector_init(), it calls into drm_connector_init_only().

Because dm_test_connector_funcs omits both the .atomic_destroy_state and
.atomic_duplicate_state hooks, this explicitly violates the core DRM check:

WARN_ON(drm_drv_uses_atomic_modeset(dev) &&
        (!funcs->atomic_destroy_state || !funcs->atomic_duplicate_state))

This causes a warning to fire unconditionally during KUnit test execution.
Should we add the required hooks to satisfy the API contract for this
test connector?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-drm-no-more-connector-reset-v1-0-586c9c026823@kernel.org?part=4