Re: [PATCH RFC 1/6] drm/vkms: Properly extract vkms_formats header

["Maíra Canal" <mcanal@igalia.com>]

Hi Louis,

Most of the code extracted in this patch was previously introduced by
you in patches that I didn't apply yet. Therefore, I believe you could
just add the code in vkms_formats.c in the original series.

Best Regards,
- Maíra

On 8/14/24 06:08, Louis Chauvet wrote:
> The vkms_format.h header was already separated from vkms_drv.h, but some
> function were missing. Move those function in vkms_format.h.
> 
> Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com>
> ---
>   drivers/gpu/drm/vkms/vkms_drv.h     | 74 +---------------------------------
>   drivers/gpu/drm/vkms/vkms_formats.c |  3 ++
>   drivers/gpu/drm/vkms/vkms_formats.h | 80 ++++++++++++++++++++++++++++++++++++-
>   3 files changed, 84 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 8f6c9e67e671..0db443924a15 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -12,6 +12,8 @@
>   #include <drm/drm_encoder.h>
>   #include <drm/drm_writeback.h>
>   
> +#include "vkms_formats.h"
> +
>   #define XRES_MIN    10
>   #define YRES_MIN    10
>   
> @@ -43,29 +45,6 @@ struct vkms_frame_info {
>   	unsigned int rotation;
>   };
>   
> -struct pixel_argb_u16 {
> -	u16 a, r, g, b;
> -};
> -
> -struct line_buffer {
> -	size_t n_pixels;
> -	struct pixel_argb_u16 *pixels;
> -};
> -
> -struct vkms_writeback_job;
> -/**
> - * typedef pixel_write_line_t - These functions are used to read a pixel line from a
> - * struct pixel_argb_u16 buffer, convert it and write it in the @wb job.
> - *
> - * @wb: the writeback job to write the output of the conversion
> - * @in_pixels: Source buffer containing the line to convert
> - * @count: The width of a line
> - * @x_start: The x (width) coordinate in the destination plane
> - * @y_start: The y (height) coordinate in the destination plane
> - */
> -typedef void (*pixel_write_line_t)(struct vkms_writeback_job *wb,
> -			      struct pixel_argb_u16 *in_pixels, int count, int x_start,
> -			      int y_start);
>   
>   struct vkms_writeback_job {
>   	struct iosys_map data[DRM_FORMAT_MAX_PLANES];
> @@ -73,53 +52,10 @@ struct vkms_writeback_job {
>   	pixel_write_line_t pixel_write;
>   };
>   
> -/**
> - * enum pixel_read_direction - Enum used internaly by VKMS to represent a reading direction in a
> - * plane.
> - */
> -enum pixel_read_direction {
> -	READ_BOTTOM_TO_TOP,
> -	READ_TOP_TO_BOTTOM,
> -	READ_RIGHT_TO_LEFT,
> -	READ_LEFT_TO_RIGHT
> -};
>   
>   struct vkms_plane_state;
>   
> -/**
> - * typedef pixel_read_line_t - These functions are used to read a pixel line in the source frame,
> - * convert it to `struct pixel_argb_u16` and write it to @out_pixel.
> - *
> - * @plane: plane used as source for the pixel value
> - * @x_start: X (width) coordinate of the first pixel to copy. The caller must ensure that x_start
> - * is non-negative and smaller than @plane->frame_info->fb->width.
> - * @y_start: Y (height) coordinate of the first pixel to copy. The caller must ensure that y_start
> - * is non-negative and smaller than @plane->frame_info->fb->height.
> - * @direction: direction to use for the copy, starting at @x_start/@y_start
> - * @count: number of pixels to copy
> - * @out_pixel: pointer where to write the pixel values. They will be written from @out_pixel[0]
> - * (included) to @out_pixel[@count] (excluded). The caller must ensure that out_pixel have a
> - * length of at least @count.
> - */
> -typedef void (*pixel_read_line_t)(const struct vkms_plane_state *plane, int x_start,
> -				  int y_start, enum pixel_read_direction direction, int count,
> -				  struct pixel_argb_u16 out_pixel[]);
>   
> -/**
> - * struct conversion_matrix - Matrix to use for a specific encoding and range
> - *
> - * @matrix: Conversion matrix from yuv to rgb. The matrix is stored in a row-major manner and is
> - * used to compute rgb values from yuv values:
> - *     [[r],[g],[b]] = @matrix * [[y],[u],[v]]
> - *   OR for yvu formats:
> - *     [[r],[g],[b]] = @matrix * [[y],[v],[u]]
> - *  The values of the matrix are signed fixed-point values with 32 bits fractional part.
> - * @y_offset: Offset to apply on the y value.
> - */
> -struct conversion_matrix {
> -	s64 matrix[3][3];
> -	int y_offset;
> -};
>   
>   /**
>    * struct vkms_plane_state - Driver specific plane state
> @@ -140,12 +76,6 @@ struct vkms_plane {
>   	struct drm_plane base;
>   };
>   
> -struct vkms_color_lut {
> -	struct drm_color_lut *base;
> -	size_t lut_length;
> -	s64 channel_value2index_ratio;
> -};
> -
>   /**
>    * struct vkms_crtc_state - Driver specific CRTC state
>    *
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
> index 65fdd3999441..5ab84801d8da 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.c
> +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> @@ -6,9 +6,12 @@
>   #include <drm/drm_blend.h>
>   #include <drm/drm_rect.h>
>   #include <drm/drm_fixed.h>
> +#include <drm/drm_fourcc.h>
> +#include <drm/drm_framebuffer.h>
>   
>   #include <kunit/visibility.h>
>   
> +#include "vkms_drv.h"
>   #include "vkms_formats.h"
>   
>   /**
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.h b/drivers/gpu/drm/vkms/vkms_formats.h
> index 852ab9a4cee5..62b06bc26e79 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.h
> +++ b/drivers/gpu/drm/vkms/vkms_formats.h
> @@ -3,7 +3,85 @@
>   #ifndef _VKMS_FORMATS_H_
>   #define _VKMS_FORMATS_H_
>   
> -#include "vkms_drv.h"
> +#include <drm/drm_color_mgmt.h>
> +
> +struct vkms_plane_state;
> +struct vkms_writeback_job;
> +
> +struct pixel_argb_u16 {
> +	u16 a, r, g, b;
> +};
> +
> +/**
> + * typedef pixel_write_line_t - These functions are used to read a pixel line from a
> + * struct pixel_argb_u16 buffer, convert it and write it in the @wb_job.
> + *
> + * @wb: the writeback job to write the output of the conversion
> + * @in_pixels: Source buffer containing the line to convert
> + * @count: The width of a line
> + * @x_start: The x (width) coordinate in the destination plane
> + * @y_start: The y (height) coordinate in the destination plane
> + */
> +typedef void (*pixel_write_line_t)(struct vkms_writeback_job *wb,
> +				   struct pixel_argb_u16 *in_pixels, int count, int x_start,
> +				   int y_start);
> +
> +struct line_buffer {
> +	size_t n_pixels;
> +	struct pixel_argb_u16 *pixels;
> +};
> +
> +/**
> + * enum pixel_read_direction - Enum used internaly by VKMS to represent a reading direction in a
> + * plane.
> + */
> +enum pixel_read_direction {
> +	READ_BOTTOM_TO_TOP,
> +	READ_TOP_TO_BOTTOM,
> +	READ_RIGHT_TO_LEFT,
> +	READ_LEFT_TO_RIGHT
> +};
> +
> +/**
> + * struct conversion_matrix - Matrix to use for a specific encoding and range
> + *
> + * @matrix: Conversion matrix from yuv to rgb. The matrix is stored in a row-major manner and is
> + * used to compute rgb values from yuv values:
> + *     [[r],[g],[b]] = @matrix * [[y],[u],[v]]
> + *   OR for yvu formats:
> + *     [[r],[g],[b]] = @matrix * [[y],[v],[u]]
> + *  The values of the matrix are signed fixed-point values with 32 bits fractional part.
> + * @y_offset: Offset to apply on the y value.
> + */
> +struct conversion_matrix {
> +	s64 matrix[3][3];
> +	int y_offset;
> +};
> +
> +struct vkms_color_lut {
> +	struct drm_color_lut *base;
> +	size_t lut_length;
> +	s64 channel_value2index_ratio;
> +};
> +
> +/**
> + * typedef pixel_read_line_t - These functions are used to read a pixel line in the source frame,
> + * convert it to `struct pixel_argb_u16` and write it to @out_pixel.
> + *
> + * @plane: plane used as source for the pixel value
> + * @x_start: X (width) coordinate of the first pixel to copy. The caller must ensure that x_start
> + * is non-negative and smaller than @plane->frame_info->fb->width.
> + * @y_start: Y (height) coordinate of the first pixel to copy. The caller must ensure that y_start
> + * is non-negative and smaller than @plane->frame_info->fb->height.
> + * @direction: direction to use for the copy, starting at @x_start/@y_start
> + * @count: number of pixels to copy
> + * @out_pixel: pointer where to write the pixel values. They will be written from @out_pixel[0]
> + * (included) to @out_pixel[@count] (excluded). The caller must ensure that out_pixel have a
> + * length of at least @count.
> + */
> +typedef void (*pixel_read_line_t)(const struct vkms_plane_state *plane, int x_start,
> +				  int y_start, enum pixel_read_direction direction, int count,
> +				  struct pixel_argb_u16 out_pixel[]);
>   
>   pixel_read_line_t get_pixel_read_line_function(u32 format);
>   
>