[PATCH 0/5] drm/omap: add color mgmt support

[PATCH 0/5] drm/omap: add color mgmt support

[Tomi Valkeinen]

Hi,

This series is based on patches sent about a year ago:

https://lists.freedesktop.org/archives/dri-devel/2019-September/233966.html
https://lists.freedesktop.org/archives/dri-devel/2019-September/233967.html

I've fixed the minor issues reported, and according to the recent
discussions with Pekka and Daniel, I have changed how the gamma works.

After these patches omapdrm will expose DEGAMMA_LUT (and no GAMMA_LUT),
and the legacy gamma-set ioctl will use DEGAMMA_LUT underneath. And on
top of that, we have the CTM and plane color mgmt.

 Tomi

Jyri Sarha (2):
  drm/omap: Implement CTM property for CRTC using OVL managers CPR
    matrix
  drm/omap: Enable COLOR_ENCODING and COLOR_RANGE properties for planes

Tomi Valkeinen (3):
  drm: add legacy support for using degamma for gamma
  drm/omap: use degamma property for gamma table
  drm/omap: rearrange includes in omapdss.h

 drivers/gpu/drm/drm_atomic_helper.c   |  81 +++++++++++++++-----
 drivers/gpu/drm/omapdrm/dss/dispc.c   | 104 ++++++++++++++++----------
 drivers/gpu/drm/omapdrm/dss/omapdss.h |  12 ++-
 drivers/gpu/drm/omapdrm/omap_crtc.c   |  51 +++++++++++--
 drivers/gpu/drm/omapdrm/omap_plane.c  |  30 ++++++++
 include/drm/drm_atomic_helper.h       |   4 +
 6 files changed, 209 insertions(+), 73 deletions(-)

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 1/5] drm: add legacy support for using degamma for gamma

[Tomi Valkeinen]

We currently have drm_atomic_helper_legacy_gamma_set() helper which can
be used to handle legacy gamma-set ioctl.
drm_atomic_helper_legacy_gamma_set() sets GAMMA_LUT, and clears
CTM and DEGAMMA_LUT. This works fine on HW where we have either:

degamma -> ctm -> gamma -> out

or

ctm -> gamma -> out

However, if the HW has gamma table before ctm, the atomic property
should be DEGAMMA_LUT, and thus we have:

degamma -> ctm -> out

This is fine for userspace which sets gamma table using the properties,
as the userspace can check for the existence of gamma & degamma, but the
legacy gamma-set ioctl does not work.

This patch adds a new helper, drm_atomic_helper_legacy_degamma_set(),
which can be used instead of drm_atomic_helper_legacy_gamma_set() when
the DEGAMMA_LUT is the underlying property that needs to be set.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 81 ++++++++++++++++++++++-------
 include/drm/drm_atomic_helper.h     |  4 ++
 2 files changed, 65 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 9e1ad493e689..5ba359114df6 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3469,24 +3469,11 @@ int drm_atomic_helper_page_flip_target(struct drm_crtc *crtc,
 }
 EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
 
-/**
- * drm_atomic_helper_legacy_gamma_set - set the legacy gamma correction table
- * @crtc: CRTC object
- * @red: red correction table
- * @green: green correction table
- * @blue: green correction table
- * @size: size of the tables
- * @ctx: lock acquire context
- *
- * Implements support for legacy gamma correction table for drivers
- * that support color management through the DEGAMMA_LUT/GAMMA_LUT
- * properties. See drm_crtc_enable_color_mgmt() and the containing chapter for
- * how the atomic color management and gamma tables work.
- */
-int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
-				       u16 *red, u16 *green, u16 *blue,
-				       uint32_t size,
-				       struct drm_modeset_acquire_ctx *ctx)
+static int legacy_gamma_degamma_set(struct drm_crtc *crtc,
+				    u16 *red, u16 *green, u16 *blue,
+				    uint32_t size,
+				    struct drm_modeset_acquire_ctx *ctx,
+				    bool use_degamma)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_atomic_state *state;
@@ -3525,9 +3512,11 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 	}
 
 	/* Reset DEGAMMA_LUT and CTM properties. */
-	replaced  = drm_property_replace_blob(&crtc_state->degamma_lut, NULL);
+	replaced  = drm_property_replace_blob(&crtc_state->degamma_lut,
+					      use_degamma ? blob : NULL);
 	replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
-	replaced |= drm_property_replace_blob(&crtc_state->gamma_lut, blob);
+	replaced |= drm_property_replace_blob(&crtc_state->gamma_lut,
+					      use_degamma ? NULL : blob);
 	crtc_state->color_mgmt_changed |= replaced;
 
 	ret = drm_atomic_commit(state);
