[PATCH 0/4] Add TCON support to the R-Car DU driver

[PATCH 0/4] Add TCON support to the R-Car DU driver

[Sergei Shtylyov]

Hello.

   Here's the set of 4 patches against David Airlie's 'linux.git' repo's
'drm-next' branch. We're adding support for the TCON (timing controller) that
 is a part of the R-Car SoC and can directly drive a LCD.

[1/4] drm_mode: add-TCON encoder connector
[2/4] rcar-du: add TCON encoder driver
[3/4] rcar-du: add TCON connector driver
[4/4] rcar-du: add R8A7794 TCON support

MBR, Sergei

[PATCH 1/4] drm_mode: add TCON encoder/connector

[Sergei Shtylyov]

TCON (Timing  Controller) usually means  a chip that drives a LCD panel.
In  our case, such controller  is a part of the Renesas R-Car SoCs. Add
the TCON encoder/connector #define's  to be used by the TCON support code
in the Renesas R-Car Display Unit (DU) driver.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 include/uapi/drm/drm_mode.h |    2 ++
 1 file changed, 2 insertions(+)

Index: linux/include/uapi/drm/drm_mode.h
===================================================================
--- linux.orig/include/uapi/drm/drm_mode.h
+++ linux/include/uapi/drm/drm_mode.h
@@ -202,6 +202,7 @@ struct drm_mode_get_plane_res {
 #define DRM_MODE_ENCODER_VIRTUAL 5
 #define DRM_MODE_ENCODER_DSI	6
 #define DRM_MODE_ENCODER_DPMST	7
+#define DRM_MODE_ENCODER_TCON	8
 
 struct drm_mode_get_encoder {
 	__u32 encoder_id;
@@ -241,6 +242,7 @@ struct drm_mode_get_encoder {
 #define DRM_MODE_CONNECTOR_eDP		14
 #define DRM_MODE_CONNECTOR_VIRTUAL      15
 #define DRM_MODE_CONNECTOR_DSI		16
+#define DRM_MODE_CONNECTOR_TCON		17
 
 struct drm_mode_get_connector {
 

[PATCH 2/4] rcar-du: add TCON encoder driver

[Sergei Shtylyov]

Renesas  R-Car SoCs  include the timing controller (TCON) that can directly
drive LCDs. It receives  the H/VSYNC, etc. from the Display Unit (DU)  and
converts  them to the set of signals  that a LCD panel can understand (the
RBG  signals are effectively passed thru).  TCON has a set of registers
that we need to  program based on the video mode timings, so we're adding
a DU encoder driver doing that...

Based on a large patch by Andrey Gusakov.

Signed-off-by: Andrey Gusakov <andrey.gusakov@cogentembedded.com>
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 drivers/gpu/drm/rcar-du/Kconfig           |    6 
 drivers/gpu/drm/rcar-du/Makefile          |    1 
 drivers/gpu/drm/rcar-du/rcar_du_drv.h     |    4 
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c |    9 +
 drivers/gpu/drm/rcar-du/rcar_du_encoder.h |    3 
 drivers/gpu/drm/rcar-du/rcar_du_kms.c     |    4 
 drivers/gpu/drm/rcar-du/rcar_du_tconenc.c |  184 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_tconenc.h |   37 ++++++
 drivers/gpu/drm/rcar-du/rcar_tcon_regs.h  |   70 +++++++++++
 9 files changed, 318 insertions(+)

Index: linux/drivers/gpu/drm/rcar-du/Kconfig
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/Kconfig
+++ linux/drivers/gpu/drm/rcar-du/Kconfig
@@ -24,6 +24,12 @@ config DRM_RCAR_LVDS
 	help
 	  Enable support for the R-Car Display Unit embedded LVDS encoders.
 
+config DRM_RCAR_TCON
+	bool "R-Car DU TCON Encoder Support"
+	depends on DRM_RCAR_DU
+	help
+	  Enable support for the R-Car Display Unit embedded TCON encoders.
+
 config DRM_RCAR_VSP
 	bool "R-Car DU VSP Compositor Support"
 	depends on DRM_RCAR_DU
Index: linux/drivers/gpu/drm/rcar-du/Makefile
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/Makefile
+++ linux/drivers/gpu/drm/rcar-du/Makefile
@@ -10,6 +10,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
 rcar-du-drm-$(CONFIG_DRM_RCAR_HDMI)	+= rcar_du_hdmicon.o \
 					   rcar_du_hdmienc.o
 rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_lvdsenc.o
+rcar-du-drm-$(CONFIG_DRM_RCAR_TCON)	+= rcar_du_tconenc.o
 
 rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
 
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -59,6 +59,7 @@ struct rcar_du_output_routing {
  * @num_crtcs: total number of CRTCs
  * @routes: array of CRTC to output routes, indexed by output (RCAR_DU_OUTPUT_*)
  * @num_lvds: number of internal LVDS encoders
+ * @num_tcon: number of internal TCON encoders
  */
 struct rcar_du_device_info {
 	unsigned int gen;
@@ -67,11 +68,13 @@ struct rcar_du_device_info {
 	unsigned int num_crtcs;
 	struct rcar_du_output_routing routes[RCAR_DU_OUTPUT_MAX];
 	unsigned int num_lvds;
+	unsigned int num_tcon;
 };
 
 #define RCAR_DU_MAX_CRTCS		4
 #define RCAR_DU_MAX_GROUPS		DIV_ROUND_UP(RCAR_DU_MAX_CRTCS, 2)
 #define RCAR_DU_MAX_LVDS		2
+#define RCAR_DU_MAX_TCON		1
 #define RCAR_DU_MAX_VSPS		4
 
 struct rcar_du_device {
@@ -99,6 +102,7 @@ struct rcar_du_device {
 	unsigned int vspd1_sink;
 
 	struct rcar_du_lvdsenc *lvds[RCAR_DU_MAX_LVDS];
+	struct rcar_du_tconenc *tcon[RCAR_DU_MAX_TCON];
 
 	struct {
 		wait_queue_head_t wait;
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -24,6 +24,7 @@
 #include "rcar_du_kms.h"
 #include "rcar_du_lvdscon.h"
 #include "rcar_du_lvdsenc.h"
+#include "rcar_du_tconenc.h"
 #include "rcar_du_vgacon.h"
 
 /* -----------------------------------------------------------------------------
@@ -48,6 +49,8 @@ static void rcar_du_encoder_disable(stru
 
 	if (renc->lvds)
 		rcar_du_lvdsenc_enable(renc->lvds, encoder->crtc, false);
+	if (renc->tcon)
+		rcar_du_tconenc_enable(renc->tcon, encoder->crtc, false);
 }
 
 static void rcar_du_encoder_enable(struct drm_encoder *encoder)
@@ -56,6 +59,8 @@ static void rcar_du_encoder_enable(struc
 
 	if (renc->lvds)
 		rcar_du_lvdsenc_enable(renc->lvds, encoder->crtc, true);
+	if (renc->tcon)
+		rcar_du_tconenc_enable(renc->tcon, encoder->crtc, true);
 }
 
 static int rcar_du_encoder_atomic_check(struct drm_encoder *encoder,
@@ -142,6 +147,10 @@ int rcar_du_encoder_init(struct rcar_du_
 		renc->lvds = rcdu->lvds[1];
 		break;
 
+	case RCAR_DU_OUTPUT_TCON:
+		renc->tcon = rcdu->tcon[0];
+		break;
+
 	default:
 		break;
 	}
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
@@ -20,6 +20,7 @@
 struct rcar_du_device;
 struct rcar_du_hdmienc;
 struct rcar_du_lvdsenc;
+struct rcar_du_tconenc;
 
 enum rcar_du_encoder_type {
 	RCAR_DU_ENCODER_UNUSED = 0,
@@ -27,6 +28,7 @@ enum rcar_du_encoder_type {
 	RCAR_DU_ENCODER_VGA,
 	RCAR_DU_ENCODER_LVDS,
 	RCAR_DU_ENCODER_HDMI,
+	RCAR_DU_ENCODER_TCON,
 };
 
 struct rcar_du_encoder {
@@ -34,6 +36,7 @@ struct rcar_du_encoder {
 	enum rcar_du_output output;
 	struct rcar_du_hdmienc *hdmi;
 	struct rcar_du_lvdsenc *lvds;
+	struct rcar_du_tconenc *tcon;
 };
 
 #define to_rcar_encoder(e) \
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_kms.c
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -27,6 +27,7 @@
 #include "rcar_du_encoder.h"
 #include "rcar_du_kms.h"
 #include "rcar_du_lvdsenc.h"
+#include "rcar_du_tconenc.h"
 #include "rcar_du_regs.h"
 #include "rcar_du_vsp.h"
 
@@ -619,6 +620,9 @@ int rcar_du_modeset_init(struct rcar_du_
 	ret = rcar_du_lvdsenc_init(rcdu);
 	if (ret < 0)
 		return ret;
+	ret = rcar_du_tconenc_init(rcdu);
+	if (ret < 0)
+		return ret;
 
 	ret = rcar_du_encoders_init(rcdu);
 	if (ret < 0)
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_tconenc.c
===================================================================
--- /dev/null
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_tconenc.c
@@ -0,0 +1,184 @@
+/*
+ * rcar_du_tconenc.c -- R-Car Display Unit TCON Encoder
+ *
+ * Copyright (C) 2015-2016 Cogent Embedded, Inc. <source@cogentembedded.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <drm/drm_encoder_slave.h>
+
+#include "rcar_du_drv.h"
+#include "rcar_du_encoder.h"
+#include "rcar_du_tconenc.h"
+#include "rcar_tcon_regs.h"
+
+struct rcar_du_tconenc {
+	struct rcar_du_device *dev;
+
+	unsigned int index;
+	void __iomem *mmio;
+	struct clk *clock;
+	bool enabled;
+};
+
+static void rcar_tcon_write(struct rcar_du_tconenc *tcon, u32 reg, u32 data)
+{
+	iowrite32(data, tcon->mmio + reg);
+}
+
+static int rcar_du_tconenc_start(struct rcar_du_tconenc *tcon,
+				 struct rcar_du_crtc *rcrtc)
+{
+	const struct drm_display_mode *mode = &rcrtc->crtc.mode;
+	int ret;
+
+	if (tcon->enabled)
+		return 0;
+
+	ret = clk_prepare_enable(tcon->clock);
+	if (ret < 0)
+		return ret;
+
+	/* Update */
+	rcar_tcon_write(tcon, OUT_V_LATCH, OUTCNT_VEN);
+	rcar_tcon_write(tcon, TCON_V_LATCH, TCON_VEN);
+
+	/* Signals:
+	 * R-Car	Display
+	 * QCLK		SSC (Source Driver Clock Input)
+	 * QSTH		SSP (Source Scanning Signal Left/Right)
+	 * QSTB		SOE (Source Driver Output Enable)
+	 * QCPV		GOE (Gate Driver Output Enable)
+	 * QPOLA	POL (Polarity Control Signal)
+	 * QPOLB	GSC (Gate Driver Scanning Clock)
+	 * QSTVA	GSP (Gate Scanning Start Signal Up/Down)
+	 * QSTVB	nope
+	 */
+	/* Setup timings */
+	rcar_tcon_write(tcon, TCON_TIM, TCON_TIMING(50, 0));
+	/* Horizontal timings */
+	rcar_tcon_write(tcon, TCON_TIM_STH1, TCON_TIMING(mode->htotal -
+							 mode->hsync_start - 1,
+							 1));
+	rcar_tcon_write(tcon, TCON_TIM_STH2, TCON_SEL_STH_SP_HS);
+	rcar_tcon_write(tcon, TCON_TIM_STB1, TCON_TIMING(mode->htotal -
+							 mode->hsync_start +
+							 mode->hdisplay + 6,
+							 3));
+	rcar_tcon_write(tcon, TCON_TIM_STB2, TCON_SEL_STB_LP_HE);
+	rcar_tcon_write(tcon, TCON_TIM_POLB1,
+			TCON_TIMING(mode->htotal - mode->hsync_start +
+				    mode->hdisplay - 8, mode->htotal / 2));
+	rcar_tcon_write(tcon, TCON_TIM_POLB2,
+			TCON_SEL_POLB | TCON_INV | TCON_MD_NORM);
+	rcar_tcon_write(tcon, TCON_TIM_CPV1,
+			TCON_TIMING(mode->htotal - mode->hsync_start +
+				    mode->hdisplay - 33, 50));
+	rcar_tcon_write(tcon, TCON_TIM_CPV2, TCON_SEL_CPV_GCK);
+	rcar_tcon_write(tcon, TCON_TIM_POLA1,
+			TCON_TIMING(mode->htotal - mode->hsync_start +
+				    mode->hdisplay + 1, 39));
+	rcar_tcon_write(tcon, TCON_TIM_POLA2,
+			TCON_SEL_POLA | TCON_INV | TCON_MD_1X1);
+
+	/* Vertical timings */
+	rcar_tcon_write(tcon, TCON_TIM_STVA1,
+			TCON_TIMING(mode->vtotal - mode->vsync_start, 1));
+	rcar_tcon_write(tcon, TCON_TIM_STVA2, TCON_SEL_STVA_VS);
+	rcar_tcon_write(tcon, TCON_TIM_STVB1, TCON_TIMING(0, 0));
+	rcar_tcon_write(tcon, TCON_TIM_STVB2, TCON_SEL_STVB_VE);
+
+	/* Data clocked out on the falling edge of QCLK, latched in LCD on
+	 * the rising edge
+	 */
+	rcar_tcon_write(tcon, OUT_CLK_PHASE, OUTCNT_LCD_EDGE);
+
+	/* Update */
+	rcar_tcon_write(tcon, OUT_V_LATCH, OUTCNT_VEN);
+	rcar_tcon_write(tcon, TCON_V_LATCH, TCON_VEN);
+
+	tcon->enabled = true;
+
+	return 0;
+}
+
+static void rcar_du_tconenc_stop(struct rcar_du_tconenc *tcon)
+{
+	if (!tcon->enabled)
+		return;
+
+	clk_disable_unprepare(tcon->clock);
+
+	tcon->enabled = false;
+}
+
+int rcar_du_tconenc_enable(struct rcar_du_tconenc *tcon, struct drm_crtc *crtc,
+			   bool enable)
+{
+	if (!enable) {
+		rcar_du_tconenc_stop(tcon);
+		return 0;
+	} else if (crtc) {
+		struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
+
+		return rcar_du_tconenc_start(tcon, rcrtc);
+	} else
+		return -EINVAL;
+}
+
+static int rcar_du_tconenc_get_resources(struct rcar_du_tconenc *tcon,
+					 struct platform_device *pdev)
+{
+	struct resource *mem;
+	char name[7];
+
+	sprintf(name, "tcon.%u", tcon->index);
+
+	mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
+	tcon->mmio = devm_ioremap_resource(&pdev->dev, mem);
+	if (IS_ERR(tcon->mmio))
+		return PTR_ERR(tcon->mmio);
+
+	tcon->clock = devm_clk_get(&pdev->dev, name);
+	if (IS_ERR(tcon->clock)) {
+		dev_err(&pdev->dev, "failed to get clock for %s\n", name);
+		return PTR_ERR(tcon->clock);
+	}
+
+	return 0;
+}
+
+int rcar_du_tconenc_init(struct rcar_du_device *rcdu)
+{
+	struct platform_device *pdev = to_platform_device(rcdu->dev);
+	struct rcar_du_tconenc *tcon;
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < rcdu->info->num_tcon; ++i) {
+		tcon = devm_kzalloc(&pdev->dev, sizeof(*tcon), GFP_KERNEL);
+		if (tcon == NULL)
+			return -ENOMEM;
+
+		tcon->dev = rcdu;
+		tcon->index = i;
+		tcon->enabled = false;
+
+		ret = rcar_du_tconenc_get_resources(tcon, pdev);
+		if (ret < 0)
+			return ret;
+
+		rcdu->tcon[i] = tcon;
+	}
+
+	return 0;
+}
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_tconenc.h
===================================================================
--- /dev/null
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_tconenc.h
@@ -0,0 +1,37 @@
+/*
+ * rcar_du_tconenc.h -- R-Car Display Unit TCON Encoder
+ *
+ * Copyright (C) 2015-2016 CogentEmbedded, Inc. <source@cogentembedded.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __RCAR_DU_TCONENC_H__
+#define __RCAR_DU_TCONENC_H__
+
+#include <linux/io.h>
+#include <linux/module.h>
+
+struct rcar_drm_crtc;
+struct rcar_du_tconenc;
+
+#if IS_ENABLED(CONFIG_DRM_RCAR_TCON)
+int rcar_du_tconenc_init(struct rcar_du_device *rcdu);
+int rcar_du_tconenc_enable(struct rcar_du_tconenc *tcon,
+			   struct drm_crtc *crtc, bool enable);
+#else
+static inline int rcar_du_tconenc_init(struct rcar_du_device *rcdu)
+{
+	return 0;
+}
+static inline int rcar_du_tconenc_enable(struct rcar_du_tconenc *tcon,
+					 struct drm_crtc *crtc, bool enable)
+{
+	return 0;
+}
+#endif
+
+#endif /* __RCAR_DU_TCONENC_H__ */
Index: linux/drivers/gpu/drm/rcar-du/rcar_tcon_regs.h
===================================================================
--- /dev/null
+++ linux/drivers/gpu/drm/rcar-du/rcar_tcon_regs.h
@@ -0,0 +1,70 @@
+/*
+ * rcar_tcon_regs.h -- R-Car TCON Registers Definitions
+ *
+ * Copyright (C) 2015-2016 CogentEmbedded, Inc. <source@cogentembedded.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ */
+
+#ifndef __RCAR_TCON_REGS_H__
+#define __RCAR_TCON_REGS_H__
+
+#define OUT_V_LATCH		0x0000
+#define OUTCNT_VEN		(1 << 0)
+
+#define OUT_CLK_PHASE		0x0024
+#define OUTCNT_LCD_EDGE		(1 << 8)	/* If set, LCDOUT[23:0] are */
+						/* output on the falling edge */
+						/* of QCLK */
+
+#define TCON_V_LATCH		0x0280
+#define TCON_VEN		(1 << 0)
+
+#define TCON_TIM		0x0284
+
+/* Synced to VSYNC */
+#define TCON_TIM_STVA1		0x0288
+#define TCON_TIM_STVA2		0x028c
+#define TCON_TIM_STVB1		0x0290
+#define TCON_TIM_STVB2		0x0294
+
+/* Synced to HSYNC */
+#define TCON_TIM_STH1		0x0298
+#define TCON_TIM_STH2		0x029c
+#define TCON_TIM_STB1		0x02a0
+#define TCON_TIM_STB2		0x02a4
+#define TCON_TIM_CPV1		0x02a8
+#define TCON_TIM_CPV2		0x02ac
+#define TCON_TIM_POLA1		0x02b0
+#define TCON_TIM_POLA2		0x02b4
+#define TCON_TIM_POLB1		0x02b8
+#define TCON_TIM_POLB2		0x02bc
+#define TCON_TIM_DE		0x02c0
+
+/* Common definitions for all TCON_TIM_*1 registers */
+#define TCON_TIMING(start, width) (((start) << 16) | ((width) << 0))
+
+/* Common definitions for all TCON_TIM_*2 registers */
+#define TCON_INV		(1 << 4)
+/* Output signal select */
+#define TCON_SEL_STVA_VS	0
+#define TCON_SEL_STVB_VE	1
+#define TCON_SEL_STH_SP_HS	2
+#define TCON_SEL_STB_LP_HE	3
+#define TCON_SEL_CPV_GCK	4
+#define TCON_SEL_POLA		5
+#define TCON_SEL_POLB		6
+#define TCON_SEL_DE		7
+/* Definitions for HSYNC-related TIM registers */
+#define TCON_HS_SEL		(1 << 8)	/* If set, horizontal sync    */
+						/* signal reference after the */
+						/* offset */
+/* Polarity generation mode */
+#define TCON_MD_NORM		(0 << 12)
+#define TCON_MD_1X1		(1 << 12)
+#define TCON_MD_1X2		(2 << 12)
+#define TCON_MD_2X2		(3 << 12)
+
+#endif /* __RCAR_TCON_REGS_H__ */

[PATCH 3/4] rcar-du: add TCON connector driver

[Sergei Shtylyov]

Renesas  R-Car SoCs include  the timing controller (TCON) that can directly
drive LCDs. It receives the  H/VSYNC, etc. from the Display Unit (DU)  and
converts  them to the set of signals  that a LCD panel can understand (the
RBG signals are  effectively passed thru). Add the DRM connector driver for
the LCDs connected via the TCON interface...

Based on a large patch by Andrey Gusakov.

Signed-off-by: Andrey Gusakov <andrey.gusakov@cogentembedded.com>
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 drivers/gpu/drm/rcar-du/Makefile          |    1 
 drivers/gpu/drm/rcar-du/rcar_du_encoder.c |    5 +
 drivers/gpu/drm/rcar-du/rcar_du_tconcon.c |  125 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/rcar-du/rcar_du_tconcon.h |   23 +++++
 4 files changed, 154 insertions(+)

Index: linux/drivers/gpu/drm/rcar-du/Makefile
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/Makefile
+++ linux/drivers/gpu/drm/rcar-du/Makefile
@@ -4,6 +4,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
 		 rcar_du_group.o \
 		 rcar_du_kms.o \
 		 rcar_du_lvdscon.o \
+		 rcar_du_tconcon.o \
 		 rcar_du_plane.o \
 		 rcar_du_vgacon.o
 
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
===================================================================
--- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
@@ -24,6 +24,7 @@
 #include "rcar_du_kms.h"
 #include "rcar_du_lvdscon.h"
 #include "rcar_du_lvdsenc.h"
+#include "rcar_du_tconcon.h"
 #include "rcar_du_tconenc.h"
 #include "rcar_du_vgacon.h"
 
@@ -194,6 +195,10 @@ int rcar_du_encoder_init(struct rcar_du_
 		ret = rcar_du_vga_connector_init(rcdu, renc);
 		break;
 
+	case RCAR_DU_ENCODER_TCON:
+		ret = rcar_du_tcon_connector_init(rcdu, renc, con_node);
+		break;
+
 	case DRM_MODE_ENCODER_TMDS:
 		ret = rcar_du_hdmi_connector_init(rcdu, renc);
 		break;
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_tconcon.c
===================================================================
--- /dev/null
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_tconcon.c
@@ -0,0 +1,125 @@
+/*
+ * rcar_du_tconcon.c -- R-Car Display Unit TCON Connector
+ *
+ * Copyright (C) 2015-2016 Cogent Embedded, Inc. <source@cogentembedded.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_encoder_slave.h>
+
+#include <video/of_display_timing.h>
+#include <video/videomode.h>
+
+#include "rcar_du_drv.h"
+#include "rcar_du_encoder.h"
+#include "rcar_du_kms.h"
+#include "rcar_du_tconcon.h"
+
+struct rcar_du_tcon_connector {
+	struct rcar_du_connector connector;
+
+	struct {
+		unsigned int width_mm;		/* Panel width in mm */
+		unsigned int height_mm;		/* Panel height in mm */
+		struct videomode mode;
+	} panel;
+};
+
+#define to_rcar_tcon_connector(c) \
+	container_of(c, struct rcar_du_tcon_connector, connector.connector)
+
+static int rcar_du_tcon_connector_get_modes(struct drm_connector *connector)
+{
+	struct rcar_du_tcon_connector *tconcon =
+		to_rcar_tcon_connector(connector);
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_create(connector->dev);
+	if (mode == NULL)
+		return 0;
+
+	mode->type = DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_DRIVER;
+
+	drm_display_mode_from_videomode(&tconcon->panel.mode, mode);
+
+	drm_mode_set_name(mode);
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static const struct drm_connector_helper_funcs connector_helper_funcs = {
+	.get_modes = rcar_du_tcon_connector_get_modes,
+	.best_encoder = rcar_du_connector_best_encoder,
+};
+
+static enum drm_connector_status
+rcar_du_tcon_connector_detect(struct drm_connector *connector, bool force)
+{
+	return connector_status_connected;
+}
+
+static const struct drm_connector_funcs connector_funcs = {
+	.dpms = drm_atomic_helper_connector_dpms,
+	.reset = drm_atomic_helper_connector_reset,
+	.detect = rcar_du_tcon_connector_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+int rcar_du_tcon_connector_init(struct rcar_du_device *rcdu,
+				struct rcar_du_encoder *renc,
+				/* TODO const */ struct device_node *np)
+{
+	struct drm_encoder *encoder = rcar_encoder_to_drm_encoder(renc);
+	struct rcar_du_tcon_connector *tconcon;
+	struct drm_connector *connector;
+	struct display_timing timing;
+	int ret;
+
+	tconcon = devm_kzalloc(rcdu->dev, sizeof(*tconcon), GFP_KERNEL);
+	if (tconcon == NULL)
+		return -ENOMEM;
+
+	ret = of_get_display_timing(np, "panel-timing", &timing);
+	if (ret < 0)
+		return ret;
+
+	videomode_from_timing(&timing, &tconcon->panel.mode);
+
+	of_property_read_u32(np, "width-mm", &tconcon->panel.width_mm);
+	of_property_read_u32(np, "height-mm", &tconcon->panel.height_mm);
+
+	connector = &tconcon->connector.connector;
+	connector->display_info.width_mm = tconcon->panel.width_mm;
+	connector->display_info.height_mm = tconcon->panel.height_mm;
+
+	ret = drm_connector_init(rcdu->ddev, connector, &connector_funcs,
+				 DRM_MODE_CONNECTOR_TCON);
+	if (ret < 0)
+		return ret;
+
+	drm_connector_helper_add(connector, &connector_helper_funcs);
+
+	connector->dpms = DRM_MODE_DPMS_OFF;
+	drm_object_property_set_value(&connector->base,
+		rcdu->ddev->mode_config.dpms_property, DRM_MODE_DPMS_OFF);
+
+	ret = drm_mode_connector_attach_encoder(connector, encoder);
+	if (ret < 0)
+		return ret;
+
+	tconcon->connector.encoder = renc;
+
+	return 0;
+}
Index: linux/drivers/gpu/drm/rcar-du/rcar_du_tconcon.h
===================================================================
--- /dev/null
+++ linux/drivers/gpu/drm/rcar-du/rcar_du_tconcon.h
@@ -0,0 +1,23 @@
+/*
+ * rcar_du_tconcon.h -- R-Car Display Unit LVDS Connector
+ *
+ * Copyright (C) 2015-2016 CogentEmbedded, Inc. <source@cogentembedded.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __RCAR_DU_TCONCON_H__
+#define __RCAR_DU_TCONCON_H__
+
+struct rcar_du_device;
+struct rcar_du_encoder;
+struct rcar_du_panel_data;
+
+int rcar_du_tcon_connector_init(struct rcar_du_device *rcdu,
+				struct rcar_du_encoder *renc,
+				struct device_node *np);
+
+#endif /* __RCAR_DU_TCONCON_H__ */

[PATCH 4/4] rcar-du: add R8A7794 TCON support

[Sergei Shtylyov]

Now that we have the TCON encoder driver, we can start enabling TCON support
for the  R-Car SoCs. We have only tested the code on R8A7794 so far, so  let
it  be the first supported SoC...

Based on a large patch by Andrey Gusakov.

Signed-off-by: Andrey Gusakov <andrey.gusakov@cogentembedded.com>
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Index: renesas/drivers/gpu/drm/rcar-du/rcar_du_drv.c
===================================================================
--- renesas.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ renesas/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -116,9 +116,7 @@ static const struct rcar_du_device_info
 		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
 	.num_crtcs = 2,
 	.routes = {
-		/* R8A7794 has two RGB outputs and one (currently unsupported)
-		 * TCON output.
-		 */
+		/* R8A7794 has two RGB outputs and one TCON output. */
 		[RCAR_DU_OUTPUT_DPAD0] = {
 			.possible_crtcs = BIT(0),
 			.encoder_type = DRM_MODE_ENCODER_NONE,
@@ -129,8 +127,14 @@ static const struct rcar_du_device_info
 			.encoder_type = DRM_MODE_ENCODER_NONE,
 			.port = 1,
 		},
+		[RCAR_DU_OUTPUT_TCON] = {
+			.possible_crtcs = BIT(0),
+			.encoder_type = DRM_MODE_ENCODER_TCON,
+			.port = 2,
+		},
 	},
 	.num_lvds = 0,
+	.num_tcon = 1,
 };
 
 static const struct rcar_du_device_info rcar_du_r8a7795_info = {

Re: [PATCH 1/4] drm_mode: add TCON encoder/connector

[Daniel Vetter]

On Fri, Apr 29, 2016 at 12:02:15AM +0300, Sergei Shtylyov wrote:
> TCON (Timing  Controller) usually means  a chip that drives a LCD panel.
> In  our case, such controller  is a part of the Renesas R-Car SoCs. Add
> the TCON encoder/connector #define's  to be used by the TCON support code
> in the Renesas R-Car Display Unit (DU) driver.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
>  include/uapi/drm/drm_mode.h |    2 ++
>  1 file changed, 2 insertions(+)
> 
> Index: linux/include/uapi/drm/drm_mode.h
> ===================================================================
> --- linux.orig/include/uapi/drm/drm_mode.h
> +++ linux/include/uapi/drm/drm_mode.h
> @@ -202,6 +202,7 @@ struct drm_mode_get_plane_res {
>  #define DRM_MODE_ENCODER_VIRTUAL 5
>  #define DRM_MODE_ENCODER_DSI	6
>  #define DRM_MODE_ENCODER_DPMST	7
> +#define DRM_MODE_ENCODER_TCON	8
>  
>  struct drm_mode_get_encoder {
>  	__u32 encoder_id;
> @@ -241,6 +242,7 @@ struct drm_mode_get_encoder {
>  #define DRM_MODE_CONNECTOR_eDP		14
>  #define DRM_MODE_CONNECTOR_VIRTUAL      15
>  #define DRM_MODE_CONNECTOR_DSI		16
> +#define DRM_MODE_CONNECTOR_TCON		17
>  
>  struct drm_mode_get_connector {

The trouble with adding more here is that everytime you do this all the
userspace needs to be extended to print something reasonable for this type
of connector. For external connectors that makes sense (since users need
to know whether it's the hdmi or DP plug), but for internal panels I'd
honestly just go with lvds for anything that's not a more complex standard
like eDP or DSI.

Just my 2 cents really, not strong opinion, and we'll not run out of
connector type numbers anytime soon ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/4] drm_mode: add TCON encoder/connector

[Daniel Vetter]

On Fri, Apr 29, 2016 at 12:02:15AM +0300, Sergei Shtylyov wrote:
> TCON (Timing  Controller) usually means  a chip that drives a LCD panel.
> In  our case, such controller  is a part of the Renesas R-Car SoCs. Add
> the TCON encoder/connector #define's  to be used by the TCON support code
> in the Renesas R-Car Display Unit (DU) driver.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
>  include/uapi/drm/drm_mode.h |    2 ++
>  1 file changed, 2 insertions(+)
> 
> Index: linux/include/uapi/drm/drm_mode.h
> ===================================================================
> --- linux.orig/include/uapi/drm/drm_mode.h
> +++ linux/include/uapi/drm/drm_mode.h
> @@ -202,6 +202,7 @@ struct drm_mode_get_plane_res {
>  #define DRM_MODE_ENCODER_VIRTUAL 5
>  #define DRM_MODE_ENCODER_DSI	6
>  #define DRM_MODE_ENCODER_DPMST	7
> +#define DRM_MODE_ENCODER_TCON	8
>  
>  struct drm_mode_get_encoder {
>  	__u32 encoder_id;
> @@ -241,6 +242,7 @@ struct drm_mode_get_encoder {
>  #define DRM_MODE_CONNECTOR_eDP		14
>  #define DRM_MODE_CONNECTOR_VIRTUAL      15
>  #define DRM_MODE_CONNECTOR_DSI		16
> +#define DRM_MODE_CONNECTOR_TCON		17
>  
>  struct drm_mode_get_connector {

The trouble with adding more here is that everytime you do this all the
userspace needs to be extended to print something reasonable for this type
of connector. For external connectors that makes sense (since users need
to know whether it's the hdmi or DP plug), but for internal panels I'd
honestly just go with lvds for anything that's not a more complex standard
like eDP or DSI.

Just my 2 cents really, not strong opinion, and we'll not run out of
connector type numbers anytime soon ;-)
-Daniel
-- 
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

Re: [PATCH 2/4] rcar-du: add TCON encoder driver

[Laurent Pinchart]

Hi Sergei,

Thank you for the patch.

On Friday 29 Apr 2016 00:03:29 Sergei Shtylyov wrote:
> Renesas  R-Car SoCs  include the timing controller (TCON) that can directly
> drive LCDs. It receives  the H/VSYNC, etc. from the Display Unit (DU)  and
> converts  them to the set of signals  that a LCD panel can understand (the
> RBG  signals are effectively passed thru).  TCON has a set of registers
> that we need to  program based on the video mode timings, so we're adding
> a DU encoder driver doing that...
> 
> Based on a large patch by Andrey Gusakov.
> 
> Signed-off-by: Andrey Gusakov <andrey.gusakov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
>  drivers/gpu/drm/rcar-du/Kconfig           |    6
>  drivers/gpu/drm/rcar-du/Makefile          |    1
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h     |    4
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c |    9 +
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.h |    3
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c     |    4
>  drivers/gpu/drm/rcar-du/rcar_du_tconenc.c |  184 ++++++++++++++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_du_tconenc.h |   37 ++++++
>  drivers/gpu/drm/rcar-du/rcar_tcon_regs.h  |   70 +++++++++++
>  9 files changed, 318 insertions(+)
> 
> Index: linux/drivers/gpu/drm/rcar-du/Kconfig
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/Kconfig
> +++ linux/drivers/gpu/drm/rcar-du/Kconfig
> @@ -24,6 +24,12 @@ config DRM_RCAR_LVDS
>  	help
>  	  Enable support for the R-Car Display Unit embedded LVDS encoders.
> 
> +config DRM_RCAR_TCON
> +	bool "R-Car DU TCON Encoder Support"
> +	depends on DRM_RCAR_DU
> +	help
> +	  Enable support for the R-Car Display Unit embedded TCON encoders.
> +
>  config DRM_RCAR_VSP
>  	bool "R-Car DU VSP Compositor Support"
>  	depends on DRM_RCAR_DU
> Index: linux/drivers/gpu/drm/rcar-du/Makefile
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/Makefile
> +++ linux/drivers/gpu/drm/rcar-du/Makefile
> @@ -10,6 +10,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
>  rcar-du-drm-$(CONFIG_DRM_RCAR_HDMI)	+= rcar_du_hdmicon.o \
>  					   rcar_du_hdmienc.o
>  rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_lvdsenc.o
> +rcar-du-drm-$(CONFIG_DRM_RCAR_TCON)	+= rcar_du_tconenc.o
> 
>  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
> 
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -59,6 +59,7 @@ struct rcar_du_output_routing {
>   * @num_crtcs: total number of CRTCs
>   * @routes: array of CRTC to output routes, indexed by output
> (RCAR_DU_OUTPUT_*) * @num_lvds: number of internal LVDS encoders
> + * @num_tcon: number of internal TCON encoders
>   */
>  struct rcar_du_device_info {
>  	unsigned int gen;
> @@ -67,11 +68,13 @@ struct rcar_du_device_info {
>  	unsigned int num_crtcs;
>  	struct rcar_du_output_routing routes[RCAR_DU_OUTPUT_MAX];
>  	unsigned int num_lvds;
> +	unsigned int num_tcon;
>  };
> 
>  #define RCAR_DU_MAX_CRTCS		4
>  #define RCAR_DU_MAX_GROUPS		DIV_ROUND_UP(RCAR_DU_MAX_CRTCS, 2)
>  #define RCAR_DU_MAX_LVDS		2
> +#define RCAR_DU_MAX_TCON		1
>  #define RCAR_DU_MAX_VSPS		4
> 
>  struct rcar_du_device {
> @@ -99,6 +102,7 @@ struct rcar_du_device {
>  	unsigned int vspd1_sink;
> 
>  	struct rcar_du_lvdsenc *lvds[RCAR_DU_MAX_LVDS];
> +	struct rcar_du_tconenc *tcon[RCAR_DU_MAX_TCON];
> 
>  	struct {
>  		wait_queue_head_t wait;
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> @@ -24,6 +24,7 @@
>  #include "rcar_du_kms.h"
>  #include "rcar_du_lvdscon.h"
>  #include "rcar_du_lvdsenc.h"
> +#include "rcar_du_tconenc.h"
>  #include "rcar_du_vgacon.h"
> 
>  /* ------------------------------------------------------------------------
> @@ -48,6 +49,8 @@ static void rcar_du_encoder_disable(stru
> 
>  	if (renc->lvds)
>  		rcar_du_lvdsenc_enable(renc->lvds, encoder->crtc, false);
> +	if (renc->tcon)
> +		rcar_du_tconenc_enable(renc->tcon, encoder->crtc, false);
>  }
> 
>  static void rcar_du_encoder_enable(struct drm_encoder *encoder)
> @@ -56,6 +59,8 @@ static void rcar_du_encoder_enable(struc
> 
>  	if (renc->lvds)
>  		rcar_du_lvdsenc_enable(renc->lvds, encoder->crtc, true);
> +	if (renc->tcon)
> +		rcar_du_tconenc_enable(renc->tcon, encoder->crtc, true);
>  }
> 
>  static int rcar_du_encoder_atomic_check(struct drm_encoder *encoder,
> @@ -142,6 +147,10 @@ int rcar_du_encoder_init(struct rcar_du_
>  		renc->lvds = rcdu->lvds[1];
>  		break;
> 
> +	case RCAR_DU_OUTPUT_TCON:
> +		renc->tcon = rcdu->tcon[0];
> +		break;
> +
>  	default:
>  		break;
>  	}
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
> @@ -20,6 +20,7 @@
>  struct rcar_du_device;
>  struct rcar_du_hdmienc;
>  struct rcar_du_lvdsenc;
> +struct rcar_du_tconenc;
> 
>  enum rcar_du_encoder_type {
>  	RCAR_DU_ENCODER_UNUSED = 0,
> @@ -27,6 +28,7 @@ enum rcar_du_encoder_type {
>  	RCAR_DU_ENCODER_VGA,
>  	RCAR_DU_ENCODER_LVDS,
>  	RCAR_DU_ENCODER_HDMI,
> +	RCAR_DU_ENCODER_TCON,
>  };
> 
>  struct rcar_du_encoder {
> @@ -34,6 +36,7 @@ struct rcar_du_encoder {
>  	enum rcar_du_output output;
>  	struct rcar_du_hdmienc *hdmi;
>  	struct rcar_du_lvdsenc *lvds;
> +	struct rcar_du_tconenc *tcon;
>  };
> 
>  #define to_rcar_encoder(e) \
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -27,6 +27,7 @@
>  #include "rcar_du_encoder.h"
>  #include "rcar_du_kms.h"
>  #include "rcar_du_lvdsenc.h"
> +#include "rcar_du_tconenc.h"
>  #include "rcar_du_regs.h"
>  #include "rcar_du_vsp.h"
> 
> @@ -619,6 +620,9 @@ int rcar_du_modeset_init(struct rcar_du_
>  	ret = rcar_du_lvdsenc_init(rcdu);
>  	if (ret < 0)
>  		return ret;
> +	ret = rcar_du_tconenc_init(rcdu);
> +	if (ret < 0)
> +		return ret;
> 
>  	ret = rcar_du_encoders_init(rcdu);
>  	if (ret < 0)
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_tconenc.c
> ===================================================================
> --- /dev/null
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_tconenc.c
> @@ -0,0 +1,184 @@
> +/*
> + * rcar_du_tconenc.c -- R-Car Display Unit TCON Encoder
> + *
> + * Copyright (C) 2015-2016 Cogent Embedded, Inc.
> <source@cogentembedded.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <drm/drm_encoder_slave.h>
> +
> +#include "rcar_du_drv.h"
> +#include "rcar_du_encoder.h"
> +#include "rcar_du_tconenc.h"
> +#include "rcar_tcon_regs.h"
> +
> +struct rcar_du_tconenc {
> +	struct rcar_du_device *dev;
> +
> +	unsigned int index;
> +	void __iomem *mmio;
> +	struct clk *clock;
> +	bool enabled;
> +};
> +
> +static void rcar_tcon_write(struct rcar_du_tconenc *tcon, u32 reg, u32
> data)
> +{
> +	iowrite32(data, tcon->mmio + reg);
> +}
> +
> +static int rcar_du_tconenc_start(struct rcar_du_tconenc *tcon,
> +				 struct rcar_du_crtc *rcrtc)
> +{
> +	const struct drm_display_mode *mode = &rcrtc->crtc.mode;
> +	int ret;
> +
> +	if (tcon->enabled)

Now that the DU driver implements the atomic API the DRM/KMS core is supposed 
to only enable/disable CRTCs and encoders when needed. Can this still happen ? 
Same comment for the stop function.

> +		return 0;
> +
> +	ret = clk_prepare_enable(tcon->clock);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Update */
> +	rcar_tcon_write(tcon, OUT_V_LATCH, OUTCNT_VEN);
> +	rcar_tcon_write(tcon, TCON_V_LATCH, TCON_VEN);

Aren't you supposed to set those bits after modifying the registers only, not 
before ?

> +	/* Signals:
> +	 * R-Car	Display
> +	 * QCLK		SSC (Source Driver Clock Input)
> +	 * QSTH		SSP (Source Scanning Signal Left/Right)
> +	 * QSTB		SOE (Source Driver Output Enable)
> +	 * QCPV		GOE (Gate Driver Output Enable)
> +	 * QPOLA	POL (Polarity Control Signal)
> +	 * QPOLB	GSC (Gate Driver Scanning Clock)
> +	 * QSTVA	GSP (Gate Scanning Start Signal Up/Down)
> +	 * QSTVB	nope
> +	 */
> +	/* Setup timings */
> +	rcar_tcon_write(tcon, TCON_TIM, TCON_TIMING(50, 0));
> +	/* Horizontal timings */
> +	rcar_tcon_write(tcon, TCON_TIM_STH1, TCON_TIMING(mode->htotal -
> +							 mode->hsync_start - 1,
> +							 1));
> +	rcar_tcon_write(tcon, TCON_TIM_STH2, TCON_SEL_STH_SP_HS);
> +	rcar_tcon_write(tcon, TCON_TIM_STB1, TCON_TIMING(mode->htotal -
> +							 mode->hsync_start +
> +							 mode->hdisplay + 6,
> +							 3));
> +	rcar_tcon_write(tcon, TCON_TIM_STB2, TCON_SEL_STB_LP_HE);
> +	rcar_tcon_write(tcon, TCON_TIM_POLB1,
> +			TCON_TIMING(mode->htotal - mode->hsync_start +
> +				    mode->hdisplay - 8, mode->htotal / 2));
> +	rcar_tcon_write(tcon, TCON_TIM_POLB2,
> +			TCON_SEL_POLB | TCON_INV | TCON_MD_NORM);
> +	rcar_tcon_write(tcon, TCON_TIM_CPV1,
> +			TCON_TIMING(mode->htotal - mode->hsync_start +
> +				    mode->hdisplay - 33, 50));
> +	rcar_tcon_write(tcon, TCON_TIM_CPV2, TCON_SEL_CPV_GCK);
> +	rcar_tcon_write(tcon, TCON_TIM_POLA1,
> +			TCON_TIMING(mode->htotal - mode->hsync_start +
> +				    mode->hdisplay + 1, 39));
> +	rcar_tcon_write(tcon, TCON_TIM_POLA2,
> +			TCON_SEL_POLA | TCON_INV | TCON_MD_1X1);
> +
> +	/* Vertical timings */
> +	rcar_tcon_write(tcon, TCON_TIM_STVA1,
> +			TCON_TIMING(mode->vtotal - mode->vsync_start, 1));
> +	rcar_tcon_write(tcon, TCON_TIM_STVA2, TCON_SEL_STVA_VS);
> +	rcar_tcon_write(tcon, TCON_TIM_STVB1, TCON_TIMING(0, 0));
> +	rcar_tcon_write(tcon, TCON_TIM_STVB2, TCON_SEL_STVB_VE);
> +
> +	/* Data clocked out on the falling edge of QCLK, latched in LCD on
> +	 * the rising edge
> +	 */
> +	rcar_tcon_write(tcon, OUT_CLK_PHASE, OUTCNT_LCD_EDGE);

I can't verify all those settings as I have no idea how TCON operates. 
However, it looks like we have lots of magic numbers, and I have a feeling 
those actually depend on the panel model. Am I wrong ?

> +	/* Update */
> +	rcar_tcon_write(tcon, OUT_V_LATCH, OUTCNT_VEN);
> +	rcar_tcon_write(tcon, TCON_V_LATCH, TCON_VEN);
> +
> +	tcon->enabled = true;
> +
> +	return 0;
> +}
> +
> +static void rcar_du_tconenc_stop(struct rcar_du_tconenc *tcon)
> +{
> +	if (!tcon->enabled)
> +		return;
> +
> +	clk_disable_unprepare(tcon->clock);
> +
> +	tcon->enabled = false;
> +}
> +
> +int rcar_du_tconenc_enable(struct rcar_du_tconenc *tcon, struct drm_crtc
> *crtc,
> +			   bool enable)
> +{
> +	if (!enable) {
> +		rcar_du_tconenc_stop(tcon);
> +		return 0;
> +	} else if (crtc) {
> +		struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> +
> +		return rcar_du_tconenc_start(tcon, rcrtc);
> +	} else
> +		return -EINVAL;

Missing { } around the last branch. Is this needed though, can enable be true 
and crtc NULL ? If so, under which circumstances ?

> +}
> +
> +static int rcar_du_tconenc_get_resources(struct rcar_du_tconenc *tcon,
> +					 struct platform_device *pdev)
> +{
> +	struct resource *mem;
> +	char name[7];
> +
> +	sprintf(name, "tcon.%u", tcon->index);
> +
> +	mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
> +	tcon->mmio = devm_ioremap_resource(&pdev->dev, mem);
> +	if (IS_ERR(tcon->mmio))
> +		return PTR_ERR(tcon->mmio);
> +
> +	tcon->clock = devm_clk_get(&pdev->dev, name);
> +	if (IS_ERR(tcon->clock)) {
> +		dev_err(&pdev->dev, "failed to get clock for %s\n", name);
> +		return PTR_ERR(tcon->clock);
> +	}

I wasn't very proud of my very similar implementation of the LVDS encoder 
code. It's a separate IP core, it should have been modeled by a separate DT 
node. I can't really ask you to do so for TCON given that I haven't done it 
for LVDS though, but I suspect we'll pay the price at some point (for instance 
when we'll have an SoC with the DU and TCON in separate power domains). If you 
have a clever idea to enhance this, please let me know.

> +	return 0;
> +}
> +
> +int rcar_du_tconenc_init(struct rcar_du_device *rcdu)
> +{
> +	struct platform_device *pdev = to_platform_device(rcdu->dev);
> +	struct rcar_du_tconenc *tcon;
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; i < rcdu->info->num_tcon; ++i) {
> +		tcon = devm_kzalloc(&pdev->dev, sizeof(*tcon), GFP_KERNEL);
> +		if (tcon == NULL)
> +			return -ENOMEM;
> +
> +		tcon->dev = rcdu;
> +		tcon->index = i;
> +		tcon->enabled = false;
> +
> +		ret = rcar_du_tconenc_get_resources(tcon, pdev);
> +		if (ret < 0)
> +			return ret;
> +
> +		rcdu->tcon[i] = tcon;
> +	}
> +
> +	return 0;
> +}
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_tconenc.h
> ===================================================================
> --- /dev/null
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_tconenc.h
> @@ -0,0 +1,37 @@
> +/*
> + * rcar_du_tconenc.h -- R-Car Display Unit TCON Encoder
> + *
> + * Copyright (C) 2015-2016 CogentEmbedded, Inc. <source@cogentembedded.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef __RCAR_DU_TCONENC_H__
> +#define __RCAR_DU_TCONENC_H__
> +
> +#include <linux/io.h>
> +#include <linux/module.h>
> +
> +struct rcar_drm_crtc;
> +struct rcar_du_tconenc;
> +
> +#if IS_ENABLED(CONFIG_DRM_RCAR_TCON)
> +int rcar_du_tconenc_init(struct rcar_du_device *rcdu);
> +int rcar_du_tconenc_enable(struct rcar_du_tconenc *tcon,
> +			   struct drm_crtc *crtc, bool enable);
> +#else
> +static inline int rcar_du_tconenc_init(struct rcar_du_device *rcdu)
> +{
> +	return 0;
> +}
> +static inline int rcar_du_tconenc_enable(struct rcar_du_tconenc *tcon,
> +					 struct drm_crtc *crtc, bool enable)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#endif /* __RCAR_DU_TCONENC_H__ */
> Index: linux/drivers/gpu/drm/rcar-du/rcar_tcon_regs.h
> ===================================================================
> --- /dev/null
> +++ linux/drivers/gpu/drm/rcar-du/rcar_tcon_regs.h
> @@ -0,0 +1,70 @@
> +/*
> + * rcar_tcon_regs.h -- R-Car TCON Registers Definitions
> + *
> + * Copyright (C) 2015-2016 CogentEmbedded, Inc. <source@cogentembedded.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + */
> +
> +#ifndef __RCAR_TCON_REGS_H__
> +#define __RCAR_TCON_REGS_H__
> +
> +#define OUT_V_LATCH		0x0000
> +#define OUTCNT_VEN		(1 << 0)
> +
> +#define OUT_CLK_PHASE		0x0024
> +#define OUTCNT_LCD_EDGE		(1 << 8)	/* If set, LCDOUT[23:0] are */
> +						/* output on the falling edge */
> +						/* of QCLK */
> +
> +#define TCON_V_LATCH		0x0280
> +#define TCON_VEN		(1 << 0)
> +
> +#define TCON_TIM		0x0284
> +
> +/* Synced to VSYNC */
> +#define TCON_TIM_STVA1		0x0288
> +#define TCON_TIM_STVA2		0x028c
> +#define TCON_TIM_STVB1		0x0290
> +#define TCON_TIM_STVB2		0x0294
> +
> +/* Synced to HSYNC */
> +#define TCON_TIM_STH1		0x0298
> +#define TCON_TIM_STH2		0x029c
> +#define TCON_TIM_STB1		0x02a0
> +#define TCON_TIM_STB2		0x02a4
> +#define TCON_TIM_CPV1		0x02a8
> +#define TCON_TIM_CPV2		0x02ac
> +#define TCON_TIM_POLA1		0x02b0
> +#define TCON_TIM_POLA2		0x02b4
> +#define TCON_TIM_POLB1		0x02b8
> +#define TCON_TIM_POLB2		0x02bc
> +#define TCON_TIM_DE		0x02c0

For the sake of completeness, could you define the TCON_DE_INV bit ?

> +
> +/* Common definitions for all TCON_TIM_*1 registers */
> +#define TCON_TIMING(start, width) (((start) << 16) | ((width) << 0))
> +
> +/* Common definitions for all TCON_TIM_*2 registers */
> +#define TCON_INV		(1 << 4)
> +/* Output signal select */
> +#define TCON_SEL_STVA_VS	0

Could you write this as (0 << 0) (and some for the other bits below) to make 
it clearer that those are bit values ?

> +#define TCON_SEL_STVB_VE	1
> +#define TCON_SEL_STH_SP_HS	2
> +#define TCON_SEL_STB_LP_HE	3
> +#define TCON_SEL_CPV_GCK	4
> +#define TCON_SEL_POLA		5
> +#define TCON_SEL_POLB		6
> +#define TCON_SEL_DE		7

I'd add a blank line here.

> +/* Definitions for HSYNC-related TIM registers */

I'd list the registers explicitly here, or would add a short comment after 
each of them above to tell what they control.

> +#define TCON_HS_SEL		(1 << 8)	/* If set, horizontal sync    */
> +						/* signal reference after the */
> +						/* offset */

And a blank linke here too.

> +/* Polarity generation mode */

Could you mention that this is applicable to POLA2 and POLB2 only ?

> +#define TCON_MD_NORM		(0 << 12)
> +#define TCON_MD_1X1		(1 << 12)
> +#define TCON_MD_1X2		(2 << 12)
> +#define TCON_MD_2X2		(3 << 12)
> +
> +#endif /* __RCAR_TCON_REGS_H__ */

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] rcar-du: add TCON encoder driver

[Laurent Pinchart]

Hi Sergei,

Thank you for the patch.

On Friday 29 Apr 2016 00:03:29 Sergei Shtylyov wrote:
> Renesas  R-Car SoCs  include the timing controller (TCON) that can directly
> drive LCDs. It receives  the H/VSYNC, etc. from the Display Unit (DU)  and
> converts  them to the set of signals  that a LCD panel can understand (the
> RBG  signals are effectively passed thru).  TCON has a set of registers
> that we need to  program based on the video mode timings, so we're adding
> a DU encoder driver doing that...
> 
> Based on a large patch by Andrey Gusakov.
> 
> Signed-off-by: Andrey Gusakov <andrey.gusakov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
>  drivers/gpu/drm/rcar-du/Kconfig           |    6
>  drivers/gpu/drm/rcar-du/Makefile          |    1
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h     |    4
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.c |    9 +
>  drivers/gpu/drm/rcar-du/rcar_du_encoder.h |    3
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c     |    4
>  drivers/gpu/drm/rcar-du/rcar_du_tconenc.c |  184 ++++++++++++++++++++++++++
>  drivers/gpu/drm/rcar-du/rcar_du_tconenc.h |   37 ++++++
>  drivers/gpu/drm/rcar-du/rcar_tcon_regs.h  |   70 +++++++++++
>  9 files changed, 318 insertions(+)
> 
> Index: linux/drivers/gpu/drm/rcar-du/Kconfig
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/Kconfig
> +++ linux/drivers/gpu/drm/rcar-du/Kconfig
> @@ -24,6 +24,12 @@ config DRM_RCAR_LVDS
>  	help
>  	  Enable support for the R-Car Display Unit embedded LVDS encoders.
> 
> +config DRM_RCAR_TCON
> +	bool "R-Car DU TCON Encoder Support"
> +	depends on DRM_RCAR_DU
> +	help
> +	  Enable support for the R-Car Display Unit embedded TCON encoders.
> +
>  config DRM_RCAR_VSP
>  	bool "R-Car DU VSP Compositor Support"
>  	depends on DRM_RCAR_DU
> Index: linux/drivers/gpu/drm/rcar-du/Makefile
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/Makefile
> +++ linux/drivers/gpu/drm/rcar-du/Makefile
> @@ -10,6 +10,7 @@ rcar-du-drm-y := rcar_du_crtc.o \
>  rcar-du-drm-$(CONFIG_DRM_RCAR_HDMI)	+= rcar_du_hdmicon.o \
>  					   rcar_du_hdmienc.o
>  rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS)	+= rcar_du_lvdsenc.o
> +rcar-du-drm-$(CONFIG_DRM_RCAR_TCON)	+= rcar_du_tconenc.o
> 
>  rcar-du-drm-$(CONFIG_DRM_RCAR_VSP)	+= rcar_du_vsp.o
> 
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -59,6 +59,7 @@ struct rcar_du_output_routing {
>   * @num_crtcs: total number of CRTCs
>   * @routes: array of CRTC to output routes, indexed by output
> (RCAR_DU_OUTPUT_*) * @num_lvds: number of internal LVDS encoders
> + * @num_tcon: number of internal TCON encoders
>   */
>  struct rcar_du_device_info {
>  	unsigned int gen;
> @@ -67,11 +68,13 @@ struct rcar_du_device_info {
>  	unsigned int num_crtcs;
>  	struct rcar_du_output_routing routes[RCAR_DU_OUTPUT_MAX];
>  	unsigned int num_lvds;
> +	unsigned int num_tcon;
>  };
> 
>  #define RCAR_DU_MAX_CRTCS		4
>  #define RCAR_DU_MAX_GROUPS		DIV_ROUND_UP(RCAR_DU_MAX_CRTCS, 2)
>  #define RCAR_DU_MAX_LVDS		2
> +#define RCAR_DU_MAX_TCON		1
>  #define RCAR_DU_MAX_VSPS		4
> 
>  struct rcar_du_device {
> @@ -99,6 +102,7 @@ struct rcar_du_device {
>  	unsigned int vspd1_sink;
> 
>  	struct rcar_du_lvdsenc *lvds[RCAR_DU_MAX_LVDS];
> +	struct rcar_du_tconenc *tcon[RCAR_DU_MAX_TCON];
> 
>  	struct {
>  		wait_queue_head_t wait;
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> @@ -24,6 +24,7 @@
>  #include "rcar_du_kms.h"
>  #include "rcar_du_lvdscon.h"
>  #include "rcar_du_lvdsenc.h"
> +#include "rcar_du_tconenc.h"
>  #include "rcar_du_vgacon.h"
> 
>  /* ------------------------------------------------------------------------
> @@ -48,6 +49,8 @@ static void rcar_du_encoder_disable(stru
> 
>  	if (renc->lvds)
>  		rcar_du_lvdsenc_enable(renc->lvds, encoder->crtc, false);
> +	if (renc->tcon)
> +		rcar_du_tconenc_enable(renc->tcon, encoder->crtc, false);
>  }
> 
>  static void rcar_du_encoder_enable(struct drm_encoder *encoder)
> @@ -56,6 +59,8 @@ static void rcar_du_encoder_enable(struc
> 
>  	if (renc->lvds)
>  		rcar_du_lvdsenc_enable(renc->lvds, encoder->crtc, true);
> +	if (renc->tcon)
> +		rcar_du_tconenc_enable(renc->tcon, encoder->crtc, true);
>  }
> 
>  static int rcar_du_encoder_atomic_check(struct drm_encoder *encoder,
> @@ -142,6 +147,10 @@ int rcar_du_encoder_init(struct rcar_du_
>  		renc->lvds = rcdu->lvds[1];
>  		break;
> 
> +	case RCAR_DU_OUTPUT_TCON:
> +		renc->tcon = rcdu->tcon[0];
> +		break;
> +
>  	default:
>  		break;
>  	}
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
> @@ -20,6 +20,7 @@
>  struct rcar_du_device;
>  struct rcar_du_hdmienc;
>  struct rcar_du_lvdsenc;
> +struct rcar_du_tconenc;
> 
>  enum rcar_du_encoder_type {
>  	RCAR_DU_ENCODER_UNUSED = 0,
> @@ -27,6 +28,7 @@ enum rcar_du_encoder_type {
>  	RCAR_DU_ENCODER_VGA,
>  	RCAR_DU_ENCODER_LVDS,
>  	RCAR_DU_ENCODER_HDMI,
> +	RCAR_DU_ENCODER_TCON,
>  };
> 
>  struct rcar_du_encoder {
> @@ -34,6 +36,7 @@ struct rcar_du_encoder {
>  	enum rcar_du_output output;
>  	struct rcar_du_hdmienc *hdmi;
>  	struct rcar_du_lvdsenc *lvds;
> +	struct rcar_du_tconenc *tcon;
>  };
> 
>  #define to_rcar_encoder(e) \
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> ===================================================================
> --- linux.orig/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -27,6 +27,7 @@
>  #include "rcar_du_encoder.h"
>  #include "rcar_du_kms.h"
>  #include "rcar_du_lvdsenc.h"
> +#include "rcar_du_tconenc.h"
>  #include "rcar_du_regs.h"
>  #include "rcar_du_vsp.h"
> 
> @@ -619,6 +620,9 @@ int rcar_du_modeset_init(struct rcar_du_
>  	ret = rcar_du_lvdsenc_init(rcdu);
>  	if (ret < 0)
>  		return ret;
> +	ret = rcar_du_tconenc_init(rcdu);
> +	if (ret < 0)
> +		return ret;
> 
>  	ret = rcar_du_encoders_init(rcdu);
>  	if (ret < 0)
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_tconenc.c
> ===================================================================
> --- /dev/null
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_tconenc.c
> @@ -0,0 +1,184 @@
> +/*
> + * rcar_du_tconenc.c -- R-Car Display Unit TCON Encoder
> + *
> + * Copyright (C) 2015-2016 Cogent Embedded, Inc.
> <source@cogentembedded.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <drm/drm_encoder_slave.h>
> +
> +#include "rcar_du_drv.h"
> +#include "rcar_du_encoder.h"
> +#include "rcar_du_tconenc.h"
> +#include "rcar_tcon_regs.h"
> +
> +struct rcar_du_tconenc {
> +	struct rcar_du_device *dev;
> +
> +	unsigned int index;
> +	void __iomem *mmio;
> +	struct clk *clock;
> +	bool enabled;
> +};
> +
> +static void rcar_tcon_write(struct rcar_du_tconenc *tcon, u32 reg, u32
> data)
> +{
> +	iowrite32(data, tcon->mmio + reg);
> +}
> +
> +static int rcar_du_tconenc_start(struct rcar_du_tconenc *tcon,
> +				 struct rcar_du_crtc *rcrtc)
> +{
> +	const struct drm_display_mode *mode = &rcrtc->crtc.mode;
> +	int ret;
> +
> +	if (tcon->enabled)

Now that the DU driver implements the atomic API the DRM/KMS core is supposed 
to only enable/disable CRTCs and encoders when needed. Can this still happen ? 
Same comment for the stop function.

> +		return 0;
> +
> +	ret = clk_prepare_enable(tcon->clock);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Update */
> +	rcar_tcon_write(tcon, OUT_V_LATCH, OUTCNT_VEN);
> +	rcar_tcon_write(tcon, TCON_V_LATCH, TCON_VEN);

Aren't you supposed to set those bits after modifying the registers only, not 
before ?

> +	/* Signals:
> +	 * R-Car	Display
> +	 * QCLK		SSC (Source Driver Clock Input)
> +	 * QSTH		SSP (Source Scanning Signal Left/Right)
> +	 * QSTB		SOE (Source Driver Output Enable)
> +	 * QCPV		GOE (Gate Driver Output Enable)
> +	 * QPOLA	POL (Polarity Control Signal)
> +	 * QPOLB	GSC (Gate Driver Scanning Clock)
> +	 * QSTVA	GSP (Gate Scanning Start Signal Up/Down)
> +	 * QSTVB	nope
> +	 */
> +	/* Setup timings */
> +	rcar_tcon_write(tcon, TCON_TIM, TCON_TIMING(50, 0));
> +	/* Horizontal timings */
> +	rcar_tcon_write(tcon, TCON_TIM_STH1, TCON_TIMING(mode->htotal -
> +							 mode->hsync_start - 1,
> +							 1));
> +	rcar_tcon_write(tcon, TCON_TIM_STH2, TCON_SEL_STH_SP_HS);
> +	rcar_tcon_write(tcon, TCON_TIM_STB1, TCON_TIMING(mode->htotal -
> +							 mode->hsync_start +
> +							 mode->hdisplay + 6,
> +							 3));
> +	rcar_tcon_write(tcon, TCON_TIM_STB2, TCON_SEL_STB_LP_HE);
> +	rcar_tcon_write(tcon, TCON_TIM_POLB1,
> +			TCON_TIMING(mode->htotal - mode->hsync_start +
> +				    mode->hdisplay - 8, mode->htotal / 2));
> +	rcar_tcon_write(tcon, TCON_TIM_POLB2,
> +			TCON_SEL_POLB | TCON_INV | TCON_MD_NORM);
> +	rcar_tcon_write(tcon, TCON_TIM_CPV1,
> +			TCON_TIMING(mode->htotal - mode->hsync_start +
> +				    mode->hdisplay - 33, 50));
> +	rcar_tcon_write(tcon, TCON_TIM_CPV2, TCON_SEL_CPV_GCK);
> +	rcar_tcon_write(tcon, TCON_TIM_POLA1,
> +			TCON_TIMING(mode->htotal - mode->hsync_start +
> +				    mode->hdisplay + 1, 39));
> +	rcar_tcon_write(tcon, TCON_TIM_POLA2,
> +			TCON_SEL_POLA | TCON_INV | TCON_MD_1X1);
> +
> +	/* Vertical timings */
> +	rcar_tcon_write(tcon, TCON_TIM_STVA1,
> +			TCON_TIMING(mode->vtotal - mode->vsync_start, 1));
> +	rcar_tcon_write(tcon, TCON_TIM_STVA2, TCON_SEL_STVA_VS);
> +	rcar_tcon_write(tcon, TCON_TIM_STVB1, TCON_TIMING(0, 0));
> +	rcar_tcon_write(tcon, TCON_TIM_STVB2, TCON_SEL_STVB_VE);
> +
> +	/* Data clocked out on the falling edge of QCLK, latched in LCD on
> +	 * the rising edge
> +	 */
> +	rcar_tcon_write(tcon, OUT_CLK_PHASE, OUTCNT_LCD_EDGE);

I can't verify all those settings as I have no idea how TCON operates. 
However, it looks like we have lots of magic numbers, and I have a feeling 
those actually depend on the panel model. Am I wrong ?

> +	/* Update */
> +	rcar_tcon_write(tcon, OUT_V_LATCH, OUTCNT_VEN);
> +	rcar_tcon_write(tcon, TCON_V_LATCH, TCON_VEN);
> +
> +	tcon->enabled = true;
> +
> +	return 0;
> +}
> +
> +static void rcar_du_tconenc_stop(struct rcar_du_tconenc *tcon)
> +{
> +	if (!tcon->enabled)
> +		return;
> +
> +	clk_disable_unprepare(tcon->clock);
> +
> +	tcon->enabled = false;
> +}
> +
> +int rcar_du_tconenc_enable(struct rcar_du_tconenc *tcon, struct drm_crtc
> *crtc,
> +			   bool enable)
> +{
> +	if (!enable) {
> +		rcar_du_tconenc_stop(tcon);
> +		return 0;
> +	} else if (crtc) {
> +		struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> +
> +		return rcar_du_tconenc_start(tcon, rcrtc);
> +	} else
> +		return -EINVAL;

Missing { } around the last branch. Is this needed though, can enable be true 
and crtc NULL ? If so, under which circumstances ?

> +}
> +
> +static int rcar_du_tconenc_get_resources(struct rcar_du_tconenc *tcon,
> +					 struct platform_device *pdev)
> +{
> +	struct resource *mem;
> +	char name[7];
> +
> +	sprintf(name, "tcon.%u", tcon->index);
> +
> +	mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
> +	tcon->mmio = devm_ioremap_resource(&pdev->dev, mem);
> +	if (IS_ERR(tcon->mmio))
> +		return PTR_ERR(tcon->mmio);
> +
> +	tcon->clock = devm_clk_get(&pdev->dev, name);
> +	if (IS_ERR(tcon->clock)) {
> +		dev_err(&pdev->dev, "failed to get clock for %s\n", name);
> +		return PTR_ERR(tcon->clock);
> +	}

I wasn't very proud of my very similar implementation of the LVDS encoder 
code. It's a separate IP core, it should have been modeled by a separate DT 
node. I can't really ask you to do so for TCON given that I haven't done it 
for LVDS though, but I suspect we'll pay the price at some point (for instance 
when we'll have an SoC with the DU and TCON in separate power domains). If you 
have a clever idea to enhance this, please let me know.

> +	return 0;
> +}
> +
> +int rcar_du_tconenc_init(struct rcar_du_device *rcdu)
> +{
> +	struct platform_device *pdev = to_platform_device(rcdu->dev);
> +	struct rcar_du_tconenc *tcon;
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; i < rcdu->info->num_tcon; ++i) {
> +		tcon = devm_kzalloc(&pdev->dev, sizeof(*tcon), GFP_KERNEL);
> +		if (tcon == NULL)
> +			return -ENOMEM;
> +
> +		tcon->dev = rcdu;
> +		tcon->index = i;
> +		tcon->enabled = false;
> +
> +		ret = rcar_du_tconenc_get_resources(tcon, pdev);
> +		if (ret < 0)
> +			return ret;
> +
> +		rcdu->tcon[i] = tcon;
> +	}
> +
> +	return 0;
> +}
> Index: linux/drivers/gpu/drm/rcar-du/rcar_du_tconenc.h
> ===================================================================
> --- /dev/null
> +++ linux/drivers/gpu/drm/rcar-du/rcar_du_tconenc.h
> @@ -0,0 +1,37 @@
> +/*
> + * rcar_du_tconenc.h -- R-Car Display Unit TCON Encoder
> + *
> + * Copyright (C) 2015-2016 CogentEmbedded, Inc. <source@cogentembedded.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#ifndef __RCAR_DU_TCONENC_H__
> +#define __RCAR_DU_TCONENC_H__
> +
> +#include <linux/io.h>
> +#include <linux/module.h>
> +
> +struct rcar_drm_crtc;
> +struct rcar_du_tconenc;
> +
> +#if IS_ENABLED(CONFIG_DRM_RCAR_TCON)
> +int rcar_du_tconenc_init(struct rcar_du_device *rcdu);
> +int rcar_du_tconenc_enable(struct rcar_du_tconenc *tcon,
> +			   struct drm_crtc *crtc, bool enable);
> +#else
> +static inline int rcar_du_tconenc_init(struct rcar_du_device *rcdu)
> +{
> +	return 0;
> +}
> +static inline int rcar_du_tconenc_enable(struct rcar_du_tconenc *tcon,
> +					 struct drm_crtc *crtc, bool enable)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#endif /* __RCAR_DU_TCONENC_H__ */
> Index: linux/drivers/gpu/drm/rcar-du/rcar_tcon_regs.h
> ===================================================================
> --- /dev/null
> +++ linux/drivers/gpu/drm/rcar-du/rcar_tcon_regs.h
> @@ -0,0 +1,70 @@
> +/*
> + * rcar_tcon_regs.h -- R-Car TCON Registers Definitions
> + *
> + * Copyright (C) 2015-2016 CogentEmbedded, Inc. <source@cogentembedded.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + */
> +
> +#ifndef __RCAR_TCON_REGS_H__
> +#define __RCAR_TCON_REGS_H__
> +
> +#define OUT_V_LATCH		0x0000
> +#define OUTCNT_VEN		(1 << 0)
> +
> +#define OUT_CLK_PHASE		0x0024
> +#define OUTCNT_LCD_EDGE		(1 << 8)	/* If set, LCDOUT[23:0] are */
> +						/* output on the falling edge */
> +						/* of QCLK */
> +
> +#define TCON_V_LATCH		0x0280
> +#define TCON_VEN		(1 << 0)
> +
> +#define TCON_TIM		0x0284
> +
> +/* Synced to VSYNC */
> +#define TCON_TIM_STVA1		0x0288
> +#define TCON_TIM_STVA2		0x028c
> +#define TCON_TIM_STVB1		0x0290
> +#define TCON_TIM_STVB2		0x0294
> +
> +/* Synced to HSYNC */
> +#define TCON_TIM_STH1		0x0298
> +#define TCON_TIM_STH2		0x029c
> +#define TCON_TIM_STB1		0x02a0
> +#define TCON_TIM_STB2		0x02a4
> +#define TCON_TIM_CPV1		0x02a8
> +#define TCON_TIM_CPV2		0x02ac
> +#define TCON_TIM_POLA1		0x02b0
> +#define TCON_TIM_POLA2		0x02b4
> +#define TCON_TIM_POLB1		0x02b8
> +#define TCON_TIM_POLB2		0x02bc
> +#define TCON_TIM_DE		0x02c0

For the sake of completeness, could you define the TCON_DE_INV bit ?

> +
> +/* Common definitions for all TCON_TIM_*1 registers */
> +#define TCON_TIMING(start, width) (((start) << 16) | ((width) << 0))
> +
> +/* Common definitions for all TCON_TIM_*2 registers */
> +#define TCON_INV		(1 << 4)
> +/* Output signal select */
> +#define TCON_SEL_STVA_VS	0

Could you write this as (0 << 0) (and some for the other bits below) to make 
it clearer that those are bit values ?

> +#define TCON_SEL_STVB_VE	1
> +#define TCON_SEL_STH_SP_HS	2
> +#define TCON_SEL_STB_LP_HE	3
> +#define TCON_SEL_CPV_GCK	4
> +#define TCON_SEL_POLA		5
> +#define TCON_SEL_POLB		6
> +#define TCON_SEL_DE		7

I'd add a blank line here.

> +/* Definitions for HSYNC-related TIM registers */

I'd list the registers explicitly here, or would add a short comment after 
each of them above to tell what they control.

> +#define TCON_HS_SEL		(1 << 8)	/* If set, horizontal sync    */
> +						/* signal reference after the */
> +						/* offset */

And a blank linke here too.

> +/* Polarity generation mode */

Could you mention that this is applicable to POLA2 and POLB2 only ?

> +#define TCON_MD_NORM		(0 << 12)
> +#define TCON_MD_1X1		(1 << 12)
> +#define TCON_MD_1X2		(2 << 12)
> +#define TCON_MD_2X2		(3 << 12)
> +
> +#endif /* __RCAR_TCON_REGS_H__ */

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 4/4] rcar-du: add R8A7794 TCON support

[Laurent Pinchart]

Hi Sergei,

Thank you for the patch.

On Friday 29 Apr 2016 00:05:33 Sergei Shtylyov wrote:
> Now that we have the TCON encoder driver, we can start enabling TCON support
> for the  R-Car SoCs. We have only tested the code on R8A7794 so far, so 
> let it  be the first supported SoC...

Please also update the DT bindings.

> Based on a large patch by Andrey Gusakov.
> 
> Signed-off-by: Andrey Gusakov <andrey.gusakov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> Index: renesas/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> ===================================================================
> --- renesas.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ renesas/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -116,9 +116,7 @@ static const struct rcar_du_device_info
> 
>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
> 
>  	.num_crtcs = 2,
>  	.routes = {
> -		/* R8A7794 has two RGB outputs and one (currently unsupported)
> -		 * TCON output.
> -		 */
> +		/* R8A7794 has two RGB outputs and one TCON output. */
>  		[RCAR_DU_OUTPUT_DPAD0] = {
>  			.possible_crtcs = BIT(0),
>  			.encoder_type = DRM_MODE_ENCODER_NONE,
> @@ -129,8 +127,14 @@ static const struct rcar_du_device_info
>  			.encoder_type = DRM_MODE_ENCODER_NONE,
>  			.port = 1,
>  		},
> +		[RCAR_DU_OUTPUT_TCON] = {
> +			.possible_crtcs = BIT(0),

According to the datasheet TCON can be connected to both DU0 and DU1.

> +			.encoder_type = DRM_MODE_ENCODER_TCON,
> +			.port = 2,
> +		},
>  	},
>  	.num_lvds = 0,
> +	.num_tcon = 1,
>  };
> 
>  static const struct rcar_du_device_info rcar_du_r8a7795_info = {

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/4] rcar-du: add R8A7794 TCON support

[Laurent Pinchart]

Hi Sergei,

Thank you for the patch.

On Friday 29 Apr 2016 00:05:33 Sergei Shtylyov wrote:
> Now that we have the TCON encoder driver, we can start enabling TCON support
> for the  R-Car SoCs. We have only tested the code on R8A7794 so far, so 
> let it  be the first supported SoC...

Please also update the DT bindings.

> Based on a large patch by Andrey Gusakov.
> 
> Signed-off-by: Andrey Gusakov <andrey.gusakov@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> Index: renesas/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> ===================================================================
> --- renesas.orig/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ renesas/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -116,9 +116,7 @@ static const struct rcar_du_device_info
> 
>  		  | RCAR_DU_FEATURE_EXT_CTRL_REGS,
> 
>  	.num_crtcs = 2,
>  	.routes = {
> -		/* R8A7794 has two RGB outputs and one (currently unsupported)
> -		 * TCON output.
> -		 */
> +		/* R8A7794 has two RGB outputs and one TCON output. */
>  		[RCAR_DU_OUTPUT_DPAD0] = {
>  			.possible_crtcs = BIT(0),
>  			.encoder_type = DRM_MODE_ENCODER_NONE,
> @@ -129,8 +127,14 @@ static const struct rcar_du_device_info
>  			.encoder_type = DRM_MODE_ENCODER_NONE,
>  			.port = 1,
>  		},
> +		[RCAR_DU_OUTPUT_TCON] = {
> +			.possible_crtcs = BIT(0),

According to the datasheet TCON can be connected to both DU0 and DU1.

> +			.encoder_type = DRM_MODE_ENCODER_TCON,
> +			.port = 2,
> +		},
>  	},
>  	.num_lvds = 0,
> +	.num_tcon = 1,
>  };
> 
>  static const struct rcar_du_device_info rcar_du_r8a7795_info = {

-- 
Regards,

Laurent Pinchart

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