Re: [PATCH 63/70] drm/sitronix: st7920: Convert to atomic_create_state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message falsely claims that the old `st7920_crtc_reset` freed the previous state.
--

--- Patch [63]: [PATCH 63/70] drm/sitronix: st7920: Convert to atomic_create_state ---
commit 51126b7e2261e0176cf52ba6e5a5273dcbb0592e
Author: Maxime Ripard <mripard@kernel.org>
drm/sitronix: st7920: Convert to atomic_create_state

This commit converts the st7920 CRTC implementation to use the
atomic_create_state hook rather than a custom reset hook.

> The st7920 crtc implementation provides a custom reset hook. However,
> this hook only allocates the state, initializes it with
> __drm_atomic_helper_crtc_reset(), and frees the previous state. It
> does not perform any hardware reset.

[Severity: Medium]
Is this description accurate regarding the previous state being freed?

Looking at the original st7920_crtc_reset() implementation before this
patch, it warns if crtc->state is non-NULL but does not appear to free
the old state memory:

	drm_WARN_ON_ONCE(crtc->dev, crtc->state);

	st7920_state = kzalloc_obj(*st7920_state);
	if (!st7920_state)
		return;

	__drm_atomic_helper_crtc_reset(crtc, &st7920_state->base);

Could the commit message be updated to clarify that the old hook was not
actually freeing the previous state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-drm-no-more-crtc-reset-v1-0-fb793475c05a@kernel.org?part=63