Re: [PATCH] drm/appletbdrm: Add suspend and resume support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/appletbdrm: Memory leak of suspend_state on reset_resume error
--

commit 60e19da684ddfb4bbf66c6f0c2624ed415d96f12
Author: Vasilij Markin <vasilij.kernel@tototo.si>
Subject: drm/appletbdrm: Add suspend and resume support

This commit introduces suspend and resume support for the appletbdrm driver.
It uses drm_mode_config_helper_suspend() and drm_mode_config_helper_resume()
to keep the device registered across cycles. In case the device was reset while
suspended, it repeats the probe-time handshake.

> diff --git a/drivers/gpu/drm/tiny/appletbdrm.c b/drivers/gpu/drm/tiny/appletbdrm.c
> index 4297d9c1125e4..024ad8bf6e1c4 100644
> --- a/drivers/gpu/drm/tiny/appletbdrm.c
> +++ b/drivers/gpu/drm/tiny/appletbdrm.c
[ ... ]
> @@ -821,6 +822,45 @@ static void appletbdrm_shutdown(struct usb_interface *intf)
>  	drm_atomic_helper_shutdown(&adev->drm);
>  }
>  
> +static int appletbdrm_suspend(struct usb_interface *intf, pm_message_t message)
> +{
> +	struct appletbdrm_device *adev = usb_get_intfdata(intf);
> +
> +	return drm_mode_config_helper_suspend(&adev->drm);
> +}
> +
> +static int appletbdrm_resume(struct usb_interface *intf)
> +{
> +	struct appletbdrm_device *adev = usb_get_intfdata(intf);
> +
> +	return drm_mode_config_helper_resume(&adev->drm);
> +}
> +
> +static int appletbdrm_reset_resume(struct usb_interface *intf)
> +{
> +	struct appletbdrm_device *adev = usb_get_intfdata(intf);
> +	struct drm_device *drm = &adev->drm;
> +	int ret;
> +
> +	/*
> +	 * A reset puts the device back into its initial state, where it
> +	 * expects the same handshake as during probe
> +	 */
> +	ret = appletbdrm_get_information(adev);
> +	if (ret) {
> +		drm_err(drm, "Failed to get display information\n");
> +		return ret;

[Severity: Medium]
During system suspend, drm_mode_config_helper_suspend() duplicates the atomic
commit state and stores it in dev->mode_config.suspend_state.

If appletbdrm_get_information() fails here, we return early and bypass the call
to drm_mode_config_helper_resume(). Does this leak the suspend_state and its
associated DRM object references, since they never get freed or put by
drm_atomic_state_put()?

> +	}
> +
> +	ret = appletbdrm_signal_readiness(adev);
> +	if (ret) {
> +		drm_err(drm, "Failed to signal readiness\n");
> +		return ret;

[Severity: Medium]
Similar to the issue above, if appletbdrm_signal_readiness() fails, does
returning early without calling drm_mode_config_helper_resume() leak the atomic
commit state stored in dev->mode_config.suspend_state?

> +	}
> +
> +	return drm_mode_config_helper_resume(drm);
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-appletbdrm-pm-v1-1-0e5432132161@tototo.si?part=1