[PATCH v2 0/3] drm/omapdrm: gamma table support

[PATCH v2 0/3] drm/omapdrm: gamma table support

[Jyri Sarha]

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.

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 (3):
  drm/omapdrm: Add gamma table support to DSS dispc
  drm/omapdrm: Work-a-round for errata i734 (LCD1 Gamma) in DSS dispc
  drm/omapdrm: Implement gamma_lut atomic crtc property

 drivers/gpu/drm/omapdrm/dss/dispc.c   | 364 ++++++++++++++++++++++++++++++++--
 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   |  20 ++
 6 files changed, 378 insertions(+), 22 deletions(-)

-- 
1.9.1

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

[PATCH v2 1/3] drm/omapdrm: Add gamma table support to DSS dispc

[Jyri Sarha]

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.

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   | 192 +++++++++++++++++++++++++++++++---
 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, 186 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index f83608b..b284622 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,128 @@ 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);
+}
+
+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;
+
+	for (i = 0; i < length; ++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;
+
+	for (channel = 0; channel < ARRAY_SIZE(dispc.gamma_table); channel++) {
+		const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma;
+		u32 *gt;
+		uint i;
+
+		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;
+
+		for (i = 0; i < gdesc->len; ++i)
+			gt[i] = (i << 2 * gdesc->bits) | (i << gdesc->bits) | i;
+
+		dispc.gamma_table[channel] = gt;
+	}
+	return 0;
+}
+
 static void _omap_dispc_initial_config(void)
 {
 	u32 l;
@@ -3829,8 +3974,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 +4086,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 +4112,7 @@ static const struct dispc_features omap54xx_dispc_feats = {
 	.has_writeback		=	true,
 	.supports_double_pixel	=	true,
 	.reverse_ilace_field_order =	true,
+	.has_gamma_tables	=	true,
 };
 
 static int dispc_init_features(struct platform_device *pdev)
@@ -4100,6 +4254,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 +4328,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

[PATCH v2 2/3] drm/omapdrm: Work-a-round for errata i734 (LCD1 Gamma) in DSS dispc

[Jyri Sarha]

Work-a-round 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 work-a-round 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
work-a-round 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.

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

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
index b284622..0e04bed 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
@@ -3931,7 +3933,7 @@ static int dispc_init_gamma_tables(void)
 	int channel;
 
 	if (!dispc.feat->has_gamma_table)
-		return;
+		return 0;
 
 	for (channel = 0; channel < ARRAY_SIZE(dispc.gamma_table); channel++) {
 		const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma;
@@ -4087,6 +4089,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 = {
@@ -4112,7 +4115,8 @@ static const struct dispc_features omap54xx_dispc_feats = {
 	.has_writeback		=	true,
 	.supports_double_pixel	=	true,
 	.reverse_ilace_field_order =	true,
-	.has_gamma_tables	=	true,
+	.has_gamma_table	=	true,
+	.has_gamma_i734_bug	=	true,
 };
 
 static int dispc_init_features(struct platform_device *pdev)
@@ -4204,6 +4208,166 @@ void dispc_free_irq(void *dev_id)
 }
 EXPORT_SYMBOL(dispc_free_irq);
 
+/*
+ * Work-a-round 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 work-a-round
+ * 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 work-a-round 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 (IS_ERR(i734_buf.vaddr)) {
+		dev_err(&dispc.pdev->dev,
+			"%s: dma_alloc_writecombine failed: %ld\n",
+			__func__, PTR_ERR(i734_buf.vaddr));
+		return PTR_ERR(i734_buf.vaddr);
+	}
+
+	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_CONTROL, 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 (can't fiddle with irq handlers in resume) */
+	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)
 {
@@ -4221,6 +4385,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");
@@ -4285,6 +4453,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 = {
@@ -4327,6 +4497,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

Re: [PATCH v2 2/3] drm/omapdrm: Work-a-round for errata i734 (LCD1 Gamma) in DSS dispc

[Jyri Sarha]

On 05/23/16 17:41, Jyri Sarha wrote:
> @@ -4112,7 +4115,8 @@ static const struct dispc_features omap54xx_dispc_feats = {
>  	.has_writeback		=	true,
>  	.supports_double_pixel	=	true,
>  	.reverse_ilace_field_order =	true,
> -	.has_gamma_tables	=	true,
> +	.has_gamma_table	=	true,
> +	.has_gamma_i734_bug	=	true,
>  };

Oops, the has_gamma_tables => has_gamma_table should be in the previous
patch.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/3] drm/omapdrm: Work-a-round for errata i734 (LCD1 Gamma) in DSS dispc

[Tomi Valkeinen]

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

On 23/05/16 17:41, Jyri Sarha wrote:
> Work-a-round for errata i734 in DSS dispc

I think it's "workaround" =).

>  - 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 work-a-round 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
> work-a-round requires that gamma table's LOADMODE is set to 0x2 in
> DISPC_CONTROL1 register.

This LOADMODE comment is a bit odd. You should say why, and how it's
handled, and what's "0x2". But then, I'm not sure if the whole comment
is needed at all. The driver is made to work only with LOADMODE=2.

Maybe the real point here is that the WA needs to happen after the
initial DSS register config.

> 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 | 176 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 174 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index b284622..0e04bed 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
> @@ -3931,7 +3933,7 @@ static int dispc_init_gamma_tables(void)
>  	int channel;
>  
>  	if (!dispc.feat->has_gamma_table)
> -		return;
> +		return 0;

This doesn't look like it belongs to this patch.

>  
>  	for (channel = 0; channel < ARRAY_SIZE(dispc.gamma_table); channel++) {
>  		const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma;
> @@ -4087,6 +4089,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 = {
> @@ -4112,7 +4115,8 @@ static const struct dispc_features omap54xx_dispc_feats = {
>  	.has_writeback		=	true,
>  	.supports_double_pixel	=	true,
>  	.reverse_ilace_field_order =	true,
> -	.has_gamma_tables	=	true,
> +	.has_gamma_table	=	true,
> +	.has_gamma_i734_bug	=	true,
>  };
>  
>  static int dispc_init_features(struct platform_device *pdev)
> @@ -4204,6 +4208,166 @@ void dispc_free_irq(void *dev_id)
>  }
>  EXPORT_SYMBOL(dispc_free_irq);
>  
> +/*
> + * Work-a-round 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 work-a-round
> + * 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 work-a-round 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 (IS_ERR(i734_buf.vaddr)) {

Hmm I don't think dma_alloc_* returns an err ptr?

> +		dev_err(&dispc.pdev->dev,
> +			"%s: dma_alloc_writecombine failed: %ld\n",
> +			__func__, PTR_ERR(i734_buf.vaddr));
> +		return PTR_ERR(i734_buf.vaddr);
> +	}
> +
> +	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_CONTROL, 8, 4);

Still wrong register.

> +
> +	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 (can't fiddle with irq handlers in resume) */
> +	count = 0;
> +	while (!(dispc_read_irqstatus() & framedone_irq))
> +		if (count++ > 10000) {
> +			dev_err(&dispc.pdev->dev, "%s: framedone timeout\n",
> +				__func__);
> +			break;
> +		}

Add { } for the 'while' too.

Busyloops are always a bit tricky... I would perhaps change the loop
above so that you first busy loop for a certain amount of loops, but
after that udelay() for a short period in each loop.

And it would be good to have a note here why busyloop is fine and what
is the calculated time the frame takes. I mean, the reason why busy loop
is preferred is what you mention, that we can't easily have a irq
handler here, but the reason busyloop is fine is that the frame is so small.

 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

Re: [PATCH v2 2/3] drm/omapdrm: Work-a-round for errata i734 (LCD1 Gamma) in DSS dispc

[Jyri Sarha]

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

On 05/23/16 18:06, Tomi Valkeinen wrote:
>> 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 work-a-round 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
>> > work-a-round requires that gamma table's LOADMODE is set to 0x2 in
>> > DISPC_CONTROL1 register.
> This LOADMODE comment is a bit odd. You should say why, and how it's
> handled, and what's "0x2". But then, I'm not sure if the whole comment
> is needed at all. The driver is made to work only with LOADMODE=2.
> 

It is there also in the comment and my point is to notify that if the
gamma support is ever changed to use some other mode, the workaround
needs to change too.

It is pretty hard to draw the line to how deep in details one should go
in comments or commit descriptions. I added the register name so that
someone is interested he can find the full description of the LOADMODE
from TRM using that as a key word.

I think I wont mention the LOADMODE in the description but leave it on
the code comments.

> Maybe the real point here is that the WA needs to happen after the
> initial DSS register config.
> 

I think that detail should be in the code comments, if it is needed at all.

BR,
Jyri


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

[PATCH v2 3/3] drm/omapdrm: Implement gamma_lut atomic crtc property

[Jyri Sarha]

Implement gamma_lut atomic crtc property, 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.

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

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 075f2bb..d5210fe 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -384,6 +384,15 @@ 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 = (struct drm_color_lut *)
+			crtc->state->gamma_lut->data;
+		unsigned int 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 +469,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 +544,16 @@ 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))
+		drm_mode_crtc_set_gamma_size(crtc, 256);
+
 	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