Re: [PATCH 63/70] drm/sitronix: st7920: Convert to atomic_create_state
| 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