Re: [PATCH v2 13/61] drm/amdgpu: Convert to atomic_create_state

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The dynamically allocated `drm_plane` structures for AMDGPU VKMS are never freed, causing a memory leak during driver teardown and on initialization error paths.
--

--- Patch [13]: [PATCH v2 13/61] drm/amdgpu: Convert to atomic_create_state ---
commit e8c110baf25c9fbe2352593883aad5a81fdc938b
Author: Maxime Ripard <[email protected]>

drm/amdgpu: Convert to atomic_create_state

This patch converts the reset hook to atomic_create_state for the plane.
This allows the plane to initialize a pristine state using
drm_atomic_helper_plane_reset(), matching what atomic_create_state expects.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> index 170adaf7e76ab..9628239855d23 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vkms.c
> @@ -144,7 +144,7 @@ static const struct drm_plane_funcs amdgpu_vkms_plane_funcs = {
>  	.update_plane		= drm_atomic_helper_update_plane,
>  	.disable_plane		= drm_atomic_helper_disable_plane,
>  	.destroy		= drm_plane_cleanup,

[Severity: High]
This isn't a bug introduced by this patch, but does this code leak the
dynamically allocated plane structure?

The .destroy hook is mapped directly to drm_plane_cleanup(), which tears down
DRM internal objects but does not free the enclosing structure. 

In amdgpu_vkms_plane_init(), the plane is dynamically allocated, but during
driver teardown the DRM core iterates over the planes and calls this .destroy
hook, which fails to free the memory. Similarly, this memory appears to leak
on the error path in amdgpu_vkms_output_init():

amdgpu_vkms_output_init() {
    ...
err_crtc:
    drm_plane_cleanup(primary);

    return ret;
}

Should there be a custom destroy hook that calls drm_plane_cleanup() followed
by kfree() to prevent this leak?

> -	.reset			= drm_atomic_helper_plane_reset,
> +	.atomic_create_state = drm_atomic_helper_plane_create_state,
>  	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
>  	.atomic_destroy_state	= drm_atomic_helper_plane_destroy_state,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13