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

[email protected]
Newsgroups org.freedesktop.lists.dri-devel
Message-ID <[email protected]>
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 <[email protected]>
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/[email protected]?part=63
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.