[PATCH 0/7] drm/exynos: panel mode info propagation

[PATCH 0/7] drm/exynos: panel mode info propagation

[Andrzej Hajda]

Hi Inki,

This patchset beside cleanup/refactoring patches (01-03) adds code to propagate
info provided by MIPI-DSI panels about its mode of work (video/command mode).
The propagation is performed for whole pipeline as every its elements uses it.

Regards
Andrzej


Andrzej Hajda (7):
  drm/exynos/decon5433: use readl_poll_timeout helpers
  drm/exynos: use helper to set possible crtcs
  drm/exynos/dsi: refactor panel detection logic
  drm/exynos: propagate info about command mode from panel
  drm/exynos/mic: use mode info stored in CRTC to detect i80 mode
  drm/exynos/decon5433: use mode info stored in CRTC to detect i80 mode
  dt-bindings: exynos5433-decon: remove i80-if-timings property

 .../bindings/display/exynos/exynos5433-decon.txt   |   6 -
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c      | 118 ++++++-----
 drivers/gpu/drm/exynos/exynos_dp.c                 |  15 +-
 drivers/gpu/drm/exynos/exynos_drm_core.c           |   1 +
 drivers/gpu/drm/exynos/exynos_drm_crtc.c           |  21 +-
 drivers/gpu/drm/exynos/exynos_drm_crtc.h           |  10 +-
 drivers/gpu/drm/exynos/exynos_drm_dpi.c            |  12 +-
 drivers/gpu/drm/exynos/exynos_drm_drv.h            |   1 +
 drivers/gpu/drm/exynos/exynos_drm_dsi.c            | 219 ++++++++++-----------
 drivers/gpu/drm/exynos/exynos_drm_mic.c            |  44 +----
 drivers/gpu/drm/exynos/exynos_drm_vidi.c           |  15 +-
 drivers/gpu/drm/exynos/exynos_hdmi.c               |  25 +--
 12 files changed, 228 insertions(+), 259 deletions(-)

-- 
2.7.4

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

[PATCH 1/7] drm/exynos/decon5433: use readl_poll_timeout helpers

[Andrzej Hajda]

Linux core provide helpers for polling with timeout, lets use them.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 5792ca88..237b4c9 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -13,6 +13,7 @@
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/component.h>
+#include <linux/iopoll.h>
 #include <linux/mfd/syscon.h>
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
@@ -407,24 +408,19 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc)
 
 static void decon_swreset(struct decon_context *ctx)
 {
-	unsigned int tries;
 	unsigned long flags;
+	u32 val;
+	int ret;
 
 	writel(0, ctx->addr + DECON_VIDCON0);
-	for (tries = 2000; tries; --tries) {
-		if (~readl(ctx->addr + DECON_VIDCON0) & VIDCON0_STOP_STATUS)
-			break;
-		udelay(10);
-	}
+	readl_poll_timeout(ctx->addr + DECON_VIDCON0, val,
+			   ~val & VIDCON0_STOP_STATUS, 12, 20000);
 
 	writel(VIDCON0_SWRESET, ctx->addr + DECON_VIDCON0);
-	for (tries = 2000; tries; --tries) {
-		if (~readl(ctx->addr + DECON_VIDCON0) & VIDCON0_SWRESET)
-			break;
-		udelay(10);
-	}
+	ret = readl_poll_timeout(ctx->addr + DECON_VIDCON0, val,
+				 ~val & VIDCON0_SWRESET, 12, 20000);
 
-	WARN(tries == 0, "failed to software reset DECON\n");
+	WARN(ret < 0, "failed to software reset DECON\n");
 
 	spin_lock_irqsave(&ctx->vblank_lock, flags);
 	ctx->frame_id = 0;
-- 
2.7.4

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

[PATCH 2/7] drm/exynos: use helper to set possible crtcs

[Andrzej Hajda]

All encoders share the same code to set encoders possible_crtcs field.
The patch creates helper to abstract out this code.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_dp.c       | 15 +++++----------
 drivers/gpu/drm/exynos/exynos_drm_core.c |  1 +
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 21 ++++++++++++++++++---
 drivers/gpu/drm/exynos/exynos_drm_crtc.h | 10 +++++++---
 drivers/gpu/drm/exynos/exynos_drm_dpi.c  | 12 ++++--------
 drivers/gpu/drm/exynos/exynos_drm_dsi.c  | 13 ++++---------
 drivers/gpu/drm/exynos/exynos_drm_vidi.c | 15 +++++----------
 drivers/gpu/drm/exynos/exynos_hdmi.c     | 25 +++++++++----------------
 8 files changed, 53 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_dp.c b/drivers/gpu/drm/exynos/exynos_dp.c
index 385537b..39629e7 100644
--- a/drivers/gpu/drm/exynos/exynos_dp.c
+++ b/drivers/gpu/drm/exynos/exynos_dp.c
@@ -155,7 +155,7 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
 	struct exynos_dp_device *dp = dev_get_drvdata(dev);
 	struct drm_encoder *encoder = &dp->encoder;
 	struct drm_device *drm_dev = data;
-	int pipe, ret;
+	int ret;
 
 	/*
 	 * Just like the probe function said, we don't need the
@@ -179,20 +179,15 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
 			return ret;
 	}
 
-	pipe = exynos_drm_crtc_get_pipe_from_type(drm_dev,
-						  EXYNOS_DISPLAY_TYPE_LCD);
-	if (pipe < 0)
-		return pipe;
-
-	encoder->possible_crtcs = 1 << pipe;
-
-	DRM_DEBUG_KMS("possible_crtcs = 0x%x\n", encoder->possible_crtcs);
-
 	drm_encoder_init(drm_dev, encoder, &exynos_dp_encoder_funcs,
 			 DRM_MODE_ENCODER_TMDS, NULL);
 
 	drm_encoder_helper_add(encoder, &exynos_dp_encoder_helper_funcs);
 
+	ret = exynos_drm_set_possible_crtcs(encoder, EXYNOS_DISPLAY_TYPE_LCD);
+	if (ret < 0)
+		return ret;
+
 	dp->plat_data.encoder = encoder;
 
 	return analogix_dp_bind(dev, dp->drm_dev, &dp->plat_data);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_core.c b/drivers/gpu/drm/exynos/exynos_drm_core.c
index edbd98f..b0c0621 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_core.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_core.c
@@ -13,6 +13,7 @@
  */
 
 #include <drm/drmP.h>
+
 #include "exynos_drm_drv.h"
 #include "exynos_drm_crtc.h"
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index d72777f..5dc1aab 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -16,6 +16,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_encoder.h>
 
 #include "exynos_drm_crtc.h"
 #include "exynos_drm_drv.h"
@@ -189,16 +190,30 @@ struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
 	return ERR_PTR(ret);
 }
 