@@ -3537,8 +3526,60 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 	drm_property_blob_put(blob);
 	return ret;
 }
+
+/**
+ * drm_atomic_helper_legacy_gamma_set - set the legacy gamma correction table using gamma_lut
+ * @crtc: CRTC object
+ * @red: red correction table
+ * @green: green correction table
+ * @blue: green correction table
+ * @size: size of the tables
+ * @ctx: lock acquire context
+ *
+ * Implements support for legacy gamma correction table for drivers
+ * that support color management through the DEGAMMA_LUT/GAMMA_LUT
+ * properties. See drm_crtc_enable_color_mgmt() and the containing chapter for
+ * how the atomic color management and gamma tables work.
+ *
+ * This function uses GAMMA_LUT property for the gamma table. This function
+ * can be used when the driver exposes either only GAMMA_LUT or both GAMMA_LUT
+ * and DEGAMMA_LUT.
+ */
+int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
+				       u16 *red, u16 *green, u16 *blue,
+				       uint32_t size,
+				       struct drm_modeset_acquire_ctx *ctx)
+{
+	return legacy_gamma_degamma_set(crtc, red, green, blue, size, ctx, false);
+}
 EXPORT_SYMBOL(drm_atomic_helper_legacy_gamma_set);
 
+/**
+ * drm_atomic_helper_legacy_degamma_set - set the legacy gamma correction table using degamma_lut
+ * @crtc: CRTC object
+ * @red: red correction table
+ * @green: green correction table
+ * @blue: green correction table
+ * @size: size of the tables
+ * @ctx: lock acquire context
+ *
+ * Implements support for legacy gamma correction table for drivers
+ * that support color management through the DEGAMMA_LUT/GAMMA_LUT
+ * properties. See drm_crtc_enable_color_mgmt() and the containing chapter for
+ * how the atomic color management and gamma tables work.
+ *
+ * This function uses DEGAMMA_LUT property for the gamma table. This function
+ * can be used when the driver exposes only DEGAMNMA_LUT.
+ */
+int drm_atomic_helper_legacy_degamma_set(struct drm_crtc *crtc,
+					 u16 *red, u16 *green, u16 *blue,
+					 uint32_t size,
+					 struct drm_modeset_acquire_ctx *ctx)
+{
+	return legacy_gamma_degamma_set(crtc, red, green, blue, size, ctx, true);
+}
+EXPORT_SYMBOL(drm_atomic_helper_legacy_degamma_set);
+
 /**
  * drm_atomic_helper_bridge_propagate_bus_fmt() - Propagate output format to
  *						  the input end of a bridge
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index b268180c97eb..e60f5a600195 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -148,6 +148,10 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 				       u16 *red, u16 *green, u16 *blue,
 				       uint32_t size,
 				       struct drm_modeset_acquire_ctx *ctx);
+int drm_atomic_helper_legacy_degamma_set(struct drm_crtc *crtc,
+					 u16 *red, u16 *green, u16 *blue,
+					 uint32_t size,
+					 struct drm_modeset_acquire_ctx *ctx);
 
 /**
  * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 2/5] drm/omap: use degamma property for gamma table

[Tomi Valkeinen]

omapdrm supports gamma via GAMMA_LUT property. However, the HW we have
is:

gamma -> ctm -> out

instead of what the model DRM framework uses:

ctm -> gamma -> out

As the following patches add CTM support for omapdrm, lets first fix the
gamma.

This patch changes the property from GAMMA_LUT to DEGAMMA_LUT, and uses
drm_atomic_helper_legacy_degamma_set for gamma_set helper. Thus we will
have:

degamma -> ctm -> out

and the legacy ioctl will continue working as before.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 328a4a74f534..6116af920660 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -573,8 +573,8 @@ static int omap_crtc_atomic_check(struct drm_crtc *crtc,
 {
 	struct drm_plane_state *pri_state;
 
-	if (state->color_mgmt_changed && state->gamma_lut) {
-		unsigned int length = state->gamma_lut->length /
+	if (state->color_mgmt_changed && state->degamma_lut) {
+		unsigned int length = state->degamma_lut->length /
 			sizeof(struct drm_color_lut);
 
 		if (length < 2)
@@ -614,10 +614,10 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 		struct drm_color_lut *lut = NULL;
 		unsigned int length = 0;
 
-		if (crtc->state->gamma_lut) {
+		if (crtc->state->degamma_lut) {
 			lut = (struct drm_color_lut *)
-				crtc->state->gamma_lut->data;
-			length = crtc->state->gamma_lut->length /
+				crtc->state->degamma_lut->data;
+			length = crtc->state->degamma_lut->length /
 				sizeof(*lut);
 		}
 		priv->dispc_ops->mgr_set_gamma(priv->dispc, omap_crtc->channel,
@@ -738,7 +738,7 @@ static const struct drm_crtc_funcs omap_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.destroy = omap_crtc_destroy,
 	.page_flip = drm_atomic_helper_page_flip,
-	.gamma_set = drm_atomic_helper_legacy_gamma_set,
+	.gamma_set = drm_atomic_helper_legacy_degamma_set,
 	.atomic_duplicate_state = omap_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 	.atomic_set_property = omap_crtc_atomic_set_property,
@@ -839,7 +839,7 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	if (priv->dispc_ops->mgr_gamma_size(priv->dispc, channel)) {
 		unsigned int gamma_lut_size = 256;
 
-		drm_crtc_enable_color_mgmt(crtc, 0, false, gamma_lut_size);
+		drm_crtc_enable_color_mgmt(crtc, gamma_lut_size, false, 0);
 		drm_mode_crtc_set_gamma_size(crtc, gamma_lut_size);
 	}
 
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 3/5] drm/omap: Implement CTM property for CRTC using OVL managers CPR matrix

[Tomi Valkeinen]

From: Jyri Sarha <jsarha@ti.com>

Implement CTM color management property for OMAP CRTC using DSS
overlay manager's Color Phase Rotation matrix. The CPR matrix does not
exactly match the CTM property documentation. On DSS the CPR matrix is
applied after gamma table look up. However, it seems stupid to add a
custom property just for that.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 39 +++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 6116af920660..d2ff50322859 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -391,6 +391,33 @@ static void omap_crtc_manual_display_update(struct work_struct *data)
 	}
 }
 
+static s16 omap_crtc_s31_32_to_s2_8(s64 coef)
+{
+	u64 sign_bit = 1ULL << 63;
+	u64 cbits = (u64)coef;
+
+	s16 ret = clamp_val(((cbits & ~sign_bit) >> 24), 0, 0x1ff);
+
+	if (cbits & sign_bit)
+		ret = -ret;
+
+	return ret;
+}
+
+static void omap_crtc_cpr_coefs_from_ctm(const struct drm_color_ctm *ctm,
+					 struct omap_dss_cpr_coefs *cpr)
+{
+	cpr->rr = omap_crtc_s31_32_to_s2_8(ctm->matrix[0]);
+	cpr->rg = omap_crtc_s31_32_to_s2_8(ctm->matrix[1]);
+	cpr->rb = omap_crtc_s31_32_to_s2_8(ctm->matrix[2]);
+	cpr->gr = omap_crtc_s31_32_to_s2_8(ctm->matrix[3]);
+	cpr->gg = omap_crtc_s31_32_to_s2_8(ctm->matrix[4]);
+	cpr->gb = omap_crtc_s31_32_to_s2_8(ctm->matrix[5]);
+	cpr->br = omap_crtc_s31_32_to_s2_8(ctm->matrix[6]);
+	cpr->bg = omap_crtc_s31_32_to_s2_8(ctm->matrix[7]);
+	cpr->bb = omap_crtc_s31_32_to_s2_8(ctm->matrix[8]);
+}
+
 static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc)
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
@@ -402,7 +429,15 @@ static void omap_crtc_write_crtc_properties(struct drm_crtc *crtc)
 	info.default_color = 0x000000;
 	info.trans_enabled = false;
 	info.partial_alpha_enabled = false;
-	info.cpr_enable = false;
+
+	if (crtc->state->ctm) {
+		struct drm_color_ctm *ctm = crtc->state->ctm->data;
+
+		info.cpr_enable = true;
+		omap_crtc_cpr_coefs_from_ctm(ctm, &info.cpr_coefs);
+	} else {
+		info.cpr_enable = false;
+	}
 
 	priv->dispc_ops->mgr_setup(priv->dispc, omap_crtc->channel, &info);
 }
@@ -839,7 +874,7 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	if (priv->dispc_ops->mgr_gamma_size(priv->dispc, channel)) {
 		unsigned int gamma_lut_size = 256;
 
-		drm_crtc_enable_color_mgmt(crtc, gamma_lut_size, false, 0);
+		drm_crtc_enable_color_mgmt(crtc, gamma_lut_size, true, 0);
 		drm_mode_crtc_set_gamma_size(crtc, gamma_lut_size);
 	}
 
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 4/5] drm/omap: rearrange includes in omapdss.h

[Tomi Valkeinen]

Drop "uapi/" and rearrange alphabetically.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/omapdss.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index ab19d4af8de7..8e9a2019f173 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -7,13 +7,13 @@
 #ifndef __OMAP_DRM_DSS_H
 #define __OMAP_DRM_DSS_H
 
-#include <linux/list.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_mode.h>
 #include <linux/device.h>
 #include <linux/interrupt.h>
-#include <video/videomode.h>
+#include <linux/list.h>
 #include <linux/platform_data/omapdss.h>
-#include <uapi/drm/drm_mode.h>
-#include <drm/drm_crtc.h>
+#include <video/videomode.h>
 
 #define DISPC_IRQ_FRAMEDONE		(1 << 0)
 #define DISPC_IRQ_VSYNC			(1 << 1)
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

[PATCH 5/5] drm/omap: Enable COLOR_ENCODING and COLOR_RANGE properties for planes

[Tomi Valkeinen]

From: Jyri Sarha <jsarha@ti.com>

Adds support for COLOR_ENCODING and COLOR_RANGE properties to
omap_plane.c and dispc.c. The supported encodings and ranges are
presets are:

For COLOR_ENCODING:
- YCbCr BT.601 (default)
- YCbCr BT.709

For COLOR_RANGE:
- YCbCr limited range
- YCbCr full range (default)

Signed-off-by: Jyri Sarha <jsarha@ti.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dispc.c   | 104 ++++++++++++++++----------
 drivers/gpu/drm/omapdrm/dss/omapdss.h |   4 +
 drivers/gpu/drm/omapdrm/omap_plane.c  |  30 ++++++++
 3 files changed, 97 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 48593932bddf..bf0c9d293077 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -874,50 +874,67 @@ static void dispc_ovl_write_color_conv_coef(struct dispc_device *dispc,
 #undef CVAL
 }
 
-static void dispc_wb_write_color_conv_coef(struct dispc_device *dispc,
-					   const struct csc_coef_rgb2yuv *ct)
-{
-	const enum omap_plane_id plane = OMAP_DSS_WB;
-
-#define CVAL(x, y) (FLD_VAL(x, 26, 16) | FLD_VAL(y, 10, 0))
+/* YUV -> RGB, ITU-R BT.601, full range */
+static const struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_full = {
+	256,   0,  358,		/* ry, rcb, rcr |1.000  0.000  1.402|*/
+	256, -88, -182,		/* gy, gcb, gcr |1.000 -0.344 -0.714|*/
+	256, 452,    0,		/* by, bcb, bcr |1.000  1.772  0.000|*/
+	true,			/* full range */
+};
 
