Hi Am 26.07.22 um 15:36 schrieb Javier Martinez Canillas:
On 7/20/22 16:27, Thomas Zimmermann wrote:Add a dedicated CRTC state to ofdrm to later store information for palette updates. Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> --- drivers/gpu/drm/tiny/ofdrm.c | 62 ++++++++++++++++++++++++++++++++++--Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx> [...]+static void ofdrm_crtc_reset(struct drm_crtc *crtc) +{ + struct ofdrm_crtc_state *ofdrm_crtc_state; + + if (crtc->state) { + ofdrm_crtc_state_destroy(to_ofdrm_crtc_state(crtc->state)); + crtc->state = NULL; /* must be set to NULL here */ + } + + ofdrm_crtc_state = kzalloc(sizeof(*ofdrm_crtc_state), GFP_KERNEL); + if (!ofdrm_crtc_state) + return; + __drm_atomic_helper_crtc_reset(crtc, &ofdrm_crtc_state->base); +} +IMO this function is hard to read, I would instead write it as following: static void ofdrm_crtc_reset(struct drm_crtc *crtc) { struct ofdrm_crtc_state *ofdrm_crtc_state = kzalloc(sizeof(*ofdrm_crtc_state), GFP_KERNEL); if (!ofdrm_crtc_state) return; if (crtc->state) { ofdrm_crtc_state_destroy(to_ofdrm_crtc_state(crtc->state)); crtc->state = NULL; /* must be set to NULL here */ } __drm_atomic_helper_crtc_reset(crtc, &ofdrm_crtc_state->base); } Also with that form I think that the crtc->state = NULL could just be dropped ?
I once had to add this line to a driver to make the DRM helpers work. But I cannot find any longer why. Maybe it's been resolved meanwhile.
Best regards Thomas
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