-int exynos_drm_crtc_get_pipe_from_type(struct drm_device *drm_dev,
+struct exynos_drm_crtc *exynos_drm_crtc_get_by_type(struct drm_device *drm_dev,
 				       enum exynos_drm_output_type out_type)
 {
 	struct drm_crtc *crtc;
 
 	drm_for_each_crtc(crtc, drm_dev)
 		if (to_exynos_crtc(crtc)->type == out_type)
-			return drm_crtc_index(crtc);
+			return to_exynos_crtc(crtc);
 
-	return -EPERM;
+	return ERR_PTR(-EPERM);
+}
+
+int exynos_drm_set_possible_crtcs(struct drm_encoder *encoder,
+		enum exynos_drm_output_type out_type)
+{
+	struct exynos_drm_crtc *crtc = exynos_drm_crtc_get_by_type(encoder->dev,
+						out_type);
+
+	if (IS_ERR(crtc))
+		return PTR_ERR(crtc);
+
+	encoder->possible_crtcs = drm_crtc_mask(&crtc->base);
+
+	return 0;
 }
 
 void exynos_drm_crtc_te_handler(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.h b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
index ef58b64..dec4461 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
@@ -15,21 +15,25 @@
 #ifndef _EXYNOS_DRM_CRTC_H_
 #define _EXYNOS_DRM_CRTC_H_
 
+
 #include "exynos_drm_drv.h"
 
 struct exynos_drm_crtc *exynos_drm_crtc_create(struct drm_device *drm_dev,
 					struct drm_plane *plane,
-					enum exynos_drm_output_type type,
+					enum exynos_drm_output_type out_type,
 					const struct exynos_drm_crtc_ops *ops,
 					void *context);
 void exynos_drm_crtc_wait_pending_update(struct exynos_drm_crtc *exynos_crtc);
 void exynos_drm_crtc_finish_update(struct exynos_drm_crtc *exynos_crtc,
 				   struct exynos_drm_plane *exynos_plane);
 
-/* This function gets pipe value to crtc device matched with out_type. */
-int exynos_drm_crtc_get_pipe_from_type(struct drm_device *drm_dev,
+/* This function gets crtc device matched with out_type. */
+struct exynos_drm_crtc *exynos_drm_crtc_get_by_type(struct drm_device *drm_dev,
 				       enum exynos_drm_output_type out_type);
 
+int exynos_drm_set_possible_crtcs(struct drm_encoder *encoder,
+		enum exynos_drm_output_type out_type);
+
 /*
  * This function calls the crtc device(manager)'s te_handler() callback
  * to trigger to transfer video image at the tearing effect synchronization
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dpi.c b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
index 63abcd2..2992491 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dpi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dpi.c
@@ -203,19 +203,15 @@ int exynos_dpi_bind(struct drm_device *dev, struct drm_encoder *encoder)
 {
 	int ret;
 
-	ret = exynos_drm_crtc_get_pipe_from_type(dev, EXYNOS_DISPLAY_TYPE_LCD);
-	if (ret < 0)
-		return ret;
-
-	encoder->possible_crtcs = 1 << ret;
-
-	DRM_DEBUG_KMS("possible_crtcs = 0x%x\n", encoder->possible_crtcs);
-
 	drm_encoder_init(dev, encoder, &exynos_dpi_encoder_funcs,
 			 DRM_MODE_ENCODER_TMDS, NULL);
 
 	drm_encoder_helper_add(encoder, &exynos_dpi_encoder_helper_funcs);
 
+	ret = exynos_drm_set_possible_crtcs(encoder, EXYNOS_DISPLAY_TYPE_LCD);
+	if (ret < 0)
+		return ret;
+
 	ret = exynos_dpi_create_connector(encoder);
 	if (ret) {
 		DRM_ERROR("failed to create connector ret = %d\n", ret);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 1690b83..3ae459f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1678,20 +1678,15 @@ static int exynos_dsi_bind(struct device *dev, struct device *master,
 	struct drm_bridge *bridge;
 	int ret;
 
-	ret = exynos_drm_crtc_get_pipe_from_type(drm_dev,
-						  EXYNOS_DISPLAY_TYPE_LCD);
-	if (ret < 0)
-		return ret;
-
-	encoder->possible_crtcs = 1 << ret;
-
-	DRM_DEBUG_KMS("possible_crtcs = 0x%x\n", encoder->possible_crtcs);
-
 	drm_encoder_init(drm_dev, encoder, &exynos_dsi_encoder_funcs,
 			 DRM_MODE_ENCODER_TMDS, NULL);
 
 	drm_encoder_helper_add(encoder, &exynos_dsi_encoder_helper_funcs);
 
+	ret = exynos_drm_set_possible_crtcs(encoder, EXYNOS_DISPLAY_TYPE_LCD);
+	if (ret < 0)
+		return ret;
+
 	ret = exynos_dsi_create_connector(encoder);
 	if (ret) {
 		DRM_ERROR("failed to create connector ret = %d\n", ret);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index cb8a728..10eada6b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -382,7 +382,7 @@ static int vidi_bind(struct device *dev, struct device *master, void *data)
 	struct exynos_drm_plane *exynos_plane;
 	struct exynos_drm_plane_config plane_config = { 0 };
 	unsigned int i;
-	int pipe, ret;
+	int ret;
 
 	ctx->drm_dev = drm_dev;
 
@@ -407,20 +407,15 @@ static int vidi_bind(struct device *dev, struct device *master, void *data)
 		return PTR_ERR(ctx->crtc);
 	}
 
-	pipe = exynos_drm_crtc_get_pipe_from_type(drm_dev,
-						  EXYNOS_DISPLAY_TYPE_VIDI);
-	if (pipe < 0)
-		return pipe;
-
-	encoder->possible_crtcs = 1 << pipe;
-
-	DRM_DEBUG_KMS("possible_crtcs = 0x%x\n", encoder->possible_crtcs);
-
 	drm_encoder_init(drm_dev, encoder, &exynos_vidi_encoder_funcs,
 			 DRM_MODE_ENCODER_TMDS, NULL);
 
 	drm_encoder_helper_add(encoder, &exynos_vidi_encoder_helper_funcs);
 
+	ret = exynos_drm_set_possible_crtcs(encoder, EXYNOS_DISPLAY_TYPE_VIDI);
+	if (ret < 0)
+		return ret;
+
 	ret = vidi_create_connector(encoder);
 	if (ret) {
 		DRM_ERROR("failed to create connector ret = %d\n", ret);
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index f50bcb8..c3edfdd 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -1699,32 +1699,25 @@ static int hdmi_bind(struct device *dev, struct device *master, void *data)
 	struct drm_device *drm_dev = data;
 	struct hdmi_context *hdata = dev_get_drvdata(dev);
 	struct drm_encoder *encoder = &hdata->encoder;
-	struct exynos_drm_crtc *exynos_crtc;
-	struct drm_crtc *crtc;
-	int ret, pipe;
+	struct exynos_drm_crtc *crtc;
+	int ret;
 
 	hdata->drm_dev = drm_dev;
 
-	pipe = exynos_drm_crtc_get_pipe_from_type(drm_dev,
-						  EXYNOS_DISPLAY_TYPE_HDMI);
-	if (pipe < 0)
-		return pipe;
-
 	hdata->phy_clk.enable = hdmiphy_clk_enable;
 
-	crtc = drm_crtc_from_index(drm_dev, pipe);
-	exynos_crtc = to_exynos_crtc(crtc);
-	exynos_crtc->pipe_clk = &hdata->phy_clk;
-
-	encoder->possible_crtcs = 1 << pipe;
-
-	DRM_DEBUG_KMS("possible_crtcs = 0x%x\n", encoder->possible_crtcs);
-
 	drm_encoder_init(drm_dev, encoder, &exynos_hdmi_encoder_funcs,
 			 DRM_MODE_ENCODER_TMDS, NULL);
 
 	drm_encoder_helper_add(encoder, &exynos_hdmi_encoder_helper_funcs);
 
+	ret = exynos_drm_set_possible_crtcs(encoder, EXYNOS_DISPLAY_TYPE_HDMI);
+	if (ret < 0)
+		return ret;
+
+	crtc = exynos_drm_crtc_get_by_type(drm_dev, EXYNOS_DISPLAY_TYPE_HDMI);
+	crtc->pipe_clk = &hdata->phy_clk;
+
 	ret = hdmi_create_connector(encoder);
 	if (ret) {
 		DRM_ERROR("failed to create connector ret = %d\n", ret);
-- 
2.7.4

[PATCH 3/7] drm/exynos/dsi: refactor panel detection logic

[Andrzej Hajda]

Description of drm_helper_hpd_irq_event clearly states that drivers
supporting hotplug events per connector should use different helper -
drm_kms_helper_hotplug_event. To achieve it following changes have
been performed:
- moved down all DSI ops - they require exynos_dsi_disable function
to be defined earlier,
- simplified exynos_dsi_detect - there is no real detection, it just
returns if panel is attached,
- DSI attach/detach callbacks attaches/detaches DRM panel and sets
connector status and other context fields accordingly, all this is
performed under mutex, as these callbacks are asynchronous.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 204 ++++++++++++++++----------------
 1 file changed, 103 insertions(+), 101 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 3ae459f..515090f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -254,7 +254,6 @@ struct exynos_dsi {
 	struct drm_encoder encoder;
 	struct mipi_dsi_host dsi_host;
 	struct drm_connector connector;
-	struct device_node *panel_node;
 	struct drm_panel *panel;
 	struct device *dev;
 
@@ -1329,12 +1328,13 @@ static int exynos_dsi_init(struct exynos_dsi *dsi)
 	return 0;
 }
 
-static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
+static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi,
+				      struct device *panel)
 {
 	int ret;
 	int te_gpio_irq;
 
-	dsi->te_gpio = of_get_named_gpio(dsi->panel_node, "te-gpios", 0);
+	dsi->te_gpio = of_get_named_gpio(panel->of_node, "te-gpios", 0);
 	if (dsi->te_gpio == -ENOENT)
 		return 0;
 
@@ -1374,85 +1374,6 @@ static void exynos_dsi_unregister_te_irq(struct exynos_dsi *dsi)
 	}
 }
 
-static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
-				  struct mipi_dsi_device *device)
-{
-	struct exynos_dsi *dsi = host_to_dsi(host);
-
-	dsi->lanes = device->lanes;
-	dsi->format = device->format;
-	dsi->mode_flags = device->mode_flags;
-	dsi->panel_node = device->dev.of_node;
-
-	/*
-	 * This is a temporary solution and should be made by more generic way.
-	 *
-	 * If attached panel device is for command mode one, dsi should register
-	 * TE interrupt handler.
-	 */
-	if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO)) {
-		int ret = exynos_dsi_register_te_irq(dsi);
-
-		if (ret)
-			return ret;
-	}
-
-	if (dsi->connector.dev)
-		drm_helper_hpd_irq_event(dsi->connector.dev);
-
-	return 0;
-}
-
-static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
-				  struct mipi_dsi_device *device)
-{
-	struct exynos_dsi *dsi = host_to_dsi(host);
-
-	exynos_dsi_unregister_te_irq(dsi);
-
-	dsi->panel_node = NULL;
-
-	if (dsi->connector.dev)
-		drm_helper_hpd_irq_event(dsi->connector.dev);
-
-	return 0;
-}
-
-static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
-				        const struct mipi_dsi_msg *msg)
-{
-	struct exynos_dsi *dsi = host_to_dsi(host);
-	struct exynos_dsi_transfer xfer;
-	int ret;
-
-	if (!(dsi->state & DSIM_STATE_ENABLED))
-		return -EINVAL;
-
-	if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
-		ret = exynos_dsi_init(dsi);
-		if (ret)
-			return ret;
-		dsi->state |= DSIM_STATE_INITIALIZED;
-	}
-
-	ret = mipi_dsi_create_packet(&xfer.packet, msg);
-	if (ret < 0)
-		return ret;
-
-	xfer.rx_len = msg->rx_len;
-	xfer.rx_payload = msg->rx_buf;
-	xfer.flags = msg->flags;
-
-	ret = exynos_dsi_transfer(dsi, &xfer);
-	return (ret < 0) ? ret : xfer.rx_done;
-}
-
-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,
-};
-
 static void exynos_dsi_enable(struct drm_encoder *encoder)
 {
 	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
@@ -1508,25 +1429,7 @@ static void exynos_dsi_disable(struct drm_encoder *encoder)
 static enum drm_connector_status
 exynos_dsi_detect(struct drm_connector *connector, bool force)
 {
-	struct exynos_dsi *dsi = connector_to_dsi(connector);
-
-	if (!dsi->panel) {
-		dsi->panel = of_drm_find_panel(dsi->panel_node);
-		if (dsi->panel)
-			drm_panel_attach(dsi->panel, &dsi->connector);
-	} else if (!dsi->panel_node) {
-		struct drm_encoder *encoder;
-
-		encoder = platform_get_drvdata(to_platform_device(dsi->dev));
-		exynos_dsi_disable(encoder);
-		drm_panel_detach(dsi->panel);
-		dsi->panel = NULL;
-	}
-
-	if (dsi->panel)
-		return connector_status_connected;
-
-	return connector_status_disconnected;
+	return connector->status;
 }
 
 static void exynos_dsi_connector_destroy(struct drm_connector *connector)
@@ -1576,6 +1479,7 @@ static int exynos_dsi_create_connector(struct drm_encoder *encoder)
 		return ret;
 	}
 
+	connector->status = connector_status_disconnected;
 	drm_connector_helper_add(connector, &exynos_dsi_connector_helper_funcs);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
