[PATCH v2 0/4] drm/komeda: Enable CRTC color-mgmt

[PATCH v2 0/4] drm/komeda: Enable CRTC color-mgmt

[james qian wang (Arm Technology China)]

This series enable CRTC color-mgmt for komeda driver, for current komeda HW
which only supports color conversion and forward gamma for CRTC.

This series actually are regrouped from:
- drm/komeda: Enable layer/plane color-mgmt:
  https://patchwork.freedesktop.org/series/60893/

- drm/komeda: Enable CRTC color-mgmt
  https://patchwork.freedesktop.org/series/61370/

For removing the dependence on:
- https://patchwork.freedesktop.org/series/30876/

Lowry Li (Arm Technology China) (1):
  drm/komeda: Adds gamma and color-transform support for DOU-IPS

james qian wang (Arm Technology China) (3):
  drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()
  drm/komeda: Add drm_lut_to_fgamma_coeffs()
  drm/komeda: Add drm_ctm_to_coeffs()

v2:
  Move the fixpoint conversion function s31_32_to_q2_12() to drm core
  as a shared helper.

 .../arm/display/komeda/d71/d71_component.c    | 24 +++++++
 .../arm/display/komeda/komeda_color_mgmt.c    | 66 +++++++++++++++++++
 .../arm/display/komeda/komeda_color_mgmt.h    | 10 ++-
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  |  2 +
 .../drm/arm/display/komeda/komeda_pipeline.h  |  3 +
 .../display/komeda/komeda_pipeline_state.c    |  6 ++
 drivers/gpu/drm/drm_color_mgmt.c              | 23 +++++++
 include/drm/drm_color_mgmt.h                  |  2 +
 8 files changed, 135 insertions(+), 1 deletion(-)

--
2.20.1

[PATCH v2 0/4] drm/komeda: Enable CRTC color-mgmt

[james qian wang (Arm Technology China)]

This series enable CRTC color-mgmt for komeda driver, for current komeda HW
which only supports color conversion and forward gamma for CRTC.

This series actually are regrouped from:
- drm/komeda: Enable layer/plane color-mgmt:
  https://patchwork.freedesktop.org/series/60893/

- drm/komeda: Enable CRTC color-mgmt
  https://patchwork.freedesktop.org/series/61370/

For removing the dependence on:
- https://patchwork.freedesktop.org/series/30876/

Lowry Li (Arm Technology China) (1):
  drm/komeda: Adds gamma and color-transform support for DOU-IPS

james qian wang (Arm Technology China) (3):
  drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()
  drm/komeda: Add drm_lut_to_fgamma_coeffs()
  drm/komeda: Add drm_ctm_to_coeffs()

v2:
  Move the fixpoint conversion function s31_32_to_q2_12() to drm core
  as a shared helper.

 .../arm/display/komeda/d71/d71_component.c    | 24 +++++++
 .../arm/display/komeda/komeda_color_mgmt.c    | 66 +++++++++++++++++++
 .../arm/display/komeda/komeda_color_mgmt.h    | 10 ++-
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  |  2 +
 .../drm/arm/display/komeda/komeda_pipeline.h  |  3 +
 .../display/komeda/komeda_pipeline_state.c    |  6 ++
 drivers/gpu/drm/drm_color_mgmt.c              | 23 +++++++
 include/drm/drm_color_mgmt.h                  |  2 +
 8 files changed, 135 insertions(+), 1 deletion(-)

--
2.20.1

[PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[james qian wang (Arm Technology China)]

Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
hardware.

Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
---
 drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
 include/drm/drm_color_mgmt.h     |  2 ++
 2 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 4ce5c6d8de99..3d533d0b45af 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
 }
 EXPORT_SYMBOL(drm_color_lut_extract);
 
+/**
+ * drm_color_ctm_s31_32_to_qm_n
+ *
+ * @user_input: input value
+ * @m: number of integer bits
+ * @n: number of fractinal bits
+ *
+ * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
+ */
+uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
+				      uint32_t m, uint32_t n)
+{
+	u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
+	bool negative = !!(user_input & BIT_ULL(63));
+	s64 val;
+
+	/* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
+	val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
+
+	return negative ? 0ll - val : val;
+}
+EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
+
 /**
  * drm_crtc_enable_color_mgmt - enable color management properties
  * @crtc: DRM CRTC
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index d1c662d92ab7..60fea5501886 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -30,6 +30,8 @@ struct drm_crtc;
 struct drm_plane;
 
 uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
+uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
+				      uint32_t m, uint32_t n);
 
 void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 				uint degamma_lut_size,
-- 
2.20.1

[PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[james qian wang (Arm Technology China)]

Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
hardware.

Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
---
 drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
 include/drm/drm_color_mgmt.h     |  2 ++
 2 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 4ce5c6d8de99..3d533d0b45af 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
 }
 EXPORT_SYMBOL(drm_color_lut_extract);
 
+/**
+ * drm_color_ctm_s31_32_to_qm_n
+ *
+ * @user_input: input value
+ * @m: number of integer bits
+ * @n: number of fractinal bits
+ *
+ * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
+ */
+uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
+				      uint32_t m, uint32_t n)
+{
+	u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
+	bool negative = !!(user_input & BIT_ULL(63));
+	s64 val;
+
+	/* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
+	val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
+
+	return negative ? 0ll - val : val;
+}
+EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
+
 /**
  * drm_crtc_enable_color_mgmt - enable color management properties
  * @crtc: DRM CRTC
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index d1c662d92ab7..60fea5501886 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -30,6 +30,8 @@ struct drm_crtc;
 struct drm_plane;
 
 uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
+uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
+				      uint32_t m, uint32_t n);
 
 void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 				uint degamma_lut_size,
-- 
2.20.1

[PATCH v2 2/4] drm/komeda: Add drm_lut_to_fgamma_coeffs()

[james qian wang (Arm Technology China)]

This function is used to convert drm 3dlut to komeda HW required 1d curve
coeffs values.

Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
---
 .../arm/display/komeda/komeda_color_mgmt.c    | 52 +++++++++++++++++++
 .../arm/display/komeda/komeda_color_mgmt.h    |  9 +++-
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
index 9d14a92dbb17..c180ce70c26c 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
@@ -65,3 +65,55 @@ const s32 *komeda_select_yuv2rgb_coeffs(u32 color_encoding, u32 color_range)
 
 	return coeffs;
 }
+
+struct gamma_curve_sector {
+	u32 boundary_start;
+	u32 num_of_segments;
+	u32 segment_width;
+};
+
+struct gamma_curve_segment {
+	u32 start;
+	u32 end;
+};
+
+static struct gamma_curve_sector sector_tbl[] = {
+	{ 0,    4,  4   },
+	{ 16,   4,  4   },
+	{ 32,   4,  8   },
+	{ 64,   4,  16  },
+	{ 128,  4,  32  },
+	{ 256,  4,  64  },
+	{ 512,  16, 32  },
+	{ 1024, 24, 128 },
+};
+
+static void
+drm_lut_to_coeffs(struct drm_property_blob *lut_blob, u32 *coeffs,
+		  struct gamma_curve_sector *sector_tbl, u32 num_sectors)
+{
+	struct drm_color_lut *lut;
+	u32 i, j, in, num = 0;
+
+	if (!lut_blob)
+		return;
+
+	lut = lut_blob->data;
+
+	for (i = 0; i < num_sectors; i++) {
+		for (j = 0; j < sector_tbl[i].num_of_segments; j++) {
+			in = sector_tbl[i].boundary_start +
+			     j * sector_tbl[i].segment_width;
+
+			coeffs[num++] = drm_color_lut_extract(lut[in].red,
+						KOMEDA_COLOR_PRECISION);
+		}
+	}
+
+	coeffs[num] = BIT(KOMEDA_COLOR_PRECISION);
+}
+
+void drm_lut_to_fgamma_coeffs(struct drm_property_blob *lut_blob, u32 *coeffs)
+{
+	drm_lut_to_coeffs(lut_blob, coeffs, sector_tbl, ARRAY_SIZE(sector_tbl));
+}
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h
index a2df218f58e7..08ab69281648 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h
@@ -11,7 +11,14 @@
 #include <drm/drm_color_mgmt.h>
 
 #define KOMEDA_N_YUV2RGB_COEFFS		12
+#define KOMEDA_N_RGB2YUV_COEFFS		12
+#define KOMEDA_COLOR_PRECISION		12
+#define KOMEDA_N_GAMMA_COEFFS		65
+#define KOMEDA_COLOR_LUT_SIZE		BIT(KOMEDA_COLOR_PRECISION)
+#define KOMEDA_N_CTM_COEFFS		9
+
+void drm_lut_to_fgamma_coeffs(struct drm_property_blob *lut_blob, u32 *coeffs);
 
 const s32 *komeda_select_yuv2rgb_coeffs(u32 color_encoding, u32 color_range);
 
-#endif
+#endif /*_KOMEDA_COLOR_MGMT_H_*/
-- 
2.20.1

[PATCH v2 2/4] drm/komeda: Add drm_lut_to_fgamma_coeffs()

[james qian wang (Arm Technology China)]

This function is used to convert drm 3dlut to komeda HW required 1d curve
coeffs values.

Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
---
 .../arm/display/komeda/komeda_color_mgmt.c    | 52 +++++++++++++++++++
 .../arm/display/komeda/komeda_color_mgmt.h    |  9 +++-
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
index 9d14a92dbb17..c180ce70c26c 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
@@ -65,3 +65,55 @@ const s32 *komeda_select_yuv2rgb_coeffs(u32 color_encoding, u32 color_range)
 
 	return coeffs;
 }
