dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] drm: automatic legacy gamma support
@ 2020-12-11 11:42 Tomi Valkeinen
  2020-12-11 11:42 ` [PATCH v4 1/2] " Tomi Valkeinen
  2020-12-11 11:42 ` [PATCH v4 2/2] drm: add legacy support for using degamma for gamma Tomi Valkeinen
  0 siblings, 2 replies; 8+ messages in thread
From: Tomi Valkeinen @ 2020-12-11 11:42 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, Ville Syrjälä, Laurent Pinchart
  Cc: Yannick Fertre, Philippe Cornu, David Airlie, Russell King,
	Sandy Huang, Paul Cercueil, Tomi Valkeinen, Thomas Zimmermann,
	Matthias Brugger, Vincent Abriou, Maxime Coquelin,
	Alexandre Torgue

Hi,

Yet another try. This time I don't touch the drm_fb_helper.c at all,
except adding a FIXME comment. Now everything is inside
drm_color_mgmt.c.

 Tomi

Tomi Valkeinen (2):
  drm: automatic legacy gamma support
  drm: add legacy support for using degamma for gamma

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   1 -
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  |   1 -
 drivers/gpu/drm/arm/malidp_crtc.c             |   1 -
 drivers/gpu/drm/armada/armada_crtc.c          |   1 -
 drivers/gpu/drm/ast/ast_mode.c                |   1 -
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |   1 -
 drivers/gpu/drm/drm_atomic_helper.c           |  70 ----------
 drivers/gpu/drm/drm_color_mgmt.c              | 125 ++++++++++++++++--
 drivers/gpu/drm/drm_fb_helper.c               |   5 +
 drivers/gpu/drm/i915/display/intel_display.c  |   1 -
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c     |   2 -
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   1 -
 drivers/gpu/drm/nouveau/dispnv50/head.c       |   2 -
 drivers/gpu/drm/omapdrm/omap_crtc.c           |   1 -
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |   1 -
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |   1 -
 drivers/gpu/drm/stm/ltdc.c                    |   1 -
 drivers/gpu/drm/vc4/vc4_crtc.c                |   1 -
 drivers/gpu/drm/vc4/vc4_txp.c                 |   1 -
 include/drm/drm_atomic_helper.h               |   4 -
 20 files changed, 121 insertions(+), 101 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 1/2] drm: automatic legacy gamma support
  2020-12-11 11:42 [PATCH v4 0/2] drm: automatic legacy gamma support Tomi Valkeinen
@ 2020-12-11 11:42 ` Tomi Valkeinen
  2020-12-11 11:50   ` Laurent Pinchart
                     ` (2 more replies)
  2020-12-11 11:42 ` [PATCH v4 2/2] drm: add legacy support for using degamma for gamma Tomi Valkeinen
  1 sibling, 3 replies; 8+ messages in thread
From: Tomi Valkeinen @ 2020-12-11 11:42 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, Ville Syrjälä, Laurent Pinchart
  Cc: Yannick Fertre, Philippe Cornu, David Airlie, Russell King,
	Sandy Huang, Paul Cercueil, Tomi Valkeinen, Thomas Zimmermann,
	Matthias Brugger, Vincent Abriou, Maxime Coquelin,
	Alexandre Torgue

To support legacy gamma ioctls the drivers need to set
drm_crtc_funcs.gamma_set either to a custom implementation or to
drm_atomic_helper_legacy_gamma_set. Most of the atomic drivers do the
latter.

We can simplify this by making the core handle it automatically.

Move the drm_atomic_helper_legacy_gamma_set() functionality into
drm_color_mgmt.c to make drm_mode_gamma_set_ioctl() use
drm_crtc_funcs.gamma_set if set or GAMMA_LUT property if not.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   1 -
 .../gpu/drm/arm/display/komeda/komeda_crtc.c  |   1 -
 drivers/gpu/drm/arm/malidp_crtc.c             |   1 -
 drivers/gpu/drm/armada/armada_crtc.c          |   1 -
 drivers/gpu/drm/ast/ast_mode.c                |   1 -
 .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |   1 -
 drivers/gpu/drm/drm_atomic_helper.c           |  70 -----------
 drivers/gpu/drm/drm_color_mgmt.c              | 111 ++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_display.c  |   1 -
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c     |   2 -
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   1 -
 drivers/gpu/drm/nouveau/dispnv50/head.c       |   2 -
 drivers/gpu/drm/omapdrm/omap_crtc.c           |   1 -
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |   1 -
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |   1 -
 drivers/gpu/drm/stm/ltdc.c                    |   1 -
 drivers/gpu/drm/vc4/vc4_crtc.c                |   1 -
 drivers/gpu/drm/vc4/vc4_txp.c                 |   1 -
 include/drm/drm_atomic_helper.h               |   4 -
 19 files changed, 102 insertions(+), 101 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 2855bb918535..848b06c51b0e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5396,7 +5396,6 @@ static void dm_disable_vblank(struct drm_crtc *crtc)
 static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = {
 	.reset = dm_crtc_reset_state,
 	.destroy = amdgpu_dm_crtc_destroy,
-	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
 	.atomic_duplicate_state = dm_crtc_duplicate_state,
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
index 4b485eb512e2..59172acb9738 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
@@ -550,7 +550,6 @@ static void komeda_crtc_vblank_disable(struct drm_crtc *crtc)
 }
 
 static const struct drm_crtc_funcs komeda_crtc_funcs = {
-	.gamma_set		= drm_atomic_helper_legacy_gamma_set,
 	.destroy		= drm_crtc_cleanup,
 	.set_config		= drm_atomic_helper_set_config,
 	.page_flip		= drm_atomic_helper_page_flip,
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
index 108e7a31bd26..494075ddbef6 100644
--- a/drivers/gpu/drm/arm/malidp_crtc.c
+++ b/drivers/gpu/drm/arm/malidp_crtc.c
@@ -510,7 +510,6 @@ static void malidp_crtc_disable_vblank(struct drm_crtc *crtc)
 }
 
 static const struct drm_crtc_funcs malidp_crtc_funcs = {
-	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 	.destroy = drm_crtc_cleanup,
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
index 3ebcf5a52c8b..b7bb90ae787f 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -820,7 +820,6 @@ static const struct drm_crtc_funcs armada_crtc_funcs = {
 	.cursor_set	= armada_drm_crtc_cursor_set,
 	.cursor_move	= armada_drm_crtc_cursor_move,
 	.destroy	= armada_drm_crtc_destroy,
-	.gamma_set	= drm_atomic_helper_legacy_gamma_set,
 	.set_config	= drm_atomic_helper_set_config,
 	.page_flip	= drm_atomic_helper_page_flip,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 9db371f4054f..5b0ec785c516 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -903,7 +903,6 @@ static void ast_crtc_atomic_destroy_state(struct drm_crtc *crtc,
 
 static const struct drm_crtc_funcs ast_crtc_funcs = {
 	.reset = ast_crtc_reset,
-	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 	.destroy = drm_crtc_cleanup,
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index c58fa00b4848..05ad75d155e8 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -473,7 +473,6 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = {
 	.atomic_destroy_state = atmel_hlcdc_crtc_destroy_state,
 	.enable_vblank = atmel_hlcdc_crtc_enable_vblank,
 	.disable_vblank = atmel_hlcdc_crtc_disable_vblank,
-	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 };
 
 int atmel_hlcdc_crtc_create(struct drm_device *dev)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index ba1507036f26..b114658100b3 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3499,76 +3499,6 @@ 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)
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_atomic_state *state;
-	struct drm_crtc_state *crtc_state;
-	struct drm_property_blob *blob = NULL;
-	struct drm_color_lut *blob_data;
-	int i, ret = 0;
-	bool replaced;
-
-	state = drm_atomic_state_alloc(crtc->dev);
-	if (!state)
-		return -ENOMEM;
-
-	blob = drm_property_create_blob(dev,
-					sizeof(struct drm_color_lut) * size,
-					NULL);
-	if (IS_ERR(blob)) {
-		ret = PTR_ERR(blob);
-		blob = NULL;
-		goto fail;
-	}
-
-	/* Prepare GAMMA_LUT with the legacy values. */
-	blob_data = blob->data;
-	for (i = 0; i < size; i++) {
-		blob_data[i].red = red[i];
-		blob_data[i].green = green[i];
-		blob_data[i].blue = blue[i];
-	}
-
-	state->acquire_ctx = ctx;
-	crtc_state = drm_atomic_get_crtc_state(state, crtc);
-	if (IS_ERR(crtc_state)) {
-		ret = PTR_ERR(crtc_state);
-		goto fail;
-	}
-
-	/* Reset DEGAMMA_LUT and CTM properties. */
-	replaced  = drm_property_replace_blob(&crtc_state->degamma_lut, NULL);
-	replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
-	replaced |= drm_property_replace_blob(&crtc_state->gamma_lut, blob);
-	crtc_state->color_mgmt_changed |= replaced;
-
-	ret = drm_atomic_commit(state);
-
-fail:
-	drm_atomic_state_put(state);
-	drm_property_blob_put(blob);
-	return ret;
-}
-EXPORT_SYMBOL(drm_atomic_helper_legacy_gamma_set);
-
 /**
  * drm_atomic_helper_bridge_propagate_bus_fmt() - Propagate output format to
  *						  the input end of a bridge
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 3bcabc2f6e0e..65eb36dc92bf 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -22,6 +22,7 @@
 
 #include <linux/uaccess.h>
 
+#include <drm/drm_atomic.h>
 #include <drm/drm_color_mgmt.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_device.h>
@@ -89,9 +90,8 @@
  *	modes) appropriately.
  *
  * There is also support for a legacy gamma table, which is set up by calling
- * drm_mode_crtc_set_gamma_size(). Drivers which support both should use
- * drm_atomic_helper_legacy_gamma_set() to alias the legacy gamma ramp with the
- * "GAMMA_LUT" property above.
+ * drm_mode_crtc_set_gamma_size(). The DRM core will then alias the legacy gamma
+ * ramp with "GAMMA_LUT".
  *
  * Support for different non RGB color encodings is controlled through
  * &drm_plane specific COLOR_ENCODING and COLOR_RANGE properties. They
@@ -156,9 +156,6 @@ EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
  * optional. The gamma and degamma properties are only attached if
  * their size is not 0 and ctm_property is only attached if has_ctm is
  * true.
- *
- * Drivers should use drm_atomic_helper_legacy_gamma_set() to implement the
- * legacy &drm_crtc_funcs.gamma_set callback.
  */
 void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 				uint degamma_lut_size,
@@ -231,6 +228,102 @@ int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
 }
 EXPORT_SYMBOL(drm_mode_crtc_set_gamma_size);
 
+/**
+ * drm_crtc_supports_legacy_gamma - does the crtc support legacy gamma correction table
+ * @crtc: CRTC object
+ *
+ * Returns true/false if the given crtc supports setting the legacy gamma
+ * correction table.
+ */
+static bool drm_crtc_supports_legacy_gamma(struct drm_crtc *crtc)
+{
+	uint32_t gamma_id = crtc->dev->mode_config.gamma_lut_property->base.id;
+
+	if (!crtc->gamma_size)
+		return false;
+
+	if (crtc->funcs->gamma_set)
+		return true;
+
+	return !!drm_mode_obj_find_prop_id(&crtc->base, gamma_id);
+}
+
+/**
+ * drm_crtc_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 have set drm_crtc_funcs.gamma_set or 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 sets the gamma using drm_crtc_funcs.gamma_set if set, or
+ * alternatively using crtc color management properties.
+ */
+static int drm_crtc_legacy_gamma_set(struct drm_crtc *crtc,
+			      u16 *red, u16 *green, u16 *blue,
+			      uint32_t size,
+			      struct drm_modeset_acquire_ctx *ctx)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_atomic_state *state;
+	struct drm_crtc_state *crtc_state;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+	int i, ret = 0;
+	bool replaced;
+
+	if (crtc->funcs->gamma_set)
+		return crtc->funcs->gamma_set(crtc, red, green, blue, size, ctx);
+
+	state = drm_atomic_state_alloc(crtc->dev);
+	if (!state)
+		return -ENOMEM;
+
+	blob = drm_property_create_blob(dev,
+					sizeof(struct drm_color_lut) * size,
+					NULL);
+	if (IS_ERR(blob)) {
+		ret = PTR_ERR(blob);
+		blob = NULL;
+		goto fail;
+	}
+
+	/* Prepare GAMMA_LUT with the legacy values. */
+	blob_data = blob->data;
+	for (i = 0; i < size; i++) {
+		blob_data[i].red = red[i];
+		blob_data[i].green = green[i];
+		blob_data[i].blue = blue[i];
+	}
+
+	state->acquire_ctx = ctx;
+	crtc_state = drm_atomic_get_crtc_state(state, crtc);
+	if (IS_ERR(crtc_state)) {
+		ret = PTR_ERR(crtc_state);
+		goto fail;
+	}
+
+	/* Set GAMMA_LUT and reset DEGAMMA_LUT and CTM */
+	replaced = drm_property_replace_blob(&crtc_state->degamma_lut, NULL);
+	replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
+	replaced |= drm_property_replace_blob(&crtc_state->gamma_lut, blob);
+	crtc_state->color_mgmt_changed |= replaced;
+
+	ret = drm_atomic_commit(state);
+
+fail:
+	drm_atomic_state_put(state);
+	drm_property_blob_put(blob);
+	return ret;
+}
+
 /**
  * drm_mode_gamma_set_ioctl - set the gamma table
  * @dev: DRM device
@@ -262,7 +355,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 	if (!crtc)
 		return -ENOENT;
 
-	if (crtc->funcs->gamma_set == NULL)
+	if (!drm_crtc_supports_legacy_gamma(crtc))
 		return -ENOSYS;
 
 	/* memcpy into gamma store */
@@ -290,8 +383,8 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 		goto out;
 	}
 
-	ret = crtc->funcs->gamma_set(crtc, r_base, g_base, b_base,
-				     crtc->gamma_size, &ctx);
+	ret = drm_crtc_legacy_gamma_set(crtc, r_base, g_base, b_base,
+					crtc->gamma_size, &ctx);
 
 out:
 	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index cddbda5303ff..1e90b8c3b1c7 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -16879,7 +16879,6 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
 }
 
 #define INTEL_CRTC_FUNCS \
-	.gamma_set = drm_atomic_helper_legacy_gamma_set, \
 	.set_config = drm_atomic_helper_set_config, \
 	.destroy = intel_crtc_destroy, \
 	.page_flip = drm_atomic_helper_page_flip, \
diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
index 42d335d3a114..7bb31fbee29d 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
@@ -774,8 +774,6 @@ static const struct drm_crtc_funcs ingenic_drm_crtc_funcs = {
 
 	.enable_vblank		= ingenic_drm_enable_vblank,
 	.disable_vblank		= ingenic_drm_disable_vblank,
-
-	.gamma_set		= drm_atomic_helper_legacy_gamma_set,
 };
 
 static const struct drm_plane_helper_funcs ingenic_drm_plane_helper_funcs = {
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index bfe994230543..b665bd498a4a 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -619,7 +619,6 @@ static const struct drm_crtc_funcs mtk_crtc_funcs = {
 	.reset			= mtk_drm_crtc_reset,
 	.atomic_duplicate_state	= mtk_drm_crtc_duplicate_state,
 	.atomic_destroy_state	= mtk_drm_crtc_destroy_state,
-	.gamma_set		= drm_atomic_helper_legacy_gamma_set,
 	.enable_vblank		= mtk_drm_crtc_enable_vblank,
 	.disable_vblank		= mtk_drm_crtc_disable_vblank,
 };
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c
index 537c1ef2e464..ec361d17e900 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -503,7 +503,6 @@ nv50_head_destroy(struct drm_crtc *crtc)
 static const struct drm_crtc_funcs
 nv50_head_func = {
 	.reset = nv50_head_reset,
-	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 	.destroy = nv50_head_destroy,
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
@@ -518,7 +517,6 @@ nv50_head_func = {
 static const struct drm_crtc_funcs
 nvd9_head_func = {
 	.reset = nv50_head_reset,
-	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 	.destroy = nv50_head_destroy,
 	.set_config = drm_atomic_helper_set_config,
 	.page_flip = drm_atomic_helper_page_flip,
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 7d66269ad998..b97f21cf4a91 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -741,7 +741,6 @@ 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,
 	.atomic_duplicate_state = omap_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 	.atomic_set_property = omap_crtc_atomic_set_property,
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index b5fb941e0f53..f93e0750431d 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -1144,7 +1144,6 @@ static const struct drm_crtc_funcs crtc_funcs_gen3 = {
 	.set_crc_source = rcar_du_crtc_set_crc_source,
 	.verify_crc_source = rcar_du_crtc_verify_crc_source,
 	.get_crc_sources = rcar_du_crtc_get_crc_sources,
-	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 };
 
 /* -----------------------------------------------------------------------------
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index d1e05482641b..8d15cabdcb02 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1643,7 +1643,6 @@ static const struct drm_crtc_funcs vop_crtc_funcs = {
 	.disable_vblank = vop_crtc_disable_vblank,
 	.set_crc_source = vop_crtc_set_crc_source,
 	.verify_crc_source = vop_crtc_verify_crc_source,
-	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 };
 
 static void vop_fb_unref_worker(struct drm_flip_work *work, void *val)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 3980677435cb..7812094f93d6 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -713,7 +713,6 @@ static const struct drm_crtc_funcs ltdc_crtc_funcs = {
 	.enable_vblank = ltdc_crtc_enable_vblank,
 	.disable_vblank = ltdc_crtc_disable_vblank,
 	.get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
-	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 };
 
 /*
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index ea710beb8e00..7495d70d6a32 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -884,7 +884,6 @@ static const struct drm_crtc_funcs vc4_crtc_funcs = {
 	.reset = vc4_crtc_reset,
 	.atomic_duplicate_state = vc4_crtc_duplicate_state,
 	.atomic_destroy_state = vc4_crtc_destroy_state,
-	.gamma_set = drm_atomic_helper_legacy_gamma_set,
 	.enable_vblank = vc4_enable_vblank,
 	.disable_vblank = vc4_disable_vblank,
 	.get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
index 8aa5220885f4..e4b782e42f69 100644
--- a/drivers/gpu/drm/vc4/vc4_txp.c
+++ b/drivers/gpu/drm/vc4/vc4_txp.c
@@ -382,7 +382,6 @@ static const struct drm_crtc_funcs vc4_txp_crtc_funcs = {
 	.reset			= vc4_crtc_reset,
 	.atomic_duplicate_state	= vc4_crtc_duplicate_state,
 	.atomic_destroy_state	= vc4_crtc_destroy_state,
-	.gamma_set		= drm_atomic_helper_legacy_gamma_set,
 	.enable_vblank		= vc4_txp_enable_vblank,
 	.disable_vblank		= vc4_txp_disable_vblank,
 };
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 5f47720440fa..4045e2507e11 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -147,10 +147,6 @@ int drm_atomic_helper_page_flip_target(
 				uint32_t flags,
 				uint32_t target,
 				struct drm_modeset_acquire_ctx *ctx);
-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);
 
 /**
  * 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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 2/2] drm: add legacy support for using degamma for gamma
  2020-12-11 11:42 [PATCH v4 0/2] drm: automatic legacy gamma support Tomi Valkeinen
  2020-12-11 11:42 ` [PATCH v4 1/2] " Tomi Valkeinen
@ 2020-12-11 11:42 ` Tomi Valkeinen
  2020-12-11 11:52   ` Laurent Pinchart
  1 sibling, 1 reply; 8+ messages in thread
From: Tomi Valkeinen @ 2020-12-11 11:42 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, Ville Syrjälä, Laurent Pinchart
  Cc: Yannick Fertre, Philippe Cornu, David Airlie, Russell King,
	Sandy Huang, Paul Cercueil, Tomi Valkeinen, Thomas Zimmermann,
	Matthias Brugger, Vincent Abriou, Maxime Coquelin,
	Alexandre Torgue

The DRM core handles legacy gamma-set ioctl by setting GAMMA_LUT and
clearing 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.

Change the DRM core to use DEGAMMA_LUT instead of GAMMA_LUT when the
latter is unavailable.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/drm_color_mgmt.c | 22 ++++++++++++++++++----
 drivers/gpu/drm/drm_fb_helper.c  |  5 +++++
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 65eb36dc92bf..9100aac1570c 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -91,7 +91,7 @@
  *
  * There is also support for a legacy gamma table, which is set up by calling
  * drm_mode_crtc_set_gamma_size(). The DRM core will then alias the legacy gamma
- * ramp with "GAMMA_LUT".
+ * ramp with "GAMMA_LUT" or, if that is unavailable, "DEGAMMA_LUT".
  *
  * Support for different non RGB color encodings is controlled through
  * &drm_plane specific COLOR_ENCODING and COLOR_RANGE properties. They
@@ -238,6 +238,7 @@ EXPORT_SYMBOL(drm_mode_crtc_set_gamma_size);
 static bool drm_crtc_supports_legacy_gamma(struct drm_crtc *crtc)
 {
 	uint32_t gamma_id = crtc->dev->mode_config.gamma_lut_property->base.id;
+	uint32_t degamma_id = crtc->dev->mode_config.degamma_lut_property->base.id;
 
 	if (!crtc->gamma_size)
 		return false;
@@ -245,7 +246,8 @@ static bool drm_crtc_supports_legacy_gamma(struct drm_crtc *crtc)
 	if (crtc->funcs->gamma_set)
 		return true;
 
-	return !!drm_mode_obj_find_prop_id(&crtc->base, gamma_id);
+	return !!(drm_mode_obj_find_prop_id(&crtc->base, gamma_id) ||
+		  drm_mode_obj_find_prop_id(&crtc->base, degamma_id));
 }
 
 /**
@@ -276,12 +278,22 @@ static int drm_crtc_legacy_gamma_set(struct drm_crtc *crtc,
 	struct drm_crtc_state *crtc_state;
 	struct drm_property_blob *blob;
 	struct drm_color_lut *blob_data;
+	uint32_t gamma_id = dev->mode_config.gamma_lut_property->base.id;
+	uint32_t degamma_id = dev->mode_config.degamma_lut_property->base.id;
+	bool use_gamma_lut;
 	int i, ret = 0;
 	bool replaced;
 
 	if (crtc->funcs->gamma_set)
 		return crtc->funcs->gamma_set(crtc, red, green, blue, size, ctx);
 
+	if (drm_mode_obj_find_prop_id(&crtc->base, gamma_id))
+		use_gamma_lut = true;
+	else if (drm_mode_obj_find_prop_id(&crtc->base, degamma_id))
+		use_gamma_lut = false;
+	else
+		return -ENODEV;
+
 	state = drm_atomic_state_alloc(crtc->dev);
 	if (!state)
 		return -ENOMEM;
@@ -311,9 +323,11 @@ static int drm_crtc_legacy_gamma_set(struct drm_crtc *crtc,
 	}
 
 	/* Set GAMMA_LUT and reset DEGAMMA_LUT and CTM */
-	replaced = drm_property_replace_blob(&crtc_state->degamma_lut, NULL);
+	replaced = drm_property_replace_blob(&crtc_state->degamma_lut,
+					     use_gamma_lut ? NULL : blob);
 	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_gamma_lut ? blob : NULL);
 	crtc_state->color_mgmt_changed |= replaced;
 
 	ret = drm_atomic_commit(state);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index e82db0f4e771..5ad19785daee 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1059,6 +1059,11 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
 			goto out_state;
 		}
 
+		/*
+		 * FIXME: This always uses gamma_lut. Some HW have only
+		 * degamma_lut, in which case we should reset gamma_lut and set
+		 * degamma_lut. See drm_crtc_legacy_gamma_set().
+		 */
 		replaced  = drm_property_replace_blob(&crtc_state->degamma_lut,
 						      NULL);
 		replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
-- 
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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] drm: automatic legacy gamma support
  2020-12-11 11:42 ` [PATCH v4 1/2] " Tomi Valkeinen
@ 2020-12-11 11:50   ` Laurent Pinchart
  2020-12-14 13:04     ` Tomi Valkeinen
  2020-12-14 13:43   ` Philippe CORNU
  2020-12-15 12:45   ` Tomi Valkeinen
  2 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2020-12-11 11:50 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Yannick Fertre, Thomas Zimmermann, Philippe Cornu, David Airlie,
	Maxime Coquelin, Russell King, dri-devel, Sandy Huang,
	Paul Cercueil, Alexandre Torgue, Matthias Brugger,
	Vincent Abriou

Hi Tomi,

Thank you for the patch.

On Fri, Dec 11, 2020 at 01:42:36PM +0200, Tomi Valkeinen wrote:
> To support legacy gamma ioctls the drivers need to set
> drm_crtc_funcs.gamma_set either to a custom implementation or to
> drm_atomic_helper_legacy_gamma_set. Most of the atomic drivers do the
> latter.
> 
> We can simplify this by making the core handle it automatically.
> 
> Move the drm_atomic_helper_legacy_gamma_set() functionality into
> drm_color_mgmt.c to make drm_mode_gamma_set_ioctl() use
> drm_crtc_funcs.gamma_set if set or GAMMA_LUT property if not.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   1 -
>  .../gpu/drm/arm/display/komeda/komeda_crtc.c  |   1 -
>  drivers/gpu/drm/arm/malidp_crtc.c             |   1 -
>  drivers/gpu/drm/armada/armada_crtc.c          |   1 -
>  drivers/gpu/drm/ast/ast_mode.c                |   1 -
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |   1 -
>  drivers/gpu/drm/drm_atomic_helper.c           |  70 -----------
>  drivers/gpu/drm/drm_color_mgmt.c              | 111 ++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_display.c  |   1 -
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c     |   2 -
>  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   1 -
>  drivers/gpu/drm/nouveau/dispnv50/head.c       |   2 -
>  drivers/gpu/drm/omapdrm/omap_crtc.c           |   1 -
>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |   1 -
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |   1 -
>  drivers/gpu/drm/stm/ltdc.c                    |   1 -
>  drivers/gpu/drm/vc4/vc4_crtc.c                |   1 -
>  drivers/gpu/drm/vc4/vc4_txp.c                 |   1 -
>  include/drm/drm_atomic_helper.h               |   4 -
>  19 files changed, 102 insertions(+), 101 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 2855bb918535..848b06c51b0e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5396,7 +5396,6 @@ static void dm_disable_vblank(struct drm_crtc *crtc)
>  static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = {
>  	.reset = dm_crtc_reset_state,
>  	.destroy = amdgpu_dm_crtc_destroy,
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  	.set_config = drm_atomic_helper_set_config,
>  	.page_flip = drm_atomic_helper_page_flip,
>  	.atomic_duplicate_state = dm_crtc_duplicate_state,
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> index 4b485eb512e2..59172acb9738 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_crtc.c
> @@ -550,7 +550,6 @@ static void komeda_crtc_vblank_disable(struct drm_crtc *crtc)
>  }
>  
>  static const struct drm_crtc_funcs komeda_crtc_funcs = {
> -	.gamma_set		= drm_atomic_helper_legacy_gamma_set,
>  	.destroy		= drm_crtc_cleanup,
>  	.set_config		= drm_atomic_helper_set_config,
>  	.page_flip		= drm_atomic_helper_page_flip,
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
> index 108e7a31bd26..494075ddbef6 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -510,7 +510,6 @@ static void malidp_crtc_disable_vblank(struct drm_crtc *crtc)
>  }
>  
>  static const struct drm_crtc_funcs malidp_crtc_funcs = {
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  	.destroy = drm_crtc_cleanup,
>  	.set_config = drm_atomic_helper_set_config,
>  	.page_flip = drm_atomic_helper_page_flip,
> diff --git a/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c
> index 3ebcf5a52c8b..b7bb90ae787f 100644
> --- a/drivers/gpu/drm/armada/armada_crtc.c
> +++ b/drivers/gpu/drm/armada/armada_crtc.c
> @@ -820,7 +820,6 @@ static const struct drm_crtc_funcs armada_crtc_funcs = {
>  	.cursor_set	= armada_drm_crtc_cursor_set,
>  	.cursor_move	= armada_drm_crtc_cursor_move,
>  	.destroy	= armada_drm_crtc_destroy,
> -	.gamma_set	= drm_atomic_helper_legacy_gamma_set,
>  	.set_config	= drm_atomic_helper_set_config,
>  	.page_flip	= drm_atomic_helper_page_flip,
>  	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 9db371f4054f..5b0ec785c516 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -903,7 +903,6 @@ static void ast_crtc_atomic_destroy_state(struct drm_crtc *crtc,
>  
>  static const struct drm_crtc_funcs ast_crtc_funcs = {
>  	.reset = ast_crtc_reset,
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  	.destroy = drm_crtc_cleanup,
>  	.set_config = drm_atomic_helper_set_config,
>  	.page_flip = drm_atomic_helper_page_flip,
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index c58fa00b4848..05ad75d155e8 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -473,7 +473,6 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = {
>  	.atomic_destroy_state = atmel_hlcdc_crtc_destroy_state,
>  	.enable_vblank = atmel_hlcdc_crtc_enable_vblank,
>  	.disable_vblank = atmel_hlcdc_crtc_disable_vblank,
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  };
>  
>  int atmel_hlcdc_crtc_create(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index ba1507036f26..b114658100b3 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3499,76 +3499,6 @@ 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)
> -{
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_atomic_state *state;
> -	struct drm_crtc_state *crtc_state;
> -	struct drm_property_blob *blob = NULL;
> -	struct drm_color_lut *blob_data;
> -	int i, ret = 0;
> -	bool replaced;
> -
> -	state = drm_atomic_state_alloc(crtc->dev);
> -	if (!state)
> -		return -ENOMEM;
> -
> -	blob = drm_property_create_blob(dev,
> -					sizeof(struct drm_color_lut) * size,
> -					NULL);
> -	if (IS_ERR(blob)) {
> -		ret = PTR_ERR(blob);
> -		blob = NULL;
> -		goto fail;
> -	}
> -
> -	/* Prepare GAMMA_LUT with the legacy values. */
> -	blob_data = blob->data;
> -	for (i = 0; i < size; i++) {
> -		blob_data[i].red = red[i];
> -		blob_data[i].green = green[i];
> -		blob_data[i].blue = blue[i];
> -	}
> -
> -	state->acquire_ctx = ctx;
> -	crtc_state = drm_atomic_get_crtc_state(state, crtc);
> -	if (IS_ERR(crtc_state)) {
> -		ret = PTR_ERR(crtc_state);
> -		goto fail;
> -	}
> -
> -	/* Reset DEGAMMA_LUT and CTM properties. */
> -	replaced  = drm_property_replace_blob(&crtc_state->degamma_lut, NULL);
> -	replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
> -	replaced |= drm_property_replace_blob(&crtc_state->gamma_lut, blob);
> -	crtc_state->color_mgmt_changed |= replaced;
> -
> -	ret = drm_atomic_commit(state);
> -
> -fail:
> -	drm_atomic_state_put(state);
> -	drm_property_blob_put(blob);
> -	return ret;
> -}
> -EXPORT_SYMBOL(drm_atomic_helper_legacy_gamma_set);
> -
>  /**
>   * drm_atomic_helper_bridge_propagate_bus_fmt() - Propagate output format to
>   *						  the input end of a bridge
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 3bcabc2f6e0e..65eb36dc92bf 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -22,6 +22,7 @@
>  
>  #include <linux/uaccess.h>
>  
> +#include <drm/drm_atomic.h>
>  #include <drm/drm_color_mgmt.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_device.h>
> @@ -89,9 +90,8 @@
>   *	modes) appropriately.
>   *
>   * There is also support for a legacy gamma table, which is set up by calling
> - * drm_mode_crtc_set_gamma_size(). Drivers which support both should use
> - * drm_atomic_helper_legacy_gamma_set() to alias the legacy gamma ramp with the
> - * "GAMMA_LUT" property above.
> + * drm_mode_crtc_set_gamma_size(). The DRM core will then alias the legacy gamma
> + * ramp with "GAMMA_LUT".
>   *
>   * Support for different non RGB color encodings is controlled through
>   * &drm_plane specific COLOR_ENCODING and COLOR_RANGE properties. They
> @@ -156,9 +156,6 @@ EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
>   * optional. The gamma and degamma properties are only attached if
>   * their size is not 0 and ctm_property is only attached if has_ctm is
>   * true.
> - *
> - * Drivers should use drm_atomic_helper_legacy_gamma_set() to implement the
> - * legacy &drm_crtc_funcs.gamma_set callback.
>   */
>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  				uint degamma_lut_size,
> @@ -231,6 +228,102 @@ int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc,
>  }
>  EXPORT_SYMBOL(drm_mode_crtc_set_gamma_size);
>  
> +/**
> + * drm_crtc_supports_legacy_gamma - does the crtc support legacy gamma correction table
> + * @crtc: CRTC object
> + *
> + * Returns true/false if the given crtc supports setting the legacy gamma
> + * correction table.
> + */
> +static bool drm_crtc_supports_legacy_gamma(struct drm_crtc *crtc)
> +{
> +	uint32_t gamma_id = crtc->dev->mode_config.gamma_lut_property->base.id;
> +
> +	if (!crtc->gamma_size)
> +		return false;
> +
> +	if (crtc->funcs->gamma_set)
> +		return true;
> +
> +	return !!drm_mode_obj_find_prop_id(&crtc->base, gamma_id);
> +}
> +
> +/**
> + * drm_crtc_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 have set drm_crtc_funcs.gamma_set or 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 sets the gamma using drm_crtc_funcs.gamma_set if set, or
> + * alternatively using crtc color management properties.
> + */
> +static int drm_crtc_legacy_gamma_set(struct drm_crtc *crtc,
> +			      u16 *red, u16 *green, u16 *blue,
> +			      uint32_t size,
> +			      struct drm_modeset_acquire_ctx *ctx)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_atomic_state *state;
> +	struct drm_crtc_state *crtc_state;
> +	struct drm_property_blob *blob;
> +	struct drm_color_lut *blob_data;
> +	int i, ret = 0;
> +	bool replaced;
> +
> +	if (crtc->funcs->gamma_set)
> +		return crtc->funcs->gamma_set(crtc, red, green, blue, size, ctx);
> +
> +	state = drm_atomic_state_alloc(crtc->dev);
> +	if (!state)
> +		return -ENOMEM;
> +
> +	blob = drm_property_create_blob(dev,
> +					sizeof(struct drm_color_lut) * size,
> +					NULL);
> +	if (IS_ERR(blob)) {
> +		ret = PTR_ERR(blob);
> +		blob = NULL;
> +		goto fail;
> +	}
> +
> +	/* Prepare GAMMA_LUT with the legacy values. */
> +	blob_data = blob->data;
> +	for (i = 0; i < size; i++) {
> +		blob_data[i].red = red[i];
> +		blob_data[i].green = green[i];
> +		blob_data[i].blue = blue[i];
> +	}
> +
> +	state->acquire_ctx = ctx;
> +	crtc_state = drm_atomic_get_crtc_state(state, crtc);
> +	if (IS_ERR(crtc_state)) {
> +		ret = PTR_ERR(crtc_state);
> +		goto fail;
> +	}
> +
> +	/* Set GAMMA_LUT and reset DEGAMMA_LUT and CTM */
> +	replaced = drm_property_replace_blob(&crtc_state->degamma_lut, NULL);
> +	replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
> +	replaced |= drm_property_replace_blob(&crtc_state->gamma_lut, blob);
> +	crtc_state->color_mgmt_changed |= replaced;
> +
> +	ret = drm_atomic_commit(state);
> +
> +fail:
> +	drm_atomic_state_put(state);
> +	drm_property_blob_put(blob);
> +	return ret;
> +}
> +
>  /**
>   * drm_mode_gamma_set_ioctl - set the gamma table
>   * @dev: DRM device
> @@ -262,7 +355,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
>  	if (!crtc)
>  		return -ENOENT;
>  
> -	if (crtc->funcs->gamma_set == NULL)
> +	if (!drm_crtc_supports_legacy_gamma(crtc))
>  		return -ENOSYS;
>  
>  	/* memcpy into gamma store */
> @@ -290,8 +383,8 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
>  		goto out;
>  	}
>  
> -	ret = crtc->funcs->gamma_set(crtc, r_base, g_base, b_base,
> -				     crtc->gamma_size, &ctx);
> +	ret = drm_crtc_legacy_gamma_set(crtc, r_base, g_base, b_base,
> +					crtc->gamma_size, &ctx);
>  
>  out:
>  	DRM_MODESET_LOCK_ALL_END(dev, ctx, ret);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index cddbda5303ff..1e90b8c3b1c7 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -16879,7 +16879,6 @@ intel_cursor_plane_create(struct drm_i915_private *dev_priv,
>  }
>  
>  #define INTEL_CRTC_FUNCS \
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set, \
>  	.set_config = drm_atomic_helper_set_config, \
>  	.destroy = intel_crtc_destroy, \
>  	.page_flip = drm_atomic_helper_page_flip, \
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> index 42d335d3a114..7bb31fbee29d 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm-drv.c
> @@ -774,8 +774,6 @@ static const struct drm_crtc_funcs ingenic_drm_crtc_funcs = {
>  
>  	.enable_vblank		= ingenic_drm_enable_vblank,
>  	.disable_vblank		= ingenic_drm_disable_vblank,
> -
> -	.gamma_set		= drm_atomic_helper_legacy_gamma_set,
>  };
>  
>  static const struct drm_plane_helper_funcs ingenic_drm_plane_helper_funcs = {
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index bfe994230543..b665bd498a4a 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -619,7 +619,6 @@ static const struct drm_crtc_funcs mtk_crtc_funcs = {
>  	.reset			= mtk_drm_crtc_reset,
>  	.atomic_duplicate_state	= mtk_drm_crtc_duplicate_state,
>  	.atomic_destroy_state	= mtk_drm_crtc_destroy_state,
> -	.gamma_set		= drm_atomic_helper_legacy_gamma_set,
>  	.enable_vblank		= mtk_drm_crtc_enable_vblank,
>  	.disable_vblank		= mtk_drm_crtc_disable_vblank,
>  };
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index 537c1ef2e464..ec361d17e900 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -503,7 +503,6 @@ nv50_head_destroy(struct drm_crtc *crtc)
>  static const struct drm_crtc_funcs
>  nv50_head_func = {
>  	.reset = nv50_head_reset,
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  	.destroy = nv50_head_destroy,
>  	.set_config = drm_atomic_helper_set_config,
>  	.page_flip = drm_atomic_helper_page_flip,
> @@ -518,7 +517,6 @@ nv50_head_func = {
>  static const struct drm_crtc_funcs
>  nvd9_head_func = {
>  	.reset = nv50_head_reset,
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  	.destroy = nv50_head_destroy,
>  	.set_config = drm_atomic_helper_set_config,
>  	.page_flip = drm_atomic_helper_page_flip,
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index 7d66269ad998..b97f21cf4a91 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -741,7 +741,6 @@ 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,
>  	.atomic_duplicate_state = omap_crtc_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>  	.atomic_set_property = omap_crtc_atomic_set_property,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index b5fb941e0f53..f93e0750431d 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -1144,7 +1144,6 @@ static const struct drm_crtc_funcs crtc_funcs_gen3 = {
>  	.set_crc_source = rcar_du_crtc_set_crc_source,
>  	.verify_crc_source = rcar_du_crtc_verify_crc_source,
>  	.get_crc_sources = rcar_du_crtc_get_crc_sources,
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  };
>  
>  /* -----------------------------------------------------------------------------
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index d1e05482641b..8d15cabdcb02 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -1643,7 +1643,6 @@ static const struct drm_crtc_funcs vop_crtc_funcs = {
>  	.disable_vblank = vop_crtc_disable_vblank,
>  	.set_crc_source = vop_crtc_set_crc_source,
>  	.verify_crc_source = vop_crtc_verify_crc_source,
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  };
>  
>  static void vop_fb_unref_worker(struct drm_flip_work *work, void *val)
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index 3980677435cb..7812094f93d6 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -713,7 +713,6 @@ static const struct drm_crtc_funcs ltdc_crtc_funcs = {
>  	.enable_vblank = ltdc_crtc_enable_vblank,
>  	.disable_vblank = ltdc_crtc_disable_vblank,
>  	.get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  };
>  
>  /*
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index ea710beb8e00..7495d70d6a32 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -884,7 +884,6 @@ static const struct drm_crtc_funcs vc4_crtc_funcs = {
>  	.reset = vc4_crtc_reset,
>  	.atomic_duplicate_state = vc4_crtc_duplicate_state,
>  	.atomic_destroy_state = vc4_crtc_destroy_state,
> -	.gamma_set = drm_atomic_helper_legacy_gamma_set,
>  	.enable_vblank = vc4_enable_vblank,
>  	.disable_vblank = vc4_disable_vblank,
>  	.get_vblank_timestamp = drm_crtc_vblank_helper_get_vblank_timestamp,
> diff --git a/drivers/gpu/drm/vc4/vc4_txp.c b/drivers/gpu/drm/vc4/vc4_txp.c
> index 8aa5220885f4..e4b782e42f69 100644
> --- a/drivers/gpu/drm/vc4/vc4_txp.c
> +++ b/drivers/gpu/drm/vc4/vc4_txp.c
> @@ -382,7 +382,6 @@ static const struct drm_crtc_funcs vc4_txp_crtc_funcs = {
>  	.reset			= vc4_crtc_reset,
>  	.atomic_duplicate_state	= vc4_crtc_duplicate_state,
>  	.atomic_destroy_state	= vc4_crtc_destroy_state,
> -	.gamma_set		= drm_atomic_helper_legacy_gamma_set,
>  	.enable_vblank		= vc4_txp_enable_vblank,
>  	.disable_vblank		= vc4_txp_disable_vblank,
>  };
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index 5f47720440fa..4045e2507e11 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -147,10 +147,6 @@ int drm_atomic_helper_page_flip_target(
>  				uint32_t flags,
>  				uint32_t target,
>  				struct drm_modeset_acquire_ctx *ctx);
> -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);
>  
>  /**
>   * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC

-- 
Regards,

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 2/2] drm: add legacy support for using degamma for gamma
  2020-12-11 11:42 ` [PATCH v4 2/2] drm: add legacy support for using degamma for gamma Tomi Valkeinen
@ 2020-12-11 11:52   ` Laurent Pinchart
  0 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2020-12-11 11:52 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Yannick Fertre, Thomas Zimmermann, Philippe Cornu, David Airlie,
	Maxime Coquelin, Russell King, dri-devel, Sandy Huang,
	Paul Cercueil, Alexandre Torgue, Matthias Brugger,
	Vincent Abriou

Hi Tomi,

Thank you for the patch.

On Fri, Dec 11, 2020 at 01:42:37PM +0200, Tomi Valkeinen wrote:
> The DRM core handles legacy gamma-set ioctl by setting GAMMA_LUT and
> clearing 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.
> 
> Change the DRM core to use DEGAMMA_LUT instead of GAMMA_LUT when the
> latter is unavailable.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/drm_color_mgmt.c | 22 ++++++++++++++++++----
>  drivers/gpu/drm/drm_fb_helper.c  |  5 +++++
>  2 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 65eb36dc92bf..9100aac1570c 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -91,7 +91,7 @@
>   *
>   * There is also support for a legacy gamma table, which is set up by calling
>   * drm_mode_crtc_set_gamma_size(). The DRM core will then alias the legacy gamma
> - * ramp with "GAMMA_LUT".
> + * ramp with "GAMMA_LUT" or, if that is unavailable, "DEGAMMA_LUT".
>   *
>   * Support for different non RGB color encodings is controlled through
>   * &drm_plane specific COLOR_ENCODING and COLOR_RANGE properties. They
> @@ -238,6 +238,7 @@ EXPORT_SYMBOL(drm_mode_crtc_set_gamma_size);
>  static bool drm_crtc_supports_legacy_gamma(struct drm_crtc *crtc)
>  {
>  	uint32_t gamma_id = crtc->dev->mode_config.gamma_lut_property->base.id;
> +	uint32_t degamma_id = crtc->dev->mode_config.degamma_lut_property->base.id;
>  
>  	if (!crtc->gamma_size)
>  		return false;
> @@ -245,7 +246,8 @@ static bool drm_crtc_supports_legacy_gamma(struct drm_crtc *crtc)
>  	if (crtc->funcs->gamma_set)
>  		return true;
>  
> -	return !!drm_mode_obj_find_prop_id(&crtc->base, gamma_id);
> +	return !!(drm_mode_obj_find_prop_id(&crtc->base, gamma_id) ||
> +		  drm_mode_obj_find_prop_id(&crtc->base, degamma_id));
>  }
>  
>  /**
> @@ -276,12 +278,22 @@ static int drm_crtc_legacy_gamma_set(struct drm_crtc *crtc,
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_property_blob *blob;
>  	struct drm_color_lut *blob_data;
> +	uint32_t gamma_id = dev->mode_config.gamma_lut_property->base.id;
> +	uint32_t degamma_id = dev->mode_config.degamma_lut_property->base.id;
> +	bool use_gamma_lut;
>  	int i, ret = 0;
>  	bool replaced;
>  
>  	if (crtc->funcs->gamma_set)
>  		return crtc->funcs->gamma_set(crtc, red, green, blue, size, ctx);
>  
> +	if (drm_mode_obj_find_prop_id(&crtc->base, gamma_id))
> +		use_gamma_lut = true;
> +	else if (drm_mode_obj_find_prop_id(&crtc->base, degamma_id))
> +		use_gamma_lut = false;
> +	else
> +		return -ENODEV;
> +
>  	state = drm_atomic_state_alloc(crtc->dev);
>  	if (!state)
>  		return -ENOMEM;
> @@ -311,9 +323,11 @@ static int drm_crtc_legacy_gamma_set(struct drm_crtc *crtc,
>  	}
>  
>  	/* Set GAMMA_LUT and reset DEGAMMA_LUT and CTM */
> -	replaced = drm_property_replace_blob(&crtc_state->degamma_lut, NULL);
> +	replaced = drm_property_replace_blob(&crtc_state->degamma_lut,
> +					     use_gamma_lut ? NULL : blob);
>  	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_gamma_lut ? blob : NULL);
>  	crtc_state->color_mgmt_changed |= replaced;
>  
>  	ret = drm_atomic_commit(state);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index e82db0f4e771..5ad19785daee 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1059,6 +1059,11 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
>  			goto out_state;
>  		}
>  
> +		/*
> +		 * FIXME: This always uses gamma_lut. Some HW have only
> +		 * degamma_lut, in which case we should reset gamma_lut and set
> +		 * degamma_lut. See drm_crtc_legacy_gamma_set().
> +		 */
>  		replaced  = drm_property_replace_blob(&crtc_state->degamma_lut,
>  						      NULL);
>  		replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);

-- 
Regards,

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] drm: automatic legacy gamma support
  2020-12-11 11:50   ` Laurent Pinchart
@ 2020-12-14 13:04     ` Tomi Valkeinen
  0 siblings, 0 replies; 8+ messages in thread
From: Tomi Valkeinen @ 2020-12-14 13:04 UTC (permalink / raw)
  To: Laurent Pinchart, dri-devel, Daniel Vetter
  Cc: Yannick Fertre, Philippe Cornu, David Airlie, Maxime Coquelin,
	Russell King, Sandy Huang, Paul Cercueil, Thomas Zimmermann,
	Matthias Brugger, Vincent Abriou, Alexandre Torgue

On 11/12/2020 13:50, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Fri, Dec 11, 2020 at 01:42:36PM +0200, Tomi Valkeinen wrote:
>> To support legacy gamma ioctls the drivers need to set
>> drm_crtc_funcs.gamma_set either to a custom implementation or to
>> drm_atomic_helper_legacy_gamma_set. Most of the atomic drivers do the
>> latter.
>>
>> We can simplify this by making the core handle it automatically.
>>
>> Move the drm_atomic_helper_legacy_gamma_set() functionality into
>> drm_color_mgmt.c to make drm_mode_gamma_set_ioctl() use
>> drm_crtc_funcs.gamma_set if set or GAMMA_LUT property if not.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks Laurent! If there are no objections, I'd like to push these to drm-misc-next tomorrow (along
with the omapdrm gamma + CTM patches).

 Tomi

-- 
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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] drm: automatic legacy gamma support
  2020-12-11 11:42 ` [PATCH v4 1/2] " Tomi Valkeinen
  2020-12-11 11:50   ` Laurent Pinchart
@ 2020-12-14 13:43   ` Philippe CORNU
  2020-12-15 12:45   ` Tomi Valkeinen
  2 siblings, 0 replies; 8+ messages in thread
From: Philippe CORNU @ 2020-12-14 13:43 UTC (permalink / raw)
  To: Tomi Valkeinen, dri-devel, Daniel Vetter, Ville Syrjälä,
	Laurent Pinchart
  Cc: Paul Cercueil, Maxime Coquelin, David Airlie, Russell King,
	Sandy Huang, Yannick FERTRE, Thomas Zimmermann, Matthias Brugger,
	Vincent ABRIOU, Alexandre TORGUE

Hi Tomi,


On 12/11/20 12:42 PM, Tomi Valkeinen wrote:
> To support legacy gamma ioctls the drivers need to set
> drm_crtc_funcs.gamma_set either to a custom implementation or to
> drm_atomic_helper_legacy_gamma_set. Most of the atomic drivers do the
> latter.
> 
> We can simplify this by making the core handle it automatically.
> 
> Move the drm_atomic_helper_legacy_gamma_set() functionality into
> drm_color_mgmt.c to make drm_mode_gamma_set_ioctl() use
> drm_crtc_funcs.gamma_set if set or GAMMA_LUT property if not.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   1 -
>   .../gpu/drm/arm/display/komeda/komeda_crtc.c  |   1 -
>   drivers/gpu/drm/arm/malidp_crtc.c             |   1 -
>   drivers/gpu/drm/armada/armada_crtc.c          |   1 -
>   drivers/gpu/drm/ast/ast_mode.c                |   1 -
>   .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c    |   1 -
>   drivers/gpu/drm/drm_atomic_helper.c           |  70 -----------
>   drivers/gpu/drm/drm_color_mgmt.c              | 111 ++++++++++++++++--
>   drivers/gpu/drm/i915/display/intel_display.c  |   1 -
>   drivers/gpu/drm/ingenic/ingenic-drm-drv.c     |   2 -
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |   1 -
>   drivers/gpu/drm/nouveau/dispnv50/head.c       |   2 -
>   drivers/gpu/drm/omapdrm/omap_crtc.c           |   1 -
>   drivers/gpu/drm/rcar-du/rcar_du_crtc.c        |   1 -
>   drivers/gpu/drm/rockchip/rockchip_drm_vop.c   |   1 -
>   drivers/gpu/drm/stm/ltdc.c                    |   1 -

For the stm part,
Reviewed-by: Philippe Cornu <philippe.cornu@st.com>

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] drm: automatic legacy gamma support
  2020-12-11 11:42 ` [PATCH v4 1/2] " Tomi Valkeinen
  2020-12-11 11:50   ` Laurent Pinchart
  2020-12-14 13:43   ` Philippe CORNU
@ 2020-12-15 12:45   ` Tomi Valkeinen
  2 siblings, 0 replies; 8+ messages in thread
From: Tomi Valkeinen @ 2020-12-15 12:45 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, Ville Syrjälä, Laurent Pinchart
  Cc: Yannick Fertre, Philippe Cornu, David Airlie, Russell King,
	Sandy Huang, Paul Cercueil, Thomas Zimmermann, Matthias Brugger,
	Vincent Abriou, Maxime Coquelin, Alexandre Torgue

On 11/12/2020 13:42, Tomi Valkeinen wrote:

> +/**
> + * drm_crtc_supports_legacy_gamma - does the crtc support legacy gamma correction table
> + * @crtc: CRTC object
> + *
> + * Returns true/false if the given crtc supports setting the legacy gamma
> + * correction table.
> + */
> +static bool drm_crtc_supports_legacy_gamma(struct drm_crtc *crtc)
> +{
> +	uint32_t gamma_id = crtc->dev->mode_config.gamma_lut_property->base.id;

Userspace-mode strikes again... I'll change uint32_t's to u32 while applying. (But in my defense, 
there were uint32_t uses already in the code touched by these patches, and drm seems to have lots of 
uint32_t all around...)

  Tomi

-- 
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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-12-15 12:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-11 11:42 [PATCH v4 0/2] drm: automatic legacy gamma support Tomi Valkeinen
2020-12-11 11:42 ` [PATCH v4 1/2] " Tomi Valkeinen
2020-12-11 11:50   ` Laurent Pinchart
2020-12-14 13:04     ` Tomi Valkeinen
2020-12-14 13:43   ` Philippe CORNU
2020-12-15 12:45   ` Tomi Valkeinen
2020-12-11 11:42 ` [PATCH v4 2/2] drm: add legacy support for using degamma for gamma Tomi Valkeinen
2020-12-11 11:52   ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).