Re: [PATCH v2 1/5] drm/format-helper: Add struct drm_xfrm_buf to cache format conversion

[Javier Martinez Canillas <javierm@redhat.com>]

Thomas Zimmermann <tzimmermann@suse.de> writes:

> Hold temporary memory for format conversion in an instance of struct
> drm_xfrm_buf. Update internal helpers of DRM's format-conversion code
> accordingly. Drivers will later be able to keep this cache across
> display updates.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---

[...]

> +int drmm_xfrm_buf_init(struct drm_device *dev, struct drm_xfrm_buf *buf)
> +{
> +	buf->mem = NULL;
> +	buf->size = 0;
> +	buf->preallocated = false;
> +
> +	return drmm_add_action_or_reset(dev, drm_xfrm_buf_init_release, buf);
> +}
> +EXPORT_SYMBOL(drmm_xfrm_buf_init);
> +

Can we find a better name than xfrm? I know that this is what's used in
the internal drm_format_helper.c helpers but if we are exposing this to
drivers, then I think that the naming is not self explanatory.

> +/**
> + * drm_xfrm_buf_reserve - Allocates storage in an xfrm buffer
> + * @buf: The xfrm buffer

At least in the kernel-doc we can say "The buffer used for pixel format
conversion" or something along those lines.

[...]

> +/**
> + * struct drm_xfrm_buf - Stores transformation and conversion state
> + *
> + * DRM helpers for format conversion store temporary state in
> + * struct drm_xfrm_buf. The buffer's resources can be reused

And same here. Maybe struct drm_fmt_conversion_buf ?

Other than this nit, the patch looks good to me.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat