Re: [PATCH V10 18/46] drm/vkms: add 3x4 matrix in color pipeline

["Nícolas F. R. A. Prado" <nfraprado@collabora.com>]

On Mon, 2025-06-16 at 22:17 -0600, Alex Hung wrote:
> From: Harry Wentland <harry.wentland@amd.com>
> 
> We add two 3x4 matrices into the VKMS color pipeline. The reason
> we're adding matrices is so that we can test that application
> of a matrix and its inverse yields an output equal to the input
> image.
> 
> One complication with the matrix implementation has to do with
> the fact that the matrix entries are in signed-magnitude fixed
> point, whereas the drm_fixed.h implementation uses 2s-complement.
> The latter one is the one that we want for easy addition and
> subtraction, so we convert all entries to 2s-complement.
> 
> Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
> Signed-off-by: Alex Hung <alex.hung@amd.com>
> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> Reviewed-by: Daniel Stone <daniels@collabora.com>
> ---
> V9:
>  - Update function names by _plane_ (Chaitanya Kumar Borah)
> 
> v8:
>  - Replace DRM_ERROR with drm_err (Louis Chauvet)
> 
> v7:
>  - Fix checkpatch warnings
>   - Change kzalloc(sizeof(struct drm_colorop) ...) to
> kzalloc(sizeof(*ops[i]) ...)
>   - Change i-1to i - 1
>   - Add a new line at EOF
> 
> v6:
>  - pre-compute colors (Louis Chauvet)
>  - round matrix output (Louis Chauvet)
> 
>  drivers/gpu/drm/vkms/vkms_colorop.c  | 34
> +++++++++++++++++++++++++++-
>  drivers/gpu/drm/vkms/vkms_composer.c | 33
> +++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c
> b/drivers/gpu/drm/vkms/vkms_colorop.c
> index 2fb74b2bd001..70be35804ea9 100644
> --- a/drivers/gpu/drm/vkms/vkms_colorop.c
> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
> @@ -12,7 +12,7 @@ static const u64 supported_tfs =
>  	BIT(DRM_COLOROP_1D_CURVE_SRGB_EOTF) |
>  	BIT(DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF);
>  
> -#define MAX_COLOR_PIPELINE_OPS 2
> +#define MAX_COLOR_PIPELINE_OPS 4
>  
>  static int vkms_initialize_color_pipeline(struct drm_plane *plane,
> struct drm_prop_enum_list *list)
>  {
> @@ -48,6 +48,38 @@ static int vkms_initialize_color_pipeline(struct
> drm_plane *plane, struct drm_pr
>  		goto cleanup;
>  	}
>  

This should also update the comment above as it still says "2nd op: 1d
curve", but 2nd op is now a 3x4 matrix.

-- 
Thanks,

Nícolas

> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane);
> +	if (ret)
> +		goto cleanup;
> +
> +	drm_colorop_set_next_property(ops[i - 1], ops[i]);
> +
> +	i++;
> +
> +	/* 3rd op: 3x4 matrix */
> +	ops[i] = kzalloc(sizeof(*ops[i]), GFP_KERNEL);
> +	if (!ops[i]) {
> +		drm_err(dev, "KMS: Failed to allocate colorop\n");
> +		ret = -ENOMEM;
> +		goto cleanup;
> +	}
> +
> +	ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane);
> +	if (ret)
> +		goto cleanup;
> +
> +	drm_colorop_set_next_property(ops[i - 1], ops[i]);
> +
> +	i++;
> +
> +	/* 4th op: 1d curve */
> +	ops[i] = kzalloc(sizeof(*ops[i]), GFP_KERNEL);
> +	if (!ops[i]) {
> +		drm_err(dev, "KMS: Failed to allocate colorop\n");
> +		ret = -ENOMEM;
> +		goto cleanup;
> +	}
> +