@@ -1612,6 +1516,104 @@ static const struct drm_encoder_funcs exynos_dsi_encoder_funcs = {
 
 MODULE_DEVICE_TABLE(of, exynos_dsi_of_match);
 
+static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
+				  struct mipi_dsi_device *device)
+{
+	struct exynos_dsi *dsi = host_to_dsi(host);
+	struct drm_device *drm = dsi->connector.dev;
+
+	/*
+	 * This is a temporary solution and should be made by more generic way.
+	 *
+	 * If attached panel device is for command mode one, dsi should register
+	 * TE interrupt handler.
+	 */
+	if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO)) {
+		int ret = exynos_dsi_register_te_irq(dsi, &device->dev);
+
+		if (ret)
+			return ret;
+	}
+
+	mutex_lock(&drm->mode_config.mutex);
+
+	dsi->lanes = device->lanes;
+	dsi->format = device->format;
+	dsi->mode_flags = device->mode_flags;
+	dsi->panel = of_drm_find_panel(device->dev.of_node);
+	if (dsi->panel) {
+		drm_panel_attach(dsi->panel, &dsi->connector);
+		dsi->connector.status = connector_status_connected;
+	}
+
+	mutex_unlock(&drm->mode_config.mutex);
+
+	if (drm->mode_config.poll_enabled)
+		drm_kms_helper_hotplug_event(drm);
+
+	return 0;
+}
+
+static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
+				  struct mipi_dsi_device *device)
+{
+	struct exynos_dsi *dsi = host_to_dsi(host);
+	struct drm_device *drm = dsi->connector.dev;
+
+	mutex_lock(&drm->mode_config.mutex);
+
+	if (dsi->panel) {
+		exynos_dsi_disable(&dsi->encoder);
+		drm_panel_detach(dsi->panel);
+		dsi->panel = NULL;
+		dsi->connector.status = connector_status_disconnected;
+	}
+
+	mutex_unlock(&drm->mode_config.mutex);
+
+	if (drm->mode_config.poll_enabled)
+		drm_kms_helper_hotplug_event(drm);
+
+	exynos_dsi_unregister_te_irq(dsi);
+
+	return 0;
+}
+
+static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
+					const struct mipi_dsi_msg *msg)
+{
+	struct exynos_dsi *dsi = host_to_dsi(host);
+	struct exynos_dsi_transfer xfer;
+	int ret;
+
+	if (!(dsi->state & DSIM_STATE_ENABLED))
+		return -EINVAL;
+
+	if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
+		ret = exynos_dsi_init(dsi);
+		if (ret)
+			return ret;
+		dsi->state |= DSIM_STATE_INITIALIZED;
+	}
+
+	ret = mipi_dsi_create_packet(&xfer.packet, msg);
+	if (ret < 0)
+		return ret;
+
+	xfer.rx_len = msg->rx_len;
+	xfer.rx_payload = msg->rx_buf;
+	xfer.flags = msg->flags;
+
+	ret = exynos_dsi_transfer(dsi, &xfer);
+	return (ret < 0) ? ret : xfer.rx_done;
+}
+
+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,
+};
+
 static int exynos_dsi_of_read_u32(const struct device_node *np,
 				  const char *propname, u32 *out_value)
 {
-- 
2.7.4

[PATCH 4/7] drm/exynos: propagate info about command mode from panel

[Andrzej Hajda]

mipi_dsi framework provides information about panel's mode of work.
This info should be propagated upstream to configure all elements of
the pipeline. As CRTC is the common nominator of the pipeline we can put
such info into its structures.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 +
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 527bf1d..96b9d49 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -165,6 +165,7 @@ struct exynos_drm_crtc {
 	const struct exynos_drm_crtc_ops	*ops;
 	void				*ctx;
 	struct exynos_drm_clk		*pipe_clk;
+	bool				i80_mode : 1;
 };
 
 static inline void exynos_drm_pipe_clk_enable(struct exynos_drm_crtc *crtc,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 515090f..79df1c9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1545,6 +1545,8 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 		drm_panel_attach(dsi->panel, &dsi->connector);
 		dsi->connector.status = connector_status_connected;
 	}
+	exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode =
+			!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO);
 
 	mutex_unlock(&drm->mode_config.mutex);
 
-- 
2.7.4

[PATCH 5/7] drm/exynos/mic: use mode info stored in CRTC to detect i80 mode

[Andrzej Hajda]

MIC driver should use info from CRTC to check mode of work instead of
illegally peeking into nodes of other devices.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_mic.c | 44 +++------------------------------
 1 file changed, 4 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c b/drivers/gpu/drm/exynos/exynos_drm_mic.c
index e457205..9f2fa8a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
@@ -21,9 +21,12 @@
 #include <linux/component.h>
 #include <linux/pm_runtime.h>
 #include <drm/drmP.h>
+#include <drm/drm_encoder.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
 
+#include <exynos_drm_drv.h>
+
 /* Sysreg registers for MIC */
 #define DSD_CFG_MUX	0x1004
 #define MIC0_RGB_MUX	(1 << 0)
@@ -85,12 +88,6 @@
 
 #define MIC_BS_SIZE_2D(x)	((x) & 0x3fff)
 
-enum {
-	ENDPOINT_DECON_NODE,
-	ENDPOINT_DSI_NODE,
-	NUM_ENDPOINTS
-};
-
 static char *clk_names[] = { "pclk_mic0", "sclk_rgb_vclk_to_mic0" };
 #define NUM_CLKS		ARRAY_SIZE(clk_names)
 static DEFINE_MUTEX(mic_mutex);
@@ -229,36 +226,6 @@ static void mic_set_reg_on(struct exynos_mic *mic, bool enable)
 	writel(reg, mic->reg + MIC_OP);
 }
 
-static int parse_dt(struct exynos_mic *mic)
-{
-	int ret = 0, i, j;
-	struct device_node *remote_node;
-	struct device_node *nodes[3];
-
-	/*
-	 * The order of endpoints does matter.
-	 * The first node must be for decon and the second one must be for dsi.
-	 */
-	for (i = 0, j = 0; i < NUM_ENDPOINTS; i++) {
-		remote_node = of_graph_get_remote_node(mic->dev->of_node, i, 0);
-		if (!remote_node) {
-			ret = -EPIPE;
-			goto exit;
-		}
-		nodes[j++] = remote_node;
-
-		if (i == ENDPOINT_DECON_NODE &&
-			of_get_child_by_name(remote_node, "i80-if-timings"))
-			mic->i80_mode = 1;
-	}
-
-exit:
-	while (--j > -1)
-		of_node_put(nodes[j]);
-
-	return ret;
-}
-
 static void mic_disable(struct drm_bridge *bridge) { }
 
 static void mic_post_disable(struct drm_bridge *bridge)
@@ -286,6 +253,7 @@ static void mic_mode_set(struct drm_bridge *bridge,
 
 	mutex_lock(&mic_mutex);
 	drm_display_mode_to_videomode(mode, &mic->vm);
+	mic->i80_mode = to_exynos_crtc(bridge->encoder->crtc)->i80_mode;
 	mutex_unlock(&mic_mutex);
 }
 
@@ -425,10 +393,6 @@ static int exynos_mic_probe(struct platform_device *pdev)
 
 	mic->dev = dev;
 
