All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/7] drm/omapdrm: gamma table support + drm_crtc_enable_color_mgmt()
@ 2016-05-25 20:43 Jyri Sarha
  2016-05-25 20:43 ` [PATCH v4 1/7] drm/omapdrm: Add gamma table support to DSS dispc Jyri Sarha
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Jyri Sarha @ 2016-05-25 20:43 UTC (permalink / raw)
  To: dri-devel; +Cc: Jyri Sarha, peter.ujfalusi, tomi.valkeinen, laurent.pinchart

Implements gamma tables for OMAP4, OMAP5, and dra7xx SoCs and adds a
work-a-round for errata that may break LCD1 channel if gamma tables
are in use.

Also adds new drm_crtc_enable_color_mgmt() as suggested[1] by Daniel
Vetter and get rid of the old drm_helper_crtc_enable_color_mgmt(). I
have not tested the change to i915 driver, only compiled it, but
functionally it should be exactly the same.

[1] http://www.spinics.net/lists/dri-devel/msg108092.html

Changes from v3 to v4
- "drm/omapdrm: Add gamma table support to DSS dispc"
  - use interpolation code in dispc_mgr_set_gamma() to produce default lut
  - restore default lut table if NULL is given as gamma lut table
- "drm/omapdrm: Implement gamma_lut atomic crtc property"
  - attach gamma_lut_property and gamma_lut_size_property to crtc if
    gamma tables are supported
  - restore default table if gamma lut property is deleted
- Adds:
  - drm: Add drm_crtc_enable_color_mgmt()
  - drm/omapdrm: Use drm_crtc_enable_color_mgmt() to enable gamma properties
  - drm/i915: Use drm_crtc_enable_color_mgmt()
  - drm: Remove obsolete drm_helper_crtc_enable_color_mgmt()

Changes from v2 to v3
- "drm/omapdrm: Add gamma table support to DSS dispc"
  - fix dispc_init_gamma_tables() to always return an int
  - omap54xx_dispc_feats initializes .has_gamma_table not .has_gamma_tables
- "drm/omapdrm: Work-a-round for errata i734 (LCD1 Gamma) in DSS dispc"
  - work-a-round -> workaround
  - Do not mention LOADMODE in description
  - dma_alloc_writecombine returns NULL on error
  - fix last wrong instrance of LCD output gating register
  - improve comment for framedone busy wait
  - add {} around busy loop's while statement

Changes from v1 to v2
- Drop "drm/omapdrm: omap_modeset_init: Separate crtc id and plane id indexing"
- "drm/omapdrm: Add gamma table support to DSS dispc"
 - Address Tomi's comments here: https://patchwork.kernel.org/patch/9128629/
- "drm/omapdrm: Work-a-round for errata i734 (LCD1 Gamma) in DSS dispc"
 - Address Tomi's comments here: https://patchwork.kernel.org/patch/9128633/

Jyri Sarha (7):
  drm/omapdrm: Add gamma table support to DSS dispc
  drm/omapdrm: Workaround for errata i734 (LCD1 Gamma) in DSS dispc
  drm/omapdrm: Implement gamma_lut atomic crtc properties
  drm: Add drm_crtc_enable_color_mgmt()
  drm/omapdrm: Use drm_crtc_enable_color_mgmt() to enable gamma
    properties
  drm/i915: Use drm_crtc_enable_color_mgmt()
  drm: Remove obsolete drm_helper_crtc_enable_color_mgmt()

 drivers/gpu/drm/drm_crtc.c            |  45 ++++
 drivers/gpu/drm/drm_crtc_helper.c     |  33 ---
 drivers/gpu/drm/i915/intel_color.c    |   3 +-
 drivers/gpu/drm/omapdrm/dss/dispc.c   | 374 ++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/dss/dispc.h   |   5 +
 drivers/gpu/drm/omapdrm/dss/hdmi4.c   |   3 -
 drivers/gpu/drm/omapdrm/dss/hdmi5.c   |   3 -
 drivers/gpu/drm/omapdrm/dss/omapdss.h |   5 +
 drivers/gpu/drm/omapdrm/omap_crtc.c   |  28 +++
 include/drm/drm_crtc.h                |   5 +-
 include/drm/drm_crtc_helper.h         |   3 -
 11 files changed, 447 insertions(+), 60 deletions(-)

-- 
1.9.1

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

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

* [PATCH v4 1/7] drm/omapdrm: Add gamma table support to DSS dispc
  2016-05-25 20:43 [PATCH v4 0/7] drm/omapdrm: gamma table support + drm_crtc_enable_color_mgmt() Jyri Sarha
@ 2016-05-25 20:43 ` Jyri Sarha
  2016-05-25 20:43 ` [PATCH v4 2/7] drm/omapdrm: Workaround for errata i734 (LCD1 Gamma) in " Jyri Sarha
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Jyri Sarha @ 2016-05-25 20:43 UTC (permalink / raw)
  To: dri-devel; +Cc: Jyri Sarha, peter.ujfalusi, tomi.valkeinen, laurent.pinchart

Add gamma table support to DSS dispc.

DSS driver initializes the default gamma table at component bind time
and holds a copy of all gamma tables in its internal data structure.

Each call to dispc_mgr_set_gamma() updates the internal table and
triggers write to the HW, if it is enabled. The tables are restored to
HW in PM resume callback. The drivers internal data structure match
the HW tables in size and in number of significant bits per color
component. The dispc_mgr_set_gamma() converts the size of any given
table for the internal data structure using linear interpolation.
Default gamma table is restored if NULL is given in place of gamma
lut.

dispc_mgr_gamma_size() gives HW gamma table size for the channel and
returns 0 if gamma table is not supported by the HW or the DSS driver.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dispc.c   | 200 +++++++++++++++++++++++++++++++---
 drivers/gpu/drm/omapdrm/dss/dispc.h   |   5 +
 drivers/gpu/drm/omapdrm/dss/hdmi4.c   |   3 -
 drivers/gpu/drm/omapdrm/dss/hdmi5.c   |   3 -
 drivers/gpu/drm/omapdrm/dss/omapdss.h |   5 +
 5 files changed, 194 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index f83608b..dbe5ab3 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -113,9 +113,12 @@ struct dispc_features {
 	 * never both, we can just use this flag for now.
 	 */
 	bool reverse_ilace_field_order:1;
+
+	bool has_gamma_table:1;
 };
 
 #define DISPC_MAX_NR_FIFOS 5
+#define DISPC_MAX_CHANNEL_GAMMA 4
 
 static struct {
 	struct platform_device *pdev;
@@ -135,6 +138,8 @@ static struct {
 	bool		ctx_valid;
 	u32		ctx[DISPC_SZ_REGS / sizeof(u32)];
 
+	u32 *gamma_table[DISPC_MAX_CHANNEL_GAMMA];
+
 	const struct dispc_features *feat;
 
 	bool is_enabled;
@@ -178,11 +183,19 @@ struct dispc_reg_field {
 	u8 low;
 };
 
+struct dispc_gamma_desc {
+	u32 len;
+	u32 bits;
+	u16 reg;
+	bool has_index;
+};
+
 static const struct {
 	const char *name;
 	u32 vsync_irq;
 	u32 framedone_irq;
 	u32 sync_lost_irq;
+	struct dispc_gamma_desc gamma;
 	struct dispc_reg_field reg_desc[DISPC_MGR_FLD_NUM];
 } mgr_desc[] = {
 	[OMAP_DSS_CHANNEL_LCD] = {
@@ -190,6 +203,12 @@ static const struct {
 		.vsync_irq	= DISPC_IRQ_VSYNC,
 		.framedone_irq	= DISPC_IRQ_FRAMEDONE,
 		.sync_lost_irq	= DISPC_IRQ_SYNC_LOST,
+		.gamma		= {
+			.len	= 256,
+			.bits	= 8,
+			.reg	= DISPC_GAMMA_TABLE0,
+			.has_index = true,
+		},
 		.reg_desc	= {
 			[DISPC_MGR_FLD_ENABLE]		= { DISPC_CONTROL,  0,  0 },
 			[DISPC_MGR_FLD_STNTFT]		= { DISPC_CONTROL,  3,  3 },
@@ -207,6 +226,12 @@ static const struct {
 		.vsync_irq	= DISPC_IRQ_EVSYNC_ODD | DISPC_IRQ_EVSYNC_EVEN,
 		.framedone_irq	= DISPC_IRQ_FRAMEDONETV,
 		.sync_lost_irq	= DISPC_IRQ_SYNC_LOST_DIGIT,
+		.gamma		= {
+			.len	= 1024,
+			.bits	= 10,
+			.reg	= DISPC_GAMMA_TABLE2,
+			.has_index = false,
+		},
 		.reg_desc	= {
 			[DISPC_MGR_FLD_ENABLE]		= { DISPC_CONTROL,  1,  1 },
 			[DISPC_MGR_FLD_STNTFT]		= { },
@@ -224,6 +249,12 @@ static const struct {
 		.vsync_irq	= DISPC_IRQ_VSYNC2,
 		.framedone_irq	= DISPC_IRQ_FRAMEDONE2,
 		.sync_lost_irq	= DISPC_IRQ_SYNC_LOST2,
+		.gamma		= {
+			.len	= 256,
+			.bits	= 8,
+			.reg	= DISPC_GAMMA_TABLE1,
+			.has_index = true,
+		},
 		.reg_desc	= {
 			[DISPC_MGR_FLD_ENABLE]		= { DISPC_CONTROL2,  0,  0 },
 			[DISPC_MGR_FLD_STNTFT]		= { DISPC_CONTROL2,  3,  3 },
@@ -241,6 +272,12 @@ static const struct {
 		.vsync_irq	= DISPC_IRQ_VSYNC3,
 		.framedone_irq	= DISPC_IRQ_FRAMEDONE3,
 		.sync_lost_irq	= DISPC_IRQ_SYNC_LOST3,
+		.gamma		= {
+			.len	= 256,
+			.bits	= 8,
+			.reg	= DISPC_GAMMA_TABLE3,
+			.has_index = true,
+		},
 		.reg_desc	= {
 			[DISPC_MGR_FLD_ENABLE]		= { DISPC_CONTROL3,  0,  0 },
 			[DISPC_MGR_FLD_STNTFT]		= { DISPC_CONTROL3,  3,  3 },
@@ -1084,20 +1121,6 @@ static u32 dispc_ovl_get_burst_size(enum omap_plane plane)
 	return unit * 8;
 }
 
-void dispc_enable_gamma_table(bool enable)
-{
-	/*
-	 * This is partially implemented to support only disabling of
-	 * the gamma table.
-	 */
-	if (enable) {
-		DSSWARN("Gamma table enabling for TV not yet supported");
-		return;
-	}
-
-	REG_FLD_MOD(DISPC_CONFIG, enable, 9, 9);
-}
-
 static void dispc_mgr_enable_cpr(enum omap_channel channel, bool enable)
 {
 	if (channel == OMAP_DSS_CHANNEL_DIGIT)
@@ -3814,6 +3837,136 @@ void dispc_disable_sidle(void)
 	REG_FLD_MOD(DISPC_SYSCONFIG, 1, 4, 3);	/* SIDLEMODE: no idle */
 }
 
+u32 dispc_mgr_gamma_size(enum omap_channel channel)
+{
+	const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma;
+
+	if (!dispc.feat->has_gamma_table)
+		return 0;
+
+	return gdesc->len;
+}
+EXPORT_SYMBOL(dispc_mgr_gamma_size);
+
+static void dispc_mgr_write_gamma_table(enum omap_channel channel)
+{
+	const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma;
+	u32 *table = dispc.gamma_table[channel];
+	unsigned int i;
+
+	DSSDBG("%s: channel %d\n", __func__, channel);
+
+	for (i = 0; i < gdesc->len; ++i) {
+		u32 v = table[i];
+
+		if (gdesc->has_index)
+			v |= i << 24;
+		else if (i == 0)
+			v |= 1 << 31;
+
+		dispc_write_reg(gdesc->reg, v);
+	}
+}
+
+static void dispc_restore_gamma_tables(void)
+{
+	DSSDBG("%s()\n", __func__);
+
+	if (!dispc.feat->has_gamma_table)
+		return;
+
+	dispc_mgr_write_gamma_table(OMAP_DSS_CHANNEL_LCD);
+
+	dispc_mgr_write_gamma_table(OMAP_DSS_CHANNEL_DIGIT);
+
+	if (dss_has_feature(FEAT_MGR_LCD2))
+		dispc_mgr_write_gamma_table(OMAP_DSS_CHANNEL_LCD2);
+
+	if (dss_has_feature(FEAT_MGR_LCD3))
+		dispc_mgr_write_gamma_table(OMAP_DSS_CHANNEL_LCD3);
+}
+
+static const struct drm_color_lut dispc_mgr_gamma_default_lut[] = {
+	{ .red = 0, .green = 0, .blue = 0, },
+	{ .red = U16_MAX, .green = U16_MAX, .blue = U16_MAX, },
+};
+
+void dispc_mgr_set_gamma(enum omap_channel channel,
+			 const struct drm_color_lut *lut,
+			 unsigned int length)
+{
+	const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma;
+	u32 *table = dispc.gamma_table[channel];
+	uint i;
+
+	DSSDBG("%s: channel %d, lut len %u, hw len %u\n", __func__,
+	       channel, length, gdesc->len);
+
+	if (!dispc.feat->has_gamma_table)
+		return;
+
+	if (lut == NULL) {
+		lut = dispc_mgr_gamma_default_lut;
+		length = ARRAY_SIZE(dispc_mgr_gamma_default_lut);
+	}
+
+	for (i = 0; i < length - 1; ++i) {
+		uint first = i * (gdesc->len - 1) / (length - 1);
+		uint last = (i + 1) * (gdesc->len - 1) / (length - 1);
+		uint w = last - first;
+		u16 r, g, b;
+		uint j;
+
+		for (j = 0; j <= w; j++) {
+			r = (lut[i].red * (w - j) + lut[i+1].red * j) / w;
+			g = (lut[i].green * (w - j) + lut[i+1].green * j) / w;
+			b = (lut[i].blue * (w - j) + lut[i+1].blue * j) / w;
+
+			r >>= 16 - gdesc->bits;
+			g >>= 16 - gdesc->bits;
+			b >>= 16 - gdesc->bits;
+
+			table[first + j] = (r << (gdesc->bits * 2)) |
+				(g << gdesc->bits) | b;
+		}
+	}
+
+	if (dispc.is_enabled)
+		dispc_mgr_write_gamma_table(channel);
+}
+EXPORT_SYMBOL(dispc_mgr_set_gamma);
+
+static int dispc_init_gamma_tables(void)
+{
+	int channel;
+
+	if (!dispc.feat->has_gamma_table)
+		return 0;
+
+	for (channel = 0; channel < ARRAY_SIZE(dispc.gamma_table); channel++) {
+		const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma;
+		u32 *gt;
+
+		if (channel == OMAP_DSS_CHANNEL_LCD2 &&
+		    !dss_has_feature(FEAT_MGR_LCD2))
+			continue;
+
+		if (channel == OMAP_DSS_CHANNEL_LCD3 &&
+		    !dss_has_feature(FEAT_MGR_LCD3))
+			continue;
+
+		gt = devm_kmalloc_array(&dispc.pdev->dev, gdesc->len,
+					   sizeof(u32), GFP_KERNEL);
+		if (!gt)
+			return -ENOMEM;
+
+		dispc.gamma_table[channel] = gt;
+
+		dispc_mgr_set_gamma(channel, NULL, 0);
+	}
+	return 0;
+}
+
 static void _omap_dispc_initial_config(void)
 {
 	u32 l;
@@ -3829,8 +3982,15 @@ static void _omap_dispc_initial_config(void)
 		dispc.core_clk_rate = dispc_fclk_rate();
 	}
 
-	/* FUNCGATED */
-	if (dss_has_feature(FEAT_FUNCGATED))
+	/* Use gamma table mode, instead of palette mode */
+	if (dispc.feat->has_gamma_table)
+		REG_FLD_MOD(DISPC_CONFIG, 1, 3, 3);
+
+	/* For older DSS versions (FEAT_FUNCGATED) this enables
+	 * func-clock auto-gating. For newer versions
+	 * (dispc.feat->has_gamma_table) this enables tv-out gamma tables.
+	 */
+	if (dss_has_feature(FEAT_FUNCGATED) || dispc.feat->has_gamma_table)
 		REG_FLD_MOD(DISPC_CONFIG, 1, 9, 9);
 
 	dispc_setup_color_conv_coef();
@@ -3934,6 +4094,7 @@ static const struct dispc_features omap44xx_dispc_feats = {
 	.has_writeback		=	true,
 	.supports_double_pixel	=	true,
 	.reverse_ilace_field_order =	true,
+	.has_gamma_table	=	true,
 };
 
 static const struct dispc_features omap54xx_dispc_feats = {
@@ -3959,6 +4120,7 @@ static const struct dispc_features omap54xx_dispc_feats = {
 	.has_writeback		=	true,
 	.supports_double_pixel	=	true,
 	.reverse_ilace_field_order =	true,
+	.has_gamma_table	=	true,
 };
 
 static int dispc_init_features(struct platform_device *pdev)
@@ -4100,6 +4262,10 @@ static int dispc_bind(struct device *dev, struct device *master, void *data)
 		}
 	}
 
+	r = dispc_init_gamma_tables();
+	if (r)
+		return r;
+
 	pm_runtime_enable(&pdev->dev);
 
 	r = dispc_runtime_get();
@@ -4170,6 +4336,8 @@ static int dispc_runtime_resume(struct device *dev)
 		_omap_dispc_initial_config();
 
 		dispc_restore_context();
+
+		dispc_restore_gamma_tables();
 	}
 
 	dispc.is_enabled = true;
diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.h b/drivers/gpu/drm/omapdrm/dss/dispc.h
index 4837442..bc1d812 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.h
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.h
@@ -42,6 +42,11 @@
 #define DISPC_MSTANDBY_CTRL		0x0858
 #define DISPC_GLOBAL_MFLAG_ATTRIBUTE	0x085C
 
+#define DISPC_GAMMA_TABLE0		0x0630
+#define DISPC_GAMMA_TABLE1		0x0634
+#define DISPC_GAMMA_TABLE2		0x0638
+#define DISPC_GAMMA_TABLE3		0x0850
+
 /* DISPC overlay registers */
 #define DISPC_OVL_BA0(n)		(DISPC_OVL_BASE(n) + \
 					DISPC_BA0_OFFSET(n))
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index f892ae15..12bef2d 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -213,9 +213,6 @@ static int hdmi_power_on_full(struct omap_dss_device *dssdev)
 
 	hdmi4_configure(&hdmi.core, &hdmi.wp, &hdmi.cfg);
 
-	/* bypass TV gamma table */
-	dispc_enable_gamma_table(0);
-
 	/* tv size */
 	dss_mgr_set_timings(channel, p);
 
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
index a43f7b1..10530c5 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
@@ -230,9 +230,6 @@ static int hdmi_power_on_full(struct omap_dss_device *dssdev)
 
 	hdmi5_configure(&hdmi.core, &hdmi.wp, &hdmi.cfg);
 
-	/* bypass TV gamma table */
-	dispc_enable_gamma_table(0);
-
 	/* tv size */
 	dss_mgr_set_timings(channel, p);
 
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index d7e7c90..adcd315 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -19,6 +19,7 @@
 #define __OMAP_DRM_DSS_H
 
 #include <video/omapdss.h>
+#include <uapi/drm/drm_mode.h>
 
 u32 dispc_read_irqstatus(void);
 void dispc_clear_irqstatus(u32 mask);
@@ -44,6 +45,10 @@ void dispc_mgr_set_timings(enum omap_channel channel,
 		const struct omap_video_timings *timings);
 void dispc_mgr_setup(enum omap_channel channel,
 		const struct omap_overlay_manager_info *info);
+u32 dispc_mgr_gamma_size(enum omap_channel channel);
+void dispc_mgr_set_gamma(enum omap_channel channel,
+			 const struct drm_color_lut *lut,
+			 unsigned int length);
 
 int dispc_ovl_enable(enum omap_plane plane, bool enable);
 bool dispc_ovl_enabled(enum omap_plane plane);
-- 
1.9.1

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

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

* [PATCH v4 2/7] drm/omapdrm: Workaround for errata i734 (LCD1 Gamma) in DSS dispc
  2016-05-25 20:43 [PATCH v4 0/7] drm/omapdrm: gamma table support + drm_crtc_enable_color_mgmt() Jyri Sarha
  2016-05-25 20:43 ` [PATCH v4 1/7] drm/omapdrm: Add gamma table support to DSS dispc Jyri Sarha
@ 2016-05-25 20:43 ` Jyri Sarha
  2016-05-25 20:43 ` [PATCH v4 3/7] drm/omapdrm: Implement gamma_lut atomic crtc properties Jyri Sarha
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Jyri Sarha @ 2016-05-25 20:43 UTC (permalink / raw)
  To: dri-devel; +Cc: Jyri Sarha, peter.ujfalusi, tomi.valkeinen, laurent.pinchart

Workaround for errata i734 in DSS dispc
 - LCD1 Gamma Correction Is Not Working When GFX Pipe Is Disabled

For gamma tables to work on LCD1 the GFX plane has to be used at least
once after DSS HW has come out of reset. The workaround sets up a
minimal LCD setup with GFX plane and waits for one vertical sync irq
before disabling the setup and continuing with the context
restore. The physical outputs are gated during the operation.

For details see:
OMAP543x Multimedia Device Silicon Revision 2.0 Silicon Errata
Literature Number: SWPZ037E
Or some other relevant errata document for the DSS IP version.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/omapdrm/dss/dispc.c | 174 ++++++++++++++++++++++++++++++++++++
 1 file changed, 174 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index dbe5ab3..eff857f 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -115,6 +115,8 @@ struct dispc_features {
 	bool reverse_ilace_field_order:1;
 
 	bool has_gamma_table:1;
+
+	bool has_gamma_i734_bug:1;
 };
 
 #define DISPC_MAX_NR_FIFOS 5
