Hi Am 05.10.23 um 15:18 schrieb Javier Martinez Canillas:
Thomas Zimmermann <tzimmermann@xxxxxxx> writes: Hello Thomas,Hold temporary memory for format conversion in an instance of struct drm_format_conv_state. Update internal helpers of DRM's format-conversion code accordingly. Drivers will later be able to maintain this cache by themselves. Besides caching, struct drm_format_conv_state will be useful to hold additional information for format conversion, such as palette data or foreground/background colors. This will enable conversion from indexed color formats to component-based formats. v3: * rename struct drm_xfrm_buf to struct drm_format_conv_state (Javier) * remove managed cleanup * add drm_format_conv_state_copy() for shadow-plane support Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> ---[...]+/** + * drm_format_conv_state_init - Initialize format-conversion state + * @state: The state to initialize + * + * Clears all fields in struct drm_format_conv_state and installs a DRM + * release action for the buffer. The buffer will be empty with no + * preallocated resources. + */ +void drm_format_conv_state_init(struct drm_format_conv_state *state) +{ + state->tmp.mem = NULL; + state->tmp.size = 0; + state->tmp.preallocated = false; +} +EXPORT_SYMBOL(drm_format_conv_state_init); + +/** + * drm_format_conv_state_copy - Copy format-conversion state + * @state: Destination state + * @old_state: Source state + * + * Copies format-conversion state from @old_state to @state; except for + * temporary storage. + */ +void drm_format_conv_state_copy(struct drm_format_conv_state *state, + const struct drm_format_conv_state *old_state) +{ + state->tmp.mem = NULL; + state->tmp.size = 0; + state->tmp.preallocated = false; +} +EXPORT_SYMBOL(drm_format_conv_state_copy); +I'm confused, the copy helper is the same than init. What's the point of this function ? Why not just call drm_format_conv_state_init() from the __drm_gem_duplicate_shadow_plane_state() function in the next patch ?
I guess that deserves a comment in the code. The reserved buffer is not to be copied to another state. So we just clear the fields. But in the future, we will likely be extra fields, such as the aforementioned palette data, that will be copied. It's just that these fields don't exist yet. Hence the copy function is different from the init.
Best regards Thomas
Other than that the patch looks good to me. After fixing the issue that Noralf pointed out: Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)
Attachment:
OpenPGP_signature.asc
Description: OpenPGP digital signature
