Re: [PATCH v5 03/16] drm/vkms: write/update the documentation for pixel conversion and pixel write functions

From: Randy Dunlap
Date: Wed Mar 13 2024 - 15:18:43 EST


Hi,

On 3/13/24 10:44, Louis Chauvet wrote:
> Add some documentation on pixel conversion functions.
> Update of outdated comments for pixel_write functions.
>
> Signed-off-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
> ---
> drivers/gpu/drm/vkms/vkms_composer.c | 7 ++++
> drivers/gpu/drm/vkms/vkms_drv.h | 13 ++++++++
> drivers/gpu/drm/vkms/vkms_formats.c | 62 ++++++++++++++++++++++++++++++------
> 3 files changed, 73 insertions(+), 9 deletions(-)
>

> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
> index 172830a3936a..6e3dc8682ff9 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.c
> +++ b/drivers/gpu/drm/vkms/vkms_formats.c


> @@ -216,6 +238,14 @@ static void argb_u16_to_RGB565(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel)
> *pixels = cpu_to_le16(r << 11 | g << 5 | b);
> }
>
> +/**

This comment is not in kernel-doc format, so either use "/*" to begin the comment
or add the function name in the first comment line, like:


+ * vkms_writeback_row - Generic loop for all supported writeback format. It is executed just after the blending to

> + * Generic loop for all supported writeback format. It is executed just after the blending to
> + * write a line in the writeback buffer.
> + *
> + * @wb: Job where to insert the final image
> + * @src_buffer: Line to write
> + * @y: Row to write in the writeback buffer
> + */
> void vkms_writeback_row(struct vkms_writeback_job *wb,
> const struct line_buffer *src_buffer, int y)
> {
> @@ -229,6 +259,13 @@ void vkms_writeback_row(struct vkms_writeback_job *wb,
> wb->pixel_write(dst_pixels, &in_pixels[x]);
> }
>
> +/**

Needs function name or don't use "/**" to begin the comment.

> + * Retrieve the correct read_pixel function for a specific format.
> + * The returned pointer is NULL for unsupported pixel formats. The caller must ensure that the
> + * pointer is valid before using it in a vkms_plane_state.
> + *
> + * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h])
> + */
> void *get_pixel_conversion_function(u32 format)
> {
> switch (format) {
> @@ -247,6 +284,13 @@ void *get_pixel_conversion_function(u32 format)
> }
> }
>
> +/**

Same here.

> + * Retrieve the correct write_pixel function for a specific format.
> + * The returned pointer is NULL for unsupported pixel formats. The caller must ensure that the
> + * pointer is valid before using it in a vkms_writeback_job.
> + *
> + * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h])
> + */
> void *get_pixel_write_function(u32 format)
> {
> switch (format) {
>

thanks.
--
#Randy