[PATCH 2/7] drm/mediatek: fix CCORR mtk_ctm_s31_32_to_s1_n function issue

Jay Liu posted 7 patches 10 months ago
Only 6 patches received!
There is a newer version of this series
[PATCH 2/7] drm/mediatek: fix CCORR mtk_ctm_s31_32_to_s1_n function issue
Posted by Jay Liu 10 months ago
if matrixbit is 11,
The range of color matrix is from 0 to (BIT(11) - 1).
Values from 0 to (BIT(11) - 1) represent positive numbers,
values from BIT(11) to (BIT(12) - 1) represent negative numbers.
For example, -1 need converted to 8191.

Fixes: 738ed4156fba ("drm/mediatek: Add matrix_bits private data for ccorr")

Signed-off-by: Jay Liu <jay.liu@mediatek.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_ccorr.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c b/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
index 94e82b3fa2d8..a9f91b71534b 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
@@ -100,6 +100,15 @@ static u16 mtk_ctm_s31_32_to_s1_n(u64 in, u32 n)
 		r |= (in >> (32 - n)) & GENMASK(n, 0);
 	}
 
+	/*
+	 *The range of r is from 0 to (BIT(n + 1) - 1),
+	 *where values from 0 to (BIT(n) - 1) represent positive numbers,
+	 *and values from BIT(n) to (BIT(n + 1) - 1) represent negative numbers.
+	 *For example, if n is 11, -1 will be converted to 8191.
+	 */
+	if (r & BIT(n + 1))
+		r = (~(r & GENMASK(n, 0)) + 1) & GENMASK(n + 1, 0);
+
 	return r;
 }
 
-- 
2.18.0
Re: [PATCH 2/7] drm/mediatek: fix CCORR mtk_ctm_s31_32_to_s1_n function issue
Posted by CK Hu (胡俊光) 9 months, 3 weeks ago
On Wed, 2025-02-19 at 17:20 +0800, Jay Liu wrote:
> if matrixbit is 11,
> The range of color matrix is from 0 to (BIT(11) - 1).
> Values from 0 to (BIT(11) - 1) represent positive numbers,
> values from BIT(11) to (BIT(12) - 1) represent negative numbers.
> For example, -1 need converted to 8191.
> 
> Fixes: 738ed4156fba ("drm/mediatek: Add matrix_bits private data for ccorr")
> 
> Signed-off-by: Jay Liu <jay.liu@mediatek.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ccorr.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c b/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> index 94e82b3fa2d8..a9f91b71534b 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> @@ -100,6 +100,15 @@ static u16 mtk_ctm_s31_32_to_s1_n(u64 in, u32 n)
>  		r |= (in >> (32 - n)) & GENMASK(n, 0);
>  	}
>  
> +	/*
> +	 *The range of r is from 0 to (BIT(n + 1) - 1),
> +	 *where values from 0 to (BIT(n) - 1) represent positive numbers,
> +	 *and values from BIT(n) to (BIT(n + 1) - 1) represent negative numbers.
> +	 *For example, if n is 11, -1 will be converted to 8191.
> +	 */
> +	if (r & BIT(n + 1))
> +		r = (~(r & GENMASK(n, 0)) + 1) & GENMASK(n + 1, 0);
> +

This function is to change s31.32 to s1.n, but it seems you change s31.32 to two's complement.
If so, use drm_color_ctm_s31_32_to_qm_n().
In addition, are you sure that MT8183 and MT8192 hardware use two's complement?

Regards,
CK

>  	return r;
>  }
>  

Re: [PATCH 2/7] drm/mediatek: fix CCORR mtk_ctm_s31_32_to_s1_n function issue
Posted by AngeloGioacchino Del Regno 10 months ago
Il 19/02/25 10:20, Jay Liu ha scritto:
> if matrixbit is 11,
> The range of color matrix is from 0 to (BIT(11) - 1).
> Values from 0 to (BIT(11) - 1) represent positive numbers,
> values from BIT(11) to (BIT(12) - 1) represent negative numbers.
> For example, -1 need converted to 8191.
> 
> Fixes: 738ed4156fba ("drm/mediatek: Add matrix_bits private data for ccorr")
> 
> Signed-off-by: Jay Liu <jay.liu@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>