@@ -4095,6 +4097,7 @@ static const struct dispc_features omap44xx_dispc_feats = {
 	.supports_double_pixel	=	true,
 	.reverse_ilace_field_order =	true,
 	.has_gamma_table	=	true,
+	.has_gamma_i734_bug	=	true,
 };
 
 static const struct dispc_features omap54xx_dispc_feats = {
@@ -4121,6 +4124,7 @@ static const struct dispc_features omap54xx_dispc_feats = {
 	.supports_double_pixel	=	true,
 	.reverse_ilace_field_order =	true,
 	.has_gamma_table	=	true,
+	.has_gamma_i734_bug	=	true,
 };
 
 static int dispc_init_features(struct platform_device *pdev)
@@ -4212,6 +4216,168 @@ void dispc_free_irq(void *dev_id)
 }
 EXPORT_SYMBOL(dispc_free_irq);
 
+/*
+ * Workaround for errata i734 in DSS dispc
+ *  - LCD1 Gamma Correction Is Not Working When GFX Pipe Is Disabled
+ *
+ * For gamma tables to work on LCD1 the GFX plane has to be used at
+ * least once after DSS HW has come out of reset. The workaround
+ * sets up a minimal LCD setup with GFX plane and waits for one
+ * vertical sync irq before disabling the setup and continuing with
+ * the context restore. The physical outputs are gated during the
+ * operation. This workaround requires that gamma table's LOADMODE
+ * is set to 0x2 in DISPC_CONTROL1 register.
+ *
+ * For details see:
+ * OMAP543x Multimedia Device Silicon Revision 2.0 Silicon Errata
+ * Literature Number: SWPZ037E
+ * Or some other relevant errata document for the DSS IP version.
+ */
+
+static const struct dispc_errata_i734_data {
+	struct omap_video_timings timings;
+	struct omap_overlay_info ovli;
+	struct omap_overlay_manager_info mgri;
+	struct dss_lcd_mgr_config lcd_conf;
+} i734 = {
+	.timings = {
+		.x_res = 8, .y_res = 1,
+		.pixelclock = 16000000,
+		.hsw = 8, .hfp = 4, .hbp = 4,
+		.vsw = 1, .vfp = 1, .vbp = 1,
+		.vsync_level = OMAPDSS_SIG_ACTIVE_LOW,
+		.hsync_level = OMAPDSS_SIG_ACTIVE_LOW,
+		.interlace = false,
+		.data_pclk_edge = OMAPDSS_DRIVE_SIG_RISING_EDGE,
+		.de_level = OMAPDSS_SIG_ACTIVE_HIGH,
+		.sync_pclk_edge = OMAPDSS_DRIVE_SIG_RISING_EDGE,
+		.double_pixel = false,
+	},
+	.ovli = {
+		.screen_width = 1,
+		.width = 1, .height = 1,
+		.color_mode = OMAP_DSS_COLOR_RGB24U,
+		.rotation = OMAP_DSS_ROT_0,
+		.rotation_type = OMAP_DSS_ROT_DMA,
+		.mirror = 0,
+		.pos_x = 0, .pos_y = 0,
+		.out_width = 0, .out_height = 0,
+		.global_alpha = 0xff,
+		.pre_mult_alpha = 0,
+		.zorder = 0,
+	},
+	.mgri = {
+		.default_color = 0,
+		.trans_enabled = false,
+		.partial_alpha_enabled = false,
+		.cpr_enable = false,
+	},
+	.lcd_conf = {
+		.io_pad_mode = DSS_IO_PAD_MODE_BYPASS,
+		.stallmode = false,
+		.fifohandcheck = false,
+		.clock_info = {
+			.lck_div = 1,
+			.pck_div = 2,
+		},
+		.video_port_width = 24,
+		.lcden_sig_polarity = 0,
+	},
+};
+
+static struct i734_buf {
+	size_t size;
+	dma_addr_t paddr;
+	void *vaddr;
+} i734_buf;
+
+static int dispc_errata_i734_wa_init(void)
+{
+	if (!dispc.feat->has_gamma_i734_bug)
+		return 0;
+
+	i734_buf.size = i734.ovli.width * i734.ovli.height *
+		color_mode_to_bpp(i734.ovli.color_mode) / 8;
+
+	i734_buf.vaddr = dma_alloc_writecombine(&dispc.pdev->dev, i734_buf.size,
+						&i734_buf.paddr, GFP_KERNEL);
+	if (!i734_buf.vaddr) {
+		dev_err(&dispc.pdev->dev, "%s: dma_alloc_writecombine failed",
+			__func__);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static void dispc_errata_i734_wa_fini(void)
+{
+	if (!dispc.feat->has_gamma_i734_bug)
+		return;
+
+	dma_free_writecombine(&dispc.pdev->dev, i734_buf.size, i734_buf.vaddr,
+			      i734_buf.paddr);
+}
+
+static void dispc_errata_i734_wa(void)
+{
+	u32 framedone_irq = dispc_mgr_get_framedone_irq(OMAP_DSS_CHANNEL_LCD);
+	struct omap_overlay_info ovli;
+	struct dss_lcd_mgr_config lcd_conf;
+	u32 gatestate;
+	unsigned int count;
+
+	if (!dispc.feat->has_gamma_i734_bug)
+		return;
+
+	gatestate = REG_GET(DISPC_CONFIG, 8, 4);
+
+	ovli = i734.ovli;
+	ovli.paddr = i734_buf.paddr;
+	lcd_conf = i734.lcd_conf;
+
+	/* Gate all LCD1 outputs */
+	REG_FLD_MOD(DISPC_CONFIG, 0x1f, 8, 4);
+
+	/* Setup and enable GFX plane */
+	dispc_ovl_set_channel_out(OMAP_DSS_GFX, OMAP_DSS_CHANNEL_LCD);
+	dispc_ovl_setup(OMAP_DSS_GFX, &ovli, false, &i734.timings, false);
+	dispc_ovl_enable(OMAP_DSS_GFX, true);
+
+	/* Set up and enable display manager for LCD1 */
+	dispc_mgr_setup(OMAP_DSS_CHANNEL_LCD, &i734.mgri);
+	dispc_calc_clock_rates(dss_get_dispc_clk_rate(),
+			       &lcd_conf.clock_info);
+	dispc_mgr_set_lcd_config(OMAP_DSS_CHANNEL_LCD, &lcd_conf);
+	dispc_mgr_set_timings(OMAP_DSS_CHANNEL_LCD, &i734.timings);
+
+	dispc_clear_irqstatus(framedone_irq);
+
+	/* Enable and shut the channel to produce just one frame */
+	dispc_mgr_enable(OMAP_DSS_CHANNEL_LCD, true);
+	dispc_mgr_enable(OMAP_DSS_CHANNEL_LCD, false);
+
+	/* Busy wait for framedone. We can't fiddle with irq handlers
+	 * in PM resume. Typically the loop runs less than 5 times and
+	 * waits less than a micro second.
+	 */
+	count = 0;
+	while (!(dispc_read_irqstatus() & framedone_irq)) {
+		if (count++ > 10000) {
+			dev_err(&dispc.pdev->dev, "%s: framedone timeout\n",
+				__func__);
+			break;
+		}
+	}
+	dispc_ovl_enable(OMAP_DSS_GFX, false);
+
+	/* Clear all irq bits before continuing */
+	dispc_clear_irqstatus(0xffffffff);
+
+	/* Restore the original state to LCD1 output gates */
+	REG_FLD_MOD(DISPC_CONFIG, gatestate, 8, 4);
+}
+
 /* DISPC HW IP initialisation */
 static int dispc_bind(struct device *dev, struct device *master, void *data)
 {
@@ -4229,6 +4395,10 @@ static int dispc_bind(struct device *dev, struct device *master, void *data)
 	if (r)
 		return r;
 
+	r = dispc_errata_i734_wa_init();
+	if (r)
+		return r;
+
 	dispc_mem = platform_get_resource(dispc.pdev, IORESOURCE_MEM, 0);
 	if (!dispc_mem) {
 		DSSERR("can't get IORESOURCE_MEM DISPC\n");
@@ -4293,6 +4463,8 @@ static void dispc_unbind(struct device *dev, struct device *master,
 			       void *data)
 {
 	pm_runtime_disable(dev);
+
+	dispc_errata_i734_wa_fini();
 }
 
 static const struct component_ops dispc_component_ops = {
@@ -4335,6 +4507,8 @@ static int dispc_runtime_resume(struct device *dev)
 	if (REG_GET(DISPC_CONFIG, 2, 1) != OMAP_DSS_LOAD_FRAME_ONLY) {
 		_omap_dispc_initial_config();
 
+		dispc_errata_i734_wa();
+
 		dispc_restore_context();
 
 		dispc_restore_gamma_tables();
-- 
1.9.1

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

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

* [PATCH v4 3/7] drm/omapdrm: Implement gamma_lut atomic crtc properties
  2016-05-25 20:43 [PATCH v4 0/7] drm/omapdrm: gamma table support + drm_crtc_enable_color_mgmt() Jyri Sarha
  2016-05-25 20:43 ` [PATCH v4 1/7] drm/omapdrm: Add gamma table support to DSS dispc Jyri Sarha
  2016-05-25 20:43 ` [PATCH v4 2/7] drm/omapdrm: Workaround for errata i734 (LCD1 Gamma) in " Jyri Sarha
@ 2016-05-25 20:43 ` Jyri Sarha
  2016-05-25 20:43 ` [PATCH v4 4/7] drm: Add drm_crtc_enable_color_mgmt() Jyri Sarha
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Jyri Sarha @ 2016-05-25 20:43 UTC (permalink / raw)
  To: dri-devel; +Cc: Jyri Sarha, peter.ujfalusi, tomi.valkeinen, laurent.pinchart

Implement gamma_lut atomic crtc properties, set crtc gamma size to 256
for all crtcs and use drm_atomic_helper_legacy_gamma_set() as
gamma_set func. The tv-out crtc has 1024 element gamma table (with
10bit precision) in HW, but current Xorg server does not accept
anything else but 256 elements so that is used for all CRTCs. The dss
dispc API converts table of any length for HW and uses linear
interpolation in the process. The default gamma table is restored
gamma_lut property is deleted.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 075f2bb..5b7f6f5 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -384,6 +384,19 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc,
 
 	WARN_ON(omap_crtc->vblank_irq.registered);
 
+	if (crtc->state->color_mgmt_changed) {
+		struct drm_color_lut *lut = NULL;
+		uint length = 0;
+
+		if (crtc->state->gamma_lut) {
+			lut = (struct drm_color_lut *)
+				crtc->state->gamma_lut->data;
+			length = crtc->state->gamma_lut->length /
+				sizeof(*lut);
+		}
+		dispc_mgr_set_gamma(omap_crtc->channel, lut, length);
+	}
+
 	if (dispc_mgr_is_enabled(omap_crtc->channel)) {
 
 		DBG("%s: GO", omap_crtc->name);
@@ -460,6 +473,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,
 	.set_property = drm_atomic_helper_crtc_set_property,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
@@ -534,6 +548,26 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 
 	drm_crtc_helper_add(crtc, &omap_crtc_helper_funcs);
 
+	/* The dispc API adapts to what ever size, but the HW supports
+	 * 256 element gamma table for LCDs and 1024 element table for
+	 * OMAP_DSS_CHANNEL_DIGIT. X server assumes 256 element gamma
+	 * tables so lets use that. Size of HW gamma table can be
+	 * extracted with dispc_mgr_gamma_size(). If it returns 0
+	 * gamma table is not supprted.
+	 */
+	if (dispc_mgr_gamma_size(channel)) {
+		struct drm_mode_config *config = &dev->mode_config;
+		uint gamma_size = 256;
+
+		drm_mode_crtc_set_gamma_size(crtc, gamma_size);
+
+		drm_object_attach_property(&crtc->base,
+					   config->gamma_lut_property, 0);
+		drm_object_attach_property(&crtc->base,
+					   config->gamma_lut_size_property,
+					   gamma_size);
+	}
+
 	omap_plane_install_properties(crtc->primary, &crtc->base);
 
 	omap_crtcs[channel] = omap_crtc;
-- 
1.9.1

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

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

* [PATCH v4 4/7] drm: Add drm_crtc_enable_color_mgmt()
  2016-05-25 20:43 [PATCH v4 0/7] drm/omapdrm: gamma table support + drm_crtc_enable_color_mgmt() Jyri Sarha
                   ` (2 preceding siblings ...)
  2016-05-25 20:43 ` [PATCH v4 3/7] drm/omapdrm: Implement gamma_lut atomic crtc properties Jyri Sarha
@ 2016-05-25 20:43 ` Jyri Sarha
  2016-05-25 21:05   ` Emil Velikov
  2016-05-25 20:43 ` [PATCH v4 5/7] drm/omapdrm: Use drm_crtc_enable_color_mgmt() to enable gamma properties Jyri Sarha
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Jyri Sarha @ 2016-05-25 20:43 UTC (permalink / raw)
  To: dri-devel; +Cc: Jyri Sarha, peter.ujfalusi, tomi.valkeinen, laurent.pinchart

Add drm_crtc_enable_color_mgmt() to. The new function makes the old
drm_helper_crtc_enable_color_mgmt() obsolete. The new function is more
flexible. It allows driver to enable only the feature it has without
forcing to enable all three color management features: gegamma lut, csc
matrix (ctm), and gamma lut.

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/drm_crtc.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |  5 ++++-
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d2a6d95..b097226 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -6065,3 +6065,48 @@ struct drm_tile_group *drm_mode_create_tile_group(struct drm_device *dev,
 	return tg;
 }
 EXPORT_SYMBOL(drm_mode_create_tile_group);
+
+/**
+ * drm_crtc_enable_color_mgmt - enable color management properties
+ * @crtc: DRM CRTC
+ * @degamma_lut_size: the size of the degamma lut (before CSC)
+ * @has_ctm: whether to attach ctm_property for CSC matrix
+ * @gamma_lut_size: the size of the gamma lut (after CSC)
+ *
+ * This function lets the driver enable the color correction
+ * properties on a CRTC. This includes 3 degamma, csc and gamma
+ * properties that userspace can set and 2 size properties to inform
+ * the userspace of the lut sizes. Each of the properties are
+ * 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.
+ */
+void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+				uint degamma_lut_size,
+				bool has_ctm,
+				uint gamma_lut_size)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	if (degamma_lut_size) {
+		drm_object_attach_property(&crtc->base,
+					   config->degamma_lut_property, 0);
+		drm_object_attach_property(&crtc->base,
+					   config->degamma_lut_size_property,
+					   degamma_lut_size);
+	}
+
+	if (has_ctm)
+		drm_object_attach_property(&crtc->base,
+					   config->ctm_property, 0);
+
+	if (gamma_lut_size) {
+		drm_object_attach_property(&crtc->base,
+					   config->gamma_lut_property, 0);
+		drm_object_attach_property(&crtc->base,
+					   config->gamma_lut_size_property,
+					   gamma_lut_size);
+	}
+}
+EXPORT_SYMBOL(drm_crtc_enable_color_mgmt);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d1559cd..36d3bbf 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -2553,7 +2553,10 @@ extern struct drm_property *drm_mode_create_rotation_property(struct drm_device
 							      unsigned int supported_rotations);
 extern unsigned int drm_rotation_simplify(unsigned int rotation,
 					  unsigned int supported_rotations);
-
+extern void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+				       uint degamma_lut_size,
+				       bool has_ctm,
+				       uint gamma_lut_size);
 /* Helpers */
 
 static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
-- 
1.9.1

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

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

* [PATCH v4 5/7] drm/omapdrm: Use drm_crtc_enable_color_mgmt() to enable gamma properties
  2016-05-25 20:43 [PATCH v4 0/7] drm/omapdrm: gamma table support + drm_crtc_enable_color_mgmt() Jyri Sarha
                   ` (3 preceding siblings ...)
  2016-05-25 20:43 ` [PATCH v4 4/7] drm: Add drm_crtc_enable_color_mgmt() Jyri Sarha
@ 2016-05-25 20:43 ` Jyri Sarha
  2016-05-26  6:33   ` Tomi Valkeinen
  2016-05-25 20:43 ` [PATCH v4 6/7] drm/i915: Use drm_crtc_enable_color_mgmt() Jyri Sarha
  2016-05-25 20:43 ` [PATCH v4 7/7] drm: Remove obsolete drm_helper_crtc_enable_color_mgmt() Jyri Sarha
  6 siblings, 1 reply; 14+ messages in thread
From: Jyri Sarha @ 2016-05-25 20:43 UTC (permalink / raw)
  To: dri-devel; +Cc: Jyri Sarha, peter.ujfalusi, tomi.valkeinen, laurent.pinchart

Use new drm_crtc_enable_color_mgmt() to enable gamma lut properties.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 5b7f6f5..e03b349 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -556,16 +556,10 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	 * gamma table is not supprted.
 	 */
 	if (dispc_mgr_gamma_size(channel)) {
-		struct drm_mode_config *config = &dev->mode_config;
-		uint gamma_size = 256;
+		uint gamma_lut_size = 256;
 
-		drm_mode_crtc_set_gamma_size(crtc, gamma_size);
-
-		drm_object_attach_property(&crtc->base,
-					   config->gamma_lut_property, 0);
-		drm_object_attach_property(&crtc->base,
-					   config->gamma_lut_size_property,
-					   gamma_size);
+		drm_crtc_enable_color_mgmt(crtc, 0, false, gamma_lut_size);
+		drm_mode_crtc_set_gamma_size(crtc, gamma_lut_size);
 	}
 
 	omap_plane_install_properties(crtc->primary, &crtc->base);
-- 
1.9.1

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

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

* [PATCH v4 6/7] drm/i915: Use drm_crtc_enable_color_mgmt()
  2016-05-25 20:43 [PATCH v4 0/7] drm/omapdrm: gamma table support + drm_crtc_enable_color_mgmt() Jyri Sarha
                   ` (4 preceding siblings ...)
  2016-05-25 20:43 ` [PATCH v4 5/7] drm/omapdrm: Use drm_crtc_enable_color_mgmt() to enable gamma properties Jyri Sarha
@ 2016-05-25 20:43 ` Jyri Sarha
  2016-05-25 20:43 ` [PATCH v4 7/7] drm: Remove obsolete drm_helper_crtc_enable_color_mgmt() Jyri Sarha
  6 siblings, 0 replies; 14+ messages in thread
From: Jyri Sarha @ 2016-05-25 20:43 UTC (permalink / raw)
  To: dri-devel; +Cc: Jyri Sarha, peter.ujfalusi, tomi.valkeinen, laurent.pinchart

Use drm_crtc_enable_color_mgmt() in intel_color_init() instead of obsolete
drm_helper_crtc_enable_color_mgmt().

Signed-off-by: Jyri Sarha <jsarha@ti.com>
CC: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
I have not tested this change, only compiled it, but functionally it
should be the same. Probably it would make sense to get rid off the
condition for calling drm_crtc_enable_color_mgmt() all together.

 drivers/gpu/drm/i915/intel_color.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 1b3f974..522f5a2 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -547,7 +547,8 @@ void intel_color_init(struct drm_crtc *crtc)
 	/* Enable color management support when we have degamma & gamma LUTs. */
 	if (INTEL_INFO(dev)->color.degamma_lut_size != 0 &&
 	    INTEL_INFO(dev)->color.gamma_lut_size != 0)
-		drm_helper_crtc_enable_color_mgmt(crtc,
+		drm_crtc_enable_color_mgmt(crtc,
 					INTEL_INFO(dev)->color.degamma_lut_size,
+					true,
 					INTEL_INFO(dev)->color.gamma_lut_size);
 }
-- 
1.9.1

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

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

* [PATCH v4 7/7] drm: Remove obsolete drm_helper_crtc_enable_color_mgmt()
  2016-05-25 20:43 [PATCH v4 0/7] drm/omapdrm: gamma table support + drm_crtc_enable_color_mgmt() Jyri Sarha
                   ` (5 preceding siblings ...)
  2016-05-25 20:43 ` [PATCH v4 6/7] drm/i915: Use drm_crtc_enable_color_mgmt() Jyri Sarha
@ 2016-05-25 20:43 ` Jyri Sarha
  2016-05-26  8:08   ` Daniel Vetter
  6 siblings, 1 reply; 14+ messages in thread
From: Jyri Sarha @ 2016-05-25 20:43 UTC (permalink / raw)
  To: dri-devel; +Cc: Jyri Sarha, peter.ujfalusi, tomi.valkeinen, laurent.pinchart

Remove obsolete drm_helper_crtc_enable_color_mgmt(). The function is
replaced by drm_crtc_enable_color_mgmt().

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/drm_crtc_helper.c | 33 ---------------------------------
 include/drm/drm_crtc_helper.h     |  3 ---
 2 files changed, 36 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index a6e4243..bf10d70 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -1121,36 +1121,3 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	return drm_plane_helper_commit(plane, plane_state, old_fb);
 }
 EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
-
-/**
- * drm_helper_crtc_enable_color_mgmt - enable color management properties
- * @crtc: DRM CRTC
- * @degamma_lut_size: the size of the degamma lut (before CSC)
- * @gamma_lut_size: the size of the gamma lut (after CSC)
- *
- * This function lets the driver enable the color correction properties on a
- * CRTC. This includes 3 degamma, csc and gamma properties that userspace can
- * set and 2 size properties to inform the userspace of the lut sizes.
- */
-void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
-				       int degamma_lut_size,
-				       int gamma_lut_size)
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_mode_config *config = &dev->mode_config;
-
-	drm_object_attach_property(&crtc->base,
-				   config->degamma_lut_property, 0);
-	drm_object_attach_property(&crtc->base,
-				   config->ctm_property, 0);
-	drm_object_attach_property(&crtc->base,
-				   config->gamma_lut_property, 0);
-
-	drm_object_attach_property(&crtc->base,
-				   config->degamma_lut_size_property,
-				   degamma_lut_size);
-	drm_object_attach_property(&crtc->base,
-				   config->gamma_lut_size_property,
-				   gamma_lut_size);
-}
-EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt);
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 97fa894..4b37afa 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -48,9 +48,6 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
 				     struct drm_display_mode *mode,
 				     int x, int y,
 				     struct drm_framebuffer *old_fb);
-extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
-					      int degamma_lut_size,
-					      int gamma_lut_size);
 extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
 extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
 
-- 
1.9.1

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

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

* Re: [PATCH v4 4/7] drm: Add drm_crtc_enable_color_mgmt()
  2016-05-25 20:43 ` [PATCH v4 4/7] drm: Add drm_crtc_enable_color_mgmt() Jyri Sarha
@ 2016-05-25 21:05   ` Emil Velikov
  2016-05-25 21:19     ` Emil Velikov
  0 siblings, 1 reply; 14+ messages in thread
From: Emil Velikov @ 2016-05-25 21:05 UTC (permalink / raw)
  To: Jyri Sarha; +Cc: peter.ujfalusi, Tomi Valkeinen, Laurent Pinchart, ML dri-devel

On 25 May 2016 at 21:43, Jyri Sarha <jsarha@ti.com> wrote:
> Add drm_crtc_enable_color_mgmt() to. The new function makes the old
> drm_helper_crtc_enable_color_mgmt() obsolete. The new function is more
> flexible. It allows driver to enable only the feature it has without
> forcing to enable all three color management features: gegamma lut, csc
> matrix (ctm), and gamma lut.
>
Why don't we just update the existing one ? In one step (patch) or two
- a) don't register property if respective _lut_size is zero b) bring
in has_ctm.

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

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

* Re: [PATCH v4 4/7] drm: Add drm_crtc_enable_color_mgmt()
  2016-05-25 21:05   ` Emil Velikov
@ 2016-05-25 21:19     ` Emil Velikov
  2016-05-26  8:07       ` Daniel Vetter
  0 siblings, 1 reply; 14+ messages in thread
From: Emil Velikov @ 2016-05-25 21:19 UTC (permalink / raw)
  To: Jyri Sarha; +Cc: peter.ujfalusi, Tomi Valkeinen, Laurent Pinchart, ML dri-devel

On 25 May 2016 at 22:05, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 25 May 2016 at 21:43, Jyri Sarha <jsarha@ti.com> wrote:
>> Add drm_crtc_enable_color_mgmt() to. The new function makes the old
>> drm_helper_crtc_enable_color_mgmt() obsolete. The new function is more
>> flexible. It allows driver to enable only the feature it has without
>> forcing to enable all three color management features: gegamma lut, csc
>> matrix (ctm), and gamma lut.
>>
> Why don't we just update the existing one ? In one step (patch) or two
> - a) don't register property if respective _lut_size is zero b) bring
> in has_ctm.
>
Ouch I missed that the goal here is to move the function out of
drm_crtc_helper.c. Sorry about that.

Perhaps the commit message can be reworded a bit - something like the
following comes to mind.

"Introduce drm_crtc_enable_color_mgmt() function which supersedes the
crtc_helper one (xxx: add name). The latter always creates all
properties (xxx: list props) and is not really a helper.
The new version allows explicit control of the properties created as
required by some hardware/drivers (xxx: name driver)."

Feel free to reuse and/tweak.

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

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

* Re: [PATCH v4 5/7] drm/omapdrm: Use drm_crtc_enable_color_mgmt() to enable gamma properties
  2016-05-25 20:43 ` [PATCH v4 5/7] drm/omapdrm: Use drm_crtc_enable_color_mgmt() to enable gamma properties Jyri Sarha
@ 2016-05-26  6:33   ` Tomi Valkeinen
  0 siblings, 0 replies; 14+ messages in thread
From: Tomi Valkeinen @ 2016-05-26  6:33 UTC (permalink / raw)
  To: Jyri Sarha, dri-devel; +Cc: peter.ujfalusi, laurent.pinchart


[-- Attachment #1.1.1: Type: text/plain, Size: 1396 bytes --]

On 25/05/16 23:43, Jyri Sarha wrote:
> Use new drm_crtc_enable_color_mgmt() to enable gamma lut properties.

You could move the color-mgmt patches to the beginning of the series,
and add the omapdrm ones on top of those.

> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_crtc.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index 5b7f6f5..e03b349 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -556,16 +556,10 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
>  	 * gamma table is not supprted.
>  	 */
>  	if (dispc_mgr_gamma_size(channel)) {
> -		struct drm_mode_config *config = &dev->mode_config;
> -		uint gamma_size = 256;
> +		uint gamma_lut_size = 256;
>  
> -		drm_mode_crtc_set_gamma_size(crtc, gamma_size);
> -
> -		drm_object_attach_property(&crtc->base,
> -					   config->gamma_lut_property, 0);
> -		drm_object_attach_property(&crtc->base,
> -					   config->gamma_lut_size_property,
> -					   gamma_size);
> +		drm_crtc_enable_color_mgmt(crtc, 0, false, gamma_lut_size);
> +		drm_mode_crtc_set_gamma_size(crtc, gamma_lut_size);

Any reason drm_crtc_enable_color_mgmt() couldn't also call
drm_mode_crtc_set_gamma_size()?

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 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

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

* Re: [PATCH v4 4/7] drm: Add drm_crtc_enable_color_mgmt()
  2016-05-25 21:19     ` Emil Velikov
@ 2016-05-26  8:07       ` Daniel Vetter
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2016-05-26  8:07 UTC (permalink / raw)
  To: Emil Velikov
  Cc: peter.ujfalusi, ML dri-devel, Tomi Valkeinen, Laurent Pinchart,
	Jyri Sarha

On Wed, May 25, 2016 at 10:19:50PM +0100, Emil Velikov wrote:
> On 25 May 2016 at 22:05, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> > On 25 May 2016 at 21:43, Jyri Sarha <jsarha@ti.com> wrote:
> >> Add drm_crtc_enable_color_mgmt() to. The new function makes the old
> >> drm_helper_crtc_enable_color_mgmt() obsolete. The new function is more
> >> flexible. It allows driver to enable only the feature it has without
> >> forcing to enable all three color management features: gegamma lut, csc
> >> matrix (ctm), and gamma lut.
> >>
> > Why don't we just update the existing one ? In one step (patch) or two
> > - a) don't register property if respective _lut_size is zero b) bring
> > in has_ctm.
> >
> Ouch I missed that the goal here is to move the function out of
> drm_crtc_helper.c. Sorry about that.

The goal still was to move it, and update the existing callers. Adding a
new without removing the old one is not what I suggested really ;-)
-Daniel

> 
> Perhaps the commit message can be reworded a bit - something like the
> following comes to mind.
> 
> "Introduce drm_crtc_enable_color_mgmt() function which supersedes the
> crtc_helper one (xxx: add name). The latter always creates all
> properties (xxx: list props) and is not really a helper.
> The new version allows explicit control of the properties created as
> required by some hardware/drivers (xxx: name driver)."
> 
> Feel free to reuse and/tweak.
> 
> Thanks
> Emil
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v4 7/7] drm: Remove obsolete drm_helper_crtc_enable_color_mgmt()
  2016-05-25 20:43 ` [PATCH v4 7/7] drm: Remove obsolete drm_helper_crtc_enable_color_mgmt() Jyri Sarha
@ 2016-05-26  8:08   ` Daniel Vetter
  2016-05-26  8:15     ` Jyri Sarha
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2016-05-26  8:08 UTC (permalink / raw)
  To: Jyri Sarha; +Cc: dri-devel, peter.ujfalusi, tomi.valkeinen, laurent.pinchart

On Wed, May 25, 2016 at 11:43:30PM +0300, Jyri Sarha wrote:
> Remove obsolete drm_helper_crtc_enable_color_mgmt(). The function is
> replaced by drm_crtc_enable_color_mgmt().
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>

Ah, here it is. Tbh this is patch splitting too far, since when you move a
function it's much better to have the removal and addition in the same
patch. If you split it like this then it's much harder to review.

So please merge this with the addition patch + the patch to update i915.
We can handle the resulting conflicts (if there are any) and
cross-maintainer depencies.
-Daniel

> ---
>  drivers/gpu/drm/drm_crtc_helper.c | 33 ---------------------------------
>  include/drm/drm_crtc_helper.h     |  3 ---
>  2 files changed, 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index a6e4243..bf10d70 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -1121,36 +1121,3 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>  	return drm_plane_helper_commit(plane, plane_state, old_fb);
>  }
>  EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
> -
> -/**
> - * drm_helper_crtc_enable_color_mgmt - enable color management properties
> - * @crtc: DRM CRTC
> - * @degamma_lut_size: the size of the degamma lut (before CSC)
> - * @gamma_lut_size: the size of the gamma lut (after CSC)
> - *
> - * This function lets the driver enable the color correction properties on a
> - * CRTC. This includes 3 degamma, csc and gamma properties that userspace can
> - * set and 2 size properties to inform the userspace of the lut sizes.
> - */
> -void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> -				       int degamma_lut_size,
> -				       int gamma_lut_size)
> -{
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_mode_config *config = &dev->mode_config;
> -
> -	drm_object_attach_property(&crtc->base,
> -				   config->degamma_lut_property, 0);
> -	drm_object_attach_property(&crtc->base,
> -				   config->ctm_property, 0);
> -	drm_object_attach_property(&crtc->base,
> -				   config->gamma_lut_property, 0);
> -
> -	drm_object_attach_property(&crtc->base,
> -				   config->degamma_lut_size_property,
> -				   degamma_lut_size);
> -	drm_object_attach_property(&crtc->base,
> -				   config->gamma_lut_size_property,
> -				   gamma_lut_size);
> -}
> -EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt);
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index 97fa894..4b37afa 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -48,9 +48,6 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>  				     struct drm_display_mode *mode,
>  				     int x, int y,
>  				     struct drm_framebuffer *old_fb);
> -extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> -					      int degamma_lut_size,
> -					      int gamma_lut_size);
>  extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
>  extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
>  
> -- 
> 1.9.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v4 7/7] drm: Remove obsolete drm_helper_crtc_enable_color_mgmt()
  2016-05-26  8:08   ` Daniel Vetter
@ 2016-05-26  8:15     ` Jyri Sarha
  0 siblings, 0 replies; 14+ messages in thread
From: Jyri Sarha @ 2016-05-26  8:15 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel, peter.ujfalusi, tomi.valkeinen, laurent.pinchart

On 05/26/16 11:08, Daniel Vetter wrote:
> On Wed, May 25, 2016 at 11:43:30PM +0300, Jyri Sarha wrote:
>> Remove obsolete drm_helper_crtc_enable_color_mgmt(). The function is
>> replaced by drm_crtc_enable_color_mgmt().
>>
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> 
> Ah, here it is. Tbh this is patch splitting too far, since when you move a
> function it's much better to have the removal and addition in the same
> patch. If you split it like this then it's much harder to review.
> 
> So please merge this with the addition patch + the patch to update i915.
> We can handle the resulting conflicts (if there are any) and
> cross-maintainer depencies.
> -Daniel

Ok, I'll do that and move the omapdrm patches after the
color_mgmt-patch, and squash omapdrm-patch #5 to patch #3.

BR,
Jyri

> 
>> ---
>>  drivers/gpu/drm/drm_crtc_helper.c | 33 ---------------------------------
>>  include/drm/drm_crtc_helper.h     |  3 ---
>>  2 files changed, 36 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
>> index a6e4243..bf10d70 100644
>> --- a/drivers/gpu/drm/drm_crtc_helper.c
>> +++ b/drivers/gpu/drm/drm_crtc_helper.c
>> @@ -1121,36 +1121,3 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>>  	return drm_plane_helper_commit(plane, plane_state, old_fb);
>>  }
>>  EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
>> -
>> -/**
>> - * drm_helper_crtc_enable_color_mgmt - enable color management properties
>> - * @crtc: DRM CRTC
>> - * @degamma_lut_size: the size of the degamma lut (before CSC)
>> - * @gamma_lut_size: the size of the gamma lut (after CSC)
>> - *
>> - * This function lets the driver enable the color correction properties on a
>> - * CRTC. This includes 3 degamma, csc and gamma properties that userspace can
>> - * set and 2 size properties to inform the userspace of the lut sizes.
>> - */
>> -void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>> -				       int degamma_lut_size,
>> -				       int gamma_lut_size)
>> -{
>> -	struct drm_device *dev = crtc->dev;
>> -	struct drm_mode_config *config = &dev->mode_config;
>> -
>> -	drm_object_attach_property(&crtc->base,
>> -				   config->degamma_lut_property, 0);
>> -	drm_object_attach_property(&crtc->base,
>> -				   config->ctm_property, 0);
>> -	drm_object_attach_property(&crtc->base,
>> -				   config->gamma_lut_property, 0);
>> -
>> -	drm_object_attach_property(&crtc->base,
>> -				   config->degamma_lut_size_property,
>> -				   degamma_lut_size);
>> -	drm_object_attach_property(&crtc->base,
>> -				   config->gamma_lut_size_property,
>> -				   gamma_lut_size);
>> -}
>> -EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt);
>> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
>> index 97fa894..4b37afa 100644
>> --- a/include/drm/drm_crtc_helper.h
>> +++ b/include/drm/drm_crtc_helper.h
>> @@ -48,9 +48,6 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>>  				     struct drm_display_mode *mode,
>>  				     int x, int y,
>>  				     struct drm_framebuffer *old_fb);
>> -extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>> -					      int degamma_lut_size,
>> -					      int gamma_lut_size);
>>  extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
>>  extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
>>  
>> -- 
>> 1.9.1
>>
> 

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

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