+
+struct gamma_curve_sector {
+	u32 boundary_start;
+	u32 num_of_segments;
+	u32 segment_width;
+};
+
+struct gamma_curve_segment {
+	u32 start;
+	u32 end;
+};
+
+static struct gamma_curve_sector sector_tbl[] = {
+	{ 0,    4,  4   },
+	{ 16,   4,  4   },
+	{ 32,   4,  8   },
+	{ 64,   4,  16  },
+	{ 128,  4,  32  },
+	{ 256,  4,  64  },
+	{ 512,  16, 32  },
+	{ 1024, 24, 128 },
+};
+
+static void
+drm_lut_to_coeffs(struct drm_property_blob *lut_blob, u32 *coeffs,
+		  struct gamma_curve_sector *sector_tbl, u32 num_sectors)
+{
+	struct drm_color_lut *lut;
+	u32 i, j, in, num = 0;
+
+	if (!lut_blob)
+		return;
+
+	lut = lut_blob->data;
+
+	for (i = 0; i < num_sectors; i++) {
+		for (j = 0; j < sector_tbl[i].num_of_segments; j++) {
+			in = sector_tbl[i].boundary_start +
+			     j * sector_tbl[i].segment_width;
+
+			coeffs[num++] = drm_color_lut_extract(lut[in].red,
+						KOMEDA_COLOR_PRECISION);
+		}
+	}
+
+	coeffs[num] = BIT(KOMEDA_COLOR_PRECISION);
+}
+
+void drm_lut_to_fgamma_coeffs(struct drm_property_blob *lut_blob, u32 *coeffs)
+{
+	drm_lut_to_coeffs(lut_blob, coeffs, sector_tbl, ARRAY_SIZE(sector_tbl));
+}
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h
index a2df218f58e7..08ab69281648 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h
@@ -11,7 +11,14 @@
 #include <drm/drm_color_mgmt.h>
 
 #define KOMEDA_N_YUV2RGB_COEFFS		12
+#define KOMEDA_N_RGB2YUV_COEFFS		12
+#define KOMEDA_COLOR_PRECISION		12
+#define KOMEDA_N_GAMMA_COEFFS		65
+#define KOMEDA_COLOR_LUT_SIZE		BIT(KOMEDA_COLOR_PRECISION)
+#define KOMEDA_N_CTM_COEFFS		9
+
+void drm_lut_to_fgamma_coeffs(struct drm_property_blob *lut_blob, u32 *coeffs);
 
 const s32 *komeda_select_yuv2rgb_coeffs(u32 color_encoding, u32 color_range);
 
-#endif
+#endif /*_KOMEDA_COLOR_MGMT_H_*/
-- 
2.20.1

[PATCH v2 3/4] drm/komeda: Add drm_ctm_to_coeffs()

[james qian wang (Arm Technology China)]

This function is for converting drm_color_ctm matrix to komeda hardware
required required Q2.12 2's complement CSC matrix.

v2:
  Move the fixpoint conversion function s31_32_to_q2_12() to drm core
  as a shared helper.

Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
---
 .../gpu/drm/arm/display/komeda/komeda_color_mgmt.c | 14 ++++++++++++++
 .../gpu/drm/arm/display/komeda/komeda_color_mgmt.h |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
index c180ce70c26c..ad668accbdf4 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
@@ -117,3 +117,17 @@ void drm_lut_to_fgamma_coeffs(struct drm_property_blob *lut_blob, u32 *coeffs)
 {
 	drm_lut_to_coeffs(lut_blob, coeffs, sector_tbl, ARRAY_SIZE(sector_tbl));
 }
+
+void drm_ctm_to_coeffs(struct drm_property_blob *ctm_blob, u32 *coeffs)
+{
+	struct drm_color_ctm *ctm;
+	u32 i;
+
+	if (!ctm_blob)
+		return;
+
+	ctm = ctm_blob->data;
+
+	for (i = 0; i < KOMEDA_N_CTM_COEFFS; i++)
+		coeffs[i] = drm_color_ctm_s31_32_to_qm_n(ctm->matrix[i], 2, 12);
+}
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h
index 08ab69281648..2f4668466112 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h
@@ -18,6 +18,7 @@
 #define KOMEDA_N_CTM_COEFFS		9
 
 void drm_lut_to_fgamma_coeffs(struct drm_property_blob *lut_blob, u32 *coeffs);
+void drm_ctm_to_coeffs(struct drm_property_blob *ctm_blob, u32 *coeffs);
 
 const s32 *komeda_select_yuv2rgb_coeffs(u32 color_encoding, u32 color_range);
 
-- 
2.20.1

[PATCH v2 3/4] drm/komeda: Add drm_ctm_to_coeffs()

[james qian wang (Arm Technology China)]

This function is for converting drm_color_ctm matrix to komeda hardware
required required Q2.12 2's complement CSC matrix.

v2:
  Move the fixpoint conversion function s31_32_to_q2_12() to drm core
  as a shared helper.

Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
---
 .../gpu/drm/arm/display/komeda/komeda_color_mgmt.c | 14 ++++++++++++++
 .../gpu/drm/arm/display/komeda/komeda_color_mgmt.h |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
index c180ce70c26c..ad668accbdf4 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
@@ -117,3 +117,17 @@ void drm_lut_to_fgamma_coeffs(struct drm_property_blob *lut_blob, u32 *coeffs)
 {
 	drm_lut_to_coeffs(lut_blob, coeffs, sector_tbl, ARRAY_SIZE(sector_tbl));
 }
+
+void drm_ctm_to_coeffs(struct drm_property_blob *ctm_blob, u32 *coeffs)
+{
+	struct drm_color_ctm *ctm;
+	u32 i;
+
+	if (!ctm_blob)
+		return;
+
+	ctm = ctm_blob->data;
+
+	for (i = 0; i < KOMEDA_N_CTM_COEFFS; i++)
+		coeffs[i] = drm_color_ctm_s31_32_to_qm_n(ctm->matrix[i], 2, 12);
+}
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h
index 08ab69281648..2f4668466112 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.h
@@ -18,6 +18,7 @@
 #define KOMEDA_N_CTM_COEFFS		9
 
 void drm_lut_to_fgamma_coeffs(struct drm_property_blob *lut_blob, u32 *coeffs);
+void drm_ctm_to_coeffs(struct drm_property_blob *ctm_blob, u32 *coeffs);
 
 const s32 *komeda_select_yuv2rgb_coeffs(u32 color_encoding, u32 color_range);
 
-- 
2.20.1

[PATCH v2 4/4] drm/komeda: Adds gamma and color-transform support for DOU-IPS

[james qian wang (Arm Technology China)]

From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>

Adds gamma and color-transform support for DOU-IPS.
Adds two caps members fgamma_coeffs and ctm_coeffs to komeda_improc_state.
If color management changed, set gamma and color-transform accordingly.

Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
---
 .../arm/display/komeda/d71/d71_component.c    | 24 +++++++++++++++++++
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  |  2 ++
 .../drm/arm/display/komeda/komeda_pipeline.h  |  3 +++
 .../display/komeda/komeda_pipeline_state.c    |  6 +++++
 4 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
index c3d29c0b051b..e7e5a8e4430e 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
@@ -942,15 +942,39 @@ static int d71_merger_init(struct d71_dev *d71,
 static void d71_improc_update(struct komeda_component *c,
 			      struct komeda_component_state *state)
 {
+	struct drm_crtc_state *crtc_st = state->crtc->state;
 	struct komeda_improc_state *st = to_improc_st(state);
+	struct d71_pipeline *pipe = to_d71_pipeline(c->pipeline);
 	u32 __iomem *reg = c->reg;
 	u32 index;
+	u32 mask = 0, ctrl = 0;
 
 	for_each_changed_input(state, index)
 		malidp_write32(reg, BLK_INPUT_ID0 + index * 4,
 			       to_d71_input_id(state, index));
 
 	malidp_write32(reg, BLK_SIZE, HV_SIZE(st->hsize, st->vsize));
+
+	if (crtc_st->color_mgmt_changed) {
+		mask |= IPS_CTRL_FT | IPS_CTRL_RGB;
+
+		if (crtc_st->gamma_lut) {
+			malidp_write_group(pipe->dou_ft_coeff_addr, FT_COEFF0,
+					   KOMEDA_N_GAMMA_COEFFS,
+					   st->fgamma_coeffs);
+			ctrl |= IPS_CTRL_FT; /* enable gamma */
+		}
+
+		if (crtc_st->ctm) {
+			malidp_write_group(reg, IPS_RGB_RGB_COEFF0,
+					   KOMEDA_N_CTM_COEFFS,
+					   st->ctm_coeffs);
+			ctrl |= IPS_CTRL_RGB; /* enable gamut */
+		}
+	}
+
+	if (mask)
+		malidp_write32_mask(reg, BLK_CONTROL, mask, ctrl);
 }
 
 static void d71_improc_dump(struct komeda_component *c, struct seq_file *sf)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 9beeda04818b..406b9d0ca058 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -590,6 +590,8 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
 
 	crtc->port = kcrtc->master->of_output_port;
 
+	drm_crtc_enable_color_mgmt(crtc, 0, true, KOMEDA_COLOR_LUT_SIZE);
+
 	return err;
 }
 
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
index b322f52ba8f2..c5ab8096c85d 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
@@ -11,6 +11,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include "malidp_utils.h"
+#include "komeda_color_mgmt.h"
 
 #define KOMEDA_MAX_PIPELINES		2
 #define KOMEDA_PIPELINE_MAX_LAYERS	4
@@ -324,6 +325,8 @@ struct komeda_improc {
 struct komeda_improc_state {
 	struct komeda_component_state base;
 	u16 hsize, vsize;
+	u32 fgamma_coeffs[KOMEDA_N_GAMMA_COEFFS];
+	u32 ctm_coeffs[KOMEDA_N_CTM_COEFFS];
 };
 
 /* display timing controller */
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index 0ba9c6aa3708..4a40b37eb1a6 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -756,6 +756,12 @@ komeda_improc_validate(struct komeda_improc *improc,
 	st->hsize = dflow->in_w;
 	st->vsize = dflow->in_h;
 
+	if (kcrtc_st->base.color_mgmt_changed) {
+		drm_lut_to_fgamma_coeffs(kcrtc_st->base.gamma_lut,
+					 st->fgamma_coeffs);
+		drm_ctm_to_coeffs(kcrtc_st->base.ctm, st->ctm_coeffs);
+	}
+
 	komeda_component_add_input(&st->base, &dflow->input, 0);
 	komeda_component_set_output(&dflow->input, &improc->base, 0);
 
-- 
2.20.1

[PATCH v2 4/4] drm/komeda: Adds gamma and color-transform support for DOU-IPS

[james qian wang (Arm Technology China)]

From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>

Adds gamma and color-transform support for DOU-IPS.
Adds two caps members fgamma_coeffs and ctm_coeffs to komeda_improc_state.
If color management changed, set gamma and color-transform accordingly.

Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
---
 .../arm/display/komeda/d71/d71_component.c    | 24 +++++++++++++++++++
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  |  2 ++
 .../drm/arm/display/komeda/komeda_pipeline.h  |  3 +++
 .../display/komeda/komeda_pipeline_state.c    |  6 +++++
 4 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
index c3d29c0b051b..e7e5a8e4430e 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
@@ -942,15 +942,39 @@ static int d71_merger_init(struct d71_dev *d71,
 static void d71_improc_update(struct komeda_component *c,
 			      struct komeda_component_state *state)
 {
+	struct drm_crtc_state *crtc_st = state->crtc->state;
 	struct komeda_improc_state *st = to_improc_st(state);
+	struct d71_pipeline *pipe = to_d71_pipeline(c->pipeline);
 	u32 __iomem *reg = c->reg;
 	u32 index;
+	u32 mask = 0, ctrl = 0;
 
 	for_each_changed_input(state, index)
 		malidp_write32(reg, BLK_INPUT_ID0 + index * 4,
 			       to_d71_input_id(state, index));
 
 	malidp_write32(reg, BLK_SIZE, HV_SIZE(st->hsize, st->vsize));
+
+	if (crtc_st->color_mgmt_changed) {
+		mask |= IPS_CTRL_FT | IPS_CTRL_RGB;
+
+		if (crtc_st->gamma_lut) {
+			malidp_write_group(pipe->dou_ft_coeff_addr, FT_COEFF0,
+					   KOMEDA_N_GAMMA_COEFFS,
+					   st->fgamma_coeffs);
+			ctrl |= IPS_CTRL_FT; /* enable gamma */
+		}
+
+		if (crtc_st->ctm) {
+			malidp_write_group(reg, IPS_RGB_RGB_COEFF0,
+					   KOMEDA_N_CTM_COEFFS,
+					   st->ctm_coeffs);
+			ctrl |= IPS_CTRL_RGB; /* enable gamut */
+		}
+	}
+
+	if (mask)
+		malidp_write32_mask(reg, BLK_CONTROL, mask, ctrl);
 }
 
 static void d71_improc_dump(struct komeda_component *c, struct seq_file *sf)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 9beeda04818b..406b9d0ca058 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -590,6 +590,8 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
 
 	crtc->port = kcrtc->master->of_output_port;
 
+	drm_crtc_enable_color_mgmt(crtc, 0, true, KOMEDA_COLOR_LUT_SIZE);
+
 	return err;
 }
 
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
index b322f52ba8f2..c5ab8096c85d 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
@@ -11,6 +11,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include "malidp_utils.h"
+#include "komeda_color_mgmt.h"
 
 #define KOMEDA_MAX_PIPELINES		2
 #define KOMEDA_PIPELINE_MAX_LAYERS	4
@@ -324,6 +325,8 @@ struct komeda_improc {
 struct komeda_improc_state {
 	struct komeda_component_state base;
 	u16 hsize, vsize;
+	u32 fgamma_coeffs[KOMEDA_N_GAMMA_COEFFS];
+	u32 ctm_coeffs[KOMEDA_N_CTM_COEFFS];
 };
 
 /* display timing controller */
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index 0ba9c6aa3708..4a40b37eb1a6 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -756,6 +756,12 @@ komeda_improc_validate(struct komeda_improc *improc,
 	st->hsize = dflow->in_w;
 	st->vsize = dflow->in_h;
 
+	if (kcrtc_st->base.color_mgmt_changed) {
+		drm_lut_to_fgamma_coeffs(kcrtc_st->base.gamma_lut,
+					 st->fgamma_coeffs);
+		drm_ctm_to_coeffs(kcrtc_st->base.ctm, st->ctm_coeffs);
+	}
+
 	komeda_component_add_input(&st->base, &dflow->input, 0);
 	komeda_component_set_output(&dflow->input, &improc->base, 0);
 
-- 
2.20.1

Re: [PATCH v2 4/4] drm/komeda: Adds gamma and color-transform support for DOU-IPS

[sandy.huang]

在 2019/10/11 下午1:43, james qian wang (Arm Technology China) 写道:
> From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
>
> Adds gamma and color-transform support for DOU-IPS.
> Adds two caps members fgamma_coeffs and ctm_coeffs to komeda_improc_state.
> If color management changed, set gamma and color-transform accordingly.
>
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
>   .../arm/display/komeda/d71/d71_component.c    | 24 +++++++++++++++++++
>   .../gpu/drm/arm/display/komeda/komeda_crtc.c  |  2 ++
>   .../drm/arm/display/komeda/komeda_pipeline.h  |  3 +++
>   .../display/komeda/komeda_pipeline_state.c    |  6 +++++
>   4 files changed, 35 insertions(+)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> index c3d29c0b051b..e7e5a8e4430e 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -942,15 +942,39 @@ static int d71_merger_init(struct d71_dev *d71,
>   static void d71_improc_update(struct komeda_component *c,
>   			      struct komeda_component_state *state)
>   {
> +	struct drm_crtc_state *crtc_st = state->crtc->state;
>   	struct komeda_improc_state *st = to_improc_st(state);
> +	struct d71_pipeline *pipe = to_d71_pipeline(c->pipeline);
>   	u32 __iomem *reg = c->reg;
>   	u32 index;
> +	u32 mask = 0, ctrl = 0;
>   
>   	for_each_changed_input(state, index)
>   		malidp_write32(reg, BLK_INPUT_ID0 + index * 4,
>   			       to_d71_input_id(state, index));
>   
>   	malidp_write32(reg, BLK_SIZE, HV_SIZE(st->hsize, st->vsize));
> +
> +	if (crtc_st->color_mgmt_changed) {
> +		mask |= IPS_CTRL_FT | IPS_CTRL_RGB;
> +
> +		if (crtc_st->gamma_lut) {
> +			malidp_write_group(pipe->dou_ft_coeff_addr, FT_COEFF0,
> +					   KOMEDA_N_GAMMA_COEFFS,
> +					   st->fgamma_coeffs);
> +			ctrl |= IPS_CTRL_FT; /* enable gamma */
> +		}
> +
> +		if (crtc_st->ctm) {
> +			malidp_write_group(reg, IPS_RGB_RGB_COEFF0,
> +					   KOMEDA_N_CTM_COEFFS,
> +					   st->ctm_coeffs);
> +			ctrl |= IPS_CTRL_RGB; /* enable gamut */
> +		}
> +	}
> +
> +	if (mask)
> +		malidp_write32_mask(reg, BLK_CONTROL, mask, ctrl);
>   }
There is no need or no method to disable/bypass the gamma and gamut?
>   
>   static void d71_improc_dump(struct komeda_component *c, struct seq_file *sf)
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 9beeda04818b..406b9d0ca058 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -590,6 +590,8 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
>   
>   	crtc->port = kcrtc->master->of_output_port;
>   
> +	drm_crtc_enable_color_mgmt(crtc, 0, true, KOMEDA_COLOR_LUT_SIZE);
> +
>   	return err;
>   }
>   
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> index b322f52ba8f2..c5ab8096c85d 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> @@ -11,6 +11,7 @@
>   #include <drm/drm_atomic.h>
>   #include <drm/drm_atomic_helper.h>
>   #include "malidp_utils.h"
> +#include "komeda_color_mgmt.h"
>   
>   #define KOMEDA_MAX_PIPELINES		2
>   #define KOMEDA_PIPELINE_MAX_LAYERS	4
> @@ -324,6 +325,8 @@ struct komeda_improc {
>   struct komeda_improc_state {
>   	struct komeda_component_state base;
>   	u16 hsize, vsize;
> +	u32 fgamma_coeffs[KOMEDA_N_GAMMA_COEFFS];
> +	u32 ctm_coeffs[KOMEDA_N_CTM_COEFFS];
>   };
>   
>   /* display timing controller */
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> index 0ba9c6aa3708..4a40b37eb1a6 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -756,6 +756,12 @@ komeda_improc_validate(struct komeda_improc *improc,
>   	st->hsize = dflow->in_w;
>   	st->vsize = dflow->in_h;
>   
> +	if (kcrtc_st->base.color_mgmt_changed) {
> +		drm_lut_to_fgamma_coeffs(kcrtc_st->base.gamma_lut,
> +					 st->fgamma_coeffs);
> +		drm_ctm_to_coeffs(kcrtc_st->base.ctm, st->ctm_coeffs);
> +	}
> +
>   	komeda_component_add_input(&st->base, &dflow->input, 0);
>   	komeda_component_set_output(&dflow->input, &improc->base, 0);
>   

Re: [PATCH v2 4/4] drm/komeda: Adds gamma and color-transform support for DOU-IPS

[sandy.huang]

在 2019/10/11 下午1:43, james qian wang (Arm Technology China) 写道:
> From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
>
> Adds gamma and color-transform support for DOU-IPS.
> Adds two caps members fgamma_coeffs and ctm_coeffs to komeda_improc_state.
> If color management changed, set gamma and color-transform accordingly.
>
> Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> ---
>   .../arm/display/komeda/d71/d71_component.c    | 24 +++++++++++++++++++
>   .../gpu/drm/arm/display/komeda/komeda_crtc.c  |  2 ++
>   .../drm/arm/display/komeda/komeda_pipeline.h  |  3 +++
>   .../display/komeda/komeda_pipeline_state.c    |  6 +++++
>   4 files changed, 35 insertions(+)
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> index c3d29c0b051b..e7e5a8e4430e 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -942,15 +942,39 @@ static int d71_merger_init(struct d71_dev *d71,
>   static void d71_improc_update(struct komeda_component *c,
>   			      struct komeda_component_state *state)
>   {
> +	struct drm_crtc_state *crtc_st = state->crtc->state;
>   	struct komeda_improc_state *st = to_improc_st(state);
> +	struct d71_pipeline *pipe = to_d71_pipeline(c->pipeline);
>   	u32 __iomem *reg = c->reg;
>   	u32 index;
> +	u32 mask = 0, ctrl = 0;
>   
>   	for_each_changed_input(state, index)
>   		malidp_write32(reg, BLK_INPUT_ID0 + index * 4,
>   			       to_d71_input_id(state, index));
>   
>   	malidp_write32(reg, BLK_SIZE, HV_SIZE(st->hsize, st->vsize));
> +
> +	if (crtc_st->color_mgmt_changed) {
> +		mask |= IPS_CTRL_FT | IPS_CTRL_RGB;
> +
> +		if (crtc_st->gamma_lut) {
> +			malidp_write_group(pipe->dou_ft_coeff_addr, FT_COEFF0,
> +					   KOMEDA_N_GAMMA_COEFFS,
> +					   st->fgamma_coeffs);
> +			ctrl |= IPS_CTRL_FT; /* enable gamma */
> +		}
> +
> +		if (crtc_st->ctm) {
> +			malidp_write_group(reg, IPS_RGB_RGB_COEFF0,
> +					   KOMEDA_N_CTM_COEFFS,
> +					   st->ctm_coeffs);
> +			ctrl |= IPS_CTRL_RGB; /* enable gamut */
> +		}
> +	}
> +
> +	if (mask)
> +		malidp_write32_mask(reg, BLK_CONTROL, mask, ctrl);
>   }
There is no need or no method to disable/bypass the gamma and gamut?
>   
>   static void d71_improc_dump(struct komeda_component *c, struct seq_file *sf)
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 9beeda04818b..406b9d0ca058 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -590,6 +590,8 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
>   
>   	crtc->port = kcrtc->master->of_output_port;
>   
> +	drm_crtc_enable_color_mgmt(crtc, 0, true, KOMEDA_COLOR_LUT_SIZE);
> +
>   	return err;
>   }
>   
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> index b322f52ba8f2..c5ab8096c85d 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> @@ -11,6 +11,7 @@
>   #include <drm/drm_atomic.h>
>   #include <drm/drm_atomic_helper.h>
>   #include "malidp_utils.h"
> +#include "komeda_color_mgmt.h"
>   
>   #define KOMEDA_MAX_PIPELINES		2
>   #define KOMEDA_PIPELINE_MAX_LAYERS	4
> @@ -324,6 +325,8 @@ struct komeda_improc {
>   struct komeda_improc_state {
>   	struct komeda_component_state base;
>   	u16 hsize, vsize;
> +	u32 fgamma_coeffs[KOMEDA_N_GAMMA_COEFFS];
> +	u32 ctm_coeffs[KOMEDA_N_CTM_COEFFS];
>   };
>   
>   /* display timing controller */
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> index 0ba9c6aa3708..4a40b37eb1a6 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -756,6 +756,12 @@ komeda_improc_validate(struct komeda_improc *improc,
>   	st->hsize = dflow->in_w;
>   	st->vsize = dflow->in_h;
>   
> +	if (kcrtc_st->base.color_mgmt_changed) {
> +		drm_lut_to_fgamma_coeffs(kcrtc_st->base.gamma_lut,
> +					 st->fgamma_coeffs);
> +		drm_ctm_to_coeffs(kcrtc_st->base.ctm, st->ctm_coeffs);
> +	}
> +
>   	komeda_component_add_input(&st->base, &dflow->input, 0);
>   	komeda_component_set_output(&dflow->input, &improc->base, 0);
>   


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 4/4] drm/komeda: Adds gamma and color-transform support for DOU-IPS

[james qian wang (Arm Technology China)]

On Fri, Oct 11, 2019 at 02:21:39PM +0800, sandy.huang wrote:
> 
> 在 2019/10/11 下午1:43, james qian wang (Arm Technology China) 写道:
> > From: "Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>
> > 
> > Adds gamma and color-transform support for DOU-IPS.
> > Adds two caps members fgamma_coeffs and ctm_coeffs to komeda_improc_state.
> > If color management changed, set gamma and color-transform accordingly.
> > 
> > Signed-off-by: Lowry Li (Arm Technology China) <lowry.li@arm.com>
> > ---
> >   .../arm/display/komeda/d71/d71_component.c    | 24 +++++++++++++++++++
> >   .../gpu/drm/arm/display/komeda/komeda_crtc.c  |  2 ++
> >   .../drm/arm/display/komeda/komeda_pipeline.h  |  3 +++
> >   .../display/komeda/komeda_pipeline_state.c    |  6 +++++
> >   4 files changed, 35 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > index c3d29c0b051b..e7e5a8e4430e 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> > @@ -942,15 +942,39 @@ static int d71_merger_init(struct d71_dev *d71,
> >   static void d71_improc_update(struct komeda_component *c,
> >   			      struct komeda_component_state *state)
> >   {
> > +	struct drm_crtc_state *crtc_st = state->crtc->state;
> >   	struct komeda_improc_state *st = to_improc_st(state);
> > +	struct d71_pipeline *pipe = to_d71_pipeline(c->pipeline);
> >   	u32 __iomem *reg = c->reg;
> >   	u32 index;
> > +	u32 mask = 0, ctrl = 0;
> >   	for_each_changed_input(state, index)
> >   		malidp_write32(reg, BLK_INPUT_ID0 + index * 4,
> >   			       to_d71_input_id(state, index));
> >   	malidp_write32(reg, BLK_SIZE, HV_SIZE(st->hsize, st->vsize));
> > +
> > +	if (crtc_st->color_mgmt_changed) {
> > +		mask |= IPS_CTRL_FT | IPS_CTRL_RGB;

NOTE: only when color_mgmt_changed we mark the mask

> > +
> > +		if (crtc_st->gamma_lut) {
> > +			malidp_write_group(pipe->dou_ft_coeff_addr, FT_COEFF0,
> > +					   KOMEDA_N_GAMMA_COEFFS,
> > +					   st->fgamma_coeffs);
> > +			ctrl |= IPS_CTRL_FT; /* enable gamma */

Here we enable the gamma, if the change include gamma_lut updating,
and if gammu_lut is NULL, the En-Gamma bit is 0, and gamma will be
disabled

> > +		}
> > +
> > +		if (crtc_st->ctm) {
> > +			malidp_write_group(reg, IPS_RGB_RGB_COEFF0,
> > +					   KOMEDA_N_CTM_COEFFS,
> > +					   st->ctm_coeffs);
> > +			ctrl |= IPS_CTRL_RGB; /* enable gamut */
> > +		}
> > +	}
> > +
> > +	if (mask)
> > +		malidp_write32_mask(reg, BLK_CONTROL, mask, ctrl);

We do have the support to disable gamma/gamut.

the ctrl is for en/dis gamma/gamut, the mask is for indicating which ctrl
bits need to be updated.
See my comments in the code.

Thanks
james

> >   }
> There is no need or no method to disable/bypass the gamma and gamut?
> >   static void d71_improc_dump(struct komeda_component *c, struct seq_file *sf)
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > index 9beeda04818b..406b9d0ca058 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> > @@ -590,6 +590,8 @@ static int komeda_crtc_add(struct komeda_kms_dev *kms,
> >   	crtc->port = kcrtc->master->of_output_port;
> > +	drm_crtc_enable_color_mgmt(crtc, 0, true, KOMEDA_COLOR_LUT_SIZE);
> > +
> >   	return err;
> >   }
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > index b322f52ba8f2..c5ab8096c85d 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> > @@ -11,6 +11,7 @@
> >   #include <drm/drm_atomic.h>
> >   #include <drm/drm_atomic_helper.h>
> >   #include "malidp_utils.h"
> > +#include "komeda_color_mgmt.h"
> >   #define KOMEDA_MAX_PIPELINES		2
> >   #define KOMEDA_PIPELINE_MAX_LAYERS	4
> > @@ -324,6 +325,8 @@ struct komeda_improc {
> >   struct komeda_improc_state {
> >   	struct komeda_component_state base;
> >   	u16 hsize, vsize;
> > +	u32 fgamma_coeffs[KOMEDA_N_GAMMA_COEFFS];
> > +	u32 ctm_coeffs[KOMEDA_N_CTM_COEFFS];
> >   };
> >   /* display timing controller */
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > index 0ba9c6aa3708..4a40b37eb1a6 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> > @@ -756,6 +756,12 @@ komeda_improc_validate(struct komeda_improc *improc,
> >   	st->hsize = dflow->in_w;
> >   	st->vsize = dflow->in_h;
> > +	if (kcrtc_st->base.color_mgmt_changed) {
> > +		drm_lut_to_fgamma_coeffs(kcrtc_st->base.gamma_lut,
> > +					 st->fgamma_coeffs);
> > +		drm_ctm_to_coeffs(kcrtc_st->base.ctm, st->ctm_coeffs);
> > +	}
> > +
> >   	komeda_component_add_input(&st->base, &dflow->input, 0);
> >   	komeda_component_set_output(&dflow->input, &improc->base, 0);
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[Daniel Vetter]

On Fri, Oct 11, 2019 at 05:43:09AM +0000, james qian wang (Arm Technology China) wrote:
> Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> hardware.
> 
> Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> ---
>  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
>  include/drm/drm_color_mgmt.h     |  2 ++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 4ce5c6d8de99..3d533d0b45af 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
>  }
>  EXPORT_SYMBOL(drm_color_lut_extract);
>  
> +/**
> + * drm_color_ctm_s31_32_to_qm_n
> + *
> + * @user_input: input value
> + * @m: number of integer bits
> + * @n: number of fractinal bits
> + *
> + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.

What's the Q meaning here? Also maybe specify that the higher bits above
m+n are cleared to all 0 or all 1. Unit test would be lovely too. Anyway:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> + */
> +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> +				      uint32_t m, uint32_t n)
> +{
> +	u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> +	bool negative = !!(user_input & BIT_ULL(63));
> +	s64 val;
> +
> +	/* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> +	val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
> +
> +	return negative ? 0ll - val : val;
> +}
> +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> +
>  /**
>   * drm_crtc_enable_color_mgmt - enable color management properties
>   * @crtc: DRM CRTC
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index d1c662d92ab7..60fea5501886 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -30,6 +30,8 @@ struct drm_crtc;
>  struct drm_plane;
>  
>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> +				      uint32_t m, uint32_t n);
>  
>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  				uint degamma_lut_size,
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[Daniel Vetter]

On Fri, Oct 11, 2019 at 05:43:09AM +0000, james qian wang (Arm Technology China) wrote:
> Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> hardware.
> 
> Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> ---
>  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
>  include/drm/drm_color_mgmt.h     |  2 ++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 4ce5c6d8de99..3d533d0b45af 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
>  }
>  EXPORT_SYMBOL(drm_color_lut_extract);
>  
> +/**
> + * drm_color_ctm_s31_32_to_qm_n
> + *
> + * @user_input: input value
> + * @m: number of integer bits
> + * @n: number of fractinal bits
> + *
> + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.

What's the Q meaning here? Also maybe specify that the higher bits above
m+n are cleared to all 0 or all 1. Unit test would be lovely too. Anyway:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> + */
> +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> +				      uint32_t m, uint32_t n)
> +{
> +	u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> +	bool negative = !!(user_input & BIT_ULL(63));
> +	s64 val;
> +
> +	/* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> +	val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
> +
> +	return negative ? 0ll - val : val;
> +}
> +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> +
>  /**
>   * drm_crtc_enable_color_mgmt - enable color management properties
>   * @crtc: DRM CRTC
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index d1c662d92ab7..60fea5501886 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -30,6 +30,8 @@ struct drm_crtc;
>  struct drm_plane;
>  
>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> +				      uint32_t m, uint32_t n);
>  
>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  				uint degamma_lut_size,
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[james qian wang (Arm Technology China)]

On Mon, Oct 14, 2019 at 10:56:05AM +0200, Daniel Vetter wrote:
> On Fri, Oct 11, 2019 at 05:43:09AM +0000, james qian wang (Arm Technology China) wrote:
> > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> > hardware.
> >
> > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> > ---
> >  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
> >  include/drm/drm_color_mgmt.h     |  2 ++
> >  2 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > index 4ce5c6d8de99..3d533d0b45af 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> >  }
> >  EXPORT_SYMBOL(drm_color_lut_extract);
> >
> > +/**
> > + * drm_color_ctm_s31_32_to_qm_n
> > + *
> > + * @user_input: input value
> > + * @m: number of integer bits
> > + * @n: number of fractinal bits
> > + *
> > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
>
> What's the Q meaning here? Also maybe specify that the higher bits above
> m+n are cleared to all 0 or all 1. Unit test would be lovely too. Anyway:

The Q used to represent signed two's complement.

For detail: https://en.wikipedia.org/wiki/Q_(number_format)

And it Q is 2's complement, so the value of higher bit equal to
sign-bit.
All 1 if it is negative
0 if it is positive.

James

> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> > + */
> > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > +                                 uint32_t m, uint32_t n)
> > +{
> > +   u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > +   bool negative = !!(user_input & BIT_ULL(63));
> > +   s64 val;
> > +
> > +   /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> > +   val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
> > +
> > +   return negative ? 0ll - val : val;
> > +}
> > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> > +
> >  /**
> >   * drm_crtc_enable_color_mgmt - enable color management properties
> >   * @crtc: DRM CRTC
> > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > index d1c662d92ab7..60fea5501886 100644
> > --- a/include/drm/drm_color_mgmt.h
> > +++ b/include/drm/drm_color_mgmt.h
> > @@ -30,6 +30,8 @@ struct drm_crtc;
> >  struct drm_plane;
> >
> >  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > +                                 uint32_t m, uint32_t n);
> >
> >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> >                             uint degamma_lut_size,
> > --
> > 2.20.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[james qian wang (Arm Technology China)]

On Mon, Oct 14, 2019 at 10:56:05AM +0200, Daniel Vetter wrote:
> On Fri, Oct 11, 2019 at 05:43:09AM +0000, james qian wang (Arm Technology China) wrote:
> > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> > hardware.
> >
> > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> > ---
> >  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
> >  include/drm/drm_color_mgmt.h     |  2 ++
> >  2 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > index 4ce5c6d8de99..3d533d0b45af 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> >  }
> >  EXPORT_SYMBOL(drm_color_lut_extract);
> >
> > +/**
> > + * drm_color_ctm_s31_32_to_qm_n
> > + *
> > + * @user_input: input value
> > + * @m: number of integer bits
> > + * @n: number of fractinal bits
> > + *
> > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
>
> What's the Q meaning here? Also maybe specify that the higher bits above
> m+n are cleared to all 0 or all 1. Unit test would be lovely too. Anyway:

The Q used to represent signed two's complement.

For detail: https://en.wikipedia.org/wiki/Q_(number_format)

And it Q is 2's complement, so the value of higher bit equal to
sign-bit.
All 1 if it is negative
0 if it is positive.

James

> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> > + */
> > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > +                                 uint32_t m, uint32_t n)
> > +{
> > +   u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > +   bool negative = !!(user_input & BIT_ULL(63));
> > +   s64 val;
> > +
> > +   /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> > +   val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
> > +
> > +   return negative ? 0ll - val : val;
> > +}
> > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> > +
> >  /**
> >   * drm_crtc_enable_color_mgmt - enable color management properties
> >   * @crtc: DRM CRTC
> > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > index d1c662d92ab7..60fea5501886 100644
> > --- a/include/drm/drm_color_mgmt.h
> > +++ b/include/drm/drm_color_mgmt.h
> > @@ -30,6 +30,8 @@ struct drm_crtc;
> >  struct drm_plane;
> >
> >  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > +                                 uint32_t m, uint32_t n);
> >
> >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> >                             uint degamma_lut_size,
> > --
> > 2.20.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[Daniel Vetter]

On Mon, Oct 14, 2019 at 09:58:20AM +0000, james qian wang (Arm Technology China) wrote:
> On Mon, Oct 14, 2019 at 10:56:05AM +0200, Daniel Vetter wrote:
> > On Fri, Oct 11, 2019 at 05:43:09AM +0000, james qian wang (Arm Technology China) wrote:
> > > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> > > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> > > hardware.
> > >
> > > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> > > ---
> > >  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
> > >  include/drm/drm_color_mgmt.h     |  2 ++
> > >  2 files changed, 25 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > > index 4ce5c6d8de99..3d533d0b45af 100644
> > > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> > >  }
> > >  EXPORT_SYMBOL(drm_color_lut_extract);
> > >
> > > +/**
> > > + * drm_color_ctm_s31_32_to_qm_n
> > > + *
> > > + * @user_input: input value
> > > + * @m: number of integer bits
> > > + * @n: number of fractinal bits
> > > + *
> > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> >
> > What's the Q meaning here? Also maybe specify that the higher bits above
> > m+n are cleared to all 0 or all 1. Unit test would be lovely too. Anyway:
> 
> The Q used to represent signed two's complement.
> 
> For detail: https://en.wikipedia.org/wiki/Q_(number_format)
> 
> And it Q is 2's complement, so the value of higher bit equal to
> sign-bit.
> All 1 if it is negative
> 0 if it is positive.

Ah I didn't know about this notation, I think in other drm docs we just
used m.n 2's complement to denote this layout. Up to you I think.
-Daniel

> 
> James
> 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > > + */
> > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > +                                 uint32_t m, uint32_t n)
> > > +{
> > > +   u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > > +   bool negative = !!(user_input & BIT_ULL(63));
> > > +   s64 val;
> > > +
> > > +   /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> > > +   val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
> > > +
> > > +   return negative ? 0ll - val : val;
> > > +}
> > > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> > > +
> > >  /**
> > >   * drm_crtc_enable_color_mgmt - enable color management properties
> > >   * @crtc: DRM CRTC
> > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > > index d1c662d92ab7..60fea5501886 100644
> > > --- a/include/drm/drm_color_mgmt.h
> > > +++ b/include/drm/drm_color_mgmt.h
> > > @@ -30,6 +30,8 @@ struct drm_crtc;
> > >  struct drm_plane;
> > >
> > >  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > +                                 uint32_t m, uint32_t n);
> > >
> > >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > >                             uint degamma_lut_size,
> > > --
> > > 2.20.1
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[Daniel Vetter]

On Mon, Oct 14, 2019 at 09:58:20AM +0000, james qian wang (Arm Technology China) wrote:
> On Mon, Oct 14, 2019 at 10:56:05AM +0200, Daniel Vetter wrote:
> > On Fri, Oct 11, 2019 at 05:43:09AM +0000, james qian wang (Arm Technology China) wrote:
> > > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> > > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> > > hardware.
> > >
> > > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> > > ---
> > >  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
> > >  include/drm/drm_color_mgmt.h     |  2 ++
> > >  2 files changed, 25 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > > index 4ce5c6d8de99..3d533d0b45af 100644
> > > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> > >  }
> > >  EXPORT_SYMBOL(drm_color_lut_extract);
> > >
> > > +/**
> > > + * drm_color_ctm_s31_32_to_qm_n
> > > + *
> > > + * @user_input: input value
> > > + * @m: number of integer bits
> > > + * @n: number of fractinal bits
> > > + *
> > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> >
> > What's the Q meaning here? Also maybe specify that the higher bits above
> > m+n are cleared to all 0 or all 1. Unit test would be lovely too. Anyway:
> 
> The Q used to represent signed two's complement.
> 
> For detail: https://en.wikipedia.org/wiki/Q_(number_format)
> 
> And it Q is 2's complement, so the value of higher bit equal to
> sign-bit.
> All 1 if it is negative
> 0 if it is positive.

Ah I didn't know about this notation, I think in other drm docs we just
used m.n 2's complement to denote this layout. Up to you I think.
-Daniel

> 
> James
> 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > > + */
> > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > +                                 uint32_t m, uint32_t n)
> > > +{
> > > +   u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > > +   bool negative = !!(user_input & BIT_ULL(63));
> > > +   s64 val;
> > > +
> > > +   /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> > > +   val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
> > > +
> > > +   return negative ? 0ll - val : val;
> > > +}
> > > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> > > +
> > >  /**
> > >   * drm_crtc_enable_color_mgmt - enable color management properties
> > >   * @crtc: DRM CRTC
> > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > > index d1c662d92ab7..60fea5501886 100644
> > > --- a/include/drm/drm_color_mgmt.h
> > > +++ b/include/drm/drm_color_mgmt.h
> > > @@ -30,6 +30,8 @@ struct drm_crtc;
> > >  struct drm_plane;
> > >
> > >  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > +                                 uint32_t m, uint32_t n);
> > >
> > >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > >                             uint degamma_lut_size,
> > > --
> > > 2.20.1
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[Ilia Mirkin]

On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
<james.qian.wang@arm.com> wrote:
>
> Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> hardware.
>
> Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> ---
>  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
>  include/drm/drm_color_mgmt.h     |  2 ++
>  2 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 4ce5c6d8de99..3d533d0b45af 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
>  }
>  EXPORT_SYMBOL(drm_color_lut_extract);
>
> +/**
> + * drm_color_ctm_s31_32_to_qm_n
> + *
> + * @user_input: input value
> + * @m: number of integer bits

Is this the full 2's complement value? i.e. including the "sign" bit
of the 2's complement representation? I'd kinda assume that m = 32, n
= 0 would just get me the integer portion of this, for example.

> + * @n: number of fractinal bits

fractional

> + *
> + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> + */
> +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> +                                     uint32_t m, uint32_t n)
> +{
> +       u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> +       bool negative = !!(user_input & BIT_ULL(63));
> +       s64 val;
> +
> +       /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */

This implies that n = 32, m = 0 would actually yield a 33-bit 2's
complement number. Is that what you meant?

> +       val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);

I'm going to play with numpy to convince myself that this is right
(esp with the endpoints), but in the meanwhile, you probably want to
use BIT_ULL in case n + m > 32 (I don't think that's the case with any
current hardware though).

> +
> +       return negative ? 0ll - val : val;

Why not just "negative ? -val : val"?

> +}
> +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> +
>  /**
>   * drm_crtc_enable_color_mgmt - enable color management properties
>   * @crtc: DRM CRTC
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index d1c662d92ab7..60fea5501886 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -30,6 +30,8 @@ struct drm_crtc;
>  struct drm_plane;
>
>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> +                                     uint32_t m, uint32_t n);
>
>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>                                 uint degamma_lut_size,
> --
> 2.20.1
>

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[Ilia Mirkin]

On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
<james.qian.wang@arm.com> wrote:
>
> Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> hardware.
>
> Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> ---
>  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
>  include/drm/drm_color_mgmt.h     |  2 ++
>  2 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 4ce5c6d8de99..3d533d0b45af 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
>  }
>  EXPORT_SYMBOL(drm_color_lut_extract);
>
> +/**
> + * drm_color_ctm_s31_32_to_qm_n
> + *
> + * @user_input: input value
> + * @m: number of integer bits

Is this the full 2's complement value? i.e. including the "sign" bit
of the 2's complement representation? I'd kinda assume that m = 32, n
= 0 would just get me the integer portion of this, for example.

> + * @n: number of fractinal bits

fractional

> + *
> + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> + */
> +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> +                                     uint32_t m, uint32_t n)
> +{
> +       u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> +       bool negative = !!(user_input & BIT_ULL(63));
> +       s64 val;
> +
> +       /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */

This implies that n = 32, m = 0 would actually yield a 33-bit 2's
complement number. Is that what you meant?

> +       val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);

I'm going to play with numpy to convince myself that this is right
(esp with the endpoints), but in the meanwhile, you probably want to
use BIT_ULL in case n + m > 32 (I don't think that's the case with any
current hardware though).

> +
> +       return negative ? 0ll - val : val;

Why not just "negative ? -val : val"?

> +}
> +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> +
>  /**
>   * drm_crtc_enable_color_mgmt - enable color management properties
>   * @crtc: DRM CRTC
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index d1c662d92ab7..60fea5501886 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -30,6 +30,8 @@ struct drm_crtc;
>  struct drm_plane;
>
>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> +                                     uint32_t m, uint32_t n);
>
>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>                                 uint degamma_lut_size,
> --
> 2.20.1
>

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[james qian wang (Arm Technology China)]

On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote:
> On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
> <james.qian.wang@arm.com> wrote:
> >
> > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> > hardware.
> >
> > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> > ---
> >  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
> >  include/drm/drm_color_mgmt.h     |  2 ++
> >  2 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > index 4ce5c6d8de99..3d533d0b45af 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> >  }
> >  EXPORT_SYMBOL(drm_color_lut_extract);
> >
> > +/**
> > + * drm_color_ctm_s31_32_to_qm_n
> > + *
> > + * @user_input: input value
> > + * @m: number of integer bits
> 
> Is this the full 2's complement value? i.e. including the "sign" bit
> of the 2's complement representation? I'd kinda assume that m = 32, n
> = 0 would just get me the integer portion of this, for example.

@m doesn't include "sign-bit"

and for this conversion only support m <= 31, n <= 32.

> > + * @n: number of fractinal bits
> 
> fractional

Thank you.
> 
> > + *
> > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> > + */
> > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > +                                     uint32_t m, uint32_t n)
> > +{
> > +       u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > +       bool negative = !!(user_input & BIT_ULL(63));
> > +       s64 val;
> > +
> > +       /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> 
> This implies that n = 32, m = 0 would actually yield a 33-bit 2's
> complement number. Is that what you meant?

Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value.

> 
> > +       val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
> 
> I'm going to play with numpy to convince myself that this is right
> (esp with the endpoints), but in the meanwhile, you probably want to
> use BIT_ULL in case n + m > 32 (I don't think that's the case with any
> current hardware though).

Yes, you are right, I need to use BIT_ULL, and Mihail also point this out.
This is function is drived from our internal s31_32_to_q2_14()

> 
> > +
> > +       return negative ? 0ll - val : val;
> 
> Why not just "negative ? -val : val"?

will correct it.

> 
> > +}
> > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> > +
> >  /**
> >   * drm_crtc_enable_color_mgmt - enable color management properties
> >   * @crtc: DRM CRTC
> > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > index d1c662d92ab7..60fea5501886 100644
> > --- a/include/drm/drm_color_mgmt.h
> > +++ b/include/drm/drm_color_mgmt.h
> > @@ -30,6 +30,8 @@ struct drm_crtc;
> >  struct drm_plane;
> >
> >  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > +                                     uint32_t m, uint32_t n);
> >
> >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> >                                 uint degamma_lut_size,
> > --
> > 2.20.1
> >

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[james qian wang (Arm Technology China)]

On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote:
> On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
> <james.qian.wang@arm.com> wrote:
> >
> > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> > hardware.
> >
> > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> > ---
> >  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
> >  include/drm/drm_color_mgmt.h     |  2 ++
> >  2 files changed, 25 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > index 4ce5c6d8de99..3d533d0b45af 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> >  }
> >  EXPORT_SYMBOL(drm_color_lut_extract);
> >
> > +/**
> > + * drm_color_ctm_s31_32_to_qm_n
> > + *
> > + * @user_input: input value
> > + * @m: number of integer bits
> 
> Is this the full 2's complement value? i.e. including the "sign" bit
> of the 2's complement representation? I'd kinda assume that m = 32, n
> = 0 would just get me the integer portion of this, for example.

@m doesn't include "sign-bit"

and for this conversion only support m <= 31, n <= 32.

> > + * @n: number of fractinal bits
> 
> fractional

Thank you.
> 
> > + *
> > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> > + */
> > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > +                                     uint32_t m, uint32_t n)
> > +{
> > +       u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > +       bool negative = !!(user_input & BIT_ULL(63));
> > +       s64 val;
> > +
> > +       /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> 
> This implies that n = 32, m = 0 would actually yield a 33-bit 2's
> complement number. Is that what you meant?

Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value.

> 
> > +       val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
> 
> I'm going to play with numpy to convince myself that this is right
> (esp with the endpoints), but in the meanwhile, you probably want to
> use BIT_ULL in case n + m > 32 (I don't think that's the case with any
> current hardware though).

Yes, you are right, I need to use BIT_ULL, and Mihail also point this out.
This is function is drived from our internal s31_32_to_q2_14()

> 
> > +
> > +       return negative ? 0ll - val : val;
> 
> Why not just "negative ? -val : val"?

will correct it.

> 
> > +}
> > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> > +
> >  /**
> >   * drm_crtc_enable_color_mgmt - enable color management properties
> >   * @crtc: DRM CRTC
> > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > index d1c662d92ab7..60fea5501886 100644
> > --- a/include/drm/drm_color_mgmt.h
> > +++ b/include/drm/drm_color_mgmt.h
> > @@ -30,6 +30,8 @@ struct drm_crtc;
> >  struct drm_plane;
> >
> >  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > +                                     uint32_t m, uint32_t n);
> >
> >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> >                                 uint degamma_lut_size,
> > --
> > 2.20.1
> >
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[Ilia Mirkin]

On Mon, Oct 14, 2019 at 9:16 PM james qian wang (Arm Technology China)
<james.qian.wang@arm.com> wrote:
> On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote:
> > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
> > <james.qian.wang@arm.com> wrote:
> > > + *
> > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> > > + */
> > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > +                                     uint32_t m, uint32_t n)
> > > +{
> > > +       u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > > +       bool negative = !!(user_input & BIT_ULL(63));
> > > +       s64 val;
> > > +
> > > +       /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> >
> > This implies that n = 32, m = 0 would actually yield a 33-bit 2's
> > complement number. Is that what you meant?
>
> Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value.

This goes counter to what the wikipedia page says [
https://en.wikipedia.org/wiki/Q_(number_format) ]:

(reformatted slightly for text-only consumption):

"""
For example, a Q15.1 format number:

- requires 15+1 = 16 bits
- its range is [-2^14, 2^14 - 2^-1] = [-16384.0, +16383.5] = [0x8000,
0x8001 ... 0xFFFF, 0x0000, 0x0001 ... 0x7FFE, 0x7FFF]
- its resolution is 2^-1 = 0.5
"""

This suggests that the proper way to represent a standard 32-bit 2's
complement integer would be Q32.0.

  -ilia

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[Ilia Mirkin]

On Mon, Oct 14, 2019 at 9:16 PM james qian wang (Arm Technology China)
<james.qian.wang@arm.com> wrote:
> On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote:
> > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
> > <james.qian.wang@arm.com> wrote:
> > > + *
> > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> > > + */
> > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > +                                     uint32_t m, uint32_t n)
> > > +{
> > > +       u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > > +       bool negative = !!(user_input & BIT_ULL(63));
> > > +       s64 val;
> > > +
> > > +       /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> >
> > This implies that n = 32, m = 0 would actually yield a 33-bit 2's
> > complement number. Is that what you meant?
>
> Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value.

This goes counter to what the wikipedia page says [
https://en.wikipedia.org/wiki/Q_(number_format) ]:

(reformatted slightly for text-only consumption):

"""
For example, a Q15.1 format number:

- requires 15+1 = 16 bits
- its range is [-2^14, 2^14 - 2^-1] = [-16384.0, +16383.5] = [0x8000,
0x8001 ... 0xFFFF, 0x0000, 0x0001 ... 0x7FFE, 0x7FFF]
- its resolution is 2^-1 = 0.5
"""

This suggests that the proper way to represent a standard 32-bit 2's
complement integer would be Q32.0.

  -ilia
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[james qian wang (Arm Technology China)]

On Mon, Oct 14, 2019 at 11:48:38PM -0400, Ilia Mirkin wrote:
> On Mon, Oct 14, 2019 at 9:16 PM james qian wang (Arm Technology China)
> <james.qian.wang@arm.com> wrote:
> > On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote:
> > > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
> > > <james.qian.wang@arm.com> wrote:
> > > > + *
> > > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> > > > + */
> > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > > +                                     uint32_t m, uint32_t n)
> > > > +{
> > > > +       u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > > > +       bool negative = !!(user_input & BIT_ULL(63));
> > > > +       s64 val;
> > > > +
> > > > +       /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> > >
> > > This implies that n = 32, m = 0 would actually yield a 33-bit 2's
> > > complement number. Is that what you meant?
> >
> > Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value.
> 
> This goes counter to what the wikipedia page says [
> https://en.wikipedia.org/wiki/Q_(number_format) ]:
> 
> (reformatted slightly for text-only consumption):
> 
> """
> For example, a Q15.1 format number:
> 
> - requires 15+1 = 16 bits
> - its range is [-2^14, 2^14 - 2^-1] = [-16384.0, +16383.5] = [0x8000,
> 0x8001 ... 0xFFFF, 0x0000, 0x0001 ... 0x7FFE, 0x7FFF]
> - its resolution is 2^-1 = 0.5
> """
> 
> This suggests that the proper way to represent a standard 32-bit 2's
> complement integer would be Q32.0.
>

Yes you're right, I will send a new version to correct this code
according to the Wiki.

Thanks
James

>   -ilia

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[james qian wang (Arm Technology China)]

On Mon, Oct 14, 2019 at 11:48:38PM -0400, Ilia Mirkin wrote:
> On Mon, Oct 14, 2019 at 9:16 PM james qian wang (Arm Technology China)
> <james.qian.wang@arm.com> wrote:
> > On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote:
> > > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
> > > <james.qian.wang@arm.com> wrote:
> > > > + *
> > > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> > > > + */
> > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > > +                                     uint32_t m, uint32_t n)
> > > > +{
> > > > +       u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > > > +       bool negative = !!(user_input & BIT_ULL(63));
> > > > +       s64 val;
> > > > +
> > > > +       /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> > >
> > > This implies that n = 32, m = 0 would actually yield a 33-bit 2's
> > > complement number. Is that what you meant?
> >
> > Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value.
> 
> This goes counter to what the wikipedia page says [
> https://en.wikipedia.org/wiki/Q_(number_format) ]:
> 
> (reformatted slightly for text-only consumption):
> 
> """
> For example, a Q15.1 format number:
> 
> - requires 15+1 = 16 bits
> - its range is [-2^14, 2^14 - 2^-1] = [-16384.0, +16383.5] = [0x8000,
> 0x8001 ... 0xFFFF, 0x0000, 0x0001 ... 0x7FFE, 0x7FFF]
> - its resolution is 2^-1 = 0.5
> """
> 
> This suggests that the proper way to represent a standard 32-bit 2's
> complement integer would be Q32.0.
>

Yes you're right, I will send a new version to correct this code
according to the Wiki.

Thanks
James

>   -ilia

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[Mihail Atanassov]

On Tuesday, 15 October 2019 02:16:11 BST james qian wang (Arm Technology China) wrote:
> On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote:
> > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
> > <james.qian.wang@arm.com> wrote:
> > >
> > > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> > > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> > > hardware.
> > >
> > > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> > > ---
> > >  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
> > >  include/drm/drm_color_mgmt.h     |  2 ++
> > >  2 files changed, 25 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > > index 4ce5c6d8de99..3d533d0b45af 100644
> > > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> > >  }
> > >  EXPORT_SYMBOL(drm_color_lut_extract);
> > >
> > > +/**
> > > + * drm_color_ctm_s31_32_to_qm_n
> > > + *
> > > + * @user_input: input value
> > > + * @m: number of integer bits
> > 
> > Is this the full 2's complement value? i.e. including the "sign" bit
> > of the 2's complement representation? I'd kinda assume that m = 32, n
> > = 0 would just get me the integer portion of this, for example.
> 
> @m doesn't include "sign-bit"
> 
> and for this conversion only support m <= 31, n <= 32.
> 
> > > + * @n: number of fractinal bits
> > 
> > fractional
> 
> Thank you.
> > 
> > > + *
> > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> > > + */
> > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > +                                     uint32_t m, uint32_t n)
> > > +{
> > > +       u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > > +       bool negative = !!(user_input & BIT_ULL(63));
> > > +       s64 val;
> > > +
> > > +       /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> > 
> > This implies that n = 32, m = 0 would actually yield a 33-bit 2's
> > complement number. Is that what you meant?
> 
> Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value.
> 

I gotta say this would be quite confusing. There is no sign bit in 2's
complement, per se. The MSbit just has a negative weight. Q16.16 is a
32-bit value, so Q0.32 should also be a 32-bit value with weights
-2^-1, +2^-2, etc.

Best to follow what Wikipedia says, right :).

> > 
> > > +       val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
> > 
> > I'm going to play with numpy to convince myself that this is right
> > (esp with the endpoints), but in the meanwhile, you probably want to
> > use BIT_ULL in case n + m > 32 (I don't think that's the case with any
> > current hardware though).
> 
> Yes, you are right, I need to use BIT_ULL, and Mihail also point this out.
> This is function is drived from our internal s31_32_to_q2_14()
> 
> > 
> > > +
> > > +       return negative ? 0ll - val : val;
> > 
> > Why not just "negative ? -val : val"?
> 
> will correct it.
> 
> > 
> > > +}
> > > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> > > +
> > >  /**
> > >   * drm_crtc_enable_color_mgmt - enable color management properties
> > >   * @crtc: DRM CRTC
> > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > > index d1c662d92ab7..60fea5501886 100644
> > > --- a/include/drm/drm_color_mgmt.h
> > > +++ b/include/drm/drm_color_mgmt.h
> > > @@ -30,6 +30,8 @@ struct drm_crtc;
> > >  struct drm_plane;
> > >
> > >  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > +                                     uint32_t m, uint32_t n);
> > >
> > >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > >                                 uint degamma_lut_size,
> > > --
> > > 2.20.1
> > >
> 


-- 
Mihail

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[Mihail Atanassov]

On Tuesday, 15 October 2019 02:16:11 BST james qian wang (Arm Technology China) wrote:
> On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote:
> > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
> > <james.qian.wang@arm.com> wrote:
> > >
> > > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> > > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> > > hardware.
> > >
> > > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> > > ---
> > >  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
> > >  include/drm/drm_color_mgmt.h     |  2 ++
> > >  2 files changed, 25 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > > index 4ce5c6d8de99..3d533d0b45af 100644
> > > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> > >  }
> > >  EXPORT_SYMBOL(drm_color_lut_extract);
> > >
> > > +/**
> > > + * drm_color_ctm_s31_32_to_qm_n
> > > + *
> > > + * @user_input: input value
> > > + * @m: number of integer bits
> > 
> > Is this the full 2's complement value? i.e. including the "sign" bit
> > of the 2's complement representation? I'd kinda assume that m = 32, n
> > = 0 would just get me the integer portion of this, for example.
> 
> @m doesn't include "sign-bit"
> 
> and for this conversion only support m <= 31, n <= 32.
> 
> > > + * @n: number of fractinal bits
> > 
> > fractional
> 
> Thank you.
> > 
> > > + *
> > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> > > + */
> > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > +                                     uint32_t m, uint32_t n)
> > > +{
> > > +       u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > > +       bool negative = !!(user_input & BIT_ULL(63));
> > > +       s64 val;
> > > +
> > > +       /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> > 
> > This implies that n = 32, m = 0 would actually yield a 33-bit 2's
> > complement number. Is that what you meant?
> 
> Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value.
> 

I gotta say this would be quite confusing. There is no sign bit in 2's
complement, per se. The MSbit just has a negative weight. Q16.16 is a
32-bit value, so Q0.32 should also be a 32-bit value with weights
-2^-1, +2^-2, etc.

Best to follow what Wikipedia says, right :).

> > 
> > > +       val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
> > 
> > I'm going to play with numpy to convince myself that this is right
> > (esp with the endpoints), but in the meanwhile, you probably want to
> > use BIT_ULL in case n + m > 32 (I don't think that's the case with any
> > current hardware though).
> 
> Yes, you are right, I need to use BIT_ULL, and Mihail also point this out.
> This is function is drived from our internal s31_32_to_q2_14()
> 
> > 
> > > +
> > > +       return negative ? 0ll - val : val;
> > 
> > Why not just "negative ? -val : val"?
> 
> will correct it.
> 
> > 
> > > +}
> > > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> > > +
> > >  /**
> > >   * drm_crtc_enable_color_mgmt - enable color management properties
> > >   * @crtc: DRM CRTC
> > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > > index d1c662d92ab7..60fea5501886 100644
> > > --- a/include/drm/drm_color_mgmt.h
> > > +++ b/include/drm/drm_color_mgmt.h
> > > @@ -30,6 +30,8 @@ struct drm_crtc;
> > >  struct drm_plane;
> > >
> > >  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > +                                     uint32_t m, uint32_t n);
> > >
> > >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > >                                 uint degamma_lut_size,
> > > --
> > > 2.20.1
> > >
> 


-- 
Mihail



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[james qian wang (Arm Technology China)]

On Tue, Oct 15, 2019 at 08:21:44AM +0000, Mihail Atanassov wrote:
> On Tuesday, 15 October 2019 02:16:11 BST james qian wang (Arm Technology China) wrote:
> > On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote:
> > > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
> > > <james.qian.wang@arm.com> wrote:
> > > >
> > > > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> > > > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> > > > hardware.
> > > >
> > > > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> > > > ---
> > > >  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
> > > >  include/drm/drm_color_mgmt.h     |  2 ++
> > > >  2 files changed, 25 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > > > index 4ce5c6d8de99..3d533d0b45af 100644
> > > > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > > > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > > > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> > > >  }
> > > >  EXPORT_SYMBOL(drm_color_lut_extract);
> > > >
> > > > +/**
> > > > + * drm_color_ctm_s31_32_to_qm_n
> > > > + *
> > > > + * @user_input: input value
> > > > + * @m: number of integer bits
> > > 
> > > Is this the full 2's complement value? i.e. including the "sign" bit
> > > of the 2's complement representation? I'd kinda assume that m = 32, n
> > > = 0 would just get me the integer portion of this, for example.
> > 
> > @m doesn't include "sign-bit"
> > 
> > and for this conversion only support m <= 31, n <= 32.
> > 
> > > > + * @n: number of fractinal bits
> > > 
> > > fractional
> > 
> > Thank you.
> > > 
> > > > + *
> > > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> > > > + */
> > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > > +                                     uint32_t m, uint32_t n)
> > > > +{
> > > > +       u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > > > +       bool negative = !!(user_input & BIT_ULL(63));
> > > > +       s64 val;
> > > > +
> > > > +       /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> > > 
> > > This implies that n = 32, m = 0 would actually yield a 33-bit 2's
> > > complement number. Is that what you meant?
> > 
> > Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value.
> > 
> 
> I gotta say this would be quite confusing. There is no sign bit in 2's
> complement, per se. The MSbit just has a negative weight. Q16.16 is a
> 32-bit value, so Q0.32 should also be a 32-bit value with weights
> -2^-1, +2^-2, etc.
> 
> Best to follow what Wikipedia says, right :).