-	ret = parse_dt(mic);
-	if (ret)
-		goto err;
-
 	ret = of_address_to_resource(dev->of_node, 0, &res);
 	if (ret) {
 		DRM_ERROR("mic: Failed to get mem region for MIC\n");
-- 
2.7.4

[PATCH 6/7] drm/exynos/decon5433: use mode info stored in CRTC to detect i80 mode

[Andrzej Hajda]

Since panel's mode of work is propagated properly from panel to DECON,
there is no need to use redundant private property. The only issue with
such approach is that check for required interrupts should be postponed
until panel communicate its requirements - ie to atomic_check.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 98 ++++++++++++++++-----------
 1 file changed, 57 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 237b4c9..7a09e03 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -34,9 +34,8 @@
 #define WINDOWS_NR	3
 #define MIN_FB_WIDTH_FOR_16WORD_BURST	128
 
-#define IFTYPE_I80	(1 << 0)
-#define I80_HW_TRG	(1 << 1)
-#define IFTYPE_HDMI	(1 << 2)
+#define I80_HW_TRG	(1 << 0)
+#define IFTYPE_HDMI	(1 << 1)
 
 static const char * const decon_clks_name[] = {
 	"pclk",
@@ -58,7 +57,9 @@ struct decon_context {
 	struct regmap			*sysreg;
 	struct clk			*clks[ARRAY_SIZE(decon_clks_name)];
 	unsigned int			irq;
-	unsigned int			te_irq;
+	unsigned int			irq_vsync;
+	unsigned int			irq_lcd_sys;
+	unsigned int			irq_te;
 	unsigned long			out_type;
 	int				first_win;
 	spinlock_t			vblank_lock;
@@ -91,7 +92,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
 	u32 val;
 
 	val = VIDINTCON0_INTEN;
-	if (ctx->out_type & IFTYPE_I80)
+	if (crtc->i80_mode)
 		val |= VIDINTCON0_FRAMEDONE;
 	else
 		val |= VIDINTCON0_INTFRMEN | VIDINTCON0_FRAMESEL_FP;
@@ -100,7 +101,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
 
 	enable_irq(ctx->irq);
 	if (!(ctx->out_type & I80_HW_TRG))
-		enable_irq(ctx->te_irq);
+		enable_irq(ctx->irq_te);
 
 	return 0;
 }
@@ -110,7 +111,7 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
 	struct decon_context *ctx = crtc->ctx;
 
 	if (!(ctx->out_type & I80_HW_TRG))
-		disable_irq_nosync(ctx->te_irq);
+		disable_irq_nosync(ctx->irq_te);
 	disable_irq_nosync(ctx->irq);
 
 	writel(0, ctx->addr + DECON_VIDINTCON0);
@@ -140,7 +141,7 @@ static u32 decon_get_frame_count(struct decon_context *ctx, bool end)
 
 	switch (status & (VIDCON1_VSTATUS_MASK | VIDCON1_I80_ACTIVE)) {
 	case VIDCON1_VSTATUS_VS:
-		if (!(ctx->out_type & IFTYPE_I80))
+		if (!(ctx->crtc->i80_mode))
 			--frm;
 		break;
 	case VIDCON1_VSTATUS_BP:
@@ -167,7 +168,7 @@ static u32 decon_get_vblank_counter(struct exynos_drm_crtc *crtc)
 
 static void decon_setup_trigger(struct decon_context *ctx)
 {
-	if (!(ctx->out_type & (IFTYPE_I80 | I80_HW_TRG)))
+	if (!ctx->crtc->i80_mode && !(ctx->out_type & I80_HW_TRG))
 		return;
 
 	if (!(ctx->out_type & I80_HW_TRG)) {
@@ -207,7 +208,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 	val = VIDOUT_LCD_ON;
 	if (interlaced)
 		val |= VIDOUT_INTERLACE_EN_F;
-	if (ctx->out_type & IFTYPE_I80) {
+	if (crtc->i80_mode) {
 		val |= VIDOUT_COMMAND_IF;
 	} else {
 		val |= VIDOUT_RGB_IF;
@@ -223,7 +224,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 			VIDTCON2_HOZVAL(m->hdisplay - 1);
 	writel(val, ctx->addr + DECON_VIDTCON2);
 
-	if (!(ctx->out_type & IFTYPE_I80)) {
+	if (!crtc->i80_mode) {
 		int vbp = m->crtc_vtotal - m->crtc_vsync_end;
 		int vfp = m->crtc_vsync_start - m->crtc_vdisplay;
 
@@ -456,7 +457,7 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 	int i;
 
 	if (!(ctx->out_type & I80_HW_TRG))
-		synchronize_irq(ctx->te_irq);
+		synchronize_irq(ctx->irq_te);
 	synchronize_irq(ctx->irq);
 
 	/*
@@ -511,6 +512,22 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc)
 		clk_disable_unprepare(ctx->clks[i]);
 }
 
+static int decon_atomic_check(struct exynos_drm_crtc *crtc,
+		struct drm_crtc_state *state)
+{
+	struct decon_context *ctx = crtc->ctx;
+
+	ctx->irq = crtc->i80_mode ? ctx->irq_lcd_sys : ctx->irq_vsync;
+
+	if (ctx->irq)
+		return 0;
+
+	dev_err(ctx->dev, "Panel requires %s mode, but appropriate interrupt is not provided.\n",
+			crtc->i80_mode ? "command" : "video");
+
+	return -EINVAL;
+}
+
 static const struct exynos_drm_crtc_ops decon_crtc_ops = {
 	.enable			= decon_enable,
 	.disable		= decon_disable,
@@ -521,6 +538,7 @@ static const struct exynos_drm_crtc_ops decon_crtc_ops = {
 	.update_plane		= decon_update_plane,
 	.disable_plane		= decon_disable_plane,
 	.atomic_flush		= decon_atomic_flush,
+	.atomic_check		= decon_atomic_check,
 };
 
 static int decon_bind(struct device *dev, struct device *master, void *data)
@@ -670,19 +688,22 @@ static const struct of_device_id exynos5433_decon_driver_dt_match[] = {
 MODULE_DEVICE_TABLE(of, exynos5433_decon_driver_dt_match);
 
 static int decon_conf_irq(struct decon_context *ctx, const char *name,
-		irq_handler_t handler, unsigned long int flags, bool required)
+		irq_handler_t handler, unsigned long int flags)
 {
 	struct platform_device *pdev = to_platform_device(ctx->dev);
 	int ret, irq = platform_get_irq_byname(pdev, name);
 
 	if (irq < 0) {
-		if (irq == -EPROBE_DEFER)
+		switch (irq) {
+		case -EPROBE_DEFER:
+			return irq;
+		case -ENODATA:
+		case -ENXIO:
+			return 0;
+		default:
+			dev_err(ctx->dev, "IRQ %s get failed, %d\n", name, irq);
 			return irq;
-		if (required)
-			dev_err(ctx->dev, "cannot get %s IRQ\n", name);
-		else
-			irq = 0;
-		return irq;
+		}
 	}
 	irq_set_status_flags(irq, IRQ_NOAUTOEN);
 	ret = devm_request_irq(ctx->dev, irq, handler, flags, "drm_decon", ctx);
@@ -710,11 +731,8 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
 	ctx->out_type = (unsigned long)of_device_get_match_data(dev);
 	spin_lock_init(&ctx->vblank_lock);
 
-	if (ctx->out_type & IFTYPE_HDMI) {
+	if (ctx->out_type & IFTYPE_HDMI)
 		ctx->first_win = 1;
-	} else if (of_get_child_by_name(dev->of_node, "i80-if-timings")) {
-		ctx->out_type |= IFTYPE_I80;
-	}
 
 	for (i = 0; i < ARRAY_SIZE(decon_clks_name); i++) {
 		struct clk *clk;
@@ -738,25 +756,23 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
 		return PTR_ERR(ctx->addr);
 	}
 
-	if (ctx->out_type & IFTYPE_I80) {
-		ret = decon_conf_irq(ctx, "lcd_sys", decon_irq_handler, 0, true);
-		if (ret < 0)
-			return ret;
-		ctx->irq = ret;
+	ret = decon_conf_irq(ctx, "vsync", decon_irq_handler, 0);
+	if (ret < 0)
+		return ret;
+	ctx->irq_vsync = ret;
 
-		ret = decon_conf_irq(ctx, "te", decon_te_irq_handler,
-				     IRQF_TRIGGER_RISING, false);
-		if (ret < 0)
-			return ret;
-		if (ret) {
-			ctx->te_irq = ret;
-			ctx->out_type &= ~I80_HW_TRG;
-		}
-	} else {
-		ret = decon_conf_irq(ctx, "vsync", decon_irq_handler, 0, true);
-		if (ret < 0)
-			return ret;
-		ctx->irq = ret;
+	ret = decon_conf_irq(ctx, "lcd_sys", decon_irq_handler, 0);
+	if (ret < 0)
+		return ret;
+	ctx->irq_lcd_sys = ret;
+
+	ret = decon_conf_irq(ctx, "te", decon_te_irq_handler,
+			IRQF_TRIGGER_RISING);
+	if (ret < 0)
+		return ret;
+	if (ret) {
+		ctx->irq_te = ret;
+		ctx->out_type &= ~I80_HW_TRG;
 	}
 
 	if (ctx->out_type & I80_HW_TRG) {
-- 
2.7.4

[PATCH 7/7] dt-bindings: exynos5433-decon: remove i80-if-timings property

[Andrzej Hajda]

Since i80/command mode is determined in runtime by propagating info
from panel this property can be removed.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 .../devicetree/bindings/display/exynos/exynos5433-decon.txt         | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/exynos/exynos5433-decon.txt b/Documentation/devicetree/bindings/display/exynos/exynos5433-decon.txt
index 549c538..26a9269 100644
--- a/Documentation/devicetree/bindings/display/exynos/exynos5433-decon.txt
+++ b/Documentation/devicetree/bindings/display/exynos/exynos5433-decon.txt
@@ -25,12 +25,6 @@ Required properties:
 	 size-cells must 1 and 0, respectively.
 - port: contains an endpoint node which is connected to the endpoint in the mic
 	node. The reg value muset be 0.
-- i80-if-timings: specify whether the panel which is connected to decon uses
-		  i80 lcd interface or mipi video interface. This node contains
-		  no timing information as that of fimd does. Because there is
-		  no register in decon to specify i80 interface timing value,
-		  it is not needed, but make it remain to use same kind of node
-		  in fimd and exynos7 decon.
 
 Example:
 SoC specific DT entry:
-- 
2.7.4

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

Re: [PATCH 0/7] drm/exynos: panel mode info propagation

[Andrzej Hajda]

On 18.04.2017 14:39, Andrzej Hajda wrote:
> Hi Inki,
>
> This patchset beside cleanup/refactoring patches (01-03) adds code to propagate
> info provided by MIPI-DSI panels about its mode of work (video/command mode).
> The propagation is performed for whole pipeline as every its elements uses it.

Gently ping.

Regards
Andrzej

>
> Regards
> Andrzej
>
>
> Andrzej Hajda (7):
>   drm/exynos/decon5433: use readl_poll_timeout helpers
>   drm/exynos: use helper to set possible crtcs
>   drm/exynos/dsi: refactor panel detection logic
>   drm/exynos: propagate info about command mode from panel
>   drm/exynos/mic: use mode info stored in CRTC to detect i80 mode
>   drm/exynos/decon5433: use mode info stored in CRTC to detect i80 mode
>   dt-bindings: exynos5433-decon: remove i80-if-timings property
>
>  .../bindings/display/exynos/exynos5433-decon.txt   |   6 -
>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c      | 118 ++++++-----
>  drivers/gpu/drm/exynos/exynos_dp.c                 |  15 +-
>  drivers/gpu/drm/exynos/exynos_drm_core.c           |   1 +
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c           |  21 +-
>  drivers/gpu/drm/exynos/exynos_drm_crtc.h           |  10 +-
>  drivers/gpu/drm/exynos/exynos_drm_dpi.c            |  12 +-
>  drivers/gpu/drm/exynos/exynos_drm_drv.h            |   1 +
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c            | 219 ++++++++++-----------
>  drivers/gpu/drm/exynos/exynos_drm_mic.c            |  44 +----
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c           |  15 +-
>  drivers/gpu/drm/exynos/exynos_hdmi.c               |  25 +--
>  12 files changed, 228 insertions(+), 259 deletions(-)
>

Re: [PATCH 0/7] drm/exynos: panel mode info propagation

[Inki Dae]

2017년 07월 26일 16:09에 Andrzej Hajda 이(가) 쓴 글:
> On 18.04.2017 14:39, Andrzej Hajda wrote:
>> Hi Inki,
>>
>> This patchset beside cleanup/refactoring patches (01-03) adds code to propagate
>> info provided by MIPI-DSI panels about its mode of work (video/command mode).
>> The propagation is performed for whole pipeline as every its elements uses it.
> 
> Gently ping.

Sorry for late. Will review them soon.

Thanks,
Inki Dae

> 
> Regards
> Andrzej
> 
>>
>> Regards
>> Andrzej
>>
>>
>> Andrzej Hajda (7):
>>   drm/exynos/decon5433: use readl_poll_timeout helpers
>>   drm/exynos: use helper to set possible crtcs
>>   drm/exynos/dsi: refactor panel detection logic
>>   drm/exynos: propagate info about command mode from panel
>>   drm/exynos/mic: use mode info stored in CRTC to detect i80 mode
>>   drm/exynos/decon5433: use mode info stored in CRTC to detect i80 mode
>>   dt-bindings: exynos5433-decon: remove i80-if-timings property
>>
>>  .../bindings/display/exynos/exynos5433-decon.txt   |   6 -
>>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c      | 118 ++++++-----
>>  drivers/gpu/drm/exynos/exynos_dp.c                 |  15 +-
>>  drivers/gpu/drm/exynos/exynos_drm_core.c           |   1 +
>>  drivers/gpu/drm/exynos/exynos_drm_crtc.c           |  21 +-
>>  drivers/gpu/drm/exynos/exynos_drm_crtc.h           |  10 +-
>>  drivers/gpu/drm/exynos/exynos_drm_dpi.c            |  12 +-
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h            |   1 +
>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c            | 219 ++++++++++-----------
>>  drivers/gpu/drm/exynos/exynos_drm_mic.c            |  44 +----
>>  drivers/gpu/drm/exynos/exynos_drm_vidi.c           |  15 +-
>>  drivers/gpu/drm/exynos/exynos_hdmi.c               |  25 +--
>>  12 files changed, 228 insertions(+), 259 deletions(-)
>>
> 
> 
> 

Re: [PATCH 3/7] drm/exynos/dsi: refactor panel detection logic

[Inki Dae]

Hi Andrzej,


2017년 04월 18일 21:39에 Andrzej Hajda 이(가) 쓴 글:
> Description of drm_helper_hpd_irq_event clearly states that drivers
> supporting hotplug events per connector should use different helper -
> drm_kms_helper_hotplug_event. To achieve it following changes have
> been performed:
> - moved down all DSI ops - they require exynos_dsi_disable function
> to be defined earlier,
> - simplified exynos_dsi_detect - there is no real detection, it just
> returns if panel is attached,
> - DSI attach/detach callbacks attaches/detaches DRM panel and sets
> connector status and other context fields accordingly, all this is
> performed under mutex, as these callbacks are asynchronous.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 204 ++++++++++++++++----------------
>  1 file changed, 103 insertions(+), 101 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 3ae459f..515090f 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -254,7 +254,6 @@ struct exynos_dsi {
>  	struct drm_encoder encoder;
>  	struct mipi_dsi_host dsi_host;
>  	struct drm_connector connector;
> -	struct device_node *panel_node;
>  	struct drm_panel *panel;
>  	struct device *dev;
>  
> @@ -1329,12 +1328,13 @@ static int exynos_dsi_init(struct exynos_dsi *dsi)
>  	return 0;
>  }
>  
> -static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
> +static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi,
> +				      struct device *panel)
>  {
>  	int ret;
>  	int te_gpio_irq;
>  
> -	dsi->te_gpio = of_get_named_gpio(dsi->panel_node, "te-gpios", 0);
> +	dsi->te_gpio = of_get_named_gpio(panel->of_node, "te-gpios", 0);
>  	if (dsi->te_gpio == -ENOENT)
>  		return 0;
>  
> @@ -1374,85 +1374,6 @@ static void exynos_dsi_unregister_te_irq(struct exynos_dsi *dsi)
>  	}
>  }
>  
> -static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
> -				  struct mipi_dsi_device *device)
> -{
> -	struct exynos_dsi *dsi = host_to_dsi(host);
> -
> -	dsi->lanes = device->lanes;
> -	dsi->format = device->format;
> -	dsi->mode_flags = device->mode_flags;
> -	dsi->panel_node = device->dev.of_node;
> -
> -	/*
> -	 * This is a temporary solution and should be made by more generic way.
> -	 *
> -	 * If attached panel device is for command mode one, dsi should register
> -	 * TE interrupt handler.
> -	 */
> -	if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO)) {
> -		int ret = exynos_dsi_register_te_irq(dsi);
> -
> -		if (ret)
> -			return ret;
> -	}
> -
> -	if (dsi->connector.dev)
> -		drm_helper_hpd_irq_event(dsi->connector.dev);
> -
> -	return 0;
> -}
> -
> -static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
> -				  struct mipi_dsi_device *device)
> -{
> -	struct exynos_dsi *dsi = host_to_dsi(host);
> -
> -	exynos_dsi_unregister_te_irq(dsi);
> -
> -	dsi->panel_node = NULL;
> -
> -	if (dsi->connector.dev)
> -		drm_helper_hpd_irq_event(dsi->connector.dev);
> -
> -	return 0;
> -}
> -
> -static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
> -				        const struct mipi_dsi_msg *msg)
> -{
> -	struct exynos_dsi *dsi = host_to_dsi(host);
> -	struct exynos_dsi_transfer xfer;
> -	int ret;
> -
> -	if (!(dsi->state & DSIM_STATE_ENABLED))
> -		return -EINVAL;
> -
> -	if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
> -		ret = exynos_dsi_init(dsi);
> -		if (ret)
> -			return ret;
> -		dsi->state |= DSIM_STATE_INITIALIZED;
> -	}
> -
> -	ret = mipi_dsi_create_packet(&xfer.packet, msg);
> -	if (ret < 0)
> -		return ret;
> -
> -	xfer.rx_len = msg->rx_len;
> -	xfer.rx_payload = msg->rx_buf;
> -	xfer.flags = msg->flags;
> -
> -	ret = exynos_dsi_transfer(dsi, &xfer);
> -	return (ret < 0) ? ret : xfer.rx_done;
> -}
> -
> -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,
> -};
> -
>  static void exynos_dsi_enable(struct drm_encoder *encoder)
>  {
>  	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
> @@ -1508,25 +1429,7 @@ static void exynos_dsi_disable(struct drm_encoder *encoder)
>  static enum drm_connector_status
>  exynos_dsi_detect(struct drm_connector *connector, bool force)
>  {
> -	struct exynos_dsi *dsi = connector_to_dsi(connector);
> -
> -	if (!dsi->panel) {
> -		dsi->panel = of_drm_find_panel(dsi->panel_node);
> -		if (dsi->panel)
> -			drm_panel_attach(dsi->panel, &dsi->connector);
> -	} else if (!dsi->panel_node) {
> -		struct drm_encoder *encoder;
> -
> -		encoder = platform_get_drvdata(to_platform_device(dsi->dev));
> -		exynos_dsi_disable(encoder);
> -		drm_panel_detach(dsi->panel);
> -		dsi->panel = NULL;
> -	}
> -
> -	if (dsi->panel)
> -		return connector_status_connected;
> -
> -	return connector_status_disconnected;
> +	return connector->status;
>  }
>  
>  static void exynos_dsi_connector_destroy(struct drm_connector *connector)
> @@ -1576,6 +1479,7 @@ static int exynos_dsi_create_connector(struct drm_encoder *encoder)
>  		return ret;
>  	}
>  
> +	connector->status = connector_status_disconnected;
>  	drm_connector_helper_add(connector, &exynos_dsi_connector_helper_funcs);
>  	drm_mode_connector_attach_encoder(connector, encoder);
>  
> @@ -1612,6 +1516,104 @@ static const struct drm_encoder_funcs exynos_dsi_encoder_funcs = {
>  
>  MODULE_DEVICE_TABLE(of, exynos_dsi_of_match);
>  
> +static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
> +				  struct mipi_dsi_device *device)
> +{
> +	struct exynos_dsi *dsi = host_to_dsi(host);
> +	struct drm_device *drm = dsi->connector.dev;
> +
> +	/*
> +	 * This is a temporary solution and should be made by more generic way.
> +	 *
> +	 * If attached panel device is for command mode one, dsi should register
> +	 * TE interrupt handler.
> +	 */
> +	if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO)) {
> +		int ret = exynos_dsi_register_te_irq(dsi, &device->dev);
> +
> +		if (ret)
> +			return ret;
> +	}
> +
> +	mutex_lock(&drm->mode_config.mutex);
> +
> +	dsi->lanes = device->lanes;
> +	dsi->format = device->format;
> +	dsi->mode_flags = device->mode_flags;
> +	dsi->panel = of_drm_find_panel(device->dev.of_node);
> +	if (dsi->panel) {
> +		drm_panel_attach(dsi->panel, &dsi->connector);
> +		dsi->connector.status = connector_status_connected;
> +	}
> +
> +	mutex_unlock(&drm->mode_config.mutex);
> +
> +	if (drm->mode_config.poll_enabled)
> +		drm_kms_helper_hotplug_event(drm);

Shouldn't drm_kms_helper_hotplug_event function be called only when connector status is changed?

Thanks,
Inki Dae 

Re: [PATCH 4/7] drm/exynos: propagate info about command mode from panel

[Inki Dae]

2017년 04월 18일 21:40에 Andrzej Hajda 이(가) 쓴 글:
> mipi_dsi framework provides information about panel's mode of work.
> This info should be propagated upstream to configure all elements of
> the pipeline. As CRTC is the common nominator of the pipeline we can put
> such info into its structures.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 +
>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index 527bf1d..96b9d49 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -165,6 +165,7 @@ struct exynos_drm_crtc {
>  	const struct exynos_drm_crtc_ops	*ops;
>  	void				*ctx;
>  	struct exynos_drm_clk		*pipe_clk;
> +	bool				i80_mode : 1;
>  };
>  
>  static inline void exynos_drm_pipe_clk_enable(struct exynos_drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 515090f..79df1c9 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1545,6 +1545,8 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>  		drm_panel_attach(dsi->panel, &dsi->connector);
>  		dsi->connector.status = connector_status_connected;
>  	}
> +	exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode =
> +			!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO);

No place to use i80_mode in this patch. Seems this patch, patch 5 and portion of using i80_mode in patch 6 should become one patch. 

Thanks,
Inki Dae

>  
>  	mutex_unlock(&drm->mode_config.mutex);
>  
> 

Re: [PATCH 6/7] drm/exynos/decon5433: use mode info stored in CRTC to detect i80 mode

[Inki Dae]

Seems you mixed some different changes below with one patch.

1. add checking panel mode with i80_mode.
2. change te_irq to irq_te.
3. add new function, decon_atomic_check. 
4. add considering more error cases in decon_conf_irq function.
5. register command and video mode interrupt handlers regardless of panel mode.

And the subject of this patch doesn't say everything to above changes.

Thanks,
Inki Dae

2017년 04월 18일 21:40에 Andrzej Hajda 이(가) 쓴 글:
> Since panel's mode of work is propagated properly from panel to DECON,
> there is no need to use redundant private property. The only issue with
> such approach is that check for required interrupts should be postponed
> until panel communicate its requirements - ie to atomic_check.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 98 ++++++++++++++++-----------
>  1 file changed, 57 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> index 237b4c9..7a09e03 100644
> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> @@ -34,9 +34,8 @@
>  #define WINDOWS_NR	3
>  #define MIN_FB_WIDTH_FOR_16WORD_BURST	128
>  
> -#define IFTYPE_I80	(1 << 0)
> -#define I80_HW_TRG	(1 << 1)
> -#define IFTYPE_HDMI	(1 << 2)
> +#define I80_HW_TRG	(1 << 0)
> +#define IFTYPE_HDMI	(1 << 1)
>  
>  static const char * const decon_clks_name[] = {
>  	"pclk",
> @@ -58,7 +57,9 @@ struct decon_context {
>  	struct regmap			*sysreg;
>  	struct clk			*clks[ARRAY_SIZE(decon_clks_name)];
>  	unsigned int			irq;
> -	unsigned int			te_irq;
> +	unsigned int			irq_vsync;
> +	unsigned int			irq_lcd_sys;
> +	unsigned int			irq_te;
>  	unsigned long			out_type;
>  	int				first_win;
>  	spinlock_t			vblank_lock;
> @@ -91,7 +92,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
>  	u32 val;
>  
>  	val = VIDINTCON0_INTEN;
> -	if (ctx->out_type & IFTYPE_I80)
> +	if (crtc->i80_mode)
>  		val |= VIDINTCON0_FRAMEDONE;
>  	else
>  		val |= VIDINTCON0_INTFRMEN | VIDINTCON0_FRAMESEL_FP;
> @@ -100,7 +101,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
>  
>  	enable_irq(ctx->irq);
>  	if (!(ctx->out_type & I80_HW_TRG))
> -		enable_irq(ctx->te_irq);
> +		enable_irq(ctx->irq_te);
>  
>  	return 0;
>  }
> @@ -110,7 +111,7 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
>  	struct decon_context *ctx = crtc->ctx;
>  
>  	if (!(ctx->out_type & I80_HW_TRG))
> -		disable_irq_nosync(ctx->te_irq);
> +		disable_irq_nosync(ctx->irq_te);
>  	disable_irq_nosync(ctx->irq);
>  
>  	writel(0, ctx->addr + DECON_VIDINTCON0);
> @@ -140,7 +141,7 @@ static u32 decon_get_frame_count(struct decon_context *ctx, bool end)
>  
>  	switch (status & (VIDCON1_VSTATUS_MASK | VIDCON1_I80_ACTIVE)) {
>  	case VIDCON1_VSTATUS_VS:
> -		if (!(ctx->out_type & IFTYPE_I80))
> +		if (!(ctx->crtc->i80_mode))
>  			--frm;
>  		break;
>  	case VIDCON1_VSTATUS_BP:
> @@ -167,7 +168,7 @@ static u32 decon_get_vblank_counter(struct exynos_drm_crtc *crtc)
>  
>  static void decon_setup_trigger(struct decon_context *ctx)
>  {
> -	if (!(ctx->out_type & (IFTYPE_I80 | I80_HW_TRG)))
> +	if (!ctx->crtc->i80_mode && !(ctx->out_type & I80_HW_TRG))
>  		return;
>  
>  	if (!(ctx->out_type & I80_HW_TRG)) {
> @@ -207,7 +208,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>  	val = VIDOUT_LCD_ON;
>  	if (interlaced)
>  		val |= VIDOUT_INTERLACE_EN_F;
> -	if (ctx->out_type & IFTYPE_I80) {
> +	if (crtc->i80_mode) {
>  		val |= VIDOUT_COMMAND_IF;
>  	} else {
>  		val |= VIDOUT_RGB_IF;
> @@ -223,7 +224,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>  			VIDTCON2_HOZVAL(m->hdisplay - 1);
>  	writel(val, ctx->addr + DECON_VIDTCON2);
>  
> -	if (!(ctx->out_type & IFTYPE_I80)) {
> +	if (!crtc->i80_mode) {
>  		int vbp = m->crtc_vtotal - m->crtc_vsync_end;
>  		int vfp = m->crtc_vsync_start - m->crtc_vdisplay;
>  
> @@ -456,7 +457,7 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
>  	int i;
>  
>  	if (!(ctx->out_type & I80_HW_TRG))
> -		synchronize_irq(ctx->te_irq);
> +		synchronize_irq(ctx->irq_te);
>  	synchronize_irq(ctx->irq);
>  
>  	/*
> @@ -511,6 +512,22 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc)
>  		clk_disable_unprepare(ctx->clks[i]);
>  }
>  
> +static int decon_atomic_check(struct exynos_drm_crtc *crtc,
> +		struct drm_crtc_state *state)
> +{
> +	struct decon_context *ctx = crtc->ctx;
> +
> +	ctx->irq = crtc->i80_mode ? ctx->irq_lcd_sys : ctx->irq_vsync;
> +
> +	if (ctx->irq)
> +		return 0;
> +
> +	dev_err(ctx->dev, "Panel requires %s mode, but appropriate interrupt is not provided.\n",
> +			crtc->i80_mode ? "command" : "video");
> +
> +	return -EINVAL;
> +}
> +
>  static const struct exynos_drm_crtc_ops decon_crtc_ops = {
>  	.enable			= decon_enable,
>  	.disable		= decon_disable,
> @@ -521,6 +538,7 @@ static const struct exynos_drm_crtc_ops decon_crtc_ops = {
>  	.update_plane		= decon_update_plane,
>  	.disable_plane		= decon_disable_plane,
>  	.atomic_flush		= decon_atomic_flush,
> +	.atomic_check		= decon_atomic_check,
>  };
>  
>  static int decon_bind(struct device *dev, struct device *master, void *data)
> @@ -670,19 +688,22 @@ static const struct of_device_id exynos5433_decon_driver_dt_match[] = {
>  MODULE_DEVICE_TABLE(of, exynos5433_decon_driver_dt_match);
>  
>  static int decon_conf_irq(struct decon_context *ctx, const char *name,
> -		irq_handler_t handler, unsigned long int flags, bool required)
> +		irq_handler_t handler, unsigned long int flags)
>  {
>  	struct platform_device *pdev = to_platform_device(ctx->dev);
>  	int ret, irq = platform_get_irq_byname(pdev, name);
>  
>  	if (irq < 0) {
> -		if (irq == -EPROBE_DEFER)
> +		switch (irq) {
> +		case -EPROBE_DEFER:
> +			return irq;
> +		case -ENODATA:
> +		case -ENXIO:
> +			return 0;
> +		default:
> +			dev_err(ctx->dev, "IRQ %s get failed, %d\n", name, irq);
>  			return irq;
> -		if (required)
> -			dev_err(ctx->dev, "cannot get %s IRQ\n", name);
> -		else
> -			irq = 0;
> -		return irq;
> +		}
>  	}
>  	irq_set_status_flags(irq, IRQ_NOAUTOEN);
>  	ret = devm_request_irq(ctx->dev, irq, handler, flags, "drm_decon", ctx);
> @@ -710,11 +731,8 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
>  	ctx->out_type = (unsigned long)of_device_get_match_data(dev);
>  	spin_lock_init(&ctx->vblank_lock);
>  
> -	if (ctx->out_type & IFTYPE_HDMI) {
> +	if (ctx->out_type & IFTYPE_HDMI)
>  		ctx->first_win = 1;
> -	} else if (of_get_child_by_name(dev->of_node, "i80-if-timings")) {
> -		ctx->out_type |= IFTYPE_I80;
> -	}
>  
>  	for (i = 0; i < ARRAY_SIZE(decon_clks_name); i++) {
>  		struct clk *clk;
> @@ -738,25 +756,23 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
>  		return PTR_ERR(ctx->addr);
>  	}
>  
> -	if (ctx->out_type & IFTYPE_I80) {
> -		ret = decon_conf_irq(ctx, "lcd_sys", decon_irq_handler, 0, true);
> -		if (ret < 0)
> -			return ret;
> -		ctx->irq = ret;
> +	ret = decon_conf_irq(ctx, "vsync", decon_irq_handler, 0);
> +	if (ret < 0)
> +		return ret;
> +	ctx->irq_vsync = ret;
>  
> -		ret = decon_conf_irq(ctx, "te", decon_te_irq_handler,
> -				     IRQF_TRIGGER_RISING, false);
> -		if (ret < 0)
> -			return ret;
> -		if (ret) {
> -			ctx->te_irq = ret;
> -			ctx->out_type &= ~I80_HW_TRG;
> -		}
> -	} else {
> -		ret = decon_conf_irq(ctx, "vsync", decon_irq_handler, 0, true);
> -		if (ret < 0)
> -			return ret;
> -		ctx->irq = ret;
> +	ret = decon_conf_irq(ctx, "lcd_sys", decon_irq_handler, 0);
> +	if (ret < 0)
> +		return ret;
> +	ctx->irq_lcd_sys = ret;
> +
> +	ret = decon_conf_irq(ctx, "te", decon_te_irq_handler,
> +			IRQF_TRIGGER_RISING);
> +	if (ret < 0)
> +		return ret;
> +	if (ret) {
> +		ctx->irq_te = ret;
> +		ctx->out_type &= ~I80_HW_TRG;
>  	}
>  
>  	if (ctx->out_type & I80_HW_TRG) {
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/7] drm/exynos/dsi: refactor panel detection logic

[Andrzej Hajda]

Hi Inki,

Sorry for late response - vacation time.

On 01.08.2017 11:29, Inki Dae wrote:
> Hi Andrzej,
>
>
> 2017년 04월 18일 21:39에 Andrzej Hajda 이(가) 쓴 글:
>> Description of drm_helper_hpd_irq_event clearly states that drivers
>> supporting hotplug events per connector should use different helper -
>> drm_kms_helper_hotplug_event. To achieve it following changes have
>> been performed:
>> - moved down all DSI ops - they require exynos_dsi_disable function
>> to be defined earlier,
>> - simplified exynos_dsi_detect - there is no real detection, it just
>> returns if panel is attached,
>> - DSI attach/detach callbacks attaches/detaches DRM panel and sets
>> connector status and other context fields accordingly, all this is
>> performed under mutex, as these callbacks are asynchronous.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 204 ++++++++++++++++----------------
>>  1 file changed, 103 insertions(+), 101 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> index 3ae459f..515090f 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> @@ -254,7 +254,6 @@ struct exynos_dsi {
>>  	struct drm_encoder encoder;
>>  	struct mipi_dsi_host dsi_host;
>>  	struct drm_connector connector;
>> -	struct device_node *panel_node;
>>  	struct drm_panel *panel;
>>  	struct device *dev;
>>  
>> @@ -1329,12 +1328,13 @@ static int exynos_dsi_init(struct exynos_dsi *dsi)
>>  	return 0;
>>  }
>>  
>> -static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi)
>> +static int exynos_dsi_register_te_irq(struct exynos_dsi *dsi,
>> +				      struct device *panel)
>>  {
>>  	int ret;
>>  	int te_gpio_irq;
>>  
>> -	dsi->te_gpio = of_get_named_gpio(dsi->panel_node, "te-gpios", 0);
>> +	dsi->te_gpio = of_get_named_gpio(panel->of_node, "te-gpios", 0);
>>  	if (dsi->te_gpio == -ENOENT)
>>  		return 0;
>>  
>> @@ -1374,85 +1374,6 @@ static void exynos_dsi_unregister_te_irq(struct exynos_dsi *dsi)
>>  	}
>>  }
>>  
>> -static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>> -				  struct mipi_dsi_device *device)
>> -{
>> -	struct exynos_dsi *dsi = host_to_dsi(host);
>> -
>> -	dsi->lanes = device->lanes;
>> -	dsi->format = device->format;
>> -	dsi->mode_flags = device->mode_flags;
>> -	dsi->panel_node = device->dev.of_node;
>> -
>> -	/*
>> -	 * This is a temporary solution and should be made by more generic way.
>> -	 *
>> -	 * If attached panel device is for command mode one, dsi should register
>> -	 * TE interrupt handler.
>> -	 */
>> -	if (!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO)) {
>> -		int ret = exynos_dsi_register_te_irq(dsi);
>> -
>> -		if (ret)
>> -			return ret;
>> -	}
>> -
>> -	if (dsi->connector.dev)
>> -		drm_helper_hpd_irq_event(dsi->connector.dev);
>> -
>> -	return 0;
>> -}
>> -
>> -static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
>> -				  struct mipi_dsi_device *device)
>> -{
>> -	struct exynos_dsi *dsi = host_to_dsi(host);
>> -
>> -	exynos_dsi_unregister_te_irq(dsi);
>> -
>> -	dsi->panel_node = NULL;
>> -
>> -	if (dsi->connector.dev)
>> -		drm_helper_hpd_irq_event(dsi->connector.dev);
>> -
>> -	return 0;
>> -}
>> -
>> -static ssize_t exynos_dsi_host_transfer(struct mipi_dsi_host *host,
>> -				        const struct mipi_dsi_msg *msg)
>> -{
>> -	struct exynos_dsi *dsi = host_to_dsi(host);
>> -	struct exynos_dsi_transfer xfer;
>> -	int ret;
>> -
>> -	if (!(dsi->state & DSIM_STATE_ENABLED))
>> -		return -EINVAL;
>> -
>> -	if (!(dsi->state & DSIM_STATE_INITIALIZED)) {
>> -		ret = exynos_dsi_init(dsi);
>> -		if (ret)
>> -			return ret;
>> -		dsi->state |= DSIM_STATE_INITIALIZED;
>> -	}
>> -
>> -	ret = mipi_dsi_create_packet(&xfer.packet, msg);
>> -	if (ret < 0)
>> -		return ret;
>> -
>> -	xfer.rx_len = msg->rx_len;
>> -	xfer.rx_payload = msg->rx_buf;
>> -	xfer.flags = msg->flags;
>> -
>> -	ret = exynos_dsi_transfer(dsi, &xfer);
>> -	return (ret < 0) ? ret : xfer.rx_done;
>> -}
>> -
>> -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,
>> -};
>> -
>>  static void exynos_dsi_enable(struct drm_encoder *encoder)
>>  {
>>  	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
>> @@ -1508,25 +1429,7 @@ static void exynos_dsi_disable(struct drm_encoder *encoder)
>>  static enum drm_connector_status
>>  exynos_dsi_detect(struct drm_connector *connector, bool force)
>>  {
>> -	struct exynos_dsi *dsi = connector_to_dsi(connector);
>> -
>> -	if (!dsi->panel) {
>> -		dsi->panel = of_drm_find_panel(dsi->panel_node);
>> -		if (dsi->panel)
>> -			drm_panel_attach(dsi->panel, &dsi->connector);
>> -	} else if (!dsi->panel_node) {
>> -		struct drm_encoder *encoder;
>> -
>> -		encoder = platform_get_drvdata(to_platform_device(dsi->dev));
>> -		exynos_dsi_disable(encoder);
>> -		drm_panel_detach(dsi->panel);
>> -		dsi->panel = NULL;
>> -	}
>> -
>> -	if (dsi->panel)
>> -		return connector_status_connected;
>> -
>> -	return connector_status_disconnected;
>> +	return connector->status;
>>  }
>>  
>>  static void exynos_dsi_connector_destroy(struct drm_connector *connector)
>> @@ -1576,6 +1479,7 @@ static int exynos_dsi_create_connector(struct drm_encoder *encoder)
>>  		return ret;
>>  	}
>>  
>> +	connector->status = connector_status_disconnected;
>>  	drm_connector_helper_add(connector, &exynos_dsi_connector_helper_funcs);
>>  	drm_mode_connector_attach_encoder(connector, encoder);
>>  
>> @@ -1612,6 +1516,104 @@ static const struct drm_encoder_funcs exynos_dsi_encoder_funcs = {
>>  
>>  MODULE_DEVICE_TABLE(of, exynos_dsi_of_match);
>>  
>> +static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>> +				  struct mipi_dsi_device *device)
>> +{
>> +	struct exynos_dsi *dsi = host_to_dsi(host);
>> +	struct drm_device *drm = dsi->connector.dev;
>> +
>> +	/*
>> +	 * This is a temporary solution and should be made by more generic way.
>> +	 *
>> +	 * If attached panel device is for command mode one, dsi should register
>> +	 * TE interrupt handler.
>> +	 */
>> +	if (!(device->mode_flags & MIPI_DSI_MODE_VIDEO)) {
>> +		int ret = exynos_dsi_register_te_irq(dsi, &device->dev);
>> +
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	mutex_lock(&drm->mode_config.mutex);
>> +
>> +	dsi->lanes = device->lanes;
>> +	dsi->format = device->format;
>> +	dsi->mode_flags = device->mode_flags;
>> +	dsi->panel = of_drm_find_panel(device->dev.of_node);
>> +	if (dsi->panel) {
>> +		drm_panel_attach(dsi->panel, &dsi->connector);
>> +		dsi->connector.status = connector_status_connected;
>> +	}
>> +
>> +	mutex_unlock(&drm->mode_config.mutex);
>> +
>> +	if (drm->mode_config.poll_enabled)
>> +		drm_kms_helper_hotplug_event(drm);
> Shouldn't drm_kms_helper_hotplug_event function be called only when connector status is changed?

exynos_dsi_host_attach is already called only on connector status change.

Regards
Andrzej

>
> Thanks,
> Inki Dae 
>
>

Re: [PATCH 4/7] drm/exynos: propagate info about command mode from panel

[Andrzej Hajda]

On 01.08.2017 11:49, Inki Dae wrote:
>
> 2017년 04월 18일 21:40에 Andrzej Hajda 이(가) 쓴 글:
>> mipi_dsi framework provides information about panel's mode of work.
>> This info should be propagated upstream to configure all elements of
>> the pipeline. As CRTC is the common nominator of the pipeline we can put
>> such info into its structures.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 +
>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c | 2 ++
>>  2 files changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> index 527bf1d..96b9d49 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> @@ -165,6 +165,7 @@ struct exynos_drm_crtc {
>>  	const struct exynos_drm_crtc_ops	*ops;
>>  	void				*ctx;
>>  	struct exynos_drm_clk		*pipe_clk;
>> +	bool				i80_mode : 1;
>>  };
>>  
>>  static inline void exynos_drm_pipe_clk_enable(struct exynos_drm_crtc *crtc,
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> index 515090f..79df1c9 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>> @@ -1545,6 +1545,8 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>>  		drm_panel_attach(dsi->panel, &dsi->connector);
>>  		dsi->connector.status = connector_status_connected;
>>  	}
>> +	exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode =
>> +			!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO);
> No place to use i80_mode in this patch. Seems this patch, patch 5 and portion of using i80_mode in patch 6 should become one patch. 

I have tried to separate propagation of panel mode of work (in DSI
driver) from using this info in upstream drivers (MIC, DECON).
But if you prefer I can combine it into one patch.

Regards
Andrzej

>
> Thanks,
> Inki Dae
>
>>  
>>  	mutex_unlock(&drm->mode_config.mutex);
>>  
>>
>

Re: [PATCH 6/7] drm/exynos/decon5433: use mode info stored in CRTC to detect i80 mode

[Andrzej Hajda]

On 01.08.2017 12:02, Inki Dae wrote:
> Seems you mixed some different changes below with one patch.
>
> 1. add checking panel mode with i80_mode.
> 2. change te_irq to irq_te.
> 3. add new function, decon_atomic_check. 
> 4. add considering more error cases in decon_conf_irq function.
> 5. register command and video mode interrupt handlers regardless of panel mode.
>
> And the subject of this patch doesn't say everything to above changes.

I will try to split these changes into smaller patches. Moreover since
recent introduction of mode_valid callback for crtc I will drop
atomic_check in favor of it.

Regards
Andrzej

>
> Thanks,
> Inki Dae
>
> 2017년 04월 18일 21:40에 Andrzej Hajda 이(가) 쓴 글:
>> Since panel's mode of work is propagated properly from panel to DECON,
>> there is no need to use redundant private property. The only issue with
>> such approach is that check for required interrupts should be postponed
>> until panel communicate its requirements - ie to atomic_check.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 98 ++++++++++++++++-----------
>>  1 file changed, 57 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> index 237b4c9..7a09e03 100644
>> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> @@ -34,9 +34,8 @@
>>  #define WINDOWS_NR	3
>>  #define MIN_FB_WIDTH_FOR_16WORD_BURST	128
>>  
>> -#define IFTYPE_I80	(1 << 0)
>> -#define I80_HW_TRG	(1 << 1)
>> -#define IFTYPE_HDMI	(1 << 2)
>> +#define I80_HW_TRG	(1 << 0)
>> +#define IFTYPE_HDMI	(1 << 1)
>>  
>>  static const char * const decon_clks_name[] = {
>>  	"pclk",
>> @@ -58,7 +57,9 @@ struct decon_context {
>>  	struct regmap			*sysreg;
>>  	struct clk			*clks[ARRAY_SIZE(decon_clks_name)];
>>  	unsigned int			irq;
>> -	unsigned int			te_irq;
>> +	unsigned int			irq_vsync;
>> +	unsigned int			irq_lcd_sys;
>> +	unsigned int			irq_te;
>>  	unsigned long			out_type;
>>  	int				first_win;
>>  	spinlock_t			vblank_lock;
>> @@ -91,7 +92,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
>>  	u32 val;
>>  
>>  	val = VIDINTCON0_INTEN;
>> -	if (ctx->out_type & IFTYPE_I80)
>> +	if (crtc->i80_mode)
>>  		val |= VIDINTCON0_FRAMEDONE;
>>  	else
>>  		val |= VIDINTCON0_INTFRMEN | VIDINTCON0_FRAMESEL_FP;
>> @@ -100,7 +101,7 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
>>  
>>  	enable_irq(ctx->irq);
>>  	if (!(ctx->out_type & I80_HW_TRG))
>> -		enable_irq(ctx->te_irq);
>> +		enable_irq(ctx->irq_te);
>>  
>>  	return 0;
>>  }
>> @@ -110,7 +111,7 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
>>  	struct decon_context *ctx = crtc->ctx;
>>  
>>  	if (!(ctx->out_type & I80_HW_TRG))
>> -		disable_irq_nosync(ctx->te_irq);
>> +		disable_irq_nosync(ctx->irq_te);
>>  	disable_irq_nosync(ctx->irq);
>>  
>>  	writel(0, ctx->addr + DECON_VIDINTCON0);
>> @@ -140,7 +141,7 @@ static u32 decon_get_frame_count(struct decon_context *ctx, bool end)
>>  
>>  	switch (status & (VIDCON1_VSTATUS_MASK | VIDCON1_I80_ACTIVE)) {
>>  	case VIDCON1_VSTATUS_VS:
>> -		if (!(ctx->out_type & IFTYPE_I80))
>> +		if (!(ctx->crtc->i80_mode))
>>  			--frm;
>>  		break;
>>  	case VIDCON1_VSTATUS_BP:
>> @@ -167,7 +168,7 @@ static u32 decon_get_vblank_counter(struct exynos_drm_crtc *crtc)
>>  
>>  static void decon_setup_trigger(struct decon_context *ctx)
>>  {
>> -	if (!(ctx->out_type & (IFTYPE_I80 | I80_HW_TRG)))
>> +	if (!ctx->crtc->i80_mode && !(ctx->out_type & I80_HW_TRG))
>>  		return;
>>  
>>  	if (!(ctx->out_type & I80_HW_TRG)) {
>> @@ -207,7 +208,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>>  	val = VIDOUT_LCD_ON;
>>  	if (interlaced)
>>  		val |= VIDOUT_INTERLACE_EN_F;
>> -	if (ctx->out_type & IFTYPE_I80) {
>> +	if (crtc->i80_mode) {
>>  		val |= VIDOUT_COMMAND_IF;
>>  	} else {
>>  		val |= VIDOUT_RGB_IF;
>> @@ -223,7 +224,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
>>  			VIDTCON2_HOZVAL(m->hdisplay - 1);
>>  	writel(val, ctx->addr + DECON_VIDTCON2);
>>  
>> -	if (!(ctx->out_type & IFTYPE_I80)) {
>> +	if (!crtc->i80_mode) {
>>  		int vbp = m->crtc_vtotal - m->crtc_vsync_end;
>>  		int vfp = m->crtc_vsync_start - m->crtc_vdisplay;
>>  
>> @@ -456,7 +457,7 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
>>  	int i;
>>  
>>  	if (!(ctx->out_type & I80_HW_TRG))
>> -		synchronize_irq(ctx->te_irq);
>> +		synchronize_irq(ctx->irq_te);
>>  	synchronize_irq(ctx->irq);
>>  
>>  	/*
>> @@ -511,6 +512,22 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc)
>>  		clk_disable_unprepare(ctx->clks[i]);
>>  }
>>  
>> +static int decon_atomic_check(struct exynos_drm_crtc *crtc,
>> +		struct drm_crtc_state *state)
>> +{
>> +	struct decon_context *ctx = crtc->ctx;
>> +
>> +	ctx->irq = crtc->i80_mode ? ctx->irq_lcd_sys : ctx->irq_vsync;
>> +
>> +	if (ctx->irq)
>> +		return 0;
>> +
>> +	dev_err(ctx->dev, "Panel requires %s mode, but appropriate interrupt is not provided.\n",
>> +			crtc->i80_mode ? "command" : "video");
>> +
>> +	return -EINVAL;
>> +}
>> +
>>  static const struct exynos_drm_crtc_ops decon_crtc_ops = {
>>  	.enable			= decon_enable,
>>  	.disable		= decon_disable,
>> @@ -521,6 +538,7 @@ static const struct exynos_drm_crtc_ops decon_crtc_ops = {
>>  	.update_plane		= decon_update_plane,
>>  	.disable_plane		= decon_disable_plane,
>>  	.atomic_flush		= decon_atomic_flush,
>> +	.atomic_check		= decon_atomic_check,
>>  };
>>  
>>  static int decon_bind(struct device *dev, struct device *master, void *data)
>> @@ -670,19 +688,22 @@ static const struct of_device_id exynos5433_decon_driver_dt_match[] = {
>>  MODULE_DEVICE_TABLE(of, exynos5433_decon_driver_dt_match);
>>  
>>  static int decon_conf_irq(struct decon_context *ctx, const char *name,
>> -		irq_handler_t handler, unsigned long int flags, bool required)
>> +		irq_handler_t handler, unsigned long int flags)
>>  {
>>  	struct platform_device *pdev = to_platform_device(ctx->dev);
>>  	int ret, irq = platform_get_irq_byname(pdev, name);
>>  
>>  	if (irq < 0) {
>> -		if (irq == -EPROBE_DEFER)
>> +		switch (irq) {
>> +		case -EPROBE_DEFER:
>> +			return irq;
>> +		case -ENODATA:
>> +		case -ENXIO:
>> +			return 0;
>> +		default:
>> +			dev_err(ctx->dev, "IRQ %s get failed, %d\n", name, irq);
>>  			return irq;
>> -		if (required)
>> -			dev_err(ctx->dev, "cannot get %s IRQ\n", name);
>> -		else
>> -			irq = 0;
>> -		return irq;
>> +		}
>>  	}
>>  	irq_set_status_flags(irq, IRQ_NOAUTOEN);
>>  	ret = devm_request_irq(ctx->dev, irq, handler, flags, "drm_decon", ctx);
>> @@ -710,11 +731,8 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
>>  	ctx->out_type = (unsigned long)of_device_get_match_data(dev);
>>  	spin_lock_init(&ctx->vblank_lock);
>>  
>> -	if (ctx->out_type & IFTYPE_HDMI) {
>> +	if (ctx->out_type & IFTYPE_HDMI)
>>  		ctx->first_win = 1;
>> -	} else if (of_get_child_by_name(dev->of_node, "i80-if-timings")) {
>> -		ctx->out_type |= IFTYPE_I80;
>> -	}
>>  
>>  	for (i = 0; i < ARRAY_SIZE(decon_clks_name); i++) {
>>  		struct clk *clk;
>> @@ -738,25 +756,23 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
>>  		return PTR_ERR(ctx->addr);
>>  	}
>>  
>> -	if (ctx->out_type & IFTYPE_I80) {
>> -		ret = decon_conf_irq(ctx, "lcd_sys", decon_irq_handler, 0, true);
>> -		if (ret < 0)
>> -			return ret;
>> -		ctx->irq = ret;
>> +	ret = decon_conf_irq(ctx, "vsync", decon_irq_handler, 0);
>> +	if (ret < 0)
>> +		return ret;
>> +	ctx->irq_vsync = ret;
>>  
>> -		ret = decon_conf_irq(ctx, "te", decon_te_irq_handler,
>> -				     IRQF_TRIGGER_RISING, false);
>> -		if (ret < 0)
>> -			return ret;
>> -		if (ret) {
>> -			ctx->te_irq = ret;
>> -			ctx->out_type &= ~I80_HW_TRG;
>> -		}
>> -	} else {
>> -		ret = decon_conf_irq(ctx, "vsync", decon_irq_handler, 0, true);
>> -		if (ret < 0)
>> -			return ret;
>> -		ctx->irq = ret;
>> +	ret = decon_conf_irq(ctx, "lcd_sys", decon_irq_handler, 0);
>> +	if (ret < 0)
>> +		return ret;
>> +	ctx->irq_lcd_sys = ret;
>> +
>> +	ret = decon_conf_irq(ctx, "te", decon_te_irq_handler,
>> +			IRQF_TRIGGER_RISING);
>> +	if (ret < 0)
>> +		return ret;
>> +	if (ret) {
>> +		ctx->irq_te = ret;
>> +		ctx->out_type &= ~I80_HW_TRG;
>>  	}
>>  
>>  	if (ctx->out_type & I80_HW_TRG) {
>>
>

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