devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/14] drm/exynos: support LCD I80 interface display
@ 2014-07-08  0:39 YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 01/14] drm/exynos: dsi: move the EoT packets configuration point YoungJun Cho
                   ` (14 more replies)
  0 siblings, 15 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

Hi,

This series adds LCD I80 interface display support for Exynos DRM driver.
The FIMD(display controller) specification describes it as "LCD I80 interface"
and the DSI specification describes it as "Command mode interface".

This is based on exynos-drm-next branch.

The previous patches,
RFC: http://www.spinics.net/lists/dri-devel/msg58898.html
V1: http://www.spinics.net/lists/dri-devel/msg59291.html
V2: http://www.spinics.net/lists/dri-devel/msg59867.html
V3: http://www.spinics.net/lists/dri-devel/msg60708.html
V4: http://www.spinics.net/lists/dri-devel/msg60943.html

Changelog v2:
- Fixes typo and removes unnecessary error log (commented by Andrzej Hazda)
- Adds missed pendlig_flip flag clear points (commented by Daniel Kurtz)

Changelog v3:
- Removes generic command mode and command mode display timing interface.
- Moves I80 interface timings from panel DT to the FIMD(display controller) DT.

Changelog v4:
- Removes exynos5 sysreg(syscon) DT bindings and node from dtsi because
  it was already updated by linux-samsung-soc (commented by Vivek Gautam)

Changelog v5:
- Fixes FIMD vidcon0 register relevant code
- Fixes panel gamma table, disable sequence
- Slitely updates for code cleanup

Patches 1 and 2 fix trivial bugs.

Patches 3, 4, 5 and 6 implement FIMD(display controller) I80 interface.
The MIPI DSI command mode based panel generates Tearing Effect synchronization
signal between MCU and FB to display video image, and FIMD should trigger to
transfer video image at this signal.
So the panel should receive the TE IRQ and call TE handler chains to notify
it to the FIMD.

Patches 7 and 8 implement to use Exynos5410 / 5420 / 5440 SoC DSI driver
which is different from previous Exynos4 SoCs for some registers control.

Patches 9 and 10 introduce MIPI DSI command mode based Samsung S6E3FA0 AMOLED
5.7" LCD drm panel driver.

The ohters add DT property nodes to support MIPI DSI command mode.

I welcome any comments.

Thank you.
Best regards YJ

YoungJun Cho (14):
  drm/exynos: dsi: move the EoT packets configuration point
  drm/exynos: use wait_event_timeout() for safety usage
  ARM: dts: samsung-fimd: add LCD I80 interface specific properties
  drm/exynos: add TE handler to support LCD I80 interface
  drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  drm/exynos: fimd: support LCD I80 interface
  ARM: dts: exynos_dsim: add exynos5410 compatible to DT bindings
  drm/exynos: dsi: add driver data to support Exynos5410/5420/5440 SoCs
  ARM: dts: s6e3fa0: add DT bindings
  drm/panel: add S6E3FA0 driver
  ARM: dts: exynos4: add system register property
  ARM: dts: exynos5: add system register property
  ARM: dts: exynos5420: add mipi-phy node
  ARM: dts: exynos5420: add dsi node

 .../devicetree/bindings/panel/samsung,s6e3fa0.txt  |  46 ++
 .../devicetree/bindings/video/exynos_dsim.txt      |   4 +-
 .../devicetree/bindings/video/samsung-fimd.txt     |  28 +
 arch/arm/boot/dts/exynos4.dtsi                     |   1 +
 arch/arm/boot/dts/exynos5.dtsi                     |   1 +
 arch/arm/boot/dts/exynos5420.dtsi                  |  20 +
 drivers/gpu/drm/exynos/Kconfig                     |   1 +
 drivers/gpu/drm/exynos/exynos_drm_crtc.c           |  15 +-
 drivers/gpu/drm/exynos/exynos_drm_crtc.h           |   7 +
 drivers/gpu/drm/exynos/exynos_drm_drv.h            |   3 +
 drivers/gpu/drm/exynos/exynos_drm_dsi.c            | 181 ++++++-
 drivers/gpu/drm/exynos/exynos_drm_fimd.c           | 276 ++++++++--
 drivers/gpu/drm/panel/Kconfig                      |   7 +
 drivers/gpu/drm/panel/Makefile                     |   1 +
 drivers/gpu/drm/panel/panel-s6e3fa0.c              | 569 +++++++++++++++++++++
 include/drm/drm_mipi_dsi.h                         |   7 +
 include/video/samsung_fimd.h                       |   3 +-
 17 files changed, 1098 insertions(+), 72 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
 create mode 100644 drivers/gpu/drm/panel/panel-s6e3fa0.c

-- 
1.9.0

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

* [PATCH v5 01/14] drm/exynos: dsi: move the EoT packets configuration point
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 02/14] drm/exynos: use wait_event_timeout() for safety usage YoungJun Cho
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

This configuration could be used in MIPI DSI command mode also.
And adds user manual description for display configuration.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 6302aa6..dad543a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -468,13 +468,19 @@ static int exynos_dsi_init_link(struct exynos_dsi *dsi)
 	/* DSI configuration */
 	reg = 0;
 
+	/* The first bit of mode_flags specifies display configuration.
+	 * If this bit is set[= MIPI_DSI_MODE_VIDEO], dsi will support video
+	 * mode, otherwise it will support command mode.
+	 */
 	if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
 		reg |= DSIM_VIDEO_MODE;
 
+		/*
+		 * The user manual describes that following bits are ignored in
+		 * command mode.
+		 */
 		if (!(dsi->mode_flags & MIPI_DSI_MODE_VSYNC_FLUSH))
 			reg |= DSIM_MFLUSH_VS;
-		if (!(dsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET))
-			reg |= DSIM_EOT_DISABLE;
 		if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
 			reg |= DSIM_SYNC_INFORM;
 		if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST)
@@ -491,6 +497,9 @@ static int exynos_dsi_init_link(struct exynos_dsi *dsi)
 			reg |= DSIM_HSA_MODE;
 	}
 
+	if (!(dsi->mode_flags & MIPI_DSI_MODE_EOT_PACKET))
+		reg |= DSIM_EOT_DISABLE;
+
 	switch (dsi->format) {
 	case MIPI_DSI_FMT_RGB888:
 		reg |= DSIM_MAIN_PIX_FORMAT_RGB888;
-- 
1.9.0

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

* [PATCH v5 02/14] drm/exynos: use wait_event_timeout() for safety usage
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 01/14] drm/exynos: dsi: move the EoT packets configuration point YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 03/14] ARM: dts: samsung-fimd: add LCD I80 interface specific properties YoungJun Cho
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: devicetree, robh+dt, pawel.moll, mark.rutland, ijc+devicetree,
	galak, linux-samsung-soc, thierry.reding, kyungmin.park, inki.dae,
	kgene.kim, jy0922.shim, sw0312.kim, a.hajda

There could be the case that the page flip operation isn't finished correctly
with some abnormal condition such as panel reset. So this patch replaces
wait_event() with wait_event_timeout() to avoid waiting for page flip completion
infinitely.
And clears exynos_crtc->pending_flip in exynos_drm_crtc_page_flip()
when exynos_drm_crtc_mode_set_commit() is failed.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 95c9435..3bf091d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -69,8 +69,10 @@ static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
 
 	if (mode > DRM_MODE_DPMS_ON) {
 		/* wait for the completion of page flip. */
-		wait_event(exynos_crtc->pending_flip_queue,
-				atomic_read(&exynos_crtc->pending_flip) == 0);
+		if (!wait_event_timeout(exynos_crtc->pending_flip_queue,
+				!atomic_read(&exynos_crtc->pending_flip),
+				HZ/20))
+			atomic_set(&exynos_crtc->pending_flip, 0);
 		drm_vblank_off(crtc->dev, exynos_crtc->pipe);
 	}
 
@@ -259,6 +261,7 @@ static int exynos_drm_crtc_page_flip(struct drm_crtc *crtc,
 			spin_lock_irq(&dev->event_lock);
 			drm_vblank_put(dev, exynos_crtc->pipe);
 			list_del(&event->base.link);
+			atomic_set(&exynos_crtc->pending_flip, 0);
 			spin_unlock_irq(&dev->event_lock);
 
 			goto out;
-- 
1.9.0

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

* [PATCH v5 03/14] ARM: dts: samsung-fimd: add LCD I80 interface specific properties
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 01/14] drm/exynos: dsi: move the EoT packets configuration point YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 02/14] drm/exynos: use wait_event_timeout() for safety usage YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 04/14] drm/exynos: add TE handler to support LCD I80 interface YoungJun Cho
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

In case of using MIPI DSI based I80 interface panel,
the relevant registers should be set.
So this patch adds relevant DT bindings.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 .../devicetree/bindings/video/samsung-fimd.txt     | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/video/samsung-fimd.txt b/Documentation/devicetree/bindings/video/samsung-fimd.txt
index 2dad41b..59ff61e 100644
--- a/Documentation/devicetree/bindings/video/samsung-fimd.txt
+++ b/Documentation/devicetree/bindings/video/samsung-fimd.txt
@@ -44,6 +44,34 @@ Optional Properties:
 - display-timings: timing settings for FIMD, as described in document [1].
 		Can be used in case timings cannot be provided otherwise
 		or to override timings provided by the panel.
+- samsung,sysreg: handle to syscon used to control the system registers
+- i80-if-timings: timing configuration for lcd i80 interface support.
+  - cs-setup: clock cycles for the active period of address signal is enabled
+              until chip select is enabled.
+              If not specified, the default value(0) will be used.
+  - wr-setup: clock cycles for the active period of CS signal is enabled until
+              write signal is enabled.
+              If not specified, the default value(0) will be used.
+  - wr-active: clock cycles for the active period of CS is enabled.
+               If not specified, the default value(1) will be used.
+  - wr-hold: clock cycles for the active period of CS is disabled until write
+             signal is disabled.
+             If not specified, the default value(0) will be used.
+
+  The parameters are defined as:
+
+    VCLK(internal)  __|¯¯¯¯¯¯|_____|¯¯¯¯¯¯|_____|¯¯¯¯¯¯|_____|¯¯¯¯¯¯|_____|¯¯
+                      :            :            :            :            :
+    Address Output  --:<XXXXXXXXXXX:XXXXXXXXXXXX:XXXXXXXXXXXX:XXXXXXXXXXXX:XX
+                      | cs-setup+1 |            :            :            :
+                      |<---------->|            :            :            :
+    Chip Select     ¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯|____________:____________:____________|¯¯
+                                   | wr-setup+1 |            | wr-hold+1  |
+                                   |<---------->|            |<---------->|
+    Write Enable    ¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯|____________|¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯
+                                                | wr-active+1|
+                                                |<---------->|
+    Video Data      ----------------------------<XXXXXXXXXXXXXXXXXXXXXXXXX>--
 
 The device node can contain 'port' child nodes according to the bindings defined
 in [2]. The following are properties specific to those nodes:
-- 
1.9.0

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

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

* [PATCH v5 04/14] drm/exynos: add TE handler to support LCD I80 interface
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
                   ` (2 preceding siblings ...)
  2014-07-08  0:39 ` [PATCH v5 03/14] ARM: dts: samsung-fimd: add LCD I80 interface specific properties YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops " YoungJun Cho
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

To support LCD I80 interface, the panel should generate
Tearing Effect synchronization signal between MCU and FB
to display video images.
And the display controller should trigger to transfer
video image at this signal.
So the panel receives the TE IRQ, then calls these handler
chains to notify it to the display controller.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 8 ++++++++
 drivers/gpu/drm/exynos/exynos_drm_crtc.h | 7 +++++++
 drivers/gpu/drm/exynos/exynos_drm_drv.h  | 3 +++
 3 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 3bf091d..b68e58f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -511,3 +511,11 @@ int exynos_drm_crtc_get_pipe_from_type(struct drm_device *drm_dev,
 
 	return -EPERM;
 }
+
+void exynos_drm_crtc_te_handler(struct drm_crtc *crtc)
+{
+	struct exynos_drm_manager *manager = to_exynos_crtc(crtc)->manager;
+
+	if (manager->ops->te_handler)
+		manager->ops->te_handler(manager);
+}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.h b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
index 9f74b10..690dcdd 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
@@ -36,4 +36,11 @@ void exynos_drm_crtc_plane_disable(struct drm_crtc *crtc, int zpos);
 int exynos_drm_crtc_get_pipe_from_type(struct drm_device *drm_dev,
 					unsigned int out_type);
 
+/*
+ * This function calls the crtc device(manager)'s te_handler() callback
+ * to trigger to transfer video image at the tearing effect synchronization
+ * signal.
+ */
+void exynos_drm_crtc_te_handler(struct drm_crtc *crtc);
+
 #endif
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 06cde45..d4e0726 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -188,6 +188,8 @@ struct exynos_drm_display {
  * @win_commit: apply hardware specific overlay data to registers.
  * @win_enable: enable hardware specific overlay.
  * @win_disable: disable hardware specific overlay.
+ * @te_handler: trigger to transfer video image at the tearing effect
+ *	synchronization signal if there is a page flip request.
  */
 struct exynos_drm_manager;
 struct exynos_drm_manager_ops {
@@ -206,6 +208,7 @@ struct exynos_drm_manager_ops {
 	void (*win_commit)(struct exynos_drm_manager *mgr, int zpos);
 	void (*win_enable)(struct exynos_drm_manager *mgr, int zpos);
 	void (*win_disable)(struct exynos_drm_manager *mgr, int zpos);
+	void (*te_handler)(struct exynos_drm_manager *mgr);
 };
 
 /*
-- 
1.9.0

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

* [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
                   ` (3 preceding siblings ...)
  2014-07-08  0:39 ` [PATCH v5 04/14] drm/exynos: add TE handler to support LCD I80 interface YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-09 15:22   ` Thierry Reding
  2014-07-08  0:39 ` [PATCH v5 06/14] drm/exynos: fimd: " YoungJun Cho
                   ` (9 subsequent siblings)
  14 siblings, 1 reply; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

To support LCD I80 interface, the DSI host calls this function
to notify the panel tearing effect synchronization signal to
the CRTC device manager to trigger to transfer video image.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++++++++
 include/drm/drm_mipi_dsi.h              |  7 +++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index dad543a..76e34ca 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -24,6 +24,7 @@
 #include <video/mipi_display.h>
 #include <video/videomode.h>
 
+#include "exynos_drm_crtc.h"
 #include "exynos_drm_drv.h"
 
 /* returns true iff both arguments logically differs */
@@ -1041,10 +1042,20 @@ static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
 	return (ret < 0) ? ret : xfer.rx_done;
 }
 
+static void exynos_dsi_host_pass_te(struct mipi_dsi_host *host)
+{
+	struct exynos_dsi *dsi = host_to_dsi(host);
+	struct drm_encoder *encoder = dsi->encoder;
+
+	if (dsi->state & DSIM_STATE_ENABLED)
+		exynos_drm_crtc_te_handler(encoder->crtc);
+}
+
 static const struct mipi_dsi_host_ops exynos_dsi_ops = {
 	.attach = exynos_dsi_host_attach,
 	.detach = exynos_dsi_host_detach,
 	.transfer = exynos_dsi_host_transfer,
+	.pass_te = exynos_dsi_host_pass_te,
 };
 
 static int exynos_dsi_poweron(struct exynos_dsi *dsi)
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 944f33f..3f21bea 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -49,6 +49,12 @@ struct mipi_dsi_msg {
  * @detach: detach DSI device from DSI host
  * @transfer: send and/or receive DSI packet, return number of received bytes,
  * 	      or error
+ * @pass_te: call the crtc te_handler() callback from DSI host.
+ *	     The panel generates tearing effect synchronization signal between
+ *	     MCU and FB to display video images. And the display controller
+ *	     should trigger to transfer video image at this signal. So the panel
+ *	     receives the TE IRQ, then calls this function to notify it to the
+ *	     display controller.
  */
 struct mipi_dsi_host_ops {
 	int (*attach)(struct mipi_dsi_host *host,
@@ -57,6 +63,7 @@ struct mipi_dsi_host_ops {
 		      struct mipi_dsi_device *dsi);
 	ssize_t (*transfer)(struct mipi_dsi_host *host,
 			    struct mipi_dsi_msg *msg);
+	void (*pass_te)(struct mipi_dsi_host *host);
 };
 
 /**
-- 
1.9.0

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

* [PATCH v5 06/14] drm/exynos: fimd: support LCD I80 interface
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
                   ` (4 preceding siblings ...)
  2014-07-08  0:39 ` [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops " YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-08 16:10   ` Inki Dae
  2014-07-08  0:39 ` [PATCH v5 07/14] ARM: dts: exynos_dsim: add exynos5410 compatible to DT bindings YoungJun Cho
                   ` (8 subsequent siblings)
  14 siblings, 1 reply; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

To support MIPI command mode based I80 interface panel,
FIMD should do followings:
- Sets LCD I80 interface timings configuration.
- Uses "lcd_sys" as an IRQ resource and sets relevant IRQ configuration.
- Sets LCD block configuration for I80 interface.
- Sets ideal(pixel) clock is 2 times faster than the original one
  to generate frame done IRQ prior to the next TE signal.
- Implements trigger feature that transfers image data if there is page
  flip request, and implements TE handler to call trigger function.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/Kconfig           |   1 +
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 276 ++++++++++++++++++++++++++-----
 include/video/samsung_fimd.h             |   3 +-
 3 files changed, 235 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index 178d2a9..9ba1aae 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -28,6 +28,7 @@ config DRM_EXYNOS_FIMD
 	bool "Exynos DRM FIMD"
 	depends on DRM_EXYNOS && !FB_S3C
 	select FB_MODE_HELPERS
+	select MFD_SYSCON
 	help
 	  Choose this option if you want to use Exynos FIMD for DRM.
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 33161ad..207872d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -20,6 +20,8 @@
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/component.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
 
 #include <video/of_display_timing.h>
 #include <video/of_videomode.h>
@@ -61,6 +63,24 @@
 /* color key value register for hardware window 1 ~ 4. */
 #define WKEYCON1_BASE(x)		((WKEYCON1 + 0x140) + ((x - 1) * 8))
 
+/* I80 / RGB trigger control register */
+#define TRIGCON				0x1A4
+#define TRGMODE_I80_RGB_ENABLE_I80	(1 << 0)
+#define SWTRGCMD_I80_RGB_ENABLE		(1 << 1)
+
+/* display mode change control register except exynos4 */
+#define VIDOUT_CON			0x000
+#define VIDOUT_CON_F_I80_LDI0		(0x2 << 8)
+
+/* I80 interface control for main LDI register */
+#define I80IFCONFAx(x)			(0x1B0 + (x) * 4)
+#define I80IFCONFBx(x)			(0x1B8 + (x) * 4)
+#define LCD_CS_SETUP(x)			((x) << 16)
+#define LCD_WR_SETUP(x)			((x) << 12)
+#define LCD_WR_ACTIVE(x)		((x) << 8)
+#define LCD_WR_HOLD(x)			((x) << 4)
+#define I80IFEN_ENABLE			(1 << 0)
+
 /* FIMD has totally five hardware windows. */
 #define WINDOWS_NR	5
 
@@ -68,10 +88,14 @@
 
 struct fimd_driver_data {
 	unsigned int timing_base;
+	unsigned int lcdblk_offset;
+	unsigned int lcdblk_vt_shift;
+	unsigned int lcdblk_bypass_shift;
 
 	unsigned int has_shadowcon:1;
 	unsigned int has_clksel:1;
 	unsigned int has_limited_fmt:1;
+	unsigned int has_vidoutcon:1;
 };
 
 static struct fimd_driver_data s3c64xx_fimd_driver_data = {
@@ -82,12 +106,19 @@ static struct fimd_driver_data s3c64xx_fimd_driver_data = {
 
 static struct fimd_driver_data exynos4_fimd_driver_data = {
 	.timing_base = 0x0,
+	.lcdblk_offset = 0x210,
+	.lcdblk_vt_shift = 10,
+	.lcdblk_bypass_shift = 1,
 	.has_shadowcon = 1,
 };
 
 static struct fimd_driver_data exynos5_fimd_driver_data = {
 	.timing_base = 0x20000,
+	.lcdblk_offset = 0x214,
+	.lcdblk_vt_shift = 24,
+	.lcdblk_bypass_shift = 15,
 	.has_shadowcon = 1,
+	.has_vidoutcon = 1,
 };
 
 struct fimd_win_data {
@@ -112,15 +143,22 @@ struct fimd_context {
 	struct clk			*bus_clk;
 	struct clk			*lcd_clk;
 	void __iomem			*regs;
+	struct regmap			*sysreg;
 	struct drm_display_mode		mode;
 	struct fimd_win_data		win_data[WINDOWS_NR];
 	unsigned int			default_win;
 	unsigned long			irq_flags;
+	u32				vidcon0;
 	u32				vidcon1;
+	u32				vidout_con;
+	u32				i80ifcon;
+	bool				i80_if;
 	bool				suspended;
 	int				pipe;
 	wait_queue_head_t		wait_vsync_queue;
 	atomic_t			wait_vsync_event;
+	atomic_t			win_updated;
+	atomic_t			triggering;
 
 	struct exynos_drm_panel_info panel;
 	struct fimd_driver_data *driver_data;
@@ -243,6 +281,14 @@ static u32 fimd_calc_clkdiv(struct fimd_context *ctx,
 	unsigned long ideal_clk = mode->htotal * mode->vtotal * mode->vrefresh;
 	u32 clkdiv;
 
+	if (ctx->i80_if) {
+		/*
+		 * The frame done interrupt should be occurred prior to the
+		 * next TE signal.
+		 */
+		ideal_clk *= 2;
+	}
+
 	/* Find the clock divider value that gets us closest to ideal_clk */
 	clkdiv = DIV_ROUND_UP(clk_get_rate(ctx->lcd_clk), ideal_clk);
 
@@ -271,11 +317,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
 {
 	struct fimd_context *ctx = mgr->ctx;
 	struct drm_display_mode *mode = &ctx->mode;
-	struct fimd_driver_data *driver_data;
-	u32 val, clkdiv, vidcon1;
-	int vsync_len, vbpd, vfpd, hsync_len, hbpd, hfpd;
+	struct fimd_driver_data *driver_data = ctx->driver_data;
+	void *timing_base = ctx->regs + driver_data->timing_base;
+	u32 val, clkdiv;
 
-	driver_data = ctx->driver_data;
 	if (ctx->suspended)
 		return;
 
@@ -283,33 +328,65 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
 	if (mode->htotal == 0 || mode->vtotal == 0)
 		return;
 
-	/* setup polarity values */
-	vidcon1 = ctx->vidcon1;
-	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
-		vidcon1 |= VIDCON1_INV_VSYNC;
-	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
-		vidcon1 |= VIDCON1_INV_HSYNC;
-	writel(vidcon1, ctx->regs + driver_data->timing_base + VIDCON1);
-
-	/* setup vertical timing values. */
-	vsync_len = mode->crtc_vsync_end - mode->crtc_vsync_start;
-	vbpd = mode->crtc_vtotal - mode->crtc_vsync_end;
-	vfpd = mode->crtc_vsync_start - mode->crtc_vdisplay;
-
-	val = VIDTCON0_VBPD(vbpd - 1) |
-		VIDTCON0_VFPD(vfpd - 1) |
-		VIDTCON0_VSPW(vsync_len - 1);
-	writel(val, ctx->regs + driver_data->timing_base + VIDTCON0);
-
-	/* setup horizontal timing values.  */
-	hsync_len = mode->crtc_hsync_end - mode->crtc_hsync_start;
-	hbpd = mode->crtc_htotal - mode->crtc_hsync_end;
-	hfpd = mode->crtc_hsync_start - mode->crtc_hdisplay;
-
-	val = VIDTCON1_HBPD(hbpd - 1) |
-		VIDTCON1_HFPD(hfpd - 1) |
-		VIDTCON1_HSPW(hsync_len - 1);
-	writel(val, ctx->regs + driver_data->timing_base + VIDTCON1);
+	if (ctx->i80_if) {
+		val = ctx->i80ifcon | I80IFEN_ENABLE;
+		writel(val, timing_base + I80IFCONFAx(0));
+
+		/* disable auto frame rate */
+		writel(0, timing_base + I80IFCONFBx(0));
+
+		if (ctx->vidout_con)
+			writel(ctx->vidout_con, timing_base + VIDOUT_CON);
+
+		/* set video type selection to I80 interface */
+		if (ctx->sysreg && regmap_update_bits(ctx->sysreg,
+					driver_data->lcdblk_offset,
+					0x3 << driver_data->lcdblk_vt_shift,
+					0x1 << driver_data->lcdblk_vt_shift)) {
+			DRM_ERROR("Failed to update sysreg for I80 i/f.\n");
+			return;
+		}
+	} else {
+		int vsync_len, vbpd, vfpd, hsync_len, hbpd, hfpd;
+		u32 vidcon1;
+
+		/* setup polarity values */
+		vidcon1 = ctx->vidcon1;
+		if (mode->flags & DRM_MODE_FLAG_NVSYNC)
+			vidcon1 |= VIDCON1_INV_VSYNC;
+		if (mode->flags & DRM_MODE_FLAG_NHSYNC)
+			vidcon1 |= VIDCON1_INV_HSYNC;
+		writel(vidcon1, ctx->regs + driver_data->timing_base + VIDCON1);
+
+		/* setup vertical timing values. */
+		vsync_len = mode->crtc_vsync_end - mode->crtc_vsync_start;
+		vbpd = mode->crtc_vtotal - mode->crtc_vsync_end;
+		vfpd = mode->crtc_vsync_start - mode->crtc_vdisplay;
+
+		val = VIDTCON0_VBPD(vbpd - 1) |
+			VIDTCON0_VFPD(vfpd - 1) |
+			VIDTCON0_VSPW(vsync_len - 1);
+		writel(val, ctx->regs + driver_data->timing_base + VIDTCON0);
+
+		/* setup horizontal timing values.  */
+		hsync_len = mode->crtc_hsync_end - mode->crtc_hsync_start;
+		hbpd = mode->crtc_htotal - mode->crtc_hsync_end;
+		hfpd = mode->crtc_hsync_start - mode->crtc_hdisplay;
+
+		val = VIDTCON1_HBPD(hbpd - 1) |
+			VIDTCON1_HFPD(hfpd - 1) |
+			VIDTCON1_HSPW(hsync_len - 1);
+		writel(val, ctx->regs + driver_data->timing_base + VIDTCON1);
+	}
+
+	/* set bypass selection */
+	if (ctx->sysreg && regmap_update_bits(ctx->sysreg,
+				driver_data->lcdblk_offset,
+				0x1 << driver_data->lcdblk_bypass_shift,
+				0x1 << driver_data->lcdblk_bypass_shift)) {
+		DRM_ERROR("Failed to update sysreg for bypass setting.\n");
+		return;
+	}
 
 	/* setup horizontal and vertical display size. */
 	val = VIDTCON2_LINEVAL(mode->vdisplay - 1) |
@@ -322,7 +399,8 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
 	 * fields of register with prefix '_F' would be updated
 	 * at vsync(same as dma start)
 	 */
-	val = VIDCON0_ENVID | VIDCON0_ENVID_F;
+	val = ctx->vidcon0;
+	val |= VIDCON0_ENVID | VIDCON0_ENVID_F;
 
 	if (ctx->driver_data->has_clksel)
 		val |= VIDCON0_CLKSEL_LCD;
@@ -660,6 +738,9 @@ static void fimd_win_commit(struct exynos_drm_manager *mgr, int zpos)
 	}
 
 	win_data->enabled = true;
+
+	if (ctx->i80_if)
+		atomic_set(&ctx->win_updated, 1);
 }
 
 static void fimd_win_disable(struct exynos_drm_manager *mgr, int zpos)
@@ -838,6 +919,58 @@ static void fimd_dpms(struct exynos_drm_manager *mgr, int mode)
 	}
 }
 
+static void fimd_trigger(struct device *dev)
+{
+	struct exynos_drm_manager *mgr = get_fimd_manager(dev);
+	struct fimd_context *ctx = mgr->ctx;
+	struct fimd_driver_data *driver_data = ctx->driver_data;
+	void *timing_base = ctx->regs + driver_data->timing_base;
+	u32 reg;
+
+	atomic_set(&ctx->triggering, 1);
+
+	reg = readl(ctx->regs + VIDINTCON0);
+	reg |= (VIDINTCON0_INT_ENABLE | VIDINTCON0_INT_I80IFDONE |
+						VIDINTCON0_INT_SYSMAINCON);
+	writel(reg, ctx->regs + VIDINTCON0);
+
+	reg = readl(timing_base + TRIGCON);
+	reg |= (TRGMODE_I80_RGB_ENABLE_I80 | SWTRGCMD_I80_RGB_ENABLE);
+	writel(reg, timing_base + TRIGCON);
+}
+
+static void fimd_te_handler(struct exynos_drm_manager *mgr)
+{
+	struct fimd_context *ctx = mgr->ctx;
+
+	/* Checks the crtc is detached already from encoder */
+	if (ctx->pipe < 0 || !ctx->drm_dev)
+		return;
+
+	 /*
+	 * Skips to trigger if in triggering state, because multiple triggering
+	 * requests can cause panel reset.
+	 */
+	if (atomic_read(&ctx->triggering))
+		return;
+
+	/*
+	 * If there is a page flip request, triggers and handles the page flip
+	 * event so that current fb can be updated into panel GRAM.
+	 */
+	if (atomic_add_unless(&ctx->win_updated, -1, 0))
+		fimd_trigger(ctx->dev);
+
+	/* Wakes up vsync event queue */
+	if (atomic_read(&ctx->wait_vsync_event)) {
+		atomic_set(&ctx->wait_vsync_event, 0);
+		wake_up(&ctx->wait_vsync_queue);
+
+		if (!atomic_read(&ctx->triggering))
+			drm_handle_vblank(ctx->drm_dev, ctx->pipe);
+	}
+}
+
 static struct exynos_drm_manager_ops fimd_manager_ops = {
 	.dpms = fimd_dpms,
 	.mode_fixup = fimd_mode_fixup,
@@ -849,6 +982,7 @@ static struct exynos_drm_manager_ops fimd_manager_ops = {
 	.win_mode_set = fimd_win_mode_set,
 	.win_commit = fimd_win_commit,
 	.win_disable = fimd_win_disable,
+	.te_handler = fimd_te_handler,
 };
 
 static struct exynos_drm_manager fimd_manager = {
@@ -859,26 +993,40 @@ static struct exynos_drm_manager fimd_manager = {
 static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
 {
 	struct fimd_context *ctx = (struct fimd_context *)dev_id;
-	u32 val;
+	u32 val, clear_bit;
 
 	val = readl(ctx->regs + VIDINTCON1);
 
-	if (val & VIDINTCON1_INT_FRAME)
-		/* VSYNC interrupt */
-		writel(VIDINTCON1_INT_FRAME, ctx->regs + VIDINTCON1);
+	clear_bit = ctx->i80_if ? VIDINTCON1_INT_I80 : VIDINTCON1_INT_FRAME;
+	if (val & clear_bit)
+		writel(clear_bit, ctx->regs + VIDINTCON1);
 
 	/* check the crtc is detached already from encoder */
 	if (ctx->pipe < 0 || !ctx->drm_dev)
 		goto out;
 
-	drm_handle_vblank(ctx->drm_dev, ctx->pipe);
-	exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
+	if (ctx->i80_if) {
+		/* unset I80 frame done interrupt */
+		val = readl(ctx->regs + VIDINTCON0);
+		val &= ~(VIDINTCON0_INT_I80IFDONE | VIDINTCON0_INT_SYSMAINCON);
+		writel(val, ctx->regs + VIDINTCON0);
 
-	/* set wait vsync event to zero and wake up queue. */
-	if (atomic_read(&ctx->wait_vsync_event)) {
-		atomic_set(&ctx->wait_vsync_event, 0);
-		wake_up(&ctx->wait_vsync_queue);
+		/* exit triggering mode */
+		atomic_set(&ctx->triggering, 0);
+
+		drm_handle_vblank(ctx->drm_dev, ctx->pipe);
+		exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
+	} else {
+		drm_handle_vblank(ctx->drm_dev, ctx->pipe);
+		exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
+
+		/* set wait vsync event to zero and wake up queue. */
+		if (atomic_read(&ctx->wait_vsync_event)) {
+			atomic_set(&ctx->wait_vsync_event, 0);
+			wake_up(&ctx->wait_vsync_queue);
+		}
 	}
+
 out:
 	return IRQ_HANDLED;
 }
@@ -923,6 +1071,7 @@ static int fimd_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct fimd_context *ctx;
+	struct device_node *i80_if_timings;
 	struct resource *res;
 	int ret = -EINVAL;
 
@@ -944,12 +1093,51 @@ static int fimd_probe(struct platform_device *pdev)
 
 	ctx->dev = dev;
 	ctx->suspended = true;
+	ctx->driver_data = drm_fimd_get_driver_data(pdev);
 
 	if (of_property_read_bool(dev->of_node, "samsung,invert-vden"))
 		ctx->vidcon1 |= VIDCON1_INV_VDEN;
 	if (of_property_read_bool(dev->of_node, "samsung,invert-vclk"))
 		ctx->vidcon1 |= VIDCON1_INV_VCLK;
 
+	i80_if_timings = of_get_child_by_name(dev->of_node, "i80-if-timings");
+	if (i80_if_timings) {
+		u32 val;
+
+		ctx->i80_if = true;
+
+		if (ctx->driver_data->has_vidoutcon)
+			ctx->vidout_con |= VIDOUT_CON_F_I80_LDI0;
+		else
+			ctx->vidcon0 |= VIDCON0_VIDOUT_I80_LDI0;
+		/*
+		 * The user manual describes that this "DSI_EN" bit is required
+		 * to enable I80 24-bit data interface.
+		 */
+		ctx->vidcon0 |= VIDCON0_DSI_EN;
+
+		if (of_property_read_u32(i80_if_timings, "cs-setup", &val))
+			val = 0;
+		ctx->i80ifcon = LCD_CS_SETUP(val);
+		if (of_property_read_u32(i80_if_timings, "wr-setup", &val))
+			val = 0;
+		ctx->i80ifcon |= LCD_WR_SETUP(val);
+		if (of_property_read_u32(i80_if_timings, "wr-active", &val))
+			val = 1;
+		ctx->i80ifcon |= LCD_WR_ACTIVE(val);
+		if (of_property_read_u32(i80_if_timings, "wr-hold", &val))
+			val = 0;
+		ctx->i80ifcon |= LCD_WR_HOLD(val);
+	}
+	of_node_put(i80_if_timings);
+
+	ctx->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
+							"samsung,sysreg");
+	if (IS_ERR(ctx->sysreg)) {
+		dev_warn(dev, "failed to get system register.\n");
+		ctx->sysreg = NULL;
+	}
+
 	ctx->bus_clk = devm_clk_get(dev, "fimd");
 	if (IS_ERR(ctx->bus_clk)) {
 		dev_err(dev, "failed to get bus clock\n");
@@ -972,7 +1160,8 @@ static int fimd_probe(struct platform_device *pdev)
 		goto err_del_component;
 	}
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "vsync");
+	res = platform_get_resource_byname(pdev, IORESOURCE_IRQ,
+					   ctx->i80_if ? "lcd_sys" : "vsync");
 	if (!res) {
 		dev_err(dev, "irq request failed.\n");
 		ret = -ENXIO;
@@ -986,7 +1175,6 @@ static int fimd_probe(struct platform_device *pdev)
 		goto err_del_component;
 	}
 
-	ctx->driver_data = drm_fimd_get_driver_data(pdev);
 	init_waitqueue_head(&ctx->wait_vsync_queue);
 	atomic_set(&ctx->wait_vsync_event, 0);
 
diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h
index b039320..eaad58b 100644
--- a/include/video/samsung_fimd.h
+++ b/include/video/samsung_fimd.h
@@ -19,6 +19,7 @@
 /* VIDCON0 */
 
 #define VIDCON0					0x00
+#define VIDCON0_DSI_EN				(1 << 30)
 #define VIDCON0_INTERLACE			(1 << 29)
 #define VIDCON0_VIDOUT_MASK			(0x7 << 26)
 #define VIDCON0_VIDOUT_SHIFT			26
@@ -355,7 +356,7 @@
 #define VIDINTCON0_INT_ENABLE			(1 << 0)
 
 #define VIDINTCON1				0x134
-#define VIDINTCON1_INT_I180			(1 << 2)
+#define VIDINTCON1_INT_I80			(1 << 2)
 #define VIDINTCON1_INT_FRAME			(1 << 1)
 #define VIDINTCON1_INT_FIFO			(1 << 0)
 
-- 
1.9.0

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

* [PATCH v5 07/14] ARM: dts: exynos_dsim: add exynos5410 compatible to DT bindings
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
                   ` (5 preceding siblings ...)
  2014-07-08  0:39 ` [PATCH v5 06/14] drm/exynos: fimd: " YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 08/14] drm/exynos: dsi: add driver data to support Exynos5410/5420/5440 SoCs YoungJun Cho
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

This patch adds relevant to exynos5410 compatible
for exynos5410 / 5420 / 5440 SoCs support.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 Documentation/devicetree/bindings/video/exynos_dsim.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/video/exynos_dsim.txt b/Documentation/devicetree/bindings/video/exynos_dsim.txt
index 33b5730..31036c6 100644
--- a/Documentation/devicetree/bindings/video/exynos_dsim.txt
+++ b/Documentation/devicetree/bindings/video/exynos_dsim.txt
@@ -1,7 +1,9 @@
 Exynos MIPI DSI Master
 
 Required properties:
-  - compatible: "samsung,exynos4210-mipi-dsi"
+  - compatible: value should be one of the following
+		"samsung,exynos4210-mipi-dsi" /* for Exynos4 SoCs */
+		"samsung,exynos5410-mipi-dsi" /* for Exynos5410/5420/5440 SoCs */
   - reg: physical base address and length of the registers set for the device
   - interrupts: should contain DSI interrupt
   - clocks: list of clock specifiers, must contain an entry for each required
-- 
1.9.0

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

* [PATCH v5 08/14] drm/exynos: dsi: add driver data to support Exynos5410/5420/5440 SoCs
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
                   ` (6 preceding siblings ...)
  2014-07-08  0:39 ` [PATCH v5 07/14] ARM: dts: exynos_dsim: add exynos5410 compatible to DT bindings YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 09/14] ARM: dts: s6e3fa0: add DT bindings YoungJun Cho
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

The offset of register DSIM_PLLTMR_REG in Exynos5410 / 5420 / 5440
SoCs is different from the one in Exynos4 SoCs.

In case of Exynos5410 / 5420 / 5440 SoCs, there is no frequency
band bit in DSIM_PLLCTRL_REG, and it uses DSIM_PHYCTRL_REG and
DSIM_PHYTIMING*_REG instead.
So this patch adds driver data to distinguish it.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 157 +++++++++++++++++++++++++++-----
 1 file changed, 135 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 76e34ca..162f74d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -17,6 +17,7 @@
 
 #include <linux/clk.h>
 #include <linux/irq.h>
+#include <linux/of_device.h>
 #include <linux/phy/phy.h>
 #include <linux/regulator/consumer.h>
 #include <linux/component.h>
@@ -55,9 +56,12 @@
 
 /* FIFO memory AC characteristic register */
 #define DSIM_PLLCTRL_REG	0x4c	/* PLL control register */
-#define DSIM_PLLTMR_REG		0x50	/* PLL timer register */
 #define DSIM_PHYACCHR_REG	0x54	/* D-PHY AC characteristic register */
 #define DSIM_PHYACCHR1_REG	0x58	/* D-PHY AC characteristic register1 */
+#define DSIM_PHYCTRL_REG	0x5c
+#define DSIM_PHYTIMING_REG	0x64
+#define DSIM_PHYTIMING1_REG	0x68
+#define DSIM_PHYTIMING2_REG	0x6c
 
 /* DSIM_STATUS */
 #define DSIM_STOP_STATE_DAT(x)		(((x) & 0xf) << 0)
@@ -201,6 +205,24 @@
 #define DSIM_PLL_M(x)			((x) << 4)
 #define DSIM_PLL_S(x)			((x) << 1)
 
+/* DSIM_PHYCTRL */
+#define DSIM_PHYCTRL_ULPS_EXIT(x)	(((x) & 0x1ff) << 0)
+
+/* DSIM_PHYTIMING */
+#define DSIM_PHYTIMING_LPX(x)		((x) << 8)
+#define DSIM_PHYTIMING_HS_EXIT(x)	((x) << 0)
+
+/* DSIM_PHYTIMING1 */
+#define DSIM_PHYTIMING1_CLK_PREPARE(x)	((x) << 24)
+#define DSIM_PHYTIMING1_CLK_ZERO(x)	((x) << 16)
+#define DSIM_PHYTIMING1_CLK_POST(x)	((x) << 8)
+#define DSIM_PHYTIMING1_CLK_TRAIL(x)	((x) << 0)
+
+/* DSIM_PHYTIMING2 */
+#define DSIM_PHYTIMING2_HS_PREPARE(x)	((x) << 16)
+#define DSIM_PHYTIMING2_HS_ZERO(x)	((x) << 8)
+#define DSIM_PHYTIMING2_HS_TRAIL(x)	((x) << 0)
+
 #define DSI_MAX_BUS_WIDTH		4
 #define DSI_NUM_VIRTUAL_CHANNELS	4
 #define DSI_TX_FIFO_SIZE		2048
@@ -234,6 +256,12 @@ struct exynos_dsi_transfer {
 #define DSIM_STATE_INITIALIZED		BIT(1)
 #define DSIM_STATE_CMD_LPM		BIT(2)
 
+struct exynos_dsi_driver_data {
+	unsigned int plltmr_reg;
+
+	unsigned int has_freqband:1;
+};
+
 struct exynos_dsi {
 	struct mipi_dsi_host dsi_host;
 	struct drm_connector connector;
@@ -263,11 +291,39 @@ struct exynos_dsi {
 
 	spinlock_t transfer_lock; /* protects transfer_list */
 	struct list_head transfer_list;
+
+	struct exynos_dsi_driver_data *driver_data;
 };
 
 #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
 #define connector_to_dsi(c) container_of(c, struct exynos_dsi, connector)
 
+static struct exynos_dsi_driver_data exynos4_dsi_driver_data = {
+	.plltmr_reg = 0x50,
+	.has_freqband = 1,
+};
+
+static struct exynos_dsi_driver_data exynos5_dsi_driver_data = {
+	.plltmr_reg = 0x58,
+};
+
+static struct of_device_id exynos_dsi_of_match[] = {
+	{ .compatible = "samsung,exynos4210-mipi-dsi",
+	  .data = &exynos4_dsi_driver_data },
+	{ .compatible = "samsung,exynos5410-mipi-dsi",
+	  .data = &exynos5_dsi_driver_data },
+	{ }
+};
+
+static inline struct exynos_dsi_driver_data *exynos_dsi_get_driver_data(
+						struct platform_device *pdev)
+{
+	const struct of_device_id *of_id =
+			of_match_device(exynos_dsi_of_match, &pdev->dev);
+
+	return (struct exynos_dsi_driver_data *)of_id->data;
+}
+
 static void exynos_dsi_wait_for_reset(struct exynos_dsi *dsi)
 {
 	if (wait_for_completion_timeout(&dsi->completed, msecs_to_jiffies(300)))
@@ -341,14 +397,9 @@ static unsigned long exynos_dsi_pll_find_pms(struct exynos_dsi *dsi,
 static unsigned long exynos_dsi_set_pll(struct exynos_dsi *dsi,
 					unsigned long freq)
 {
-	static const unsigned long freq_bands[] = {
-		100 * MHZ, 120 * MHZ, 160 * MHZ, 200 * MHZ,
-		270 * MHZ, 320 * MHZ, 390 * MHZ, 450 * MHZ,
-		510 * MHZ, 560 * MHZ, 640 * MHZ, 690 * MHZ,
-		770 * MHZ, 870 * MHZ, 950 * MHZ,
-	};
+	struct exynos_dsi_driver_data *driver_data = dsi->driver_data;
 	unsigned long fin, fout;
-	int timeout, band;
+	int timeout;
 	u8 p, s;
 	u16 m;
 	u32 reg;
@@ -369,18 +420,30 @@ static unsigned long exynos_dsi_set_pll(struct exynos_dsi *dsi,
 			"failed to find PLL PMS for requested frequency\n");
 		return -EFAULT;
 	}
+	dev_dbg(dsi->dev, "PLL freq %lu, (p %d, m %d, s %d)\n", fout, p, m, s);
 
-	for (band = 0; band < ARRAY_SIZE(freq_bands); ++band)
-		if (fout < freq_bands[band])
-			break;
+	writel(500, dsi->reg_base + driver_data->plltmr_reg);
 
-	dev_dbg(dsi->dev, "PLL freq %lu, (p %d, m %d, s %d), band %d\n", fout,
-		p, m, s, band);
+	reg = DSIM_PLL_EN | DSIM_PLL_P(p) | DSIM_PLL_M(m) | DSIM_PLL_S(s);
 
-	writel(500, dsi->reg_base + DSIM_PLLTMR_REG);
+	if (driver_data->has_freqband) {
+		static const unsigned long freq_bands[] = {
+			100 * MHZ, 120 * MHZ, 160 * MHZ, 200 * MHZ,
+			270 * MHZ, 320 * MHZ, 390 * MHZ, 450 * MHZ,
+			510 * MHZ, 560 * MHZ, 640 * MHZ, 690 * MHZ,
+			770 * MHZ, 870 * MHZ, 950 * MHZ,
+		};
+		int band;
+
+		for (band = 0; band < ARRAY_SIZE(freq_bands); ++band)
+			if (fout < freq_bands[band])
+				break;
+
+		dev_dbg(dsi->dev, "band %d\n", band);
+
+		reg |= DSIM_FREQ_BAND(band);
+	}
 
-	reg = DSIM_FREQ_BAND(band) | DSIM_PLL_EN
-			| DSIM_PLL_P(p) | DSIM_PLL_M(m) | DSIM_PLL_S(s);
 	writel(reg, dsi->reg_base + DSIM_PLLCTRL_REG);
 
 	timeout = 1000;
@@ -434,6 +497,59 @@ static int exynos_dsi_enable_clock(struct exynos_dsi *dsi)
 	return 0;
 }
 
+static void exynos_dsi_set_phy_ctrl(struct exynos_dsi *dsi)
+{
+	struct exynos_dsi_driver_data *driver_data = dsi->driver_data;
+	u32 reg;
+
+	if (driver_data->has_freqband)
+		return;
+
+	/* B D-PHY: D-PHY Master & Slave Analog Block control */
+	reg = DSIM_PHYCTRL_ULPS_EXIT(0x0af);
+	writel(reg, dsi->reg_base + DSIM_PHYCTRL_REG);
+
+	/*
+	 * T LPX: Transmitted length of any Low-Power state period
+	 * T HS-EXIT: Time that the transmitter drives LP-11 following a HS
+	 *	burst
+	 */
+	reg = DSIM_PHYTIMING_LPX(0x06) | DSIM_PHYTIMING_HS_EXIT(0x0b);
+	writel(reg, dsi->reg_base + DSIM_PHYTIMING_REG);
+
+	/*
+	 * T CLK-PREPARE: Time that the transmitter drives the Clock Lane LP-00
+	 *	Line state immediately before the HS-0 Line state starting the
+	 *	HS transmission
+	 * T CLK-ZERO: Time that the transmitter drives the HS-0 state prior to
+	 *	transmitting the Clock.
+	 * T CLK_POST: Time that the transmitter continues to send HS clock
+	 *	after the last associated Data Lane has transitioned to LP Mode
+	 *	Interval is defined as the period from the end of T HS-TRAIL to
+	 *	the beginning of T CLK-TRAIL
+	 * T CLK-TRAIL: Time that the transmitter drives the HS-0 state after
+	 *	the last payload clock bit of a HS transmission burst
+	 */
+	reg = DSIM_PHYTIMING1_CLK_PREPARE(0x07) |
+			DSIM_PHYTIMING1_CLK_ZERO(0x27) |
+			DSIM_PHYTIMING1_CLK_POST(0x0d) |
+			DSIM_PHYTIMING1_CLK_TRAIL(0x08);
+	writel(reg, dsi->reg_base + DSIM_PHYTIMING1_REG);
+
+	/*
+	 * T HS-PREPARE: Time that the transmitter drives the Data Lane LP-00
+	 *	Line state immediately before the HS-0 Line state starting the
+	 *	HS transmission
+	 * T HS-ZERO: Time that the transmitter drives the HS-0 state prior to
+	 *	transmitting the Sync sequence.
+	 * T HS-TRAIL: Time that the transmitter drives the flipped differential
+	 *	state after last payload data bit of a HS transmission burst
+	 */
+	reg = DSIM_PHYTIMING2_HS_PREPARE(0x09) | DSIM_PHYTIMING2_HS_ZERO(0x0d) |
+			DSIM_PHYTIMING2_HS_TRAIL(0x0b);
+	writel(reg, dsi->reg_base + DSIM_PHYTIMING2_REG);
+}
+
 static void exynos_dsi_disable_clock(struct exynos_dsi *dsi)
 {
 	u32 reg;
@@ -956,10 +1072,11 @@ static irqreturn_t exynos_dsi_irq(int irq, void *dev_id)
 
 static int exynos_dsi_init(struct exynos_dsi *dsi)
 {
-	exynos_dsi_enable_clock(dsi);
 	exynos_dsi_reset(dsi);
 	enable_irq(dsi->irq);
+	exynos_dsi_enable_clock(dsi);
 	exynos_dsi_wait_for_reset(dsi);
+	exynos_dsi_set_phy_ctrl(dsi);
 	exynos_dsi_init_link(dsi);
 
 	return 0;
@@ -1463,6 +1580,7 @@ static int exynos_dsi_probe(struct platform_device *pdev)
 	dsi->dsi_host.dev = &pdev->dev;
 
 	dsi->dev = &pdev->dev;
+	dsi->driver_data = exynos_dsi_get_driver_data(pdev);
 
 	ret = exynos_dsi_parse_dt(dsi);
 	if (ret)
@@ -1545,11 +1663,6 @@ static int exynos_dsi_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct of_device_id exynos_dsi_of_match[] = {
-	{ .compatible = "samsung,exynos4210-mipi-dsi" },
-	{ }
-};
-
 struct platform_driver dsi_driver = {
 	.probe = exynos_dsi_probe,
 	.remove = exynos_dsi_remove,
-- 
1.9.0

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

* [PATCH v5 09/14] ARM: dts: s6e3fa0: add DT bindings
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
                   ` (7 preceding siblings ...)
  2014-07-08  0:39 ` [PATCH v5 08/14] drm/exynos: dsi: add driver data to support Exynos5410/5420/5440 SoCs YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 10/14] drm/panel: add S6E3FA0 driver YoungJun Cho
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

This patch adds DT bindings for s6e3fa0 panel.
The bindings describes panel resources and display timings.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 .../devicetree/bindings/panel/samsung,s6e3fa0.txt  | 46 ++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt

diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
new file mode 100644
index 0000000..2cd32f5
--- /dev/null
+++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
@@ -0,0 +1,46 @@
+Samsung S6E3FA0 AMOLED LCD 5.7 inch panel
+
+Required properties:
+  - compatible: "samsung,s6e3fa0"
+  - reg: the virtual channel number of a DSI peripheral
+  - vdd3-supply: core voltage supply
+  - vci-supply: voltage supply for analog circuits
+  - reset-gpios: a GPIO spec for the reset pin
+  - det-gpios: a GPIO spec for the OLED detection pin
+  - te-gpios: a GPIO spec for the TE pin
+  - display-timings: timings for the connected panel as described by [1]
+
+Optional properties:
+
+The device node can contain one 'port' child node with one child 'endpoint'
+node, according to the bindings defined in [2]. This node should describe
+panel's video bus.
+
+[1]: Documentation/devicetree/bindings/video/display-timing.txt
+[2]: Documentation/devicetree/bindings/media/video-interfaces.txt
+
+Example:
+
+	panel@0 {
+		compatible = "samsung,s6e3fa0";
+		reg = <0>;
+		vdd3-supply = <&vcclcd_reg>;
+		vci-supply = <&vlcd_reg>;
+		reset-gpios = <&gpy7 4 0>;
+		det-gpios = <&gpg0 6 0>;
+		te-gpios = <&gpd1 7 0>;
+
+		display-timings {
+			timings0 {
+				clock-frequency = <0>;
+				hactive = <1080>;
+				vactive = <1920>;
+				hfront-porch = <2>;
+				hback-porch = <2>;
+				hsync-len = <1>;
+				vfront-porch = <1>;
+				vback-porch = <4>;
+				vsync-len = <1>;
+			};
+		};
+	};
-- 
1.9.0

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

* [PATCH v5 10/14] drm/panel: add S6E3FA0 driver
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
                   ` (8 preceding siblings ...)
  2014-07-08  0:39 ` [PATCH v5 09/14] ARM: dts: s6e3fa0: add DT bindings YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 11/14] ARM: dts: exynos4: add system register property YoungJun Cho
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

This patch adds MIPI DSI command mode based
S6E3FA0 AMOLED LCD Panel driver.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/panel/Kconfig         |   7 +
 drivers/gpu/drm/panel/Makefile        |   1 +
 drivers/gpu/drm/panel/panel-s6e3fa0.c | 569 ++++++++++++++++++++++++++++++++++
 3 files changed, 577 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-s6e3fa0.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 4ec874d..be1392e 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -30,4 +30,11 @@ config DRM_PANEL_S6E8AA0
 	select DRM_MIPI_DSI
 	select VIDEOMODE_HELPERS
 
+config DRM_PANEL_S6E3FA0
+	tristate "S6E3FA0 DSI command mode panel"
+	depends on DRM && DRM_PANEL
+	depends on OF
+	select DRM_MIPI_DSI
+	select VIDEOMODE_HELPERS
+
 endmenu
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 8b92921..85c6738 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
 obj-$(CONFIG_DRM_PANEL_LD9040) += panel-ld9040.o
 obj-$(CONFIG_DRM_PANEL_S6E8AA0) += panel-s6e8aa0.o
+obj-$(CONFIG_DRM_PANEL_S6E3FA0) += panel-s6e3fa0.o
diff --git a/drivers/gpu/drm/panel/panel-s6e3fa0.c b/drivers/gpu/drm/panel/panel-s6e3fa0.c
new file mode 100644
index 0000000..66058a7
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-s6e3fa0.c
@@ -0,0 +1,569 @@
+/*
+ * MIPI DSI command mode based s6e3fa0 AMOLED LCD 5.7 inch drm panel driver.
+ *
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd
+ *
+ * YoungJun Cho <yj44.cho@samsung.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.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_mipi_dsi.h>
+#include <drm/drm_panel.h>
+
+#include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
+
+#include <video/mipi_display.h>
+#include <video/of_videomode.h>
+#include <video/videomode.h>
+
+/* Manufacturer Command Set */
+#define MCS_GLOBAL_PARAMETER	0xb0
+#define MCS_AID			0xb2
+#define MCS_ELVSSOPT		0xb6
+#define MCS_TEMPERATURE_SET	0xb8
+#define MCS_PENTILE_CTRL	0xc0
+#define MCS_GAMMA_MODE		0xca
+#define MCS_VDDM		0xd7
+#define MCS_ALS			0xe3
+#define MCS_ERR_FG		0xed
+#define MCS_KEY_LEV1		0xf0
+#define MCS_GAMMA_UPDATE	0xf7
+#define MCS_KEY_LEV2		0xfc
+#define MCS_RE			0xfe
+#define MCS_TOUT2_HSYNC		0xff
+
+/* Content Adaptive Brightness Control */
+#define DCS_WRITE_CABC		0x55
+
+#define MTP_ID_LEN		3
+#define GAMMA_LEVEL_NUM		30
+
+#define DEFAULT_VDDM_VAL	0x15
+
+struct s6e3fa0 {
+	struct device				*dev;
+	struct drm_panel			panel;
+
+	struct regulator_bulk_data		supplies[2];
+	struct gpio_desc			*reset_gpio;
+	struct gpio_desc			*det_gpio;
+	struct gpio_desc			*te_gpio;
+	struct videomode			vm;
+
+	unsigned int				power_on_delay;
+	unsigned int				reset_delay;
+	unsigned int				init_delay;
+	unsigned int				width_mm;
+	unsigned int				height_mm;
+
+	unsigned char				id;
+	unsigned char				vddm;
+	unsigned int				brightness;
+};
+
+#define panel_to_s6e3fa0(p) container_of(p, struct s6e3fa0, panel)
+
+/* VDD Memory Lookup Table contains pairs of {ReadValue, WriteValue} */
+static const unsigned char s6e3fa0_vddm_lut[][2] = {
+	{0x00, 0x0d}, {0x01, 0x0d}, {0x02, 0x0e}, {0x03, 0x0f}, {0x04, 0x10},
+	{0x05, 0x11}, {0x06, 0x12}, {0x07, 0x13}, {0x08, 0x14}, {0x09, 0x15},
+	{0x0a, 0x16}, {0x0b, 0x17}, {0x0c, 0x18}, {0x0d, 0x19}, {0x0e, 0x1a},
+	{0x0f, 0x1b}, {0x10, 0x1c}, {0x11, 0x1d}, {0x12, 0x1e}, {0x13, 0x1f},
+	{0x14, 0x20}, {0x15, 0x21}, {0x16, 0x22}, {0x17, 0x23}, {0x18, 0x24},
+	{0x19, 0x25}, {0x1a, 0x26}, {0x1b, 0x27}, {0x1c, 0x28}, {0x1d, 0x29},
+	{0x1e, 0x2a}, {0x1f, 0x2b}, {0x20, 0x2c}, {0x21, 0x2d}, {0x22, 0x2e},
+	{0x23, 0x2f}, {0x24, 0x30}, {0x25, 0x31}, {0x26, 0x32}, {0x27, 0x33},
+	{0x28, 0x34}, {0x29, 0x35}, {0x2a, 0x36}, {0x2b, 0x37}, {0x2c, 0x38},
+	{0x2d, 0x39}, {0x2e, 0x3a}, {0x2f, 0x3b}, {0x30, 0x3c}, {0x31, 0x3d},
+	{0x32, 0x3e}, {0x33, 0x3f}, {0x34, 0x3f}, {0x35, 0x3f}, {0x36, 0x3f},
+	{0x37, 0x3f}, {0x38, 0x3f}, {0x39, 0x3f}, {0x3a, 0x3f}, {0x3b, 0x3f},
+	{0x3c, 0x3f}, {0x3d, 0x3f}, {0x3e, 0x3f}, {0x3f, 0x3f}, {0x40, 0x0c},
+	{0x41, 0x0b}, {0x42, 0x0a}, {0x43, 0x09}, {0x44, 0x08}, {0x45, 0x07},
+	{0x46, 0x06}, {0x47, 0x05}, {0x48, 0x04}, {0x49, 0x03}, {0x4a, 0x02},
+	{0x4b, 0x01}, {0x4c, 0x40}, {0x4d, 0x41}, {0x4e, 0x42}, {0x4f, 0x43},
+	{0x50, 0x44}, {0x51, 0x45}, {0x52, 0x46}, {0x53, 0x47}, {0x54, 0x48},
+	{0x55, 0x49}, {0x56, 0x4a}, {0x57, 0x4b}, {0x58, 0x4c}, {0x59, 0x4d},
+	{0x5a, 0x4e}, {0x5b, 0x4f}, {0x5c, 0x50}, {0x5d, 0x51}, {0x5e, 0x52},
+	{0x5f, 0x53}, {0x60, 0x54}, {0x61, 0x55}, {0x62, 0x56}, {0x63, 0x57},
+	{0x64, 0x58}, {0x65, 0x59}, {0x66, 0x5a}, {0x67, 0x5b}, {0x68, 0x5c},
+	{0x69, 0x5d}, {0x6a, 0x5e}, {0x6b, 0x5f}, {0x6c, 0x60}, {0x6d, 0x61},
+	{0x6e, 0x62}, {0x6f, 0x63}, {0x70, 0x64}, {0x71, 0x65}, {0x72, 0x66},
+	{0x73, 0x67}, {0x74, 0x68}, {0x75, 0x69}, {0x76, 0x6a}, {0x77, 0x6b},
+	{0x78, 0x6c}, {0x79, 0x6d}, {0x7a, 0x6e}, {0x7b, 0x6f}, {0x7c, 0x70},
+	{0x7d, 0x71}, {0x7e, 0x72}, {0x7f, 0x73},
+};
+
+static int s6e3fa0_dcs_read(struct s6e3fa0 *ctx, unsigned char cmd,
+							void *data, size_t len)
+{
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+
+	return mipi_dsi_dcs_read(dsi, dsi->channel, cmd, data, len);
+}
+
+static void s6e3fa0_dcs_write(struct s6e3fa0 *ctx, const void *data, size_t len)
+{
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+
+	mipi_dsi_dcs_write(dsi, dsi->channel, data, len);
+}
+
+#define s6e3fa0_dcs_write_seq(ctx, seq...)				\
+do {									\
+	const unsigned char d[] = { seq };				\
+	BUILD_BUG_ON_MSG(ARRAY_SIZE(d) > 64, "too big seq for stack");	\
+	s6e3fa0_dcs_write(ctx, d, ARRAY_SIZE(d));			\
+} while (0)
+
+#define s6e3fa0_dcs_write_seq_static(ctx, seq...)			\
+do {									\
+	static const unsigned char d[] = { seq };			\
+	s6e3fa0_dcs_write(ctx, d, ARRAY_SIZE(d));			\
+} while (0)
+
+static void s6e3fa0_apply_level_1_key(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_dcs_write_seq_static(ctx, MCS_KEY_LEV1, 0x5a, 0x5a);
+}
+
+static void s6e3fa0_apply_level_2_key(struct s6e3fa0 *ctx, bool on)
+{
+	if (on)
+		s6e3fa0_dcs_write_seq_static(ctx, MCS_KEY_LEV2, 0x5a, 0x5a);
+	else
+		s6e3fa0_dcs_write_seq_static(ctx, MCS_KEY_LEV2, 0xa5, 0xa5);
+}
+
+static void s6e3fa0_set_maximum_return_packet_size(struct s6e3fa0 *ctx,
+							unsigned int size)
+{
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
+
+	if (ops && ops->transfer) {
+		unsigned char buf[] = {size, 0};
+		struct mipi_dsi_msg msg = {
+			.channel = dsi->channel,
+			.type = MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE,
+			.tx_len = sizeof(buf),
+			.tx_buf = buf
+		};
+
+		ops->transfer(dsi->host, &msg);
+	}
+}
+
+static void s6e3fa0_read_mtp_id(struct s6e3fa0 *ctx)
+{
+	unsigned char id[MTP_ID_LEN];
+	int ret;
+
+	s6e3fa0_set_maximum_return_packet_size(ctx, MTP_ID_LEN);
+	ret = s6e3fa0_dcs_read(ctx, MIPI_DCS_GET_DISPLAY_ID, id, MTP_ID_LEN);
+	if (ret < MTP_ID_LEN || id[0] == 0x00) {
+		dev_err(ctx->dev, "failed to read id\n");
+		return;
+	}
+
+	dev_info(ctx->dev, "ID: 0x%02x, 0x%02x, 0x%02x\n", id[0], id[1], id[2]);
+
+	ctx->id = id[2];
+}
+
+static void s6e3fa0_read_vddm(struct s6e3fa0 *ctx)
+{
+	unsigned char vddm;
+	int ret;
+
+	s6e3fa0_apply_level_2_key(ctx, true);
+	s6e3fa0_dcs_write_seq_static(ctx, MCS_GLOBAL_PARAMETER, 0x16);
+	s6e3fa0_set_maximum_return_packet_size(ctx, 1);
+	ret = s6e3fa0_dcs_read(ctx, MCS_VDDM, &vddm, 1);
+	s6e3fa0_apply_level_2_key(ctx, false);
+
+	if (ret < 1 || vddm > 0x7f) {
+		dev_err(ctx->dev, "failed to read vddm, use default val.\n");
+		vddm = DEFAULT_VDDM_VAL;
+	}
+
+	ctx->vddm = s6e3fa0_vddm_lut[vddm][1];
+}
+
+static void s6e3fa0_set_pentile_control(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_dcs_write_seq_static(ctx, MCS_PENTILE_CTRL, 0x00, 0x02, 0x03,
+					0x32, 0x03, 0x44, 0x44, 0xc0, 0x00,
+					0x1c, 0x20, 0xe8);
+}
+
+static void s6e3fa0_write_ambient_light_sensor(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_dcs_write_seq_static(ctx, MCS_ALS, 0xff, 0xff, 0xff, 0xff);
+}
+
+static void s6e3fa0_set_readability_enhancement(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_dcs_write_seq_static(ctx, MCS_RE, 0x00, 0x03,
+					MCS_GLOBAL_PARAMETER, 0x2b,
+					MCS_RE, 0xe4);
+}
+
+static void s6e3fa0_set_common_control(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_set_pentile_control(ctx);
+	s6e3fa0_write_ambient_light_sensor(ctx);
+	s6e3fa0_set_readability_enhancement(ctx);
+}
+
+static void s6e3fa0_set_gamma(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_dcs_write_seq_static(ctx, MCS_GAMMA_MODE, 0x01, 0x00, 0x01,
+					0x00, 0x01, 0x00, 0x80, 0x80, 0x80,
+					0x80, 0x80, 0x80, 0x80, 0x80, 0x80,
+					0x80, 0x80, 0x80, 0x80, 0x80, 0x80,
+					0x80, 0x80, 0x80, 0x80, 0x80, 0x80,
+					0x80, 0x80, 0x80, 0x00, 0x00, 0x00);
+}
+
+static void s6e3fa0_set_amoled_impulse_driving(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_dcs_write_seq_static(ctx, MCS_AID, 0x00, 0x06, 0x00, 0x06);
+}
+
+static void s6e3fa0_set_elvss(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_dcs_write_seq_static(ctx, MCS_ELVSSOPT, 0x88, 0x0a);
+}
+
+static void s6e3fa0_update_gamma(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_dcs_write_seq_static(ctx, MCS_GAMMA_UPDATE, 0x03);
+}
+
+static void s6e3fa0_write_automatic_current_limit(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_dcs_write_seq_static(ctx, DCS_WRITE_CABC, 0x02);
+}
+
+static void s6e3fa0_set_brightness_control(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_set_gamma(ctx);
+	s6e3fa0_set_amoled_impulse_driving(ctx);
+	s6e3fa0_set_elvss(ctx);
+	s6e3fa0_update_gamma(ctx);
+	s6e3fa0_write_automatic_current_limit(ctx);
+}
+
+static void s6e3fa0_set_temperature(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_dcs_write_seq_static(ctx, MCS_GLOBAL_PARAMETER, 0x05,
+					MCS_TEMPERATURE_SET, 0x19);
+}
+
+static void s6e3fa0_set_elvss_control(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_set_temperature(ctx);
+	s6e3fa0_set_elvss(ctx);
+}
+
+static void s6e3fa0_set_te_on(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_dcs_write_seq_static(ctx, MIPI_DCS_SET_TEAR_ON, 0x00);
+}
+
+static void s6e3fa0_set_etc_and_write_vddm(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_apply_level_2_key(ctx, true);
+	s6e3fa0_dcs_write_seq(ctx, MCS_ERR_FG, 0x0c, 0x04,
+				MCS_TOUT2_HSYNC, 0x01,
+				MCS_GLOBAL_PARAMETER, 0x16,
+				MCS_VDDM, ctx->vddm);
+	s6e3fa0_apply_level_2_key(ctx, false);
+}
+
+static void s6e3fa0_set_etc_condition(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_set_te_on(ctx);
+	s6e3fa0_set_etc_and_write_vddm(ctx);
+}
+
+static void s6e3fa0_panel_init(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_set_common_control(ctx);
+	s6e3fa0_set_brightness_control(ctx);
+	s6e3fa0_set_elvss_control(ctx);
+	s6e3fa0_set_etc_condition(ctx);
+
+	msleep(ctx->init_delay);
+}
+
+static int s6e3fa0_power_off(struct s6e3fa0 *ctx)
+{
+	gpiod_set_value(ctx->reset_gpio, 0);
+
+	return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+}
+
+static int s6e3fa0_power_on(struct s6e3fa0 *ctx)
+{
+	int ret;
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+	if (ret)
+		return ret;
+
+	msleep(ctx->power_on_delay);
+
+	gpiod_set_value(ctx->reset_gpio, 1);
+	gpiod_set_value(ctx->reset_gpio, 0);
+	usleep_range(1000, 2000);
+	gpiod_set_value(ctx->reset_gpio, 1);
+
+	msleep(ctx->reset_delay);
+
+	return 0;
+}
+
+static void s6e3fa0_set_sequence(struct s6e3fa0 *ctx)
+{
+	s6e3fa0_apply_level_1_key(ctx);
+	s6e3fa0_dcs_write_seq_static(ctx, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(20);
+
+	s6e3fa0_read_mtp_id(ctx);
+	s6e3fa0_read_vddm(ctx);
+
+	s6e3fa0_panel_init(ctx);
+
+	s6e3fa0_dcs_write_seq_static(ctx, MIPI_DCS_SET_DISPLAY_ON);
+}
+
+static int s6e3fa0_disable(struct drm_panel *panel)
+{
+	struct s6e3fa0 *ctx = panel_to_s6e3fa0(panel);
+
+	s6e3fa0_dcs_write_seq_static(ctx, MIPI_DCS_SET_DISPLAY_OFF);
+	msleep(35);
+	s6e3fa0_dcs_write_seq_static(ctx, MIPI_DCS_ENTER_SLEEP_MODE);
+	msleep(120);
+
+	return s6e3fa0_power_off(ctx);
+}
+
+static int s6e3fa0_enable(struct drm_panel *panel)
+{
+	struct s6e3fa0 *ctx = panel_to_s6e3fa0(panel);
+	int ret;
+
+	ret = s6e3fa0_power_on(ctx);
+	if (ret)
+		return ret;
+
+	s6e3fa0_set_sequence(ctx);
+
+	return ret;
+}
+
+static int s6e3fa0_get_modes(struct drm_panel *panel)
+{
+	struct drm_connector *connector = panel->connector;
+	struct s6e3fa0 *ctx = panel_to_s6e3fa0(panel);
+	struct drm_display_mode *mode;
+
+	mode = drm_mode_create(connector->dev);
+	if (!mode) {
+		DRM_ERROR("failed to create a new display mode\n");
+		return 0;
+	}
+
+	drm_display_mode_from_videomode(&ctx->vm, mode);
+	mode->width_mm = ctx->width_mm;
+	mode->height_mm = ctx->height_mm;
+	connector->display_info.width_mm = mode->width_mm;
+	connector->display_info.height_mm = mode->height_mm;
+
+	mode->type = DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED;
+	drm_mode_probed_add(connector, mode);
+
+	return 1;
+}
+
+static const struct drm_panel_funcs s6e3fa0_drm_funcs = {
+	.disable	= s6e3fa0_disable,
+	.enable		= s6e3fa0_enable,
+	.get_modes	= s6e3fa0_get_modes,
+};
+
+static int s6e3fa0_parse_dt(struct s6e3fa0 *ctx)
+{
+	struct device *dev = ctx->dev;
+	int ret;
+
+	ret = of_get_videomode(dev->of_node, &ctx->vm, 0);
+	if (ret) {
+		dev_err(dev, "failed to get video mode: %d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+irqreturn_t s6e3fa0_det_interrupt(int irq, void *dev_id)
+{
+	/* TODO */
+	return IRQ_HANDLED;
+}
+
+irqreturn_t s6e3fa0_te_interrupt(int irq, void *dev_id)
+{
+	struct s6e3fa0 *ctx = dev_id;
+	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
+	struct mipi_dsi_host *host = dsi->host;
+	const struct mipi_dsi_host_ops *ops = host->ops;
+
+	if (ops && ops->pass_te)
+		ops->pass_te(host);
+
+	return IRQ_HANDLED;
+}
+
+static int s6e3fa0_probe(struct mipi_dsi_device *dsi)
+{
+	struct device *dev = &dsi->dev;
+	struct s6e3fa0 *ctx;
+	int ret;
+
+	ctx = devm_kzalloc(dev, sizeof(struct s6e3fa0), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	mipi_dsi_set_drvdata(dsi, ctx);
+
+	ctx->dev = dev;
+
+	dsi->lanes = 4;
+	dsi->format = MIPI_DSI_FMT_RGB888;
+
+	ret = s6e3fa0_parse_dt(ctx);
+	if (ret)
+		return ret;
+
+	ctx->supplies[0].supply = "vdd3";
+	ctx->supplies[1].supply = "vci";
+	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ctx->supplies),
+					ctx->supplies);
+	if (ret) {
+		dev_err(dev, "failed to get regulators: %d\n", ret);
+		return ret;
+	}
+
+	ctx->reset_gpio = devm_gpiod_get(dev, "reset");
+	if (IS_ERR(ctx->reset_gpio)) {
+		dev_err(dev, "failed to get reset gpio: %ld\n",
+						PTR_ERR(ctx->reset_gpio));
+		return PTR_ERR(ctx->reset_gpio);
+	}
+	ret = gpiod_direction_output(ctx->reset_gpio, 1);
+	if (ret < 0) {
+		dev_err(dev, "failed to configure reset gpio: %d\n", ret);
+		return ret;
+	}
+
+	ctx->det_gpio = devm_gpiod_get(dev, "det");
+	if (IS_ERR(ctx->det_gpio)) {
+		dev_err(dev, "failed to get det gpio: %ld\n",
+						PTR_ERR(ctx->det_gpio));
+		return PTR_ERR(ctx->det_gpio);
+	}
+	ret = gpiod_direction_input(ctx->det_gpio);
+	if (ret < 0) {
+		dev_err(dev, "failed to configure det gpio: %d\n", ret);
+		return ret;
+	}
+	ret = devm_request_irq(dev, gpiod_to_irq(ctx->det_gpio),
+						s6e3fa0_det_interrupt,
+						IRQF_TRIGGER_FALLING,
+						"oled-det", ctx);
+	if (ret) {
+		dev_err(dev, "failed to request det irq: %d\n", ret);
+		return ret;
+	}
+
+	ctx->te_gpio = devm_gpiod_get(dev, "te");
+	if (IS_ERR(ctx->te_gpio)) {
+		dev_err(dev, "failed to get te gpio: %ld\n",
+						PTR_ERR(ctx->te_gpio));
+		return PTR_ERR(ctx->te_gpio);
+	}
+	ret = gpiod_direction_input(ctx->te_gpio);
+	if (ret < 0) {
+		dev_err(dev, "failed to configure te gpio: %d\n", ret);
+		return ret;
+	}
+	ret = devm_request_irq(dev, gpiod_to_irq(ctx->te_gpio),
+						s6e3fa0_te_interrupt,
+						IRQF_TRIGGER_RISING,
+						"TE", ctx);
+	if (ret) {
+		dev_err(dev, "failed to request det irq: %d\n", ret);
+		return ret;
+	}
+
+	ctx->power_on_delay = 10;
+	ctx->reset_delay = 5;
+	ctx->init_delay = 120;
+	ctx->width_mm = 71;
+	ctx->height_mm = 126;
+
+	ctx->brightness = GAMMA_LEVEL_NUM - 1;
+
+	drm_panel_init(&ctx->panel);
+	ctx->panel.dev = dev;
+	ctx->panel.funcs = &s6e3fa0_drm_funcs;
+
+	ret = drm_panel_add(&ctx->panel);
+	if (ret)
+		return ret;
+
+	ret = mipi_dsi_attach(dsi);
+	if (ret)
+		drm_panel_remove(&ctx->panel);
+
+	return ret;
+}
+
+static int s6e3fa0_remove(struct mipi_dsi_device *dsi)
+{
+	struct s6e3fa0 *ctx = mipi_dsi_get_drvdata(dsi);
+
+	mipi_dsi_detach(dsi);
+	drm_panel_remove(&ctx->panel);
+
+	return 0;
+}
+
+static struct of_device_id s6e3fa0_of_match[] = {
+	{ .compatible = "samsung,s6e3fa0" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, s6e3fa0_of_match);
+
+static struct mipi_dsi_driver s6e3fa0_driver = {
+	.probe = s6e3fa0_probe,
+	.remove = s6e3fa0_remove,
+	.driver = {
+		.name = "panel_s6e3fa0",
+		.owner = THIS_MODULE,
+		.of_match_table = s6e3fa0_of_match,
+	},
+};
+module_mipi_dsi_driver(s6e3fa0_driver);
+
+MODULE_AUTHOR("YoungJun Cho <yj44.cho@samsung.com>");
+MODULE_DESCRIPTION("MIPI DSI command mode based s6e3fa0 AMOLED LCD Driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.0

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

* [PATCH v5 11/14] ARM: dts: exynos4: add system register property
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
                   ` (9 preceding siblings ...)
  2014-07-08  0:39 ` [PATCH v5 10/14] drm/panel: add S6E3FA0 driver YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 12/14] ARM: dts: exynos5: " YoungJun Cho
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

This patch adds sysreg property to fimd device node
which is required to use I80 interface.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos4.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index b8ece4b..92ee786 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -608,6 +608,7 @@
 		clocks = <&clock CLK_SCLK_FIMD0>, <&clock CLK_FIMD0>;
 		clock-names = "sclk_fimd", "fimd";
 		samsung,power-domain = <&pd_lcd0>;
+		samsung,sysreg = <&sys_reg>;
 		status = "disabled";
 	};
 };
-- 
1.9.0

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

* [PATCH v5 12/14] ARM: dts: exynos5: add system register property
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
                   ` (10 preceding siblings ...)
  2014-07-08  0:39 ` [PATCH v5 11/14] ARM: dts: exynos4: add system register property YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 13/14] ARM: dts: exynos5420: add mipi-phy node YoungJun Cho
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

This patch adds sysreg property to fimd device node
which is required to use I80 interface.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos5.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos5.dtsi b/arch/arm/boot/dts/exynos5.dtsi
index 79d0608..fdead12 100644
--- a/arch/arm/boot/dts/exynos5.dtsi
+++ b/arch/arm/boot/dts/exynos5.dtsi
@@ -87,6 +87,7 @@
 		reg = <0x14400000 0x40000>;
 		interrupt-names = "fifo", "vsync", "lcd_sys";
 		interrupts = <18 4>, <18 5>, <18 6>;
+		samsung,sysreg = <&sysreg_system_controller>;
 		status = "disabled";
 	};
 
-- 
1.9.0

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

* [PATCH v5 13/14] ARM: dts: exynos5420: add mipi-phy node
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
                   ` (11 preceding siblings ...)
  2014-07-08  0:39 ` [PATCH v5 12/14] ARM: dts: exynos5: " YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-08  0:39 ` [PATCH v5 14/14] ARM: dts: exynos5420: add dsi node YoungJun Cho
  2014-07-09 15:07 ` [PATCH v5 00/14] drm/exynos: support LCD I80 interface display Inki Dae
  14 siblings, 0 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

This patch adds mipi-phy node for MIPI DSI device.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos5420.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index e385322..0b9d15d 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -517,6 +517,12 @@
 		phy-names = "dp";
 	};
 
+	mipi_phy: video-phy@10040714 {
+		compatible = "samsung,s5pv210-mipi-video-phy";
+		reg = <0x10040714 12>;
+		#phy-cells = <1>;
+	};
+
 	fimd: fimd@14400000 {
 		samsung,power-domain = <&disp_pd>;
 		clocks = <&clock CLK_SCLK_FIMD1>, <&clock CLK_FIMD1>;
-- 
1.9.0

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

* [PATCH v5 14/14] ARM: dts: exynos5420: add dsi node
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
                   ` (12 preceding siblings ...)
  2014-07-08  0:39 ` [PATCH v5 13/14] ARM: dts: exynos5420: add mipi-phy node YoungJun Cho
@ 2014-07-08  0:39 ` YoungJun Cho
  2014-07-09 15:07 ` [PATCH v5 00/14] drm/exynos: support LCD I80 interface display Inki Dae
  14 siblings, 0 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-08  0:39 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

This patch adds common part of dsi node.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/boot/dts/exynos5420.dtsi | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 0b9d15d..3a7862b 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -523,6 +523,20 @@
 		#phy-cells = <1>;
 	};
 
+	dsi@14500000 {
+		compatible = "samsung,exynos5410-mipi-dsi";
+		reg = <0x14500000 0x10000>;
+		interrupts = <0 82 0>;
+		samsung,power-domain = <&disp_pd>;
+		phys = <&mipi_phy 1>;
+		phy-names = "dsim";
+		clocks = <&clock CLK_DSIM1>, <&clock CLK_SCLK_MIPI1>;
+		clock-names = "bus_clk", "pll_clk";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
+
 	fimd: fimd@14400000 {
 		samsung,power-domain = <&disp_pd>;
 		clocks = <&clock CLK_SCLK_FIMD1>, <&clock CLK_FIMD1>;
-- 
1.9.0

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

* Re: [PATCH v5 06/14] drm/exynos: fimd: support LCD I80 interface
  2014-07-08  0:39 ` [PATCH v5 06/14] drm/exynos: fimd: " YoungJun Cho
@ 2014-07-08 16:10   ` Inki Dae
  2014-07-09  0:07     ` YoungJun Cho
  0 siblings, 1 reply; 34+ messages in thread
From: Inki Dae @ 2014-07-08 16:10 UTC (permalink / raw)
  To: YoungJun Cho
  Cc: airlied, dri-devel, mark.rutland, devicetree, linux-samsung-soc,
	pawel.moll, ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park,
	robh+dt, galak, kgene.kim

2014-07-08 9:39 GMT+09:00 YoungJun Cho <yj44.cho@samsung.com>:
> To support MIPI command mode based I80 interface panel,
> FIMD should do followings:
> - Sets LCD I80 interface timings configuration.
> - Uses "lcd_sys" as an IRQ resource and sets relevant IRQ configuration.
> - Sets LCD block configuration for I80 interface.
> - Sets ideal(pixel) clock is 2 times faster than the original one
>   to generate frame done IRQ prior to the next TE signal.
> - Implements trigger feature that transfers image data if there is page
>   flip request, and implements TE handler to call trigger function.
>
> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> Acked-by: Inki Dae <inki.dae@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/gpu/drm/exynos/Kconfig           |   1 +
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 276 ++++++++++++++++++++++++++-----
>  include/video/samsung_fimd.h             |   3 +-
>  3 files changed, 235 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
> index 178d2a9..9ba1aae 100644
> --- a/drivers/gpu/drm/exynos/Kconfig
> +++ b/drivers/gpu/drm/exynos/Kconfig
> @@ -28,6 +28,7 @@ config DRM_EXYNOS_FIMD
>         bool "Exynos DRM FIMD"
>         depends on DRM_EXYNOS && !FB_S3C
>         select FB_MODE_HELPERS
> +       select MFD_SYSCON
>         help
>           Choose this option if you want to use Exynos FIMD for DRM.
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 33161ad..207872d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -20,6 +20,8 @@
>  #include <linux/of_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/component.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
>
>  #include <video/of_display_timing.h>
>  #include <video/of_videomode.h>
> @@ -61,6 +63,24 @@
>  /* color key value register for hardware window 1 ~ 4. */
>  #define WKEYCON1_BASE(x)               ((WKEYCON1 + 0x140) + ((x - 1) * 8))
>
> +/* I80 / RGB trigger control register */
> +#define TRIGCON                                0x1A4
> +#define TRGMODE_I80_RGB_ENABLE_I80     (1 << 0)
> +#define SWTRGCMD_I80_RGB_ENABLE                (1 << 1)
> +
> +/* display mode change control register except exynos4 */
> +#define VIDOUT_CON                     0x000
> +#define VIDOUT_CON_F_I80_LDI0          (0x2 << 8)
> +
> +/* I80 interface control for main LDI register */
> +#define I80IFCONFAx(x)                 (0x1B0 + (x) * 4)
> +#define I80IFCONFBx(x)                 (0x1B8 + (x) * 4)
> +#define LCD_CS_SETUP(x)                        ((x) << 16)
> +#define LCD_WR_SETUP(x)                        ((x) << 12)
> +#define LCD_WR_ACTIVE(x)               ((x) << 8)
> +#define LCD_WR_HOLD(x)                 ((x) << 4)
> +#define I80IFEN_ENABLE                 (1 << 0)
> +
>  /* FIMD has totally five hardware windows. */
>  #define WINDOWS_NR     5
>
> @@ -68,10 +88,14 @@
>
>  struct fimd_driver_data {
>         unsigned int timing_base;
> +       unsigned int lcdblk_offset;
> +       unsigned int lcdblk_vt_shift;
> +       unsigned int lcdblk_bypass_shift;
>
>         unsigned int has_shadowcon:1;
>         unsigned int has_clksel:1;
>         unsigned int has_limited_fmt:1;
> +       unsigned int has_vidoutcon:1;
>  };
>
>  static struct fimd_driver_data s3c64xx_fimd_driver_data = {
> @@ -82,12 +106,19 @@ static struct fimd_driver_data s3c64xx_fimd_driver_data = {
>
>  static struct fimd_driver_data exynos4_fimd_driver_data = {
>         .timing_base = 0x0,
> +       .lcdblk_offset = 0x210,
> +       .lcdblk_vt_shift = 10,
> +       .lcdblk_bypass_shift = 1,
>         .has_shadowcon = 1,
>  };
>
>  static struct fimd_driver_data exynos5_fimd_driver_data = {
>         .timing_base = 0x20000,
> +       .lcdblk_offset = 0x214,
> +       .lcdblk_vt_shift = 24,
> +       .lcdblk_bypass_shift = 15,
>         .has_shadowcon = 1,
> +       .has_vidoutcon = 1,
>  };
>
>  struct fimd_win_data {
> @@ -112,15 +143,22 @@ struct fimd_context {
>         struct clk                      *bus_clk;
>         struct clk                      *lcd_clk;
>         void __iomem                    *regs;
> +       struct regmap                   *sysreg;
>         struct drm_display_mode         mode;
>         struct fimd_win_data            win_data[WINDOWS_NR];
>         unsigned int                    default_win;
>         unsigned long                   irq_flags;
> +       u32                             vidcon0;
>         u32                             vidcon1;
> +       u32                             vidout_con;
> +       u32                             i80ifcon;
> +       bool                            i80_if;
>         bool                            suspended;
>         int                             pipe;
>         wait_queue_head_t               wait_vsync_queue;
>         atomic_t                        wait_vsync_event;
> +       atomic_t                        win_updated;
> +       atomic_t                        triggering;
>
>         struct exynos_drm_panel_info panel;
>         struct fimd_driver_data *driver_data;
> @@ -243,6 +281,14 @@ static u32 fimd_calc_clkdiv(struct fimd_context *ctx,
>         unsigned long ideal_clk = mode->htotal * mode->vtotal * mode->vrefresh;
>         u32 clkdiv;
>
> +       if (ctx->i80_if) {
> +               /*
> +                * The frame done interrupt should be occurred prior to the
> +                * next TE signal.
> +                */
> +               ideal_clk *= 2;
> +       }
> +
>         /* Find the clock divider value that gets us closest to ideal_clk */
>         clkdiv = DIV_ROUND_UP(clk_get_rate(ctx->lcd_clk), ideal_clk);
>
> @@ -271,11 +317,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
>  {
>         struct fimd_context *ctx = mgr->ctx;
>         struct drm_display_mode *mode = &ctx->mode;
> -       struct fimd_driver_data *driver_data;
> -       u32 val, clkdiv, vidcon1;
> -       int vsync_len, vbpd, vfpd, hsync_len, hbpd, hfpd;
> +       struct fimd_driver_data *driver_data = ctx->driver_data;
> +       void *timing_base = ctx->regs + driver_data->timing_base;
> +       u32 val, clkdiv;
>
> -       driver_data = ctx->driver_data;
>         if (ctx->suspended)
>                 return;
>
> @@ -283,33 +328,65 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
>         if (mode->htotal == 0 || mode->vtotal == 0)
>                 return;
>
> -       /* setup polarity values */
> -       vidcon1 = ctx->vidcon1;
> -       if (mode->flags & DRM_MODE_FLAG_NVSYNC)
> -               vidcon1 |= VIDCON1_INV_VSYNC;
> -       if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> -               vidcon1 |= VIDCON1_INV_HSYNC;
> -       writel(vidcon1, ctx->regs + driver_data->timing_base + VIDCON1);
> -
> -       /* setup vertical timing values. */
> -       vsync_len = mode->crtc_vsync_end - mode->crtc_vsync_start;
> -       vbpd = mode->crtc_vtotal - mode->crtc_vsync_end;
> -       vfpd = mode->crtc_vsync_start - mode->crtc_vdisplay;
> -
> -       val = VIDTCON0_VBPD(vbpd - 1) |
> -               VIDTCON0_VFPD(vfpd - 1) |
> -               VIDTCON0_VSPW(vsync_len - 1);
> -       writel(val, ctx->regs + driver_data->timing_base + VIDTCON0);
> -
> -       /* setup horizontal timing values.  */
> -       hsync_len = mode->crtc_hsync_end - mode->crtc_hsync_start;
> -       hbpd = mode->crtc_htotal - mode->crtc_hsync_end;
> -       hfpd = mode->crtc_hsync_start - mode->crtc_hdisplay;
> -
> -       val = VIDTCON1_HBPD(hbpd - 1) |
> -               VIDTCON1_HFPD(hfpd - 1) |
> -               VIDTCON1_HSPW(hsync_len - 1);
> -       writel(val, ctx->regs + driver_data->timing_base + VIDTCON1);
> +       if (ctx->i80_if) {
> +               val = ctx->i80ifcon | I80IFEN_ENABLE;
> +               writel(val, timing_base + I80IFCONFAx(0));
> +
> +               /* disable auto frame rate */
> +               writel(0, timing_base + I80IFCONFBx(0));
> +
> +               if (ctx->vidout_con)
> +                       writel(ctx->vidout_con, timing_base + VIDOUT_CON);

VIDOUT_CON register should be set in case of video mode also if
has_vidoutcon is 1. Can you re-send only this patch fixing it up?

Thanks,
Inki Dae

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

* [PATCH v5 06/14] drm/exynos: fimd: support LCD I80 interface
  2014-07-08 16:10   ` Inki Dae
@ 2014-07-09  0:07     ` YoungJun Cho
  0 siblings, 0 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-09  0:07 UTC (permalink / raw)
  To: airlied, dri-devel
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, a.hajda, kyungmin.park, robh+dt,
	galak, kgene.kim

To support MIPI command mode based I80 interface panel,
FIMD should do followings:
- Sets LCD I80 interface timings configuration.
- Uses "lcd_sys" as an IRQ resource and sets relevant IRQ configuration.
- Sets LCD block configuration for I80 interface.
- Sets ideal(pixel) clock is 2 times faster than the original one
  to generate frame done IRQ prior to the next TE signal.
- Implements trigger feature that transfers image data if there is page
  flip request, and implements TE handler to call trigger function.

Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
Acked-by: Inki Dae <inki.dae@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/gpu/drm/exynos/Kconfig           |   1 +
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 276 ++++++++++++++++++++++++++-----
 include/video/samsung_fimd.h             |   3 +-
 3 files changed, 235 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index 178d2a9..9ba1aae 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -28,6 +28,7 @@ config DRM_EXYNOS_FIMD
 	bool "Exynos DRM FIMD"
 	depends on DRM_EXYNOS && !FB_S3C
 	select FB_MODE_HELPERS
+	select MFD_SYSCON
 	help
 	  Choose this option if you want to use Exynos FIMD for DRM.
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 33161ad..28a3168 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -20,6 +20,8 @@
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/component.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
 
 #include <video/of_display_timing.h>
 #include <video/of_videomode.h>
@@ -61,6 +63,24 @@
 /* color key value register for hardware window 1 ~ 4. */
 #define WKEYCON1_BASE(x)		((WKEYCON1 + 0x140) + ((x - 1) * 8))
 
+/* I80 / RGB trigger control register */
+#define TRIGCON				0x1A4
+#define TRGMODE_I80_RGB_ENABLE_I80	(1 << 0)
+#define SWTRGCMD_I80_RGB_ENABLE		(1 << 1)
+
+/* display mode change control register except exynos4 */
+#define VIDOUT_CON			0x000
+#define VIDOUT_CON_F_I80_LDI0		(0x2 << 8)
+
+/* I80 interface control for main LDI register */
+#define I80IFCONFAx(x)			(0x1B0 + (x) * 4)
+#define I80IFCONFBx(x)			(0x1B8 + (x) * 4)
+#define LCD_CS_SETUP(x)			((x) << 16)
+#define LCD_WR_SETUP(x)			((x) << 12)
+#define LCD_WR_ACTIVE(x)		((x) << 8)
+#define LCD_WR_HOLD(x)			((x) << 4)
+#define I80IFEN_ENABLE			(1 << 0)
+
 /* FIMD has totally five hardware windows. */
 #define WINDOWS_NR	5
 
@@ -68,10 +88,14 @@
 
 struct fimd_driver_data {
 	unsigned int timing_base;
+	unsigned int lcdblk_offset;
+	unsigned int lcdblk_vt_shift;
+	unsigned int lcdblk_bypass_shift;
 
 	unsigned int has_shadowcon:1;
 	unsigned int has_clksel:1;
 	unsigned int has_limited_fmt:1;
+	unsigned int has_vidoutcon:1;
 };
 
 static struct fimd_driver_data s3c64xx_fimd_driver_data = {
@@ -82,12 +106,19 @@ static struct fimd_driver_data s3c64xx_fimd_driver_data = {
 
 static struct fimd_driver_data exynos4_fimd_driver_data = {
 	.timing_base = 0x0,
+	.lcdblk_offset = 0x210,
+	.lcdblk_vt_shift = 10,
+	.lcdblk_bypass_shift = 1,
 	.has_shadowcon = 1,
 };
 
 static struct fimd_driver_data exynos5_fimd_driver_data = {
 	.timing_base = 0x20000,
+	.lcdblk_offset = 0x214,
+	.lcdblk_vt_shift = 24,
+	.lcdblk_bypass_shift = 15,
 	.has_shadowcon = 1,
+	.has_vidoutcon = 1,
 };
 
 struct fimd_win_data {
@@ -112,15 +143,22 @@ struct fimd_context {
 	struct clk			*bus_clk;
 	struct clk			*lcd_clk;
 	void __iomem			*regs;
+	struct regmap			*sysreg;
 	struct drm_display_mode		mode;
 	struct fimd_win_data		win_data[WINDOWS_NR];
 	unsigned int			default_win;
 	unsigned long			irq_flags;
+	u32				vidcon0;
 	u32				vidcon1;
+	u32				vidout_con;
+	u32				i80ifcon;
+	bool				i80_if;
 	bool				suspended;
 	int				pipe;
 	wait_queue_head_t		wait_vsync_queue;
 	atomic_t			wait_vsync_event;
+	atomic_t			win_updated;
+	atomic_t			triggering;
 
 	struct exynos_drm_panel_info panel;
 	struct fimd_driver_data *driver_data;
@@ -243,6 +281,14 @@ static u32 fimd_calc_clkdiv(struct fimd_context *ctx,
 	unsigned long ideal_clk = mode->htotal * mode->vtotal * mode->vrefresh;
 	u32 clkdiv;
 
+	if (ctx->i80_if) {
+		/*
+		 * The frame done interrupt should be occurred prior to the
+		 * next TE signal.
+		 */
+		ideal_clk *= 2;
+	}
+
 	/* Find the clock divider value that gets us closest to ideal_clk */
 	clkdiv = DIV_ROUND_UP(clk_get_rate(ctx->lcd_clk), ideal_clk);
 
@@ -271,11 +317,10 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
 {
 	struct fimd_context *ctx = mgr->ctx;
 	struct drm_display_mode *mode = &ctx->mode;
-	struct fimd_driver_data *driver_data;
-	u32 val, clkdiv, vidcon1;
-	int vsync_len, vbpd, vfpd, hsync_len, hbpd, hfpd;
+	struct fimd_driver_data *driver_data = ctx->driver_data;
+	void *timing_base = ctx->regs + driver_data->timing_base;
+	u32 val, clkdiv;
 
-	driver_data = ctx->driver_data;
 	if (ctx->suspended)
 		return;
 
@@ -283,33 +328,65 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
 	if (mode->htotal == 0 || mode->vtotal == 0)
 		return;
 
-	/* setup polarity values */
-	vidcon1 = ctx->vidcon1;
-	if (mode->flags & DRM_MODE_FLAG_NVSYNC)
-		vidcon1 |= VIDCON1_INV_VSYNC;
-	if (mode->flags & DRM_MODE_FLAG_NHSYNC)
-		vidcon1 |= VIDCON1_INV_HSYNC;
-	writel(vidcon1, ctx->regs + driver_data->timing_base + VIDCON1);
-
-	/* setup vertical timing values. */
-	vsync_len = mode->crtc_vsync_end - mode->crtc_vsync_start;
-	vbpd = mode->crtc_vtotal - mode->crtc_vsync_end;
-	vfpd = mode->crtc_vsync_start - mode->crtc_vdisplay;
-
-	val = VIDTCON0_VBPD(vbpd - 1) |
-		VIDTCON0_VFPD(vfpd - 1) |
-		VIDTCON0_VSPW(vsync_len - 1);
-	writel(val, ctx->regs + driver_data->timing_base + VIDTCON0);
-
-	/* setup horizontal timing values.  */
-	hsync_len = mode->crtc_hsync_end - mode->crtc_hsync_start;
-	hbpd = mode->crtc_htotal - mode->crtc_hsync_end;
-	hfpd = mode->crtc_hsync_start - mode->crtc_hdisplay;
-
-	val = VIDTCON1_HBPD(hbpd - 1) |
-		VIDTCON1_HFPD(hfpd - 1) |
-		VIDTCON1_HSPW(hsync_len - 1);
-	writel(val, ctx->regs + driver_data->timing_base + VIDTCON1);
+	if (ctx->i80_if) {
+		val = ctx->i80ifcon | I80IFEN_ENABLE;
+		writel(val, timing_base + I80IFCONFAx(0));
+
+		/* disable auto frame rate */
+		writel(0, timing_base + I80IFCONFBx(0));
+
+		/* set video type selection to I80 interface */
+		if (ctx->sysreg && regmap_update_bits(ctx->sysreg,
+					driver_data->lcdblk_offset,
+					0x3 << driver_data->lcdblk_vt_shift,
+					0x1 << driver_data->lcdblk_vt_shift)) {
+			DRM_ERROR("Failed to update sysreg for I80 i/f.\n");
+			return;
+		}
+	} else {
+		int vsync_len, vbpd, vfpd, hsync_len, hbpd, hfpd;
+		u32 vidcon1;
+
+		/* setup polarity values */
+		vidcon1 = ctx->vidcon1;
+		if (mode->flags & DRM_MODE_FLAG_NVSYNC)
+			vidcon1 |= VIDCON1_INV_VSYNC;
+		if (mode->flags & DRM_MODE_FLAG_NHSYNC)
+			vidcon1 |= VIDCON1_INV_HSYNC;
+		writel(vidcon1, ctx->regs + driver_data->timing_base + VIDCON1);
+
+		/* setup vertical timing values. */
+		vsync_len = mode->crtc_vsync_end - mode->crtc_vsync_start;
+		vbpd = mode->crtc_vtotal - mode->crtc_vsync_end;
+		vfpd = mode->crtc_vsync_start - mode->crtc_vdisplay;
+
+		val = VIDTCON0_VBPD(vbpd - 1) |
+			VIDTCON0_VFPD(vfpd - 1) |
+			VIDTCON0_VSPW(vsync_len - 1);
+		writel(val, ctx->regs + driver_data->timing_base + VIDTCON0);
+
+		/* setup horizontal timing values.  */
+		hsync_len = mode->crtc_hsync_end - mode->crtc_hsync_start;
+		hbpd = mode->crtc_htotal - mode->crtc_hsync_end;
+		hfpd = mode->crtc_hsync_start - mode->crtc_hdisplay;
+
+		val = VIDTCON1_HBPD(hbpd - 1) |
+			VIDTCON1_HFPD(hfpd - 1) |
+			VIDTCON1_HSPW(hsync_len - 1);
+		writel(val, ctx->regs + driver_data->timing_base + VIDTCON1);
+	}
+
+	if (driver_data->has_vidoutcon)
+		writel(ctx->vidout_con, timing_base + VIDOUT_CON);
+
+	/* set bypass selection */
+	if (ctx->sysreg && regmap_update_bits(ctx->sysreg,
+				driver_data->lcdblk_offset,
+				0x1 << driver_data->lcdblk_bypass_shift,
+				0x1 << driver_data->lcdblk_bypass_shift)) {
+		DRM_ERROR("Failed to update sysreg for bypass setting.\n");
+		return;
+	}
 
 	/* setup horizontal and vertical display size. */
 	val = VIDTCON2_LINEVAL(mode->vdisplay - 1) |
@@ -322,7 +399,8 @@ static void fimd_commit(struct exynos_drm_manager *mgr)
 	 * fields of register with prefix '_F' would be updated
 	 * at vsync(same as dma start)
 	 */
-	val = VIDCON0_ENVID | VIDCON0_ENVID_F;
+	val = ctx->vidcon0;
+	val |= VIDCON0_ENVID | VIDCON0_ENVID_F;
 
 	if (ctx->driver_data->has_clksel)
 		val |= VIDCON0_CLKSEL_LCD;
@@ -660,6 +738,9 @@ static void fimd_win_commit(struct exynos_drm_manager *mgr, int zpos)
 	}
 
 	win_data->enabled = true;
+
+	if (ctx->i80_if)
+		atomic_set(&ctx->win_updated, 1);
 }
 
 static void fimd_win_disable(struct exynos_drm_manager *mgr, int zpos)
@@ -838,6 +919,58 @@ static void fimd_dpms(struct exynos_drm_manager *mgr, int mode)
 	}
 }
 
+static void fimd_trigger(struct device *dev)
+{
+	struct exynos_drm_manager *mgr = get_fimd_manager(dev);
+	struct fimd_context *ctx = mgr->ctx;
+	struct fimd_driver_data *driver_data = ctx->driver_data;
+	void *timing_base = ctx->regs + driver_data->timing_base;
+	u32 reg;
+
+	atomic_set(&ctx->triggering, 1);
+
+	reg = readl(ctx->regs + VIDINTCON0);
+	reg |= (VIDINTCON0_INT_ENABLE | VIDINTCON0_INT_I80IFDONE |
+						VIDINTCON0_INT_SYSMAINCON);
+	writel(reg, ctx->regs + VIDINTCON0);
+
+	reg = readl(timing_base + TRIGCON);
+	reg |= (TRGMODE_I80_RGB_ENABLE_I80 | SWTRGCMD_I80_RGB_ENABLE);
+	writel(reg, timing_base + TRIGCON);
+}
+
+static void fimd_te_handler(struct exynos_drm_manager *mgr)
+{
+	struct fimd_context *ctx = mgr->ctx;
+
+	/* Checks the crtc is detached already from encoder */
+	if (ctx->pipe < 0 || !ctx->drm_dev)
+		return;
+
+	 /*
+	 * Skips to trigger if in triggering state, because multiple triggering
+	 * requests can cause panel reset.
+	 */
+	if (atomic_read(&ctx->triggering))
+		return;
+
+	/*
+	 * If there is a page flip request, triggers and handles the page flip
+	 * event so that current fb can be updated into panel GRAM.
+	 */
+	if (atomic_add_unless(&ctx->win_updated, -1, 0))
+		fimd_trigger(ctx->dev);
+
+	/* Wakes up vsync event queue */
+	if (atomic_read(&ctx->wait_vsync_event)) {
+		atomic_set(&ctx->wait_vsync_event, 0);
+		wake_up(&ctx->wait_vsync_queue);
+
+		if (!atomic_read(&ctx->triggering))
+			drm_handle_vblank(ctx->drm_dev, ctx->pipe);
+	}
+}
+
 static struct exynos_drm_manager_ops fimd_manager_ops = {
 	.dpms = fimd_dpms,
 	.mode_fixup = fimd_mode_fixup,
@@ -849,6 +982,7 @@ static struct exynos_drm_manager_ops fimd_manager_ops = {
 	.win_mode_set = fimd_win_mode_set,
 	.win_commit = fimd_win_commit,
 	.win_disable = fimd_win_disable,
+	.te_handler = fimd_te_handler,
 };
 
 static struct exynos_drm_manager fimd_manager = {
@@ -859,26 +993,40 @@ static struct exynos_drm_manager fimd_manager = {
 static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
 {
 	struct fimd_context *ctx = (struct fimd_context *)dev_id;
-	u32 val;
+	u32 val, clear_bit;
 
 	val = readl(ctx->regs + VIDINTCON1);
 
-	if (val & VIDINTCON1_INT_FRAME)
-		/* VSYNC interrupt */
-		writel(VIDINTCON1_INT_FRAME, ctx->regs + VIDINTCON1);
+	clear_bit = ctx->i80_if ? VIDINTCON1_INT_I80 : VIDINTCON1_INT_FRAME;
+	if (val & clear_bit)
+		writel(clear_bit, ctx->regs + VIDINTCON1);
 
 	/* check the crtc is detached already from encoder */
 	if (ctx->pipe < 0 || !ctx->drm_dev)
 		goto out;
 
-	drm_handle_vblank(ctx->drm_dev, ctx->pipe);
-	exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
+	if (ctx->i80_if) {
+		/* unset I80 frame done interrupt */
+		val = readl(ctx->regs + VIDINTCON0);
+		val &= ~(VIDINTCON0_INT_I80IFDONE | VIDINTCON0_INT_SYSMAINCON);
+		writel(val, ctx->regs + VIDINTCON0);
 
-	/* set wait vsync event to zero and wake up queue. */
-	if (atomic_read(&ctx->wait_vsync_event)) {
-		atomic_set(&ctx->wait_vsync_event, 0);
-		wake_up(&ctx->wait_vsync_queue);
+		/* exit triggering mode */
+		atomic_set(&ctx->triggering, 0);
+
+		drm_handle_vblank(ctx->drm_dev, ctx->pipe);
+		exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
+	} else {
+		drm_handle_vblank(ctx->drm_dev, ctx->pipe);
+		exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
+
+		/* set wait vsync event to zero and wake up queue. */
+		if (atomic_read(&ctx->wait_vsync_event)) {
+			atomic_set(&ctx->wait_vsync_event, 0);
+			wake_up(&ctx->wait_vsync_queue);
+		}
 	}
+
 out:
 	return IRQ_HANDLED;
 }
@@ -923,6 +1071,7 @@ static int fimd_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct fimd_context *ctx;
+	struct device_node *i80_if_timings;
 	struct resource *res;
 	int ret = -EINVAL;
 
@@ -944,12 +1093,51 @@ static int fimd_probe(struct platform_device *pdev)
 
 	ctx->dev = dev;
 	ctx->suspended = true;
+	ctx->driver_data = drm_fimd_get_driver_data(pdev);
 
 	if (of_property_read_bool(dev->of_node, "samsung,invert-vden"))
 		ctx->vidcon1 |= VIDCON1_INV_VDEN;
 	if (of_property_read_bool(dev->of_node, "samsung,invert-vclk"))
 		ctx->vidcon1 |= VIDCON1_INV_VCLK;
 
+	i80_if_timings = of_get_child_by_name(dev->of_node, "i80-if-timings");
+	if (i80_if_timings) {
+		u32 val;
+
+		ctx->i80_if = true;
+
+		if (ctx->driver_data->has_vidoutcon)
+			ctx->vidout_con |= VIDOUT_CON_F_I80_LDI0;
+		else
+			ctx->vidcon0 |= VIDCON0_VIDOUT_I80_LDI0;
+		/*
+		 * The user manual describes that this "DSI_EN" bit is required
+		 * to enable I80 24-bit data interface.
+		 */
+		ctx->vidcon0 |= VIDCON0_DSI_EN;
+
+		if (of_property_read_u32(i80_if_timings, "cs-setup", &val))
+			val = 0;
+		ctx->i80ifcon = LCD_CS_SETUP(val);
+		if (of_property_read_u32(i80_if_timings, "wr-setup", &val))
+			val = 0;
+		ctx->i80ifcon |= LCD_WR_SETUP(val);
+		if (of_property_read_u32(i80_if_timings, "wr-active", &val))
+			val = 1;
+		ctx->i80ifcon |= LCD_WR_ACTIVE(val);
+		if (of_property_read_u32(i80_if_timings, "wr-hold", &val))
+			val = 0;
+		ctx->i80ifcon |= LCD_WR_HOLD(val);
+	}
+	of_node_put(i80_if_timings);
+
+	ctx->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
+							"samsung,sysreg");
+	if (IS_ERR(ctx->sysreg)) {
+		dev_warn(dev, "failed to get system register.\n");
+		ctx->sysreg = NULL;
+	}
+
 	ctx->bus_clk = devm_clk_get(dev, "fimd");
 	if (IS_ERR(ctx->bus_clk)) {
 		dev_err(dev, "failed to get bus clock\n");
@@ -972,7 +1160,8 @@ static int fimd_probe(struct platform_device *pdev)
 		goto err_del_component;
 	}
 
-	res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "vsync");
+	res = platform_get_resource_byname(pdev, IORESOURCE_IRQ,
+					   ctx->i80_if ? "lcd_sys" : "vsync");
 	if (!res) {
 		dev_err(dev, "irq request failed.\n");
 		ret = -ENXIO;
@@ -986,7 +1175,6 @@ static int fimd_probe(struct platform_device *pdev)
 		goto err_del_component;
 	}
 
-	ctx->driver_data = drm_fimd_get_driver_data(pdev);
 	init_waitqueue_head(&ctx->wait_vsync_queue);
 	atomic_set(&ctx->wait_vsync_event, 0);
 
diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h
index b039320..eaad58b 100644
--- a/include/video/samsung_fimd.h
+++ b/include/video/samsung_fimd.h
@@ -19,6 +19,7 @@
 /* VIDCON0 */
 
 #define VIDCON0					0x00
+#define VIDCON0_DSI_EN				(1 << 30)
 #define VIDCON0_INTERLACE			(1 << 29)
 #define VIDCON0_VIDOUT_MASK			(0x7 << 26)
 #define VIDCON0_VIDOUT_SHIFT			26
@@ -355,7 +356,7 @@
 #define VIDINTCON0_INT_ENABLE			(1 << 0)
 
 #define VIDINTCON1				0x134
-#define VIDINTCON1_INT_I180			(1 << 2)
+#define VIDINTCON1_INT_I80			(1 << 2)
 #define VIDINTCON1_INT_FRAME			(1 << 1)
 #define VIDINTCON1_INT_FIFO			(1 << 0)
 
-- 
1.9.0

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

* Re: [PATCH v5 00/14] drm/exynos: support LCD I80 interface display
  2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
                   ` (13 preceding siblings ...)
  2014-07-08  0:39 ` [PATCH v5 14/14] ARM: dts: exynos5420: add dsi node YoungJun Cho
@ 2014-07-09 15:07 ` Inki Dae
  2014-07-09 15:23   ` Thierry Reding
  14 siblings, 1 reply; 34+ messages in thread
From: Inki Dae @ 2014-07-09 15:07 UTC (permalink / raw)
  To: YoungJun Cho
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, dri-devel, a.hajda, kyungmin.park,
	robh+dt, galak, kgene.kim

On 2014년 07월 08일 09:39, YoungJun Cho wrote:
> Hi,
> 
> This series adds LCD I80 interface display support for Exynos DRM driver.
> The FIMD(display controller) specification describes it as "LCD I80 interface"
> and the DSI specification describes it as "Command mode interface".
> 
> This is based on exynos-drm-next branch.

Thanks for contributions. Picked them up.

Thanks,
Inki Dae

> 
> The previous patches,
> RFC: http://www.spinics.net/lists/dri-devel/msg58898.html
> V1: http://www.spinics.net/lists/dri-devel/msg59291.html
> V2: http://www.spinics.net/lists/dri-devel/msg59867.html
> V3: http://www.spinics.net/lists/dri-devel/msg60708.html
> V4: http://www.spinics.net/lists/dri-devel/msg60943.html
> 
> Changelog v2:
> - Fixes typo and removes unnecessary error log (commented by Andrzej Hazda)
> - Adds missed pendlig_flip flag clear points (commented by Daniel Kurtz)
> 
> Changelog v3:
> - Removes generic command mode and command mode display timing interface.
> - Moves I80 interface timings from panel DT to the FIMD(display controller) DT.
> 
> Changelog v4:
> - Removes exynos5 sysreg(syscon) DT bindings and node from dtsi because
>   it was already updated by linux-samsung-soc (commented by Vivek Gautam)
> 
> Changelog v5:
> - Fixes FIMD vidcon0 register relevant code
> - Fixes panel gamma table, disable sequence
> - Slitely updates for code cleanup
> 
> Patches 1 and 2 fix trivial bugs.
> 
> Patches 3, 4, 5 and 6 implement FIMD(display controller) I80 interface.
> The MIPI DSI command mode based panel generates Tearing Effect synchronization
> signal between MCU and FB to display video image, and FIMD should trigger to
> transfer video image at this signal.
> So the panel should receive the TE IRQ and call TE handler chains to notify
> it to the FIMD.
> 
> Patches 7 and 8 implement to use Exynos5410 / 5420 / 5440 SoC DSI driver
> which is different from previous Exynos4 SoCs for some registers control.
> 
> Patches 9 and 10 introduce MIPI DSI command mode based Samsung S6E3FA0 AMOLED
> 5.7" LCD drm panel driver.
> 
> The ohters add DT property nodes to support MIPI DSI command mode.
> 
> I welcome any comments.
> 
> Thank you.
> Best regards YJ
> 
> YoungJun Cho (14):
>   drm/exynos: dsi: move the EoT packets configuration point
>   drm/exynos: use wait_event_timeout() for safety usage
>   ARM: dts: samsung-fimd: add LCD I80 interface specific properties
>   drm/exynos: add TE handler to support LCD I80 interface
>   drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
>   drm/exynos: fimd: support LCD I80 interface
>   ARM: dts: exynos_dsim: add exynos5410 compatible to DT bindings
>   drm/exynos: dsi: add driver data to support Exynos5410/5420/5440 SoCs
>   ARM: dts: s6e3fa0: add DT bindings
>   drm/panel: add S6E3FA0 driver
>   ARM: dts: exynos4: add system register property
>   ARM: dts: exynos5: add system register property
>   ARM: dts: exynos5420: add mipi-phy node
>   ARM: dts: exynos5420: add dsi node
> 
>  .../devicetree/bindings/panel/samsung,s6e3fa0.txt  |  46 ++
>  .../devicetree/bindings/video/exynos_dsim.txt      |   4 +-
>  .../devicetree/bindings/video/samsung-fimd.txt     |  28 +
>  arch/arm/boot/dts/exynos4.dtsi                     |   1 +
>  arch/arm/boot/dts/exynos5.dtsi                     |   1 +
>  arch/arm/boot/dts/exynos5420.dtsi                  |  20 +
>  drivers/gpu/drm/exynos/Kconfig                     |   1 +
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c           |  15 +-
>  drivers/gpu/drm/exynos/exynos_drm_crtc.h           |   7 +
>  drivers/gpu/drm/exynos/exynos_drm_drv.h            |   3 +
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c            | 181 ++++++-
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c           | 276 ++++++++--
>  drivers/gpu/drm/panel/Kconfig                      |   7 +
>  drivers/gpu/drm/panel/Makefile                     |   1 +
>  drivers/gpu/drm/panel/panel-s6e3fa0.c              | 569 +++++++++++++++++++++
>  include/drm/drm_mipi_dsi.h                         |   7 +
>  include/video/samsung_fimd.h                       |   3 +-
>  17 files changed, 1098 insertions(+), 72 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt
>  create mode 100644 drivers/gpu/drm/panel/panel-s6e3fa0.c
> 

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

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-08  0:39 ` [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops " YoungJun Cho
@ 2014-07-09 15:22   ` Thierry Reding
  2014-07-09 16:03     ` Inki Dae
  2014-07-10  1:06     ` YoungJun Cho
  0 siblings, 2 replies; 34+ messages in thread
From: Thierry Reding @ 2014-07-09 15:22 UTC (permalink / raw)
  To: YoungJun Cho
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, dri-devel, a.hajda, kyungmin.park,
	robh+dt, galak, kgene.kim


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

On Tue, Jul 08, 2014 at 09:39:38AM +0900, YoungJun Cho wrote:
> To support LCD I80 interface, the DSI host calls this function
> to notify the panel tearing effect synchronization signal to
> the CRTC device manager to trigger to transfer video image.
> 
> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> Acked-by: Inki Dae <inki.dae@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++++++++
>  include/drm/drm_mipi_dsi.h              |  7 +++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index dad543a..76e34ca 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -24,6 +24,7 @@
>  #include <video/mipi_display.h>
>  #include <video/videomode.h>
>  
> +#include "exynos_drm_crtc.h"
>  #include "exynos_drm_drv.h"
>  
>  /* returns true iff both arguments logically differs */
> @@ -1041,10 +1042,20 @@ static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
>  	return (ret < 0) ? ret : xfer.rx_done;
>  }
>  
> +static void exynos_dsi_host_pass_te(struct mipi_dsi_host *host)
> +{
> +	struct exynos_dsi *dsi = host_to_dsi(host);
> +	struct drm_encoder *encoder = dsi->encoder;
> +
> +	if (dsi->state & DSIM_STATE_ENABLED)
> +		exynos_drm_crtc_te_handler(encoder->crtc);
> +}
> +
>  static const struct mipi_dsi_host_ops exynos_dsi_ops = {
>  	.attach = exynos_dsi_host_attach,
>  	.detach = exynos_dsi_host_detach,
>  	.transfer = exynos_dsi_host_transfer,
> +	.pass_te = exynos_dsi_host_pass_te,
>  };
>  
>  static int exynos_dsi_poweron(struct exynos_dsi *dsi)
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 944f33f..3f21bea 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -49,6 +49,12 @@ struct mipi_dsi_msg {
>   * @detach: detach DSI device from DSI host
>   * @transfer: send and/or receive DSI packet, return number of received bytes,
>   * 	      or error
> + * @pass_te: call the crtc te_handler() callback from DSI host.
> + *	     The panel generates tearing effect synchronization signal between
> + *	     MCU and FB to display video images. And the display controller
> + *	     should trigger to transfer video image at this signal. So the panel
> + *	     receives the TE IRQ, then calls this function to notify it to the
> + *	     display controller.
>   */
>  struct mipi_dsi_host_ops {
>  	int (*attach)(struct mipi_dsi_host *host,
> @@ -57,6 +63,7 @@ struct mipi_dsi_host_ops {
>  		      struct mipi_dsi_device *dsi);
>  	ssize_t (*transfer)(struct mipi_dsi_host *host,
>  			    struct mipi_dsi_msg *msg);
> +	void (*pass_te)(struct mipi_dsi_host *host);

I've objected to this particular change before and that objection still
stands. I don't see how this is related to DSI. It seems like an
implementation detail of this particular setup and I think it should be
handled differently (within the Exynos DSI controller implementation
possibly).

Laurent also asked you to split this up into two patches, one for the
core part, the other for the Exynos driver parts, yet this patch
contains both changes.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

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

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

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

* Re: [PATCH v5 00/14] drm/exynos: support LCD I80 interface display
  2014-07-09 15:07 ` [PATCH v5 00/14] drm/exynos: support LCD I80 interface display Inki Dae
@ 2014-07-09 15:23   ` Thierry Reding
  0 siblings, 0 replies; 34+ messages in thread
From: Thierry Reding @ 2014-07-09 15:23 UTC (permalink / raw)
  To: Inki Dae
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, dri-devel, a.hajda, kyungmin.park,
	robh+dt, galak, kgene.kim


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

On Thu, Jul 10, 2014 at 12:07:08AM +0900, Inki Dae wrote:
> On 2014년 07월 08일 09:39, YoungJun Cho wrote:
> > Hi,
> > 
> > This series adds LCD I80 interface display support for Exynos DRM driver.
> > The FIMD(display controller) specification describes it as "LCD I80 interface"
> > and the DSI specification describes it as "Command mode interface".
> > 
> > This is based on exynos-drm-next branch.
> 
> Thanks for contributions. Picked them up.

I'd prefer if you didn't pick them up. See comments to patch 5.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

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

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

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-09 15:22   ` Thierry Reding
@ 2014-07-09 16:03     ` Inki Dae
  2014-07-10  1:06     ` YoungJun Cho
  1 sibling, 0 replies; 34+ messages in thread
From: Inki Dae @ 2014-07-09 16:03 UTC (permalink / raw)
  To: Thierry Reding
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, dri-devel, a.hajda, kyungmin.park,
	robh+dt, galak, kgene.kim

On 2014년 07월 10일 00:22, Thierry Reding wrote:
> On Tue, Jul 08, 2014 at 09:39:38AM +0900, YoungJun Cho wrote:
>> To support LCD I80 interface, the DSI host calls this function
>> to notify the panel tearing effect synchronization signal to
>> the CRTC device manager to trigger to transfer video image.
>>
>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
>> Acked-by: Inki Dae <inki.dae@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++++++++
>>  include/drm/drm_mipi_dsi.h              |  7 +++++++
>>  2 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> index dad543a..76e34ca 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> @@ -24,6 +24,7 @@
>>  #include <video/mipi_display.h>
>>  #include <video/videomode.h>
>>  
>> +#include "exynos_drm_crtc.h"
>>  #include "exynos_drm_drv.h"
>>  
>>  /* returns true iff both arguments logically differs */
>> @@ -1041,10 +1042,20 @@ static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
>>  	return (ret < 0) ? ret : xfer.rx_done;
>>  }
>>  
>> +static void exynos_dsi_host_pass_te(struct mipi_dsi_host *host)
>> +{
>> +	struct exynos_dsi *dsi = host_to_dsi(host);
>> +	struct drm_encoder *encoder = dsi->encoder;
>> +
>> +	if (dsi->state & DSIM_STATE_ENABLED)
>> +		exynos_drm_crtc_te_handler(encoder->crtc);
>> +}
>> +
>>  static const struct mipi_dsi_host_ops exynos_dsi_ops = {
>>  	.attach = exynos_dsi_host_attach,
>>  	.detach = exynos_dsi_host_detach,
>>  	.transfer = exynos_dsi_host_transfer,
>> +	.pass_te = exynos_dsi_host_pass_te,
>>  };
>>  
>>  static int exynos_dsi_poweron(struct exynos_dsi *dsi)
>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>> index 944f33f..3f21bea 100644
>> --- a/include/drm/drm_mipi_dsi.h
>> +++ b/include/drm/drm_mipi_dsi.h
>> @@ -49,6 +49,12 @@ struct mipi_dsi_msg {
>>   * @detach: detach DSI device from DSI host
>>   * @transfer: send and/or receive DSI packet, return number of received bytes,
>>   * 	      or error
>> + * @pass_te: call the crtc te_handler() callback from DSI host.
>> + *	     The panel generates tearing effect synchronization signal between
>> + *	     MCU and FB to display video images. And the display controller
>> + *	     should trigger to transfer video image at this signal. So the panel
>> + *	     receives the TE IRQ, then calls this function to notify it to the
>> + *	     display controller.
>>   */
>>  struct mipi_dsi_host_ops {
>>  	int (*attach)(struct mipi_dsi_host *host,
>> @@ -57,6 +63,7 @@ struct mipi_dsi_host_ops {
>>  		      struct mipi_dsi_device *dsi);
>>  	ssize_t (*transfer)(struct mipi_dsi_host *host,
>>  			    struct mipi_dsi_msg *msg);
>> +	void (*pass_te)(struct mipi_dsi_host *host);
> 
> I've objected to this particular change before and that objection still
> stands. I don't see how this is related to DSI. It seems like an
> implementation detail of this particular setup and I think it should be
> handled differently (within the Exynos DSI controller implementation
> possibly).

I thought we had enough review but yes,  I missed it. It seems better to
handle pass_te callback within Exynos side as of now. That could cause
controversy. However, the obvious one is that te signal should be
notified to display controller driver somehow but there is no a good way
for it. Anyway, got it.  Reverted.

Thanks,
Inki Dae

> 
> Laurent also asked you to split this up into two patches, one for the
> core part, the other for the Exynos driver parts, yet this patch
> contains both changes.
> 
> Thierry
> 

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

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-09 15:22   ` Thierry Reding
  2014-07-09 16:03     ` Inki Dae
@ 2014-07-10  1:06     ` YoungJun Cho
  2014-07-10  1:20       ` Inki Dae
  2014-07-10  7:38       ` Thierry Reding
  1 sibling, 2 replies; 34+ messages in thread
From: YoungJun Cho @ 2014-07-10  1:06 UTC (permalink / raw)
  To: Thierry Reding
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, dri-devel, a.hajda, kyungmin.park,
	robh+dt, galak, kgene.kim

On 07/10/2014 12:22 AM, Thierry Reding wrote:
> On Tue, Jul 08, 2014 at 09:39:38AM +0900, YoungJun Cho wrote:
>> To support LCD I80 interface, the DSI host calls this function
>> to notify the panel tearing effect synchronization signal to
>> the CRTC device manager to trigger to transfer video image.
>>
>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
>> Acked-by: Inki Dae <inki.dae@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++++++++
>>   include/drm/drm_mipi_dsi.h              |  7 +++++++
>>   2 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> index dad543a..76e34ca 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> @@ -24,6 +24,7 @@
>>   #include <video/mipi_display.h>
>>   #include <video/videomode.h>
>>
>> +#include "exynos_drm_crtc.h"
>>   #include "exynos_drm_drv.h"
>>
>>   /* returns true iff both arguments logically differs */
>> @@ -1041,10 +1042,20 @@ static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
>>   	return (ret < 0) ? ret : xfer.rx_done;
>>   }
>>
>> +static void exynos_dsi_host_pass_te(struct mipi_dsi_host *host)
>> +{
>> +	struct exynos_dsi *dsi = host_to_dsi(host);
>> +	struct drm_encoder *encoder = dsi->encoder;
>> +
>> +	if (dsi->state & DSIM_STATE_ENABLED)
>> +		exynos_drm_crtc_te_handler(encoder->crtc);
>> +}
>> +
>>   static const struct mipi_dsi_host_ops exynos_dsi_ops = {
>>   	.attach = exynos_dsi_host_attach,
>>   	.detach = exynos_dsi_host_detach,
>>   	.transfer = exynos_dsi_host_transfer,
>> +	.pass_te = exynos_dsi_host_pass_te,
>>   };
>>
>>   static int exynos_dsi_poweron(struct exynos_dsi *dsi)
>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>> index 944f33f..3f21bea 100644
>> --- a/include/drm/drm_mipi_dsi.h
>> +++ b/include/drm/drm_mipi_dsi.h
>> @@ -49,6 +49,12 @@ struct mipi_dsi_msg {
>>    * @detach: detach DSI device from DSI host
>>    * @transfer: send and/or receive DSI packet, return number of received bytes,
>>    * 	      or error
>> + * @pass_te: call the crtc te_handler() callback from DSI host.
>> + *	     The panel generates tearing effect synchronization signal between
>> + *	     MCU and FB to display video images. And the display controller
>> + *	     should trigger to transfer video image at this signal. So the panel
>> + *	     receives the TE IRQ, then calls this function to notify it to the
>> + *	     display controller.
>>    */
>>   struct mipi_dsi_host_ops {
>>   	int (*attach)(struct mipi_dsi_host *host,
>> @@ -57,6 +63,7 @@ struct mipi_dsi_host_ops {
>>   		      struct mipi_dsi_device *dsi);
>>   	ssize_t (*transfer)(struct mipi_dsi_host *host,
>>   			    struct mipi_dsi_msg *msg);
>> +	void (*pass_te)(struct mipi_dsi_host *host);
>
> I've objected to this particular change before and that objection still
> stands. I don't see how this is related to DSI. It seems like an
> implementation detail of this particular setup and I think it should be
> handled differently (within the Exynos DSI controller implementation
> possibly).
>

Okay, I understand what you mean.
As you know, this function is called by panel TE interrupt handler, so 
it could be accessed by panel.
Do you have any good idea for panel to access exynos_drm_dsi directly 
without mipi_dis_host_ops?

Thank you.
Best regards YJ

> Laurent also asked you to split this up into two patches, one for the
> core part, the other for the Exynos driver parts, yet this patch
> contains both changes.
>
> Thierry
>

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-10  1:06     ` YoungJun Cho
@ 2014-07-10  1:20       ` Inki Dae
  2014-07-10  2:29         ` Inki Dae
  2014-07-10  7:38       ` Thierry Reding
  1 sibling, 1 reply; 34+ messages in thread
From: Inki Dae @ 2014-07-10  1:20 UTC (permalink / raw)
  To: YoungJun Cho
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, robh+dt, a.hajda, kyungmin.park,
	dri-devel, galak, kgene.kim

On 2014년 07월 10일 10:06, YoungJun Cho wrote:
> On 07/10/2014 12:22 AM, Thierry Reding wrote:
>> On Tue, Jul 08, 2014 at 09:39:38AM +0900, YoungJun Cho wrote:
>>> To support LCD I80 interface, the DSI host calls this function
>>> to notify the panel tearing effect synchronization signal to
>>> the CRTC device manager to trigger to transfer video image.
>>>
>>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
>>> Acked-by: Inki Dae <inki.dae@samsung.com>
>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++++++++
>>>   include/drm/drm_mipi_dsi.h              |  7 +++++++
>>>   2 files changed, 18 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> index dad543a..76e34ca 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> @@ -24,6 +24,7 @@
>>>   #include <video/mipi_display.h>
>>>   #include <video/videomode.h>
>>>
>>> +#include "exynos_drm_crtc.h"
>>>   #include "exynos_drm_drv.h"
>>>
>>>   /* returns true iff both arguments logically differs */
>>> @@ -1041,10 +1042,20 @@ static ssize_t
>>> exynos_dsi_host_transfer(struct mipi_dsi_host *host,
>>>       return (ret < 0) ? ret : xfer.rx_done;
>>>   }
>>>
>>> +static void exynos_dsi_host_pass_te(struct mipi_dsi_host *host)
>>> +{
>>> +    struct exynos_dsi *dsi = host_to_dsi(host);
>>> +    struct drm_encoder *encoder = dsi->encoder;
>>> +
>>> +    if (dsi->state & DSIM_STATE_ENABLED)
>>> +        exynos_drm_crtc_te_handler(encoder->crtc);
>>> +}
>>> +
>>>   static const struct mipi_dsi_host_ops exynos_dsi_ops = {
>>>       .attach = exynos_dsi_host_attach,
>>>       .detach = exynos_dsi_host_detach,
>>>       .transfer = exynos_dsi_host_transfer,
>>> +    .pass_te = exynos_dsi_host_pass_te,
>>>   };
>>>
>>>   static int exynos_dsi_poweron(struct exynos_dsi *dsi)
>>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>>> index 944f33f..3f21bea 100644
>>> --- a/include/drm/drm_mipi_dsi.h
>>> +++ b/include/drm/drm_mipi_dsi.h
>>> @@ -49,6 +49,12 @@ struct mipi_dsi_msg {
>>>    * @detach: detach DSI device from DSI host
>>>    * @transfer: send and/or receive DSI packet, return number of
>>> received bytes,
>>>    *           or error
>>> + * @pass_te: call the crtc te_handler() callback from DSI host.
>>> + *         The panel generates tearing effect synchronization signal
>>> between
>>> + *         MCU and FB to display video images. And the display
>>> controller
>>> + *         should trigger to transfer video image at this signal. So
>>> the panel
>>> + *         receives the TE IRQ, then calls this function to notify
>>> it to the
>>> + *         display controller.
>>>    */
>>>   struct mipi_dsi_host_ops {
>>>       int (*attach)(struct mipi_dsi_host *host,
>>> @@ -57,6 +63,7 @@ struct mipi_dsi_host_ops {
>>>                 struct mipi_dsi_device *dsi);
>>>       ssize_t (*transfer)(struct mipi_dsi_host *host,
>>>                   struct mipi_dsi_msg *msg);
>>> +    void (*pass_te)(struct mipi_dsi_host *host);
>>
>> I've objected to this particular change before and that objection still
>> stands. I don't see how this is related to DSI. It seems like an
>> implementation detail of this particular setup and I think it should be
>> handled differently (within the Exynos DSI controller implementation
>> possibly).
>>
> 
> Okay, I understand what you mean.
> As you know, this function is called by panel TE interrupt handler, so
> it could be accessed by panel.
> Do you have any good idea for panel to access exynos_drm_dsi directly
> without mipi_dis_host_ops?
> 

Mr. Cho, let's just use notifier callback for the meantime: fimd driver
registers a te handler to Exynos mipi dsi driver, and then Exynos mipi
dsi driver calls the te handler when te interrupt occurs from Panel device.

I think all we can consider for it is to use mipi_dsi_host_ops or core
framework - connector/encoder and crtc. However, it seems that it's not
really easy for we have a consensus with other maintainers until other
i80 users appear and they need common something for it.


Thanks,
Inki Dae

> Thank you.
> Best regards YJ
> 
>> Laurent also asked you to split this up into two patches, one for the
>> core part, the other for the Exynos driver parts, yet this patch
>> contains both changes.
>>
>> Thierry
>>
> 
> 

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

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-10  1:20       ` Inki Dae
@ 2014-07-10  2:29         ` Inki Dae
  0 siblings, 0 replies; 34+ messages in thread
From: Inki Dae @ 2014-07-10  2:29 UTC (permalink / raw)
  To: YoungJun Cho
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, dri-devel, a.hajda, kyungmin.park,
	robh+dt, galak, kgene.kim

On 2014년 07월 10일 10:20, Inki Dae wrote:
> On 2014년 07월 10일 10:06, YoungJun Cho wrote:
>> On 07/10/2014 12:22 AM, Thierry Reding wrote:
>>> On Tue, Jul 08, 2014 at 09:39:38AM +0900, YoungJun Cho wrote:
>>>> To support LCD I80 interface, the DSI host calls this function
>>>> to notify the panel tearing effect synchronization signal to
>>>> the CRTC device manager to trigger to transfer video image.
>>>>
>>>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
>>>> Acked-by: Inki Dae <inki.dae@samsung.com>
>>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>> ---
>>>>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++++++++
>>>>   include/drm/drm_mipi_dsi.h              |  7 +++++++
>>>>   2 files changed, 18 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>> b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>> index dad543a..76e34ca 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>> @@ -24,6 +24,7 @@
>>>>   #include <video/mipi_display.h>
>>>>   #include <video/videomode.h>
>>>>
>>>> +#include "exynos_drm_crtc.h"
>>>>   #include "exynos_drm_drv.h"
>>>>
>>>>   /* returns true iff both arguments logically differs */
>>>> @@ -1041,10 +1042,20 @@ static ssize_t
>>>> exynos_dsi_host_transfer(struct mipi_dsi_host *host,
>>>>       return (ret < 0) ? ret : xfer.rx_done;
>>>>   }
>>>>
>>>> +static void exynos_dsi_host_pass_te(struct mipi_dsi_host *host)
>>>> +{
>>>> +    struct exynos_dsi *dsi = host_to_dsi(host);
>>>> +    struct drm_encoder *encoder = dsi->encoder;
>>>> +
>>>> +    if (dsi->state & DSIM_STATE_ENABLED)
>>>> +        exynos_drm_crtc_te_handler(encoder->crtc);
>>>> +}
>>>> +
>>>>   static const struct mipi_dsi_host_ops exynos_dsi_ops = {
>>>>       .attach = exynos_dsi_host_attach,
>>>>       .detach = exynos_dsi_host_detach,
>>>>       .transfer = exynos_dsi_host_transfer,
>>>> +    .pass_te = exynos_dsi_host_pass_te,
>>>>   };
>>>>
>>>>   static int exynos_dsi_poweron(struct exynos_dsi *dsi)
>>>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>>>> index 944f33f..3f21bea 100644
>>>> --- a/include/drm/drm_mipi_dsi.h
>>>> +++ b/include/drm/drm_mipi_dsi.h
>>>> @@ -49,6 +49,12 @@ struct mipi_dsi_msg {
>>>>    * @detach: detach DSI device from DSI host
>>>>    * @transfer: send and/or receive DSI packet, return number of
>>>> received bytes,
>>>>    *           or error
>>>> + * @pass_te: call the crtc te_handler() callback from DSI host.
>>>> + *         The panel generates tearing effect synchronization signal
>>>> between
>>>> + *         MCU and FB to display video images. And the display
>>>> controller
>>>> + *         should trigger to transfer video image at this signal. So
>>>> the panel
>>>> + *         receives the TE IRQ, then calls this function to notify
>>>> it to the
>>>> + *         display controller.
>>>>    */
>>>>   struct mipi_dsi_host_ops {
>>>>       int (*attach)(struct mipi_dsi_host *host,
>>>> @@ -57,6 +63,7 @@ struct mipi_dsi_host_ops {
>>>>                 struct mipi_dsi_device *dsi);
>>>>       ssize_t (*transfer)(struct mipi_dsi_host *host,
>>>>                   struct mipi_dsi_msg *msg);
>>>> +    void (*pass_te)(struct mipi_dsi_host *host);
>>>
>>> I've objected to this particular change before and that objection still
>>> stands. I don't see how this is related to DSI. It seems like an
>>> implementation detail of this particular setup and I think it should be
>>> handled differently (within the Exynos DSI controller implementation
>>> possibly).
>>>
>>
>> Okay, I understand what you mean.
>> As you know, this function is called by panel TE interrupt handler, so
>> it could be accessed by panel.
>> Do you have any good idea for panel to access exynos_drm_dsi directly
>> without mipi_dis_host_ops?
>>
> 
> Mr. Cho, let's just use notifier callback for the meantime: fimd driver
> registers a te handler to Exynos mipi dsi driver, and then Exynos mipi
> dsi driver calls the te handler when te interrupt occurs from Panel device.
> 

Again, there was my missing point. there wouldn't be way that Exynos
mipi dsi driver can receive te signal from panel without mipi_dsi_host_ops.

So let's use remote-endpoint property of DSI device node. DSI driver can
get te gpio from Panel node, and can register te irq handler in case of
i80 Panel. After that, te signal could be notified from te interrupt
handler of DSI driver to FIMD driver through notifier callback
registered already.

Thanks,
Inki Dae

> I think all we can consider for it is to use mipi_dsi_host_ops or core
> framework - connector/encoder and crtc. However, it seems that it's not
> really easy for we have a consensus with other maintainers until other
> i80 users appear and they need common something for it.
> 
> 
> Thanks,
> Inki Dae
> 
>> Thank you.
>> Best regards YJ
>>
>>> Laurent also asked you to split this up into two patches, one for the
>>> core part, the other for the Exynos driver parts, yet this patch
>>> contains both changes.
>>>
>>> Thierry
>>>
>>
>>
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

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

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-10  1:06     ` YoungJun Cho
  2014-07-10  1:20       ` Inki Dae
@ 2014-07-10  7:38       ` Thierry Reding
  2014-07-14  9:22         ` YoungJun Cho
  1 sibling, 1 reply; 34+ messages in thread
From: Thierry Reding @ 2014-07-10  7:38 UTC (permalink / raw)
  To: YoungJun Cho
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, dri-devel, a.hajda, kyungmin.park,
	robh+dt, galak, kgene.kim


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

On Thu, Jul 10, 2014 at 10:06:07AM +0900, YoungJun Cho wrote:
> On 07/10/2014 12:22 AM, Thierry Reding wrote:
> >On Tue, Jul 08, 2014 at 09:39:38AM +0900, YoungJun Cho wrote:
> >>To support LCD I80 interface, the DSI host calls this function
> >>to notify the panel tearing effect synchronization signal to
> >>the CRTC device manager to trigger to transfer video image.
> >>
> >>Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> >>Acked-by: Inki Dae <inki.dae@samsung.com>
> >>Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> >>---
> >>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++++++++
> >>  include/drm/drm_mipi_dsi.h              |  7 +++++++
> >>  2 files changed, 18 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> >>index dad543a..76e34ca 100644
> >>--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> >>+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> >>@@ -24,6 +24,7 @@
> >>  #include <video/mipi_display.h>
> >>  #include <video/videomode.h>
> >>
> >>+#include "exynos_drm_crtc.h"
> >>  #include "exynos_drm_drv.h"
> >>
> >>  /* returns true iff both arguments logically differs */
> >>@@ -1041,10 +1042,20 @@ static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
> >>  	return (ret < 0) ? ret : xfer.rx_done;
> >>  }
> >>
> >>+static void exynos_dsi_host_pass_te(struct mipi_dsi_host *host)
> >>+{
> >>+	struct exynos_dsi *dsi = host_to_dsi(host);
> >>+	struct drm_encoder *encoder = dsi->encoder;
> >>+
> >>+	if (dsi->state & DSIM_STATE_ENABLED)
> >>+		exynos_drm_crtc_te_handler(encoder->crtc);
> >>+}
> >>+
> >>  static const struct mipi_dsi_host_ops exynos_dsi_ops = {
> >>  	.attach = exynos_dsi_host_attach,
> >>  	.detach = exynos_dsi_host_detach,
> >>  	.transfer = exynos_dsi_host_transfer,
> >>+	.pass_te = exynos_dsi_host_pass_te,
> >>  };
> >>
> >>  static int exynos_dsi_poweron(struct exynos_dsi *dsi)
> >>diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> >>index 944f33f..3f21bea 100644
> >>--- a/include/drm/drm_mipi_dsi.h
> >>+++ b/include/drm/drm_mipi_dsi.h
> >>@@ -49,6 +49,12 @@ struct mipi_dsi_msg {
> >>   * @detach: detach DSI device from DSI host
> >>   * @transfer: send and/or receive DSI packet, return number of received bytes,
> >>   * 	      or error
> >>+ * @pass_te: call the crtc te_handler() callback from DSI host.
> >>+ *	     The panel generates tearing effect synchronization signal between
> >>+ *	     MCU and FB to display video images. And the display controller
> >>+ *	     should trigger to transfer video image at this signal. So the panel
> >>+ *	     receives the TE IRQ, then calls this function to notify it to the
> >>+ *	     display controller.
> >>   */
> >>  struct mipi_dsi_host_ops {
> >>  	int (*attach)(struct mipi_dsi_host *host,
> >>@@ -57,6 +63,7 @@ struct mipi_dsi_host_ops {
> >>  		      struct mipi_dsi_device *dsi);
> >>  	ssize_t (*transfer)(struct mipi_dsi_host *host,
> >>  			    struct mipi_dsi_msg *msg);
> >>+	void (*pass_te)(struct mipi_dsi_host *host);
> >
> >I've objected to this particular change before and that objection still
> >stands. I don't see how this is related to DSI. It seems like an
> >implementation detail of this particular setup and I think it should be
> >handled differently (within the Exynos DSI controller implementation
> >possibly).
> >
> 
> Okay, I understand what you mean.
> As you know, this function is called by panel TE interrupt handler, so it
> could be accessed by panel.
> Do you have any good idea for panel to access exynos_drm_dsi directly
> without mipi_dis_host_ops?

I've gone through the DSI specification again and the only mention of
the tearing effect is in section 8.12 "TE Signaling in DSI". That says
the following:

	A Command Mode display module has its own timing controller and
	local frame buffer for display refresh. In some cases the host
	processor needs to be notified of timing events on the display
	module, e.g. the start of vertical blanking or similar timing
	information. In a traditional parallel-bus interface like DBI-2,
	a dedicated signal wire labeled TE (Tearing Effect) is provided
	to convey such timing information to the host processor. In a
	DSI system, the same information, with reasonably low latency,
	shall be transmitted from the display module to the host
	processor when requested, using the bidirectional Data Lane.

My interpretation of that is that a DSI peripheral doesn't have a
dedicated TE signal. Now the panel that you want to support here seems
to have one, so I'm wondering if maybe it isn't a DSI panel at all but
rather DBI.

The specification goes into further detail about how to perform the TE
reporting in DSI. Essentially it consists of giving the peripheral
control of the bus via a BTA and then waiting for the peripheral to
report back with the TE event.

It would really help if somebody could find a datasheet for the panel so
that we don't have to keep guessing what the actual interface is and how
it's supposed to work.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

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

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

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-10  7:38       ` Thierry Reding
@ 2014-07-14  9:22         ` YoungJun Cho
  2014-07-14  9:41           ` Thierry Reding
  0 siblings, 1 reply; 34+ messages in thread
From: YoungJun Cho @ 2014-07-14  9:22 UTC (permalink / raw)
  To: Thierry Reding
  Cc: airlied, dri-devel, devicetree, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, linux-samsung-soc, kyungmin.park, inki.dae,
	kgene.kim, jy0922.shim, sw0312.kim, a.hajda

Hi Thierry,

Thank you for comment.

On 07/10/2014 04:38 PM, Thierry Reding wrote:
> On Thu, Jul 10, 2014 at 10:06:07AM +0900, YoungJun Cho wrote:
>> On 07/10/2014 12:22 AM, Thierry Reding wrote:
>>> On Tue, Jul 08, 2014 at 09:39:38AM +0900, YoungJun Cho wrote:
>>>> To support LCD I80 interface, the DSI host calls this function
>>>> to notify the panel tearing effect synchronization signal to
>>>> the CRTC device manager to trigger to transfer video image.
>>>>
>>>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
>>>> Acked-by: Inki Dae <inki.dae@samsung.com>
>>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>> ---
>>>>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++++++++
>>>>   include/drm/drm_mipi_dsi.h              |  7 +++++++
>>>>   2 files changed, 18 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>> index dad543a..76e34ca 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>> @@ -24,6 +24,7 @@
>>>>   #include <video/mipi_display.h>
>>>>   #include <video/videomode.h>
>>>>
>>>> +#include "exynos_drm_crtc.h"
>>>>   #include "exynos_drm_drv.h"
>>>>
>>>>   /* returns true iff both arguments logically differs */
>>>> @@ -1041,10 +1042,20 @@ static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
>>>>   	return (ret < 0) ? ret : xfer.rx_done;
>>>>   }
>>>>
>>>> +static void exynos_dsi_host_pass_te(struct mipi_dsi_host *host)
>>>> +{
>>>> +	struct exynos_dsi *dsi = host_to_dsi(host);
>>>> +	struct drm_encoder *encoder = dsi->encoder;
>>>> +
>>>> +	if (dsi->state & DSIM_STATE_ENABLED)
>>>> +		exynos_drm_crtc_te_handler(encoder->crtc);
>>>> +}
>>>> +
>>>>   static const struct mipi_dsi_host_ops exynos_dsi_ops = {
>>>>   	.attach = exynos_dsi_host_attach,
>>>>   	.detach = exynos_dsi_host_detach,
>>>>   	.transfer = exynos_dsi_host_transfer,
>>>> +	.pass_te = exynos_dsi_host_pass_te,
>>>>   };
>>>>
>>>>   static int exynos_dsi_poweron(struct exynos_dsi *dsi)
>>>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>>>> index 944f33f..3f21bea 100644
>>>> --- a/include/drm/drm_mipi_dsi.h
>>>> +++ b/include/drm/drm_mipi_dsi.h
>>>> @@ -49,6 +49,12 @@ struct mipi_dsi_msg {
>>>>    * @detach: detach DSI device from DSI host
>>>>    * @transfer: send and/or receive DSI packet, return number of received bytes,
>>>>    * 	      or error
>>>> + * @pass_te: call the crtc te_handler() callback from DSI host.
>>>> + *	     The panel generates tearing effect synchronization signal between
>>>> + *	     MCU and FB to display video images. And the display controller
>>>> + *	     should trigger to transfer video image at this signal. So the panel
>>>> + *	     receives the TE IRQ, then calls this function to notify it to the
>>>> + *	     display controller.
>>>>    */
>>>>   struct mipi_dsi_host_ops {
>>>>   	int (*attach)(struct mipi_dsi_host *host,
>>>> @@ -57,6 +63,7 @@ struct mipi_dsi_host_ops {
>>>>   		      struct mipi_dsi_device *dsi);
>>>>   	ssize_t (*transfer)(struct mipi_dsi_host *host,
>>>>   			    struct mipi_dsi_msg *msg);
>>>> +	void (*pass_te)(struct mipi_dsi_host *host);
>>>
>>> I've objected to this particular change before and that objection still
>>> stands. I don't see how this is related to DSI. It seems like an
>>> implementation detail of this particular setup and I think it should be
>>> handled differently (within the Exynos DSI controller implementation
>>> possibly).
>>>
>>
>> Okay, I understand what you mean.
>> As you know, this function is called by panel TE interrupt handler, so it
>> could be accessed by panel.
>> Do you have any good idea for panel to access exynos_drm_dsi directly
>> without mipi_dis_host_ops?
>
> I've gone through the DSI specification again and the only mention of
> the tearing effect is in section 8.12 "TE Signaling in DSI". That says
> the following:
>
> 	A Command Mode display module has its own timing controller and
> 	local frame buffer for display refresh. In some cases the host
> 	processor needs to be notified of timing events on the display
> 	module, e.g. the start of vertical blanking or similar timing
> 	information. In a traditional parallel-bus interface like DBI-2,
> 	a dedicated signal wire labeled TE (Tearing Effect) is provided
> 	to convey such timing information to the host processor. In a
> 	DSI system, the same information, with reasonably low latency,
> 	shall be transmitted from the display module to the host
> 	processor when requested, using the bidirectional Data Lane.
>
> My interpretation of that is that a DSI peripheral doesn't have a
> dedicated TE signal. Now the panel that you want to support here seems
> to have one, so I'm wondering if maybe it isn't a DSI panel at all but
> rather DBI.

Uhm, this panel is DSI panel right. It provides TE external pad to 
transfer TE pulse to host AP and it also provides TE relevant 3 DCS, so 
host AP could choose either of them.
But I think it's better to use IRQ instead of polling method.

As Inki commented before, I'll try to use remote-endpoint property of 
DSI device node in exynos DSIM driver and call FIMD notifier.

Thank you.
Best regards YJ

>
> The specification goes into further detail about how to perform the TE
> reporting in DSI. Essentially it consists of giving the peripheral
> control of the bus via a BTA and then waiting for the peripheral to
> report back with the TE event.
>
> It would really help if somebody could find a datasheet for the panel so
> that we don't have to keep guessing what the actual interface is and how
> it's supposed to work.
>
> Thierry
>

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-14  9:22         ` YoungJun Cho
@ 2014-07-14  9:41           ` Thierry Reding
  2014-07-14 10:45             ` YoungJun Cho
  0 siblings, 1 reply; 34+ messages in thread
From: Thierry Reding @ 2014-07-14  9:41 UTC (permalink / raw)
  To: YoungJun Cho
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, dri-devel, a.hajda, kyungmin.park,
	robh+dt, galak, kgene.kim


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

On Mon, Jul 14, 2014 at 06:22:39PM +0900, YoungJun Cho wrote:
> Hi Thierry,
> 
> Thank you for comment.
> 
> On 07/10/2014 04:38 PM, Thierry Reding wrote:
> >On Thu, Jul 10, 2014 at 10:06:07AM +0900, YoungJun Cho wrote:
> >>On 07/10/2014 12:22 AM, Thierry Reding wrote:
> >>>On Tue, Jul 08, 2014 at 09:39:38AM +0900, YoungJun Cho wrote:
> >>>>To support LCD I80 interface, the DSI host calls this function
> >>>>to notify the panel tearing effect synchronization signal to
> >>>>the CRTC device manager to trigger to transfer video image.
> >>>>
> >>>>Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> >>>>Acked-by: Inki Dae <inki.dae@samsung.com>
> >>>>Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> >>>>---
> >>>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++++++++
> >>>>  include/drm/drm_mipi_dsi.h              |  7 +++++++
> >>>>  2 files changed, 18 insertions(+)
> >>>>
> >>>>diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> >>>>index dad543a..76e34ca 100644
> >>>>--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> >>>>+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> >>>>@@ -24,6 +24,7 @@
> >>>>  #include <video/mipi_display.h>
> >>>>  #include <video/videomode.h>
> >>>>
> >>>>+#include "exynos_drm_crtc.h"
> >>>>  #include "exynos_drm_drv.h"
> >>>>
> >>>>  /* returns true iff both arguments logically differs */
> >>>>@@ -1041,10 +1042,20 @@ static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
> >>>>  	return (ret < 0) ? ret : xfer.rx_done;
> >>>>  }
> >>>>
> >>>>+static void exynos_dsi_host_pass_te(struct mipi_dsi_host *host)
> >>>>+{
> >>>>+	struct exynos_dsi *dsi = host_to_dsi(host);
> >>>>+	struct drm_encoder *encoder = dsi->encoder;
> >>>>+
> >>>>+	if (dsi->state & DSIM_STATE_ENABLED)
> >>>>+		exynos_drm_crtc_te_handler(encoder->crtc);
> >>>>+}
> >>>>+
> >>>>  static const struct mipi_dsi_host_ops exynos_dsi_ops = {
> >>>>  	.attach = exynos_dsi_host_attach,
> >>>>  	.detach = exynos_dsi_host_detach,
> >>>>  	.transfer = exynos_dsi_host_transfer,
> >>>>+	.pass_te = exynos_dsi_host_pass_te,
> >>>>  };
> >>>>
> >>>>  static int exynos_dsi_poweron(struct exynos_dsi *dsi)
> >>>>diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> >>>>index 944f33f..3f21bea 100644
> >>>>--- a/include/drm/drm_mipi_dsi.h
> >>>>+++ b/include/drm/drm_mipi_dsi.h
> >>>>@@ -49,6 +49,12 @@ struct mipi_dsi_msg {
> >>>>   * @detach: detach DSI device from DSI host
> >>>>   * @transfer: send and/or receive DSI packet, return number of received bytes,
> >>>>   * 	      or error
> >>>>+ * @pass_te: call the crtc te_handler() callback from DSI host.
> >>>>+ *	     The panel generates tearing effect synchronization signal between
> >>>>+ *	     MCU and FB to display video images. And the display controller
> >>>>+ *	     should trigger to transfer video image at this signal. So the panel
> >>>>+ *	     receives the TE IRQ, then calls this function to notify it to the
> >>>>+ *	     display controller.
> >>>>   */
> >>>>  struct mipi_dsi_host_ops {
> >>>>  	int (*attach)(struct mipi_dsi_host *host,
> >>>>@@ -57,6 +63,7 @@ struct mipi_dsi_host_ops {
> >>>>  		      struct mipi_dsi_device *dsi);
> >>>>  	ssize_t (*transfer)(struct mipi_dsi_host *host,
> >>>>  			    struct mipi_dsi_msg *msg);
> >>>>+	void (*pass_te)(struct mipi_dsi_host *host);
> >>>
> >>>I've objected to this particular change before and that objection still
> >>>stands. I don't see how this is related to DSI. It seems like an
> >>>implementation detail of this particular setup and I think it should be
> >>>handled differently (within the Exynos DSI controller implementation
> >>>possibly).
> >>>
> >>
> >>Okay, I understand what you mean.
> >>As you know, this function is called by panel TE interrupt handler, so it
> >>could be accessed by panel.
> >>Do you have any good idea for panel to access exynos_drm_dsi directly
> >>without mipi_dis_host_ops?
> >
> >I've gone through the DSI specification again and the only mention of
> >the tearing effect is in section 8.12 "TE Signaling in DSI". That says
> >the following:
> >
> >	A Command Mode display module has its own timing controller and
> >	local frame buffer for display refresh. In some cases the host
> >	processor needs to be notified of timing events on the display
> >	module, e.g. the start of vertical blanking or similar timing
> >	information. In a traditional parallel-bus interface like DBI-2,
> >	a dedicated signal wire labeled TE (Tearing Effect) is provided
> >	to convey such timing information to the host processor. In a
> >	DSI system, the same information, with reasonably low latency,
> >	shall be transmitted from the display module to the host
> >	processor when requested, using the bidirectional Data Lane.
> >
> >My interpretation of that is that a DSI peripheral doesn't have a
> >dedicated TE signal. Now the panel that you want to support here seems
> >to have one, so I'm wondering if maybe it isn't a DSI panel at all but
> >rather DBI.
> 
> Uhm, this panel is DSI panel right. It provides TE external pad to transfer
> TE pulse to host AP and it also provides TE relevant 3 DCS, so host AP could
> choose either of them.
> But I think it's better to use IRQ instead of polling method.

According to the specification you don't have to rely on polling. You
can simply pass control of the bus to the peripheral (via a BTA
sequence) and then wait for the peripheral to signal TE.

That said, I've been doing some research and it seems like we have a
somewhat similar feature on Tegra. What happens there is that there are
three GPIO pins that can be repurposed for TE signalling. But as opposed
to using them as interrupts the display controller can be configured to
use them, upon which it will automatically handle the TE signal by
sending the next frame.

So we have at least two very different implementations of this on two
different SoCs. Further the specification explicitly recommends using
the BTA sequence and DSI protocol to wait for TE. So I still think that
controllers that provide an additional, non-spec compliant method to
signal TE should handle it separately rather than within DSI. Otherwise
we essentially need to make the DSI "core" aware of all these quirks,
and I'd rather avoid that.

> As Inki commented before, I'll try to use remote-endpoint property of DSI
> device node in exynos DSIM driver and call FIMD notifier.

Sounds like it matches what I said above. I'm not a huge fan of
notifiers, but if it works for you I suppose that's fine. The
alternative would be to directly call a FIMD function, which is
somewhat more explicit than a notifier.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

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

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

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-14  9:41           ` Thierry Reding
@ 2014-07-14 10:45             ` YoungJun Cho
  2014-07-14 11:03               ` Thierry Reding
  0 siblings, 1 reply; 34+ messages in thread
From: YoungJun Cho @ 2014-07-14 10:45 UTC (permalink / raw)
  To: Thierry Reding
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, dri-devel, a.hajda, kyungmin.park,
	robh+dt, galak, kgene.kim

Hi Thierry,

On 07/14/2014 06:41 PM, Thierry Reding wrote:
> On Mon, Jul 14, 2014 at 06:22:39PM +0900, YoungJun Cho wrote:
>> Hi Thierry,
>>
>> Thank you for comment.
>>
>> On 07/10/2014 04:38 PM, Thierry Reding wrote:
>>> On Thu, Jul 10, 2014 at 10:06:07AM +0900, YoungJun Cho wrote:
>>>> On 07/10/2014 12:22 AM, Thierry Reding wrote:
>>>>> On Tue, Jul 08, 2014 at 09:39:38AM +0900, YoungJun Cho wrote:
>>>>>> To support LCD I80 interface, the DSI host calls this function
>>>>>> to notify the panel tearing effect synchronization signal to
>>>>>> the CRTC device manager to trigger to transfer video image.
>>>>>>
>>>>>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
>>>>>> Acked-by: Inki Dae <inki.dae@samsung.com>
>>>>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++++++++
>>>>>>   include/drm/drm_mipi_dsi.h              |  7 +++++++
>>>>>>   2 files changed, 18 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>>>> index dad543a..76e34ca 100644
>>>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>>>>> @@ -24,6 +24,7 @@
>>>>>>   #include <video/mipi_display.h>
>>>>>>   #include <video/videomode.h>
>>>>>>
>>>>>> +#include "exynos_drm_crtc.h"
>>>>>>   #include "exynos_drm_drv.h"
>>>>>>
>>>>>>   /* returns true iff both arguments logically differs */
>>>>>> @@ -1041,10 +1042,20 @@ static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
>>>>>>   	return (ret < 0) ? ret : xfer.rx_done;
>>>>>>   }
>>>>>>
>>>>>> +static void exynos_dsi_host_pass_te(struct mipi_dsi_host *host)
>>>>>> +{
>>>>>> +	struct exynos_dsi *dsi = host_to_dsi(host);
>>>>>> +	struct drm_encoder *encoder = dsi->encoder;
>>>>>> +
>>>>>> +	if (dsi->state & DSIM_STATE_ENABLED)
>>>>>> +		exynos_drm_crtc_te_handler(encoder->crtc);
>>>>>> +}
>>>>>> +
>>>>>>   static const struct mipi_dsi_host_ops exynos_dsi_ops = {
>>>>>>   	.attach = exynos_dsi_host_attach,
>>>>>>   	.detach = exynos_dsi_host_detach,
>>>>>>   	.transfer = exynos_dsi_host_transfer,
>>>>>> +	.pass_te = exynos_dsi_host_pass_te,
>>>>>>   };
>>>>>>
>>>>>>   static int exynos_dsi_poweron(struct exynos_dsi *dsi)
>>>>>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>>>>>> index 944f33f..3f21bea 100644
>>>>>> --- a/include/drm/drm_mipi_dsi.h
>>>>>> +++ b/include/drm/drm_mipi_dsi.h
>>>>>> @@ -49,6 +49,12 @@ struct mipi_dsi_msg {
>>>>>>    * @detach: detach DSI device from DSI host
>>>>>>    * @transfer: send and/or receive DSI packet, return number of received bytes,
>>>>>>    * 	      or error
>>>>>> + * @pass_te: call the crtc te_handler() callback from DSI host.
>>>>>> + *	     The panel generates tearing effect synchronization signal between
>>>>>> + *	     MCU and FB to display video images. And the display controller
>>>>>> + *	     should trigger to transfer video image at this signal. So the panel
>>>>>> + *	     receives the TE IRQ, then calls this function to notify it to the
>>>>>> + *	     display controller.
>>>>>>    */
>>>>>>   struct mipi_dsi_host_ops {
>>>>>>   	int (*attach)(struct mipi_dsi_host *host,
>>>>>> @@ -57,6 +63,7 @@ struct mipi_dsi_host_ops {
>>>>>>   		      struct mipi_dsi_device *dsi);
>>>>>>   	ssize_t (*transfer)(struct mipi_dsi_host *host,
>>>>>>   			    struct mipi_dsi_msg *msg);
>>>>>> +	void (*pass_te)(struct mipi_dsi_host *host);
>>>>>
>>>>> I've objected to this particular change before and that objection still
>>>>> stands. I don't see how this is related to DSI. It seems like an
>>>>> implementation detail of this particular setup and I think it should be
>>>>> handled differently (within the Exynos DSI controller implementation
>>>>> possibly).
>>>>>
>>>>
>>>> Okay, I understand what you mean.
>>>> As you know, this function is called by panel TE interrupt handler, so it
>>>> could be accessed by panel.
>>>> Do you have any good idea for panel to access exynos_drm_dsi directly
>>>> without mipi_dis_host_ops?
>>>
>>> I've gone through the DSI specification again and the only mention of
>>> the tearing effect is in section 8.12 "TE Signaling in DSI". That says
>>> the following:
>>>
>>> 	A Command Mode display module has its own timing controller and
>>> 	local frame buffer for display refresh. In some cases the host
>>> 	processor needs to be notified of timing events on the display
>>> 	module, e.g. the start of vertical blanking or similar timing
>>> 	information. In a traditional parallel-bus interface like DBI-2,
>>> 	a dedicated signal wire labeled TE (Tearing Effect) is provided
>>> 	to convey such timing information to the host processor. In a
>>> 	DSI system, the same information, with reasonably low latency,
>>> 	shall be transmitted from the display module to the host
>>> 	processor when requested, using the bidirectional Data Lane.
>>>
>>> My interpretation of that is that a DSI peripheral doesn't have a
>>> dedicated TE signal. Now the panel that you want to support here seems
>>> to have one, so I'm wondering if maybe it isn't a DSI panel at all but
>>> rather DBI.
>>
>> Uhm, this panel is DSI panel right. It provides TE external pad to transfer
>> TE pulse to host AP and it also provides TE relevant 3 DCS, so host AP could
>> choose either of them.
>> But I think it's better to use IRQ instead of polling method.
>
> According to the specification you don't have to rely on polling. You
> can simply pass control of the bus to the peripheral (via a BTA
> sequence) and then wait for the peripheral to signal TE.
>

I need to check that the Exynos DSIM driver supports this BTA sequence.

> That said, I've been doing some research and it seems like we have a
> somewhat similar feature on Tegra. What happens there is that there are
> three GPIO pins that can be repurposed for TE signalling. But as opposed
> to using them as interrupts the display controller can be configured to
> use them, upon which it will automatically handle the TE signal by
> sending the next frame.

Could you explain more detail how the Tegra display controller could be 
configured with this GPIO pins?
I have no idea except that the display controller registers this GPIO as 
an IRQ.

>
> So we have at least two very different implementations of this on two
> different SoCs. Further the specification explicitly recommends using
> the BTA sequence and DSI protocol to wait for TE. So I still think that
> controllers that provide an additional, non-spec compliant method to
> signal TE should handle it separately rather than within DSI. Otherwise
> we essentially need to make the DSI "core" aware of all these quirks,
> and I'd rather avoid that.

You mean, the DSI specification guides to use BTA, so it's better to use 
display controller rather than DSIM, right?

>
>> As Inki commented before, I'll try to use remote-endpoint property of DSI
>> device node in exynos DSIM driver and call FIMD notifier.
>
> Sounds like it matches what I said above. I'm not a huge fan of
> notifiers, but if it works for you I suppose that's fine. The
> alternative would be to directly call a FIMD function, which is
> somewhat more explicit than a notifier.

Yes, I also like explicit call, so I want to use dsi_host_ops and calls 
it in panel. But there is an objection to use dis_host_ops, we think 
notifier in exynos dsim for fimd(display controller).

Thank you.
Best regards YJ

>
> Thierry
>

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-14 10:45             ` YoungJun Cho
@ 2014-07-14 11:03               ` Thierry Reding
  2014-07-15  2:34                 ` Inki Dae
  0 siblings, 1 reply; 34+ messages in thread
From: Thierry Reding @ 2014-07-14 11:03 UTC (permalink / raw)
  To: YoungJun Cho
  Cc: airlied, dri-devel, devicetree, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, linux-samsung-soc, kyungmin.park, inki.dae,
	kgene.kim, jy0922.shim, sw0312.kim, a.hajda

[-- Attachment #1: Type: text/plain, Size: 3919 bytes --]

On Mon, Jul 14, 2014 at 07:45:28PM +0900, YoungJun Cho wrote:
> On 07/14/2014 06:41 PM, Thierry Reding wrote:
[...]
> >That said, I've been doing some research and it seems like we have a
> >somewhat similar feature on Tegra. What happens there is that there are
> >three GPIO pins that can be repurposed for TE signalling. But as opposed
> >to using them as interrupts the display controller can be configured to
> >use them, upon which it will automatically handle the TE signal by
> >sending the next frame.
> 
> Could you explain more detail how the Tegra display controller could be
> configured with this GPIO pins?
> I have no idea except that the display controller registers this GPIO as an
> IRQ.

On Tegra the display controller has a special register that can be
programmed to use one of the three GPIOs as TE signal. Then the display
controller can be configured in one-shot (non-continuous) mode, which
means that software needs to explicitly set a trigger bit to tell the
display controller to send a new frame. If TE signalling is enabled,
then the display controller will not immediately send a new frame when
triggered but wait for signalling of this GPIO.

> >So we have at least two very different implementations of this on two
> >different SoCs. Further the specification explicitly recommends using
> >the BTA sequence and DSI protocol to wait for TE. So I still think that
> >controllers that provide an additional, non-spec compliant method to
> >signal TE should handle it separately rather than within DSI. Otherwise
> >we essentially need to make the DSI "core" aware of all these quirks,
> >and I'd rather avoid that.
> 
> You mean, the DSI specification guides to use BTA, so it's better to use
> display controller rather than DSIM, right?

What I'm saying is that there's nothing about a side-band TE wire in the
DSI spec. In fact the spec explicitly says that this mechanism of an
external TE wire from older protocols (DBI) was replaced by the BTA
sequence over the protocol.

Now, my understanding is that using the BTA sequence over the DSI
protocol would introduce some latency and that forces some panel vendors
to still provide a side-band TE wire even in DSI compliant panels. But
since this is not part of the specification there is no standard way to
do this (as evidenced by Tegra and Exynos). Therefore putting such
functionality into the core DSI code is bad.

But that doesn't mean that you have to put this functionality into the
display controller driver on Exynos. What I'm saying is that it should
be handled by the SoC driver rather than the core. Where exactly
probably depends on the particular case.

> >>As Inki commented before, I'll try to use remote-endpoint property of DSI
> >>device node in exynos DSIM driver and call FIMD notifier.
> >
> >Sounds like it matches what I said above. I'm not a huge fan of
> >notifiers, but if it works for you I suppose that's fine. The
> >alternative would be to directly call a FIMD function, which is
> >somewhat more explicit than a notifier.
> 
> Yes, I also like explicit call, so I want to use dsi_host_ops and calls it
> in panel. But there is an objection to use dis_host_ops, we think notifier
> in exynos dsim for fimd(display controller).

There are other ways to explicitly call into the display controller. You
could for example get access to the CRTC that DSIM is currently
connected to (via exynos_dsi.encoder->crtc) and then cast that to a
struct exynos_drm_crtc and call a function to trigger a new frame to be
sent (for example exynos_drm_crtc_send_frame()). This assumes that you
can safely cast struct drm_crtc * to struct exynos_drm_crtc *, but that
shouldn't be a problem.

With the above, you could make the DSIM handle the TE signal interrupts
and trigger the DC using the new exynos_drm_crtc_send_frame() function.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-14 11:03               ` Thierry Reding
@ 2014-07-15  2:34                 ` Inki Dae
  2014-07-16  2:23                   ` YoungJun Cho
  0 siblings, 1 reply; 34+ messages in thread
From: Inki Dae @ 2014-07-15  2:34 UTC (permalink / raw)
  To: Thierry Reding
  Cc: YoungJun Cho, airlied, dri-devel, devicetree, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, linux-samsung-soc,
	kyungmin.park, kgene.kim, jy0922.shim, sw0312.kim, a.hajda

On 2014년 07월 14일 20:03, Thierry Reding wrote:
> On Mon, Jul 14, 2014 at 07:45:28PM +0900, YoungJun Cho wrote:
>> On 07/14/2014 06:41 PM, Thierry Reding wrote:
> [...]
>>> That said, I've been doing some research and it seems like we have a
>>> somewhat similar feature on Tegra. What happens there is that there are
>>> three GPIO pins that can be repurposed for TE signalling. But as opposed
>>> to using them as interrupts the display controller can be configured to
>>> use them, upon which it will automatically handle the TE signal by
>>> sending the next frame.
>>
>> Could you explain more detail how the Tegra display controller could be
>> configured with this GPIO pins?
>> I have no idea except that the display controller registers this GPIO as an
>> IRQ.
> 
> On Tegra the display controller has a special register that can be
> programmed to use one of the three GPIOs as TE signal. Then the display
> controller can be configured in one-shot (non-continuous) mode, which
> means that software needs to explicitly set a trigger bit to tell the
> display controller to send a new frame. If TE signalling is enabled,
> then the display controller will not immediately send a new frame when
> triggered but wait for signalling of this GPIO.
> 
>>> So we have at least two very different implementations of this on two
>>> different SoCs. Further the specification explicitly recommends using
>>> the BTA sequence and DSI protocol to wait for TE. So I still think that
>>> controllers that provide an additional, non-spec compliant method to
>>> signal TE should handle it separately rather than within DSI. Otherwise
>>> we essentially need to make the DSI "core" aware of all these quirks,
>>> and I'd rather avoid that.
>>
>> You mean, the DSI specification guides to use BTA, so it's better to use
>> display controller rather than DSIM, right?
> 
> What I'm saying is that there's nothing about a side-band TE wire in the
> DSI spec. In fact the spec explicitly says that this mechanism of an
> external TE wire from older protocols (DBI) was replaced by the BTA
> sequence over the protocol.
> 
> Now, my understanding is that using the BTA sequence over the DSI
> protocol would introduce some latency and that forces some panel vendors
> to still provide a side-band TE wire even in DSI compliant panels. But
> since this is not part of the specification there is no standard way to
> do this (as evidenced by Tegra and Exynos). Therefore putting such
> functionality into the core DSI code is bad.
> 
> But that doesn't mean that you have to put this functionality into the
> display controller driver on Exynos. What I'm saying is that it should
> be handled by the SoC driver rather than the core. Where exactly
> probably depends on the particular case.
> 
>>>> As Inki commented before, I'll try to use remote-endpoint property of DSI
>>>> device node in exynos DSIM driver and call FIMD notifier.
>>>
>>> Sounds like it matches what I said above. I'm not a huge fan of
>>> notifiers, but if it works for you I suppose that's fine. The
>>> alternative would be to directly call a FIMD function, which is
>>> somewhat more explicit than a notifier.
>>
>> Yes, I also like explicit call, so I want to use dsi_host_ops and calls it
>> in panel. But there is an objection to use dis_host_ops, we think notifier
>> in exynos dsim for fimd(display controller).
> 
> There are other ways to explicitly call into the display controller. You
> could for example get access to the CRTC that DSIM is currently
> connected to (via exynos_dsi.encoder->crtc) and then cast that to a
> struct exynos_drm_crtc and call a function to trigger a new frame to be
> sent (for example exynos_drm_crtc_send_frame()). This assumes that you
> can safely cast struct drm_crtc * to struct exynos_drm_crtc *, but that
> shouldn't be a problem.
> 
> With the above, you could make the DSIM handle the TE signal interrupts
> and trigger the DC using the new exynos_drm_crtc_send_frame() function.
> 

It seems better than the use of notifier. Actually, original patch used
this way except TE event.
Mr. Cho, let's use remote-endpoint property and this way instead of
notifier.

Thanks,
Inki Dae

> Thierry
> 

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-15  2:34                 ` Inki Dae
@ 2014-07-16  2:23                   ` YoungJun Cho
  2014-07-16  7:54                     ` Thierry Reding
  0 siblings, 1 reply; 34+ messages in thread
From: YoungJun Cho @ 2014-07-16  2:23 UTC (permalink / raw)
  To: Inki Dae, Thierry Reding
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, dri-devel, a.hajda, kyungmin.park,
	robh+dt, galak, kgene.kim

Hi Inki,

On 07/15/2014 11:34 AM, Inki Dae wrote:
> On 2014년 07월 14일 20:03, Thierry Reding wrote:
>> On Mon, Jul 14, 2014 at 07:45:28PM +0900, YoungJun Cho wrote:
>>> On 07/14/2014 06:41 PM, Thierry Reding wrote:
>> [...]
>>>> That said, I've been doing some research and it seems like we have a
>>>> somewhat similar feature on Tegra. What happens there is that there are
>>>> three GPIO pins that can be repurposed for TE signalling. But as opposed
>>>> to using them as interrupts the display controller can be configured to
>>>> use them, upon which it will automatically handle the TE signal by
>>>> sending the next frame.
>>>
>>> Could you explain more detail how the Tegra display controller could be
>>> configured with this GPIO pins?
>>> I have no idea except that the display controller registers this GPIO as an
>>> IRQ.
>>
>> On Tegra the display controller has a special register that can be
>> programmed to use one of the three GPIOs as TE signal. Then the display
>> controller can be configured in one-shot (non-continuous) mode, which
>> means that software needs to explicitly set a trigger bit to tell the
>> display controller to send a new frame. If TE signalling is enabled,
>> then the display controller will not immediately send a new frame when
>> triggered but wait for signalling of this GPIO.
>>
>>>> So we have at least two very different implementations of this on two
>>>> different SoCs. Further the specification explicitly recommends using
>>>> the BTA sequence and DSI protocol to wait for TE. So I still think that
>>>> controllers that provide an additional, non-spec compliant method to
>>>> signal TE should handle it separately rather than within DSI. Otherwise
>>>> we essentially need to make the DSI "core" aware of all these quirks,
>>>> and I'd rather avoid that.
>>>
>>> You mean, the DSI specification guides to use BTA, so it's better to use
>>> display controller rather than DSIM, right?
>>
>> What I'm saying is that there's nothing about a side-band TE wire in the
>> DSI spec. In fact the spec explicitly says that this mechanism of an
>> external TE wire from older protocols (DBI) was replaced by the BTA
>> sequence over the protocol.
>>
>> Now, my understanding is that using the BTA sequence over the DSI
>> protocol would introduce some latency and that forces some panel vendors
>> to still provide a side-band TE wire even in DSI compliant panels. But
>> since this is not part of the specification there is no standard way to
>> do this (as evidenced by Tegra and Exynos). Therefore putting such
>> functionality into the core DSI code is bad.
>>
>> But that doesn't mean that you have to put this functionality into the
>> display controller driver on Exynos. What I'm saying is that it should
>> be handled by the SoC driver rather than the core. Where exactly
>> probably depends on the particular case.
>>
>>>>> As Inki commented before, I'll try to use remote-endpoint property of DSI
>>>>> device node in exynos DSIM driver and call FIMD notifier.
>>>>
>>>> Sounds like it matches what I said above. I'm not a huge fan of
>>>> notifiers, but if it works for you I suppose that's fine. The
>>>> alternative would be to directly call a FIMD function, which is
>>>> somewhat more explicit than a notifier.
>>>
>>> Yes, I also like explicit call, so I want to use dsi_host_ops and calls it
>>> in panel. But there is an objection to use dis_host_ops, we think notifier
>>> in exynos dsim for fimd(display controller).
>>
>> There are other ways to explicitly call into the display controller. You
>> could for example get access to the CRTC that DSIM is currently
>> connected to (via exynos_dsi.encoder->crtc) and then cast that to a
>> struct exynos_drm_crtc and call a function to trigger a new frame to be
>> sent (for example exynos_drm_crtc_send_frame()). This assumes that you
>> can safely cast struct drm_crtc * to struct exynos_drm_crtc *, but that
>> shouldn't be a problem.
>>
>> With the above, you could make the DSIM handle the TE signal interrupts
>> and trigger the DC using the new exynos_drm_crtc_send_frame() function.
>>
>
> It seems better than the use of notifier. Actually, original patch used
> this way except TE event.
> Mr. Cho, let's use remote-endpoint property and this way instead of
> notifier.
>

The struct exynos_dsi has panel_node, which is valid by 
exynos_dsi_host_attach() is called from panel, we could use it instead 
of getting new remote-endpoint node.

So after called exynos_dsi_host_attach(), the dsi driver could know that 
the panel supports mipi command mode or video mode,
and if the panel is for mipi command mode one, dsi driver gets panel te 
gpio and registers its irq.

I'll try.

Thank you.
Best regards YJ

> Thanks,
> Inki Dae
>
>> Thierry
>>
>
>

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

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-16  2:23                   ` YoungJun Cho
@ 2014-07-16  7:54                     ` Thierry Reding
  2014-07-16 10:12                       ` YoungJun Cho
  0 siblings, 1 reply; 34+ messages in thread
From: Thierry Reding @ 2014-07-16  7:54 UTC (permalink / raw)
  To: YoungJun Cho
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, dri-devel, a.hajda, kyungmin.park,
	robh+dt, galak, kgene.kim


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

On Wed, Jul 16, 2014 at 11:23:09AM +0900, YoungJun Cho wrote:
> Hi Inki,
> 
> On 07/15/2014 11:34 AM, Inki Dae wrote:
> >On 2014년 07월 14일 20:03, Thierry Reding wrote:
> >>On Mon, Jul 14, 2014 at 07:45:28PM +0900, YoungJun Cho wrote:
> >>>On 07/14/2014 06:41 PM, Thierry Reding wrote:
> >>[...]
> >>>>That said, I've been doing some research and it seems like we have a
> >>>>somewhat similar feature on Tegra. What happens there is that there are
> >>>>three GPIO pins that can be repurposed for TE signalling. But as opposed
> >>>>to using them as interrupts the display controller can be configured to
> >>>>use them, upon which it will automatically handle the TE signal by
> >>>>sending the next frame.
> >>>
> >>>Could you explain more detail how the Tegra display controller could be
> >>>configured with this GPIO pins?
> >>>I have no idea except that the display controller registers this GPIO as an
> >>>IRQ.
> >>
> >>On Tegra the display controller has a special register that can be
> >>programmed to use one of the three GPIOs as TE signal. Then the display
> >>controller can be configured in one-shot (non-continuous) mode, which
> >>means that software needs to explicitly set a trigger bit to tell the
> >>display controller to send a new frame. If TE signalling is enabled,
> >>then the display controller will not immediately send a new frame when
> >>triggered but wait for signalling of this GPIO.
> >>
> >>>>So we have at least two very different implementations of this on two
> >>>>different SoCs. Further the specification explicitly recommends using
> >>>>the BTA sequence and DSI protocol to wait for TE. So I still think that
> >>>>controllers that provide an additional, non-spec compliant method to
> >>>>signal TE should handle it separately rather than within DSI. Otherwise
> >>>>we essentially need to make the DSI "core" aware of all these quirks,
> >>>>and I'd rather avoid that.
> >>>
> >>>You mean, the DSI specification guides to use BTA, so it's better to use
> >>>display controller rather than DSIM, right?
> >>
> >>What I'm saying is that there's nothing about a side-band TE wire in the
> >>DSI spec. In fact the spec explicitly says that this mechanism of an
> >>external TE wire from older protocols (DBI) was replaced by the BTA
> >>sequence over the protocol.
> >>
> >>Now, my understanding is that using the BTA sequence over the DSI
> >>protocol would introduce some latency and that forces some panel vendors
> >>to still provide a side-band TE wire even in DSI compliant panels. But
> >>since this is not part of the specification there is no standard way to
> >>do this (as evidenced by Tegra and Exynos). Therefore putting such
> >>functionality into the core DSI code is bad.
> >>
> >>But that doesn't mean that you have to put this functionality into the
> >>display controller driver on Exynos. What I'm saying is that it should
> >>be handled by the SoC driver rather than the core. Where exactly
> >>probably depends on the particular case.
> >>
> >>>>>As Inki commented before, I'll try to use remote-endpoint property of DSI
> >>>>>device node in exynos DSIM driver and call FIMD notifier.
> >>>>
> >>>>Sounds like it matches what I said above. I'm not a huge fan of
> >>>>notifiers, but if it works for you I suppose that's fine. The
> >>>>alternative would be to directly call a FIMD function, which is
> >>>>somewhat more explicit than a notifier.
> >>>
> >>>Yes, I also like explicit call, so I want to use dsi_host_ops and calls it
> >>>in panel. But there is an objection to use dis_host_ops, we think notifier
> >>>in exynos dsim for fimd(display controller).
> >>
> >>There are other ways to explicitly call into the display controller. You
> >>could for example get access to the CRTC that DSIM is currently
> >>connected to (via exynos_dsi.encoder->crtc) and then cast that to a
> >>struct exynos_drm_crtc and call a function to trigger a new frame to be
> >>sent (for example exynos_drm_crtc_send_frame()). This assumes that you
> >>can safely cast struct drm_crtc * to struct exynos_drm_crtc *, but that
> >>shouldn't be a problem.
> >>
> >>With the above, you could make the DSIM handle the TE signal interrupts
> >>and trigger the DC using the new exynos_drm_crtc_send_frame() function.
> >>
> >
> >It seems better than the use of notifier. Actually, original patch used
> >this way except TE event.
> >Mr. Cho, let's use remote-endpoint property and this way instead of
> >notifier.
> >
> 
> The struct exynos_dsi has panel_node, which is valid by
> exynos_dsi_host_attach() is called from panel, we could use it instead of
> getting new remote-endpoint node.
> 
> So after called exynos_dsi_host_attach(), the dsi driver could know that the
> panel supports mipi command mode or video mode,
> and if the panel is for mipi command mode one, dsi driver gets panel te gpio
> and registers its irq.

Why does the TE GPIO even need to be in the panel's device tree node?
Wouldn't it make more sense for it to be in the DSIM node? After all
that's what the GPIO is connected to, right? Well, at least logically
if not physically.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

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

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

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-16  7:54                     ` Thierry Reding
@ 2014-07-16 10:12                       ` YoungJun Cho
  2014-07-17  2:15                         ` Inki Dae
  0 siblings, 1 reply; 34+ messages in thread
From: YoungJun Cho @ 2014-07-16 10:12 UTC (permalink / raw)
  To: Thierry Reding
  Cc: mark.rutland, devicetree, linux-samsung-soc, pawel.moll,
	ijc+devicetree, sw0312.kim, dri-devel, a.hajda, kyungmin.park,
	robh+dt, galak, kgene.kim

Hi Thierry,

On 07/16/2014 04:54 PM, Thierry Reding wrote:
> On Wed, Jul 16, 2014 at 11:23:09AM +0900, YoungJun Cho wrote:
>> Hi Inki,
>>
>> On 07/15/2014 11:34 AM, Inki Dae wrote:
>>> On 2014년 07월 14일 20:03, Thierry Reding wrote:
>>>> On Mon, Jul 14, 2014 at 07:45:28PM +0900, YoungJun Cho wrote:
>>>>> On 07/14/2014 06:41 PM, Thierry Reding wrote:
>>>> [...]
>>>>>> That said, I've been doing some research and it seems like we have a
>>>>>> somewhat similar feature on Tegra. What happens there is that there are
>>>>>> three GPIO pins that can be repurposed for TE signalling. But as opposed
>>>>>> to using them as interrupts the display controller can be configured to
>>>>>> use them, upon which it will automatically handle the TE signal by
>>>>>> sending the next frame.
>>>>>
>>>>> Could you explain more detail how the Tegra display controller could be
>>>>> configured with this GPIO pins?
>>>>> I have no idea except that the display controller registers this GPIO as an
>>>>> IRQ.
>>>>
>>>> On Tegra the display controller has a special register that can be
>>>> programmed to use one of the three GPIOs as TE signal. Then the display
>>>> controller can be configured in one-shot (non-continuous) mode, which
>>>> means that software needs to explicitly set a trigger bit to tell the
>>>> display controller to send a new frame. If TE signalling is enabled,
>>>> then the display controller will not immediately send a new frame when
>>>> triggered but wait for signalling of this GPIO.
>>>>
>>>>>> So we have at least two very different implementations of this on two
>>>>>> different SoCs. Further the specification explicitly recommends using
>>>>>> the BTA sequence and DSI protocol to wait for TE. So I still think that
>>>>>> controllers that provide an additional, non-spec compliant method to
>>>>>> signal TE should handle it separately rather than within DSI. Otherwise
>>>>>> we essentially need to make the DSI "core" aware of all these quirks,
>>>>>> and I'd rather avoid that.
>>>>>
>>>>> You mean, the DSI specification guides to use BTA, so it's better to use
>>>>> display controller rather than DSIM, right?
>>>>
>>>> What I'm saying is that there's nothing about a side-band TE wire in the
>>>> DSI spec. In fact the spec explicitly says that this mechanism of an
>>>> external TE wire from older protocols (DBI) was replaced by the BTA
>>>> sequence over the protocol.
>>>>
>>>> Now, my understanding is that using the BTA sequence over the DSI
>>>> protocol would introduce some latency and that forces some panel vendors
>>>> to still provide a side-band TE wire even in DSI compliant panels. But
>>>> since this is not part of the specification there is no standard way to
>>>> do this (as evidenced by Tegra and Exynos). Therefore putting such
>>>> functionality into the core DSI code is bad.
>>>>
>>>> But that doesn't mean that you have to put this functionality into the
>>>> display controller driver on Exynos. What I'm saying is that it should
>>>> be handled by the SoC driver rather than the core. Where exactly
>>>> probably depends on the particular case.
>>>>
>>>>>>> As Inki commented before, I'll try to use remote-endpoint property of DSI
>>>>>>> device node in exynos DSIM driver and call FIMD notifier.
>>>>>>
>>>>>> Sounds like it matches what I said above. I'm not a huge fan of
>>>>>> notifiers, but if it works for you I suppose that's fine. The
>>>>>> alternative would be to directly call a FIMD function, which is
>>>>>> somewhat more explicit than a notifier.
>>>>>
>>>>> Yes, I also like explicit call, so I want to use dsi_host_ops and calls it
>>>>> in panel. But there is an objection to use dis_host_ops, we think notifier
>>>>> in exynos dsim for fimd(display controller).
>>>>
>>>> There are other ways to explicitly call into the display controller. You
>>>> could for example get access to the CRTC that DSIM is currently
>>>> connected to (via exynos_dsi.encoder->crtc) and then cast that to a
>>>> struct exynos_drm_crtc and call a function to trigger a new frame to be
>>>> sent (for example exynos_drm_crtc_send_frame()). This assumes that you
>>>> can safely cast struct drm_crtc * to struct exynos_drm_crtc *, but that
>>>> shouldn't be a problem.
>>>>
>>>> With the above, you could make the DSIM handle the TE signal interrupts
>>>> and trigger the DC using the new exynos_drm_crtc_send_frame() function.
>>>>
>>>
>>> It seems better than the use of notifier. Actually, original patch used
>>> this way except TE event.
>>> Mr. Cho, let's use remote-endpoint property and this way instead of
>>> notifier.
>>>
>>
>> The struct exynos_dsi has panel_node, which is valid by
>> exynos_dsi_host_attach() is called from panel, we could use it instead of
>> getting new remote-endpoint node.
>>
>> So after called exynos_dsi_host_attach(), the dsi driver could know that the
>> panel supports mipi command mode or video mode,
>> and if the panel is for mipi command mode one, dsi driver gets panel te gpio
>> and registers its irq.
>
> Why does the TE GPIO even need to be in the panel's device tree node?
> Wouldn't it make more sense for it to be in the DSIM node? After all
> that's what the GPIO is connected to, right? Well, at least logically
> if not physically.
>

I also agree that the GPIO in DT means the connection after all.

But the panel provides the TE pin(this is obvious) and decides the DSIM 
mode(command or video).
This TE pin is useless in video mode.

So I think it's better for panel to decide and provide all related things.

Thank you.
Best regards YJ

> Thierry
>

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

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

* Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface
  2014-07-16 10:12                       ` YoungJun Cho
@ 2014-07-17  2:15                         ` Inki Dae
  0 siblings, 0 replies; 34+ messages in thread
From: Inki Dae @ 2014-07-17  2:15 UTC (permalink / raw)
  To: YoungJun Cho
  Cc: Thierry Reding, airlied, dri-devel, devicetree, robh+dt,
	pawel.moll, mark.rutland, ijc+devicetree, galak,
	linux-samsung-soc, kyungmin.park, kgene.kim, jy0922.shim,
	sw0312.kim, a.hajda

On 2014년 07월 16일 19:12, YoungJun Cho wrote:
> Hi Thierry,
> 
> On 07/16/2014 04:54 PM, Thierry Reding wrote:
>> On Wed, Jul 16, 2014 at 11:23:09AM +0900, YoungJun Cho wrote:
>>> Hi Inki,
>>>
>>> On 07/15/2014 11:34 AM, Inki Dae wrote:
>>>> On 2014년 07월 14일 20:03, Thierry Reding wrote:
>>>>> On Mon, Jul 14, 2014 at 07:45:28PM +0900, YoungJun Cho wrote:
>>>>>> On 07/14/2014 06:41 PM, Thierry Reding wrote:
>>>>> [...]
>>>>>>> That said, I've been doing some research and it seems like we have a
>>>>>>> somewhat similar feature on Tegra. What happens there is that
>>>>>>> there are
>>>>>>> three GPIO pins that can be repurposed for TE signalling. But as
>>>>>>> opposed
>>>>>>> to using them as interrupts the display controller can be
>>>>>>> configured to
>>>>>>> use them, upon which it will automatically handle the TE signal by
>>>>>>> sending the next frame.
>>>>>>
>>>>>> Could you explain more detail how the Tegra display controller
>>>>>> could be
>>>>>> configured with this GPIO pins?
>>>>>> I have no idea except that the display controller registers this
>>>>>> GPIO as an
>>>>>> IRQ.
>>>>>
>>>>> On Tegra the display controller has a special register that can be
>>>>> programmed to use one of the three GPIOs as TE signal. Then the
>>>>> display
>>>>> controller can be configured in one-shot (non-continuous) mode, which
>>>>> means that software needs to explicitly set a trigger bit to tell the
>>>>> display controller to send a new frame. If TE signalling is enabled,
>>>>> then the display controller will not immediately send a new frame when
>>>>> triggered but wait for signalling of this GPIO.
>>>>>
>>>>>>> So we have at least two very different implementations of this on
>>>>>>> two
>>>>>>> different SoCs. Further the specification explicitly recommends
>>>>>>> using
>>>>>>> the BTA sequence and DSI protocol to wait for TE. So I still
>>>>>>> think that
>>>>>>> controllers that provide an additional, non-spec compliant method to
>>>>>>> signal TE should handle it separately rather than within DSI.
>>>>>>> Otherwise
>>>>>>> we essentially need to make the DSI "core" aware of all these
>>>>>>> quirks,
>>>>>>> and I'd rather avoid that.
>>>>>>
>>>>>> You mean, the DSI specification guides to use BTA, so it's better
>>>>>> to use
>>>>>> display controller rather than DSIM, right?
>>>>>
>>>>> What I'm saying is that there's nothing about a side-band TE wire
>>>>> in the
>>>>> DSI spec. In fact the spec explicitly says that this mechanism of an
>>>>> external TE wire from older protocols (DBI) was replaced by the BTA
>>>>> sequence over the protocol.
>>>>>
>>>>> Now, my understanding is that using the BTA sequence over the DSI
>>>>> protocol would introduce some latency and that forces some panel
>>>>> vendors
>>>>> to still provide a side-band TE wire even in DSI compliant panels. But
>>>>> since this is not part of the specification there is no standard
>>>>> way to
>>>>> do this (as evidenced by Tegra and Exynos). Therefore putting such
>>>>> functionality into the core DSI code is bad.
>>>>>
>>>>> But that doesn't mean that you have to put this functionality into the
>>>>> display controller driver on Exynos. What I'm saying is that it should
>>>>> be handled by the SoC driver rather than the core. Where exactly
>>>>> probably depends on the particular case.
>>>>>
>>>>>>>> As Inki commented before, I'll try to use remote-endpoint
>>>>>>>> property of DSI
>>>>>>>> device node in exynos DSIM driver and call FIMD notifier.
>>>>>>>
>>>>>>> Sounds like it matches what I said above. I'm not a huge fan of
>>>>>>> notifiers, but if it works for you I suppose that's fine. The
>>>>>>> alternative would be to directly call a FIMD function, which is
>>>>>>> somewhat more explicit than a notifier.
>>>>>>
>>>>>> Yes, I also like explicit call, so I want to use dsi_host_ops and
>>>>>> calls it
>>>>>> in panel. But there is an objection to use dis_host_ops, we think
>>>>>> notifier
>>>>>> in exynos dsim for fimd(display controller).
>>>>>
>>>>> There are other ways to explicitly call into the display
>>>>> controller. You
>>>>> could for example get access to the CRTC that DSIM is currently
>>>>> connected to (via exynos_dsi.encoder->crtc) and then cast that to a
>>>>> struct exynos_drm_crtc and call a function to trigger a new frame
>>>>> to be
>>>>> sent (for example exynos_drm_crtc_send_frame()). This assumes that you
>>>>> can safely cast struct drm_crtc * to struct exynos_drm_crtc *, but
>>>>> that
>>>>> shouldn't be a problem.
>>>>>
>>>>> With the above, you could make the DSIM handle the TE signal
>>>>> interrupts
>>>>> and trigger the DC using the new exynos_drm_crtc_send_frame()
>>>>> function.
>>>>>
>>>>
>>>> It seems better than the use of notifier. Actually, original patch used
>>>> this way except TE event.
>>>> Mr. Cho, let's use remote-endpoint property and this way instead of
>>>> notifier.
>>>>
>>>
>>> The struct exynos_dsi has panel_node, which is valid by
>>> exynos_dsi_host_attach() is called from panel, we could use it
>>> instead of
>>> getting new remote-endpoint node.
>>>
>>> So after called exynos_dsi_host_attach(), the dsi driver could know
>>> that the
>>> panel supports mipi command mode or video mode,
>>> and if the panel is for mipi command mode one, dsi driver gets panel
>>> te gpio
>>> and registers its irq.
>>
>> Why does the TE GPIO even need to be in the panel's device tree node?
>> Wouldn't it make more sense for it to be in the DSIM node? After all
>> that's what the GPIO is connected to, right? Well, at least logically
>> if not physically.
>>
> 
> I also agree that the GPIO in DT means the connection after all.
> 
> But the panel provides the TE pin(this is obvious) and decides the DSIM
> mode(command or video).
> This TE pin is useless in video mode.
> 
> So I think it's better for panel to decide and provide all related things.
> 

+1

device tree already connects them using endpoint node logically. And it
wouldn't make sense for TE gpio pin to be in dsi node: TE pin belongs to
panel device, not dsi.

And if dsi node has te gpio property, where i80 based parallel panel's
te pin should be in? Display controller? if TE pin is placed in panel
node, and fimd and parallel panel or dsi and mipi panel are connected
logically using device tree (graph), that case would be clear.

Thanks,
Inki Dae

> Thank you.
> Best regards YJ
> 
>> Thierry
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

end of thread, other threads:[~2014-07-17  2:15 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-08  0:39 [PATCH v5 00/14] drm/exynos: support LCD I80 interface display YoungJun Cho
2014-07-08  0:39 ` [PATCH v5 01/14] drm/exynos: dsi: move the EoT packets configuration point YoungJun Cho
2014-07-08  0:39 ` [PATCH v5 02/14] drm/exynos: use wait_event_timeout() for safety usage YoungJun Cho
2014-07-08  0:39 ` [PATCH v5 03/14] ARM: dts: samsung-fimd: add LCD I80 interface specific properties YoungJun Cho
2014-07-08  0:39 ` [PATCH v5 04/14] drm/exynos: add TE handler to support LCD I80 interface YoungJun Cho
2014-07-08  0:39 ` [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops " YoungJun Cho
2014-07-09 15:22   ` Thierry Reding
2014-07-09 16:03     ` Inki Dae
2014-07-10  1:06     ` YoungJun Cho
2014-07-10  1:20       ` Inki Dae
2014-07-10  2:29         ` Inki Dae
2014-07-10  7:38       ` Thierry Reding
2014-07-14  9:22         ` YoungJun Cho
2014-07-14  9:41           ` Thierry Reding
2014-07-14 10:45             ` YoungJun Cho
2014-07-14 11:03               ` Thierry Reding
2014-07-15  2:34                 ` Inki Dae
2014-07-16  2:23                   ` YoungJun Cho
2014-07-16  7:54                     ` Thierry Reding
2014-07-16 10:12                       ` YoungJun Cho
2014-07-17  2:15                         ` Inki Dae
2014-07-08  0:39 ` [PATCH v5 06/14] drm/exynos: fimd: " YoungJun Cho
2014-07-08 16:10   ` Inki Dae
2014-07-09  0:07     ` YoungJun Cho
2014-07-08  0:39 ` [PATCH v5 07/14] ARM: dts: exynos_dsim: add exynos5410 compatible to DT bindings YoungJun Cho
2014-07-08  0:39 ` [PATCH v5 08/14] drm/exynos: dsi: add driver data to support Exynos5410/5420/5440 SoCs YoungJun Cho
2014-07-08  0:39 ` [PATCH v5 09/14] ARM: dts: s6e3fa0: add DT bindings YoungJun Cho
2014-07-08  0:39 ` [PATCH v5 10/14] drm/panel: add S6E3FA0 driver YoungJun Cho
2014-07-08  0:39 ` [PATCH v5 11/14] ARM: dts: exynos4: add system register property YoungJun Cho
2014-07-08  0:39 ` [PATCH v5 12/14] ARM: dts: exynos5: " YoungJun Cho
2014-07-08  0:39 ` [PATCH v5 13/14] ARM: dts: exynos5420: add mipi-phy node YoungJun Cho
2014-07-08  0:39 ` [PATCH v5 14/14] ARM: dts: exynos5420: add dsi node YoungJun Cho
2014-07-09 15:07 ` [PATCH v5 00/14] drm/exynos: support LCD I80 interface display Inki Dae
2014-07-09 15:23   ` Thierry Reding

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