Sorry, My fault! will correct in v5.

> > > 
> > > > +       val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
> > > 
> > > I'm going to play with numpy to convince myself that this is right
> > > (esp with the endpoints), but in the meanwhile, you probably want to
> > > use BIT_ULL in case n + m > 32 (I don't think that's the case with any
> > > current hardware though).
> > 
> > Yes, you are right, I need to use BIT_ULL, and Mihail also point this out.
> > This is function is drived from our internal s31_32_to_q2_14()
> > 
> > > 
> > > > +
> > > > +       return negative ? 0ll - val : val;
> > > 
> > > Why not just "negative ? -val : val"?
> > 
> > will correct it.
> > 
> > > 
> > > > +}
> > > > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> > > > +
> > > >  /**
> > > >   * drm_crtc_enable_color_mgmt - enable color management properties
> > > >   * @crtc: DRM CRTC
> > > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > > > index d1c662d92ab7..60fea5501886 100644
> > > > --- a/include/drm/drm_color_mgmt.h
> > > > +++ b/include/drm/drm_color_mgmt.h
> > > > @@ -30,6 +30,8 @@ struct drm_crtc;
> > > >  struct drm_plane;
> > > >
> > > >  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > > +                                     uint32_t m, uint32_t n);
> > > >
> > > >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > > >                                 uint degamma_lut_size,
> > > > --
> > > > 2.20.1
> > > >
> > 
> 
> 
> -- 
> Mihail
> 
> 

Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()

[james qian wang (Arm Technology China)]

On Tue, Oct 15, 2019 at 08:21:44AM +0000, Mihail Atanassov wrote:
> On Tuesday, 15 October 2019 02:16:11 BST james qian wang (Arm Technology China) wrote:
> > On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote:
> > > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
> > > <james.qian.wang@arm.com> wrote:
> > > >
> > > > Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> > > > convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> > > > hardware.
> > > >
> > > > Signed-off-by: james qian wang (Arm Technology China) <james.qian.wang@arm.com>
> > > > ---
> > > >  drivers/gpu/drm/drm_color_mgmt.c | 23 +++++++++++++++++++++++
> > > >  include/drm/drm_color_mgmt.h     |  2 ++
> > > >  2 files changed, 25 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > > > index 4ce5c6d8de99..3d533d0b45af 100644
> > > > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > > > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > > > @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> > > >  }
> > > >  EXPORT_SYMBOL(drm_color_lut_extract);
> > > >
> > > > +/**
> > > > + * drm_color_ctm_s31_32_to_qm_n
> > > > + *
> > > > + * @user_input: input value
> > > > + * @m: number of integer bits
> > > 
> > > Is this the full 2's complement value? i.e. including the "sign" bit
> > > of the 2's complement representation? I'd kinda assume that m = 32, n
> > > = 0 would just get me the integer portion of this, for example.
> > 
> > @m doesn't include "sign-bit"
> > 
> > and for this conversion only support m <= 31, n <= 32.
> > 
> > > > + * @n: number of fractinal bits
> > > 
> > > fractional
> > 
> > Thank you.
> > > 
> > > > + *
> > > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> > > > + */
> > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > > +                                     uint32_t m, uint32_t n)
> > > > +{
> > > > +       u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > > > +       bool negative = !!(user_input & BIT_ULL(63));
> > > > +       s64 val;
> > > > +
> > > > +       /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> > > 
> > > This implies that n = 32, m = 0 would actually yield a 33-bit 2's
> > > complement number. Is that what you meant?
> > 
> > Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value.
> > 
> 
> I gotta say this would be quite confusing. There is no sign bit in 2's
> complement, per se. The MSbit just has a negative weight. Q16.16 is a
> 32-bit value, so Q0.32 should also be a 32-bit value with weights
> -2^-1, +2^-2, etc.
> 
> Best to follow what Wikipedia says, right :).