end of thread, other threads:[~2016-05-26  8:15 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-25 20:43 [PATCH v4 0/7] drm/omapdrm: gamma table support + drm_crtc_enable_color_mgmt() Jyri Sarha
2016-05-25 20:43 ` [PATCH v4 1/7] drm/omapdrm: Add gamma table support to DSS dispc Jyri Sarha
2016-05-25 20:43 ` [PATCH v4 2/7] drm/omapdrm: Workaround for errata i734 (LCD1 Gamma) in " Jyri Sarha
2016-05-25 20:43 ` [PATCH v4 3/7] drm/omapdrm: Implement gamma_lut atomic crtc properties Jyri Sarha
2016-05-25 20:43 ` [PATCH v4 4/7] drm: Add drm_crtc_enable_color_mgmt() Jyri Sarha
2016-05-25 21:05   ` Emil Velikov
2016-05-25 21:19     ` Emil Velikov
2016-05-26  8:07       ` Daniel Vetter
2016-05-25 20:43 ` [PATCH v4 5/7] drm/omapdrm: Use drm_crtc_enable_color_mgmt() to enable gamma properties Jyri Sarha
2016-05-26  6:33   ` Tomi Valkeinen
2016-05-25 20:43 ` [PATCH v4 6/7] drm/i915: Use drm_crtc_enable_color_mgmt() Jyri Sarha
2016-05-25 20:43 ` [PATCH v4 7/7] drm: Remove obsolete drm_helper_crtc_enable_color_mgmt() Jyri Sarha
2016-05-26  8:08   ` Daniel Vetter
2016-05-26  8:15     ` Jyri Sarha

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.