-	dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 0), CVAL(ct->yg,  ct->yr));
-	dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 1), CVAL(ct->crr, ct->yb));
-	dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 2), CVAL(ct->crb, ct->crg));
-	dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 3), CVAL(ct->cbg, ct->cbr));
-	dispc_write_reg(dispc, DISPC_OVL_CONV_COEF(plane, 4), CVAL(0, ct->cbb));
+/* YUV -> RGB, ITU-R BT.601, limited range */
+static const struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_lim = {
+	298,    0,  409,	/* ry, rcb, rcr |1.164  0.000  1.596|*/
+	298, -100, -208,	/* gy, gcb, gcr |1.164 -0.392 -0.813|*/
+	298,  516,    0,	/* by, bcb, bcr |1.164  2.017  0.000|*/
+	false,			/* limited range */
+};
 
-	REG_FLD_MOD(dispc, DISPC_OVL_ATTRIBUTES(plane), ct->full_range, 11, 11);
+/* YUV -> RGB, ITU-R BT.709, full range */
+static const struct csc_coef_yuv2rgb coefs_yuv2rgb_bt709_full = {
+	256,    0,  402,        /* ry, rcb, rcr |1.000  0.000  1.570|*/
+	256,  -48, -120,        /* gy, gcb, gcr |1.000 -0.187 -0.467|*/
+	256,  475,    0,        /* by, bcb, bcr |1.000  1.856  0.000|*/
+	true,                   /* full range */
+};
 
-#undef CVAL
-}
+/* YUV -> RGB, ITU-R BT.709, limited range */
+static const struct csc_coef_yuv2rgb coefs_yuv2rgb_bt709_lim = {
+	298,    0,  459,	/* ry, rcb, rcr |1.164  0.000  1.793|*/
+	298,  -55, -136,	/* gy, gcb, gcr |1.164 -0.213 -0.533|*/
+	298,  541,    0,	/* by, bcb, bcr |1.164  2.112  0.000|*/
+	false,			/* limited range */
+};
 
-static void dispc_setup_color_conv_coef(struct dispc_device *dispc)
+static int dispc_ovl_set_csc(struct dispc_device *dispc,
+			     enum omap_plane_id plane,
+			     enum drm_color_encoding color_encoding,
+			     enum drm_color_range color_range)
 {
-	int i;
-	int num_ovl = dispc_get_num_ovls(dispc);
-
-	/* YUV -> RGB, ITU-R BT.601, limited range */
-	const struct csc_coef_yuv2rgb coefs_yuv2rgb_bt601_lim = {
-		298,    0,  409,	/* ry, rcb, rcr */
-		298, -100, -208,	/* gy, gcb, gcr */
-		298,  516,    0,	/* by, bcb, bcr */
-		false,			/* limited range */
-	};
+	const struct csc_coef_yuv2rgb *csc;
 
-	/* RGB -> YUV, ITU-R BT.601, limited range */
-	const struct csc_coef_rgb2yuv coefs_rgb2yuv_bt601_lim = {
-		 66, 129,  25,		/* yr,   yg,  yb */
-		-38, -74, 112,		/* cbr, cbg, cbb */
-		112, -94, -18,		/* crr, crg, crb */
-		false,			/* limited range */
-	};
+	switch (color_encoding) {
+	case DRM_COLOR_YCBCR_BT601:
+		if (color_range == DRM_COLOR_YCBCR_FULL_RANGE)
+			csc = &coefs_yuv2rgb_bt601_full;
+		else
+			csc = &coefs_yuv2rgb_bt601_lim;
+		break;
+	case DRM_COLOR_YCBCR_BT709:
+		if (color_range == DRM_COLOR_YCBCR_FULL_RANGE)
+			csc = &coefs_yuv2rgb_bt709_full;
+		else
+			csc = &coefs_yuv2rgb_bt709_lim;
+		break;
+	default:
+		DSSERR("Unsupported CSC mode %d for plane %d\n",
+		       color_encoding, plane);
+		return -EINVAL;
+	}
 
-	for (i = 1; i < num_ovl; i++)
-		dispc_ovl_write_color_conv_coef(dispc, i, &coefs_yuv2rgb_bt601_lim);
+	dispc_ovl_write_color_conv_coef(dispc, plane, csc);
 
-	if (dispc->feat->has_writeback)
-		dispc_wb_write_color_conv_coef(dispc, &coefs_rgb2yuv_bt601_lim);
+	return 0;
 }
 
 static void dispc_ovl_set_ba0(struct dispc_device *dispc,
@@ -2598,7 +2615,9 @@ static int dispc_ovl_setup_common(struct dispc_device *dispc,
 				  u8 pre_mult_alpha, u8 global_alpha,
 				  enum omap_dss_rotation_type rotation_type,
 				  bool replication, const struct videomode *vm,
-				  bool mem_to_mem)
+				  bool mem_to_mem,
+				  enum drm_color_encoding color_encoding,
+				  enum drm_color_range color_range)
 {
 	bool five_taps = true;
 	bool fieldmode = false;
@@ -2747,6 +2766,9 @@ static int dispc_ovl_setup_common(struct dispc_device *dispc,
 				      fieldmode, fourcc, rotation);
 		dispc_ovl_set_output_size(dispc, plane, out_width, out_height);
 		dispc_ovl_set_vid_color_conv(dispc, plane, cconv);
+
+		if (plane != OMAP_DSS_WB)
+			dispc_ovl_set_csc(dispc, plane, color_encoding, color_range);
 	}
 
 	dispc_ovl_set_rotation_attrs(dispc, plane, rotation, rotation_type,
@@ -2783,7 +2805,8 @@ static int dispc_ovl_setup(struct dispc_device *dispc,
 		oi->screen_width, oi->pos_x, oi->pos_y, oi->width, oi->height,
 		oi->out_width, oi->out_height, oi->fourcc, oi->rotation,
 		oi->zorder, oi->pre_mult_alpha, oi->global_alpha,
-		oi->rotation_type, replication, vm, mem_to_mem);
+		oi->rotation_type, replication, vm, mem_to_mem,
+		oi->color_encoding, oi->color_range);
 
 	return r;
 }
@@ -2816,7 +2839,8 @@ static int dispc_wb_setup(struct dispc_device *dispc,
 		wi->buf_width, pos_x, pos_y, in_width, in_height, wi->width,
 		wi->height, wi->fourcc, wi->rotation, zorder,
 		wi->pre_mult_alpha, global_alpha, wi->rotation_type,
-		replication, vm, mem_to_mem);
+		replication, vm, mem_to_mem, DRM_COLOR_YCBCR_BT601,
+		DRM_COLOR_YCBCR_LIMITED_RANGE);
 	if (r)
 		return r;
 
@@ -3927,8 +3951,6 @@ static void _omap_dispc_initial_config(struct dispc_device *dispc)
 	    dispc->feat->has_gamma_table)
 		REG_FLD_MOD(dispc, DISPC_CONFIG, 1, 9, 9);
 
-	dispc_setup_color_conv_coef(dispc);
-
 	dispc_set_loadmode(dispc, OMAP_DSS_LOAD_FRAME_ONLY);
 
 	dispc_init_fifos(dispc);
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index 8e9a2019f173..816424eb2d41 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -7,6 +7,7 @@
 #ifndef __OMAP_DRM_DSS_H
 #define __OMAP_DRM_DSS_H
 
+#include <drm/drm_color_mgmt.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_mode.h>
 #include <linux/device.h>
@@ -243,6 +244,9 @@ struct omap_overlay_info {
 	u8 global_alpha;
 	u8 pre_mult_alpha;
 	u8 zorder;
+
+	enum drm_color_encoding color_encoding;
+	enum drm_color_range color_range;
 };
 
 struct omap_overlay_manager_info {
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 73ec99819a3d..1f433fb5f207 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -59,6 +59,8 @@ static void omap_plane_atomic_update(struct drm_plane *plane,
 		info.pre_mult_alpha = 1;
 	else
 		info.pre_mult_alpha = 0;
+	info.color_encoding = state->color_encoding;
+	info.color_range = state->color_range;
 
 	/* update scanout: */
 	omap_framebuffer_update_scanout(state->fb, state, &info);
@@ -189,6 +191,8 @@ static void omap_plane_reset(struct drm_plane *plane)
 	 */
 	plane->state->zpos = plane->type == DRM_PLANE_TYPE_PRIMARY
 			   ? 0 : omap_plane->id;
+	plane->state->color_encoding = DRM_COLOR_YCBCR_BT601;
+	plane->state->color_range = DRM_COLOR_YCBCR_FULL_RANGE;
 }
 
 static int omap_plane_atomic_set_property(struct drm_plane *plane,
@@ -232,6 +236,23 @@ static const struct drm_plane_funcs omap_plane_funcs = {
 	.atomic_get_property = omap_plane_atomic_get_property,
 };
 
+static bool omap_plane_supports_yuv(struct drm_plane *plane)
+{
+	struct omap_drm_private *priv = plane->dev->dev_private;
+	struct omap_plane *omap_plane = to_omap_plane(plane);
+	const u32 *formats =
+		priv->dispc_ops->ovl_get_color_modes(priv->dispc, omap_plane->id);
+	u32 i;
+
+	for (i = 0; formats[i]; i++)
+		if (formats[i] == DRM_FORMAT_YUYV ||
+		    formats[i] == DRM_FORMAT_UYVY ||
+		    formats[i] == DRM_FORMAT_NV12)
+			return true;
+
+	return false;
+}
+
 static const char *plane_id_to_name[] = {
 	[OMAP_DSS_GFX] = "gfx",
 	[OMAP_DSS_VIDEO1] = "vid1",
@@ -293,6 +314,15 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 	drm_plane_create_blend_mode_property(plane, BIT(DRM_MODE_BLEND_PREMULTI) |
 					     BIT(DRM_MODE_BLEND_COVERAGE));
 
+	if (omap_plane_supports_yuv(plane))
+		drm_plane_create_color_properties(plane,
+						  BIT(DRM_COLOR_YCBCR_BT601) |
+						  BIT(DRM_COLOR_YCBCR_BT709),
+						  BIT(DRM_COLOR_YCBCR_FULL_RANGE) |
+						  BIT(DRM_COLOR_YCBCR_LIMITED_RANGE),
+						  DRM_COLOR_YCBCR_BT601,
+						  DRM_COLOR_YCBCR_FULL_RANGE);
+
 	return plane;
 
 error:
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

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

Re: [PATCH 1/5] drm: add legacy support for using degamma for gamma

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 6688 bytes --]

On Wed, 23 Sep 2020 14:57:23 +0300
Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> We currently have drm_atomic_helper_legacy_gamma_set() helper which can
> be used to handle legacy gamma-set ioctl.
> drm_atomic_helper_legacy_gamma_set() sets GAMMA_LUT, and clears
> CTM and DEGAMMA_LUT. This works fine on HW where we have either:
> 
> degamma -> ctm -> gamma -> out
> 
> or
> 
> ctm -> gamma -> out
> 
> However, if the HW has gamma table before ctm, the atomic property
> should be DEGAMMA_LUT, and thus we have:
> 
> degamma -> ctm -> out
> 
> This is fine for userspace which sets gamma table using the properties,
> as the userspace can check for the existence of gamma & degamma, but the
> legacy gamma-set ioctl does not work.
> 
> This patch adds a new helper, drm_atomic_helper_legacy_degamma_set(),
> which can be used instead of drm_atomic_helper_legacy_gamma_set() when
> the DEGAMMA_LUT is the underlying property that needs to be set.
> 

This sounds and looks right to me fwiw, so

Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>


Thanks,
pq

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 81 ++++++++++++++++++++++-------
>  include/drm/drm_atomic_helper.h     |  4 ++
>  2 files changed, 65 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 9e1ad493e689..5ba359114df6 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3469,24 +3469,11 @@ int drm_atomic_helper_page_flip_target(struct drm_crtc *crtc,
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
>  
> -/**
> - * drm_atomic_helper_legacy_gamma_set - set the legacy gamma correction table
> - * @crtc: CRTC object
> - * @red: red correction table
> - * @green: green correction table
> - * @blue: green correction table
> - * @size: size of the tables
> - * @ctx: lock acquire context
> - *
> - * Implements support for legacy gamma correction table for drivers
> - * that support color management through the DEGAMMA_LUT/GAMMA_LUT
> - * properties. See drm_crtc_enable_color_mgmt() and the containing chapter for
> - * how the atomic color management and gamma tables work.
> - */
> -int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> -				       u16 *red, u16 *green, u16 *blue,
> -				       uint32_t size,
> -				       struct drm_modeset_acquire_ctx *ctx)
> +static int legacy_gamma_degamma_set(struct drm_crtc *crtc,
> +				    u16 *red, u16 *green, u16 *blue,
> +				    uint32_t size,
> +				    struct drm_modeset_acquire_ctx *ctx,
> +				    bool use_degamma)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_atomic_state *state;
> @@ -3525,9 +3512,11 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
>  	}
>  
>  	/* Reset DEGAMMA_LUT and CTM properties. */
> -	replaced  = drm_property_replace_blob(&crtc_state->degamma_lut, NULL);
> +	replaced  = drm_property_replace_blob(&crtc_state->degamma_lut,
> +					      use_degamma ? blob : NULL);
>  	replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
> -	replaced |= drm_property_replace_blob(&crtc_state->gamma_lut, blob);
> +	replaced |= drm_property_replace_blob(&crtc_state->gamma_lut,
> +					      use_degamma ? NULL : blob);
>  	crtc_state->color_mgmt_changed |= replaced;
>  
>  	ret = drm_atomic_commit(state);
> @@ -3537,8 +3526,60 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
>  	drm_property_blob_put(blob);
>  	return ret;
>  }
> +
> +/**
> + * drm_atomic_helper_legacy_gamma_set - set the legacy gamma correction table using gamma_lut
> + * @crtc: CRTC object
> + * @red: red correction table
> + * @green: green correction table
> + * @blue: green correction table
> + * @size: size of the tables
> + * @ctx: lock acquire context
> + *
> + * Implements support for legacy gamma correction table for drivers
> + * that support color management through the DEGAMMA_LUT/GAMMA_LUT
> + * properties. See drm_crtc_enable_color_mgmt() and the containing chapter for
> + * how the atomic color management and gamma tables work.
> + *
> + * This function uses GAMMA_LUT property for the gamma table. This function
> + * can be used when the driver exposes either only GAMMA_LUT or both GAMMA_LUT
> + * and DEGAMMA_LUT.
> + */
> +int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> +				       u16 *red, u16 *green, u16 *blue,
> +				       uint32_t size,
> +				       struct drm_modeset_acquire_ctx *ctx)
> +{
> +	return legacy_gamma_degamma_set(crtc, red, green, blue, size, ctx, false);
> +}
>  EXPORT_SYMBOL(drm_atomic_helper_legacy_gamma_set);
>  
> +/**
> + * drm_atomic_helper_legacy_degamma_set - set the legacy gamma correction table using degamma_lut
> + * @crtc: CRTC object
> + * @red: red correction table
> + * @green: green correction table
> + * @blue: green correction table
> + * @size: size of the tables
> + * @ctx: lock acquire context
> + *
> + * Implements support for legacy gamma correction table for drivers
> + * that support color management through the DEGAMMA_LUT/GAMMA_LUT
> + * properties. See drm_crtc_enable_color_mgmt() and the containing chapter for
> + * how the atomic color management and gamma tables work.
> + *
> + * This function uses DEGAMMA_LUT property for the gamma table. This function
> + * can be used when the driver exposes only DEGAMNMA_LUT.
> + */
> +int drm_atomic_helper_legacy_degamma_set(struct drm_crtc *crtc,
> +					 u16 *red, u16 *green, u16 *blue,
> +					 uint32_t size,
> +					 struct drm_modeset_acquire_ctx *ctx)
> +{
> +	return legacy_gamma_degamma_set(crtc, red, green, blue, size, ctx, true);
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_legacy_degamma_set);
> +
>  /**
>   * drm_atomic_helper_bridge_propagate_bus_fmt() - Propagate output format to
>   *						  the input end of a bridge
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index b268180c97eb..e60f5a600195 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -148,6 +148,10 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
>  				       u16 *red, u16 *green, u16 *blue,
>  				       uint32_t size,
>  				       struct drm_modeset_acquire_ctx *ctx);
> +int drm_atomic_helper_legacy_degamma_set(struct drm_crtc *crtc,
> +					 u16 *red, u16 *green, u16 *blue,
> +					 uint32_t size,
> +					 struct drm_modeset_acquire_ctx *ctx);
>  
>  /**
>   * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH 2/5] drm/omap: use degamma property for gamma table

[Pekka Paalanen]

[-- Attachment #1.1: Type: text/plain, Size: 2941 bytes --]

On Wed, 23 Sep 2020 14:57:24 +0300
Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

> omapdrm supports gamma via GAMMA_LUT property. However, the HW we have
> is:
> 
> gamma -> ctm -> out
> 
> instead of what the model DRM framework uses:
> 
> ctm -> gamma -> out
> 
> As the following patches add CTM support for omapdrm, lets first fix the
> gamma.
> 
> This patch changes the property from GAMMA_LUT to DEGAMMA_LUT, and uses
> drm_atomic_helper_legacy_degamma_set for gamma_set helper. Thus we will
> have:
> 
> degamma -> ctm -> out
> 
> and the legacy ioctl will continue working as before.
> 

Makes sense to me:

Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com>


Thanks,
pq

> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index 328a4a74f534..6116af920660 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -573,8 +573,8 @@ static int omap_crtc_atomic_check(struct drm_crtc *crtc,
>  {
>  	struct drm_plane_state *pri_state;
>  
> -	if (state->color_mgmt_changed && state->gamma_lut) {
> -		unsigned int length = state->gamma_lut->length /
> +	if (state->color_mgmt_changed && state->degamma_lut) {
> +		unsigned int length = state->degamma_lut->length /
>  			sizeof(struct drm_color_lut);
>  
>  		if (length < 2)
> @@ -614,10 +614,10 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
>  		struct drm_color_lut *lut = NULL;
>  		unsigned int length = 0;
>  
> -		if (crtc->state->gamma_lut) {
> +		if (crtc->state->degamma_lut) {
>  			lut = (struct drm_color_lut *)
> -				crtc->state->gamma_lut->data;
> -			length = crtc->state->gamma_lut->length /
> +				crtc->state->degamma_lut->data;
> +			length = crtc->state->degamma_lut->length /
>  				sizeof(*lut);
>  		}
>  		priv->dispc_ops->mgr_set_gamma(priv->dispc, omap_crtc->channel,
> @@ -738,7 +738,7 @@ static const struct drm_crtc_funcs omap_crtc_funcs = {
>  	.set_config = drm_atomic_helper_set_config,
>  	.destroy = omap_crtc_destroy,
>  	.page_flip = drm_atomic_helper_page_flip,
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
> +	.gamma_set = drm_atomic_helper_legacy_degamma_set,
>  	.atomic_duplicate_state = omap_crtc_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>  	.atomic_set_property = omap_crtc_atomic_set_property,
> @@ -839,7 +839,7 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
>  	if (priv->dispc_ops->mgr_gamma_size(priv->dispc, channel)) {
>  		unsigned int gamma_lut_size = 256;
>  
> -		drm_crtc_enable_color_mgmt(crtc, 0, false, gamma_lut_size);
> +		drm_crtc_enable_color_mgmt(crtc, gamma_lut_size, false, 0);
>  		drm_mode_crtc_set_gamma_size(crtc, gamma_lut_size);
>  	}
>  


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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