Sorry, My fault! will correct in v5.

> > > 
> > > > +       val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
> > > 
> > > I'm going to play with numpy to convince myself that this is right
> > > (esp with the endpoints), but in the meanwhile, you probably want to
> > > use BIT_ULL in case n + m > 32 (I don't think that's the case with any
> > > current hardware though).
> > 
> > Yes, you are right, I need to use BIT_ULL, and Mihail also point this out.
> > This is function is drived from our internal s31_32_to_q2_14()
> > 
> > > 
> > > > +
> > > > +       return negative ? 0ll - val : val;
> > > 
> > > Why not just "negative ? -val : val"?
> > 
> > will correct it.
> > 
> > > 
> > > > +}
> > > > +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> > > > +
> > > >  /**
> > > >   * drm_crtc_enable_color_mgmt - enable color management properties
> > > >   * @crtc: DRM CRTC
> > > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > > > index d1c662d92ab7..60fea5501886 100644
> > > > --- a/include/drm/drm_color_mgmt.h
> > > > +++ b/include/drm/drm_color_mgmt.h
> > > > @@ -30,6 +30,8 @@ struct drm_crtc;
> > > >  struct drm_plane;
> > > >
> > > >  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > > +                                     uint32_t m, uint32_t n);
> > > >
> > > >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > > >                                 uint degamma_lut_size,
> > > > --
> > > > 2.20.1
> > > >
> > 
> 
> 
> -- 
> Mihail
> 
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel