[PATCH 0/7] drm/exynos: add Ultra HD and interlace modes support to Exynos5433

[PATCH 0/7] drm/exynos: add Ultra HD and interlace modes support to Exynos5433

[Andrzej Hajda]

Hi Inki,

This patchset adds support for UltraHD and intrelace modes on DECON and HDMI
drivers of Exynos5433 chipset. To fully support it on TM2(e) platforms MHL
patches are alse required which will be posted in separate patchset.

Krzysztof, DTS patch do not depend on the previous patches, so there is
no need to synchronize branches.

Regards
Andrzej


Andrzej Hajda (7):
  drm/exynos/hdmi: add 297MHz pixel clock support
  drm/exynos/hdmi: fix VSI infoframe registers
  drm/exynos/hdmi: fix PLL for 27MHz settings
  drm/exynos/hdmi: add bridge support
  drm/exynos/decon5433: add support for interlace modes
  drm/exynos/decon5433: signal vblank only on odd fields
  arm64: dts: exynos: configure TV path clocks for Ultra HD modes

 arch/arm64/boot/dts/exynos/exynos5433.dtsi    |  3 +
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 52 ++++++++++++-----
 drivers/gpu/drm/exynos/exynos_hdmi.c          | 80 +++++++++++++++++++++------
 include/video/exynos5433_decon.h              |  2 +
 4 files changed, 107 insertions(+), 30 deletions(-)

-- 
2.7.4

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

[PATCH 0/7] drm/exynos: add Ultra HD and interlace modes support to Exynos5433

[Andrzej Hajda]

Hi Inki,

This patchset adds support for UltraHD and intrelace modes on DECON and HDMI
drivers of Exynos5433 chipset. To fully support it on TM2(e) platforms MHL
patches are alse required which will be posted in separate patchset.

Krzysztof, DTS patch do not depend on the previous patches, so there is
no need to synchronize branches.

Regards
Andrzej


Andrzej Hajda (7):
  drm/exynos/hdmi: add 297MHz pixel clock support
  drm/exynos/hdmi: fix VSI infoframe registers
  drm/exynos/hdmi: fix PLL for 27MHz settings
  drm/exynos/hdmi: add bridge support
  drm/exynos/decon5433: add support for interlace modes
  drm/exynos/decon5433: signal vblank only on odd fields
  arm64: dts: exynos: configure TV path clocks for Ultra HD modes

 arch/arm64/boot/dts/exynos/exynos5433.dtsi    |  3 +
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 52 ++++++++++++-----
 drivers/gpu/drm/exynos/exynos_hdmi.c          | 80 +++++++++++++++++++++------
 include/video/exynos5433_decon.h              |  2 +
 4 files changed, 107 insertions(+), 30 deletions(-)

-- 
2.7.4

[PATCH 1/7] drm/exynos/hdmi: add 297MHz pixel clock support

[Andrzej Hajda]

297MHz is used by Ultra HD modes.

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

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 5ed8b1e..bef8965 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -587,6 +587,15 @@ static const struct hdmiphy_config hdmiphy_5433_configs[] = {
 			0x08, 0x10, 0x01, 0x01, 0x48, 0x4a, 0x00, 0x40,
 		},
 	},
+	{
+		.pixel_clock = 297000000,
+		.conf = {
+			0x01, 0x51, 0x3E, 0x05, 0x40, 0xF0, 0x88, 0xC2,
+			0x52, 0x53, 0x44, 0x8C, 0x27, 0x00, 0x7C, 0xAC,
+			0xD6, 0x2B, 0x67, 0x00, 0x00, 0x04, 0x00, 0x30,
+			0x08, 0x10, 0x01, 0x01, 0x48, 0x40, 0x00, 0x40,
+		},
+	},
 };
 
 static const char * const hdmi_clk_gates4[] = {
-- 
2.7.4

[PATCH 2/7] drm/exynos/hdmi: fix VSI infoframe registers

[Andrzej Hajda]

VSI infoframe registers address space is non-contiguous, so infoframe write
should be split into two chunks.

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

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index bef8965..223b872 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -797,7 +797,8 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
 				sizeof(buf));
 	if (ret > 0) {
 		hdmi_reg_writeb(hdata, HDMI_VSI_CON, HDMI_VSI_CON_EVERY_VSYNC);
-		hdmi_reg_write_buf(hdata, HDMI_VSI_HEADER0, buf, ret);
+		hdmi_reg_write_buf(hdata, HDMI_VSI_HEADER0, buf, 3);
+		hdmi_reg_write_buf(hdata, HDMI_VSI_DATA(0), buf + 3, ret - 3);
 	}
 
 	ret = hdmi_audio_infoframe_init(&frm.audio);
-- 
2.7.4

[PATCH 3/7] drm/exynos/hdmi: fix PLL for 27MHz settings

[Andrzej Hajda]

Current settings for 27MHz and 27.027MHz do not work. Use the settings from
vendor code instead.

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

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 223b872..a73b192 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -509,9 +509,9 @@ static const struct hdmiphy_config hdmiphy_5433_configs[] = {
 	{
 		.pixel_clock = 27000000,
 		.conf = {
-			0x01, 0x51, 0x22, 0x51, 0x08, 0xfc, 0x88, 0x46,
-			0x72, 0x50, 0x24, 0x0c, 0x24, 0x0f, 0x7c, 0xa5,
-			0xd4, 0x2b, 0x87, 0x00, 0x00, 0x04, 0x00, 0x30,
+			0x01, 0x51, 0x2d, 0x75, 0x01, 0x00, 0x88, 0x02,
+			0x72, 0x50, 0x44, 0x8c, 0x27, 0x00, 0x7c, 0xac,
+			0xd6, 0x2b, 0x67, 0x00, 0x00, 0x04, 0x00, 0x30,
 			0x08, 0x10, 0x01, 0x01, 0x48, 0x40, 0x00, 0x40,
 		},
 	},
@@ -519,9 +519,9 @@ static const struct hdmiphy_config hdmiphy_5433_configs[] = {
 		.pixel_clock = 27027000,
 		.conf = {
 			0x01, 0x51, 0x2d, 0x72, 0x64, 0x09, 0x88, 0xc3,
-			0x71, 0x50, 0x24, 0x14, 0x24, 0x0f, 0x7c, 0xa5,
-			0xd4, 0x2b, 0x87, 0x00, 0x00, 0x04, 0x00, 0x30,
-			0x28, 0x10, 0x01, 0x01, 0x48, 0x40, 0x00, 0x40,
+			0x71, 0x50, 0x44, 0x8c, 0x27, 0x00, 0x7c, 0xac,
+			0xd6, 0x2b, 0x67, 0x00, 0x00, 0x04, 0x00, 0x30,
+			0x08, 0x10, 0x01, 0x01, 0x48, 0x40, 0x00, 0x40,
 		},
 	},
 	{
-- 
2.7.4

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

[PATCH 3/7] drm/exynos/hdmi: fix PLL for 27MHz settings

[Andrzej Hajda]

Current settings for 27MHz and 27.027MHz do not work. Use the settings from
vendor code instead.

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

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 223b872..a73b192 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -509,9 +509,9 @@ static const struct hdmiphy_config hdmiphy_5433_configs[] = {
 	{
 		.pixel_clock = 27000000,
 		.conf = {
-			0x01, 0x51, 0x22, 0x51, 0x08, 0xfc, 0x88, 0x46,
-			0x72, 0x50, 0x24, 0x0c, 0x24, 0x0f, 0x7c, 0xa5,
-			0xd4, 0x2b, 0x87, 0x00, 0x00, 0x04, 0x00, 0x30,
+			0x01, 0x51, 0x2d, 0x75, 0x01, 0x00, 0x88, 0x02,
+			0x72, 0x50, 0x44, 0x8c, 0x27, 0x00, 0x7c, 0xac,
+			0xd6, 0x2b, 0x67, 0x00, 0x00, 0x04, 0x00, 0x30,
 			0x08, 0x10, 0x01, 0x01, 0x48, 0x40, 0x00, 0x40,
 		},
 	},
@@ -519,9 +519,9 @@ static const struct hdmiphy_config hdmiphy_5433_configs[] = {
 		.pixel_clock = 27027000,
 		.conf = {
 			0x01, 0x51, 0x2d, 0x72, 0x64, 0x09, 0x88, 0xc3,
-			0x71, 0x50, 0x24, 0x14, 0x24, 0x0f, 0x7c, 0xa5,
-			0xd4, 0x2b, 0x87, 0x00, 0x00, 0x04, 0x00, 0x30,
-			0x28, 0x10, 0x01, 0x01, 0x48, 0x40, 0x00, 0x40,
+			0x71, 0x50, 0x44, 0x8c, 0x27, 0x00, 0x7c, 0xac,
+			0xd6, 0x2b, 0x67, 0x00, 0x00, 0x04, 0x00, 0x30,
+			0x08, 0x10, 0x01, 0x01, 0x48, 0x40, 0x00, 0x40,
 		},
 	},
 	{
-- 
2.7.4

[PATCH 4/7] drm/exynos/hdmi: add bridge support

[Andrzej Hajda]

In some platforms there is attached another device to the end of HDMI.
The patch adds support for it.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 56 +++++++++++++++++++++++++++++-------
 1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index a73b192..41fb894 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -35,6 +35,7 @@
 #include <linux/io.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
+#include <linux/of_graph.h>
 #include <linux/hdmi.h>
 #include <linux/component.h>
 #include <linux/mfd/syscon.h>
@@ -133,6 +134,7 @@ struct hdmi_context {
 	struct regulator_bulk_data	regul_bulk[ARRAY_SIZE(supply)];
 	struct regulator		*reg_hdmi_en;
 	struct exynos_drm_clk		phy_clk;
+	struct drm_bridge		*bridge;
 };
 
 static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
@@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
 	drm_connector_register(connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
-	return 0;
+	if (hdata->bridge) {
+		encoder->bridge = hdata->bridge;
+		hdata->bridge->encoder = encoder;
+		ret = drm_bridge_attach(encoder->dev, hdata->bridge);
+		if (ret)
+			DRM_ERROR("Failed to attach bridge\n");
+	}
+
+	return ret;
 }
 
 static bool hdmi_mode_fixup(struct drm_encoder *encoder,
@@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk *clk, bool enable)
 		hdmiphy_disable(hdata);
 }
 
+static int hdmi_bridge_init(struct hdmi_context *hdata)
+{
+	struct device *dev = hdata->dev;
+	struct device_node *ep, *np;
+
+	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
+	if (!ep)
+		return 0;
+
+	np = of_graph_get_remote_port_parent(ep);
+	of_node_put(ep);
+	if (!np) {
+		DRM_ERROR("failed to get remote port parent");
+		return -EINVAL;
+	}
+
+	hdata->bridge = of_drm_find_bridge(np);
+	of_node_put(np);
+
+	if (!hdata->bridge)
+		return -EPROBE_DEFER;
+
+	return 0;
+}
+
 static int hdmi_resources_init(struct hdmi_context *hdata)
 {
 	struct device *dev = hdata->dev;
@@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
 
 	hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
 
-	if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
-		return 0;
+	if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
+		if (IS_ERR(hdata->reg_hdmi_en))
+			return PTR_ERR(hdata->reg_hdmi_en);
 
-	if (IS_ERR(hdata->reg_hdmi_en))
-		return PTR_ERR(hdata->reg_hdmi_en);
-
-	ret = regulator_enable(hdata->reg_hdmi_en);
-	if (ret)
-		DRM_ERROR("failed to enable hdmi-en regulator\n");
+		ret = regulator_enable(hdata->reg_hdmi_en);
+		if (ret) {
+			DRM_ERROR("failed to enable hdmi-en regulator\n");
+			return ret;
+		}
+	}
 
-	return ret;
+	return hdmi_bridge_init(hdata);
 }
 
 static struct of_device_id hdmi_match_types[] = {
-- 
2.7.4

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

[PATCH 4/7] drm/exynos/hdmi: add bridge support

[Andrzej Hajda]

In some platforms there is attached another device to the end of HDMI.
The patch adds support for it.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 56 +++++++++++++++++++++++++++++-------
 1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index a73b192..41fb894 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -35,6 +35,7 @@
 #include <linux/io.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
+#include <linux/of_graph.h>
 #include <linux/hdmi.h>
 #include <linux/component.h>
 #include <linux/mfd/syscon.h>
@@ -133,6 +134,7 @@ struct hdmi_context {
 	struct regulator_bulk_data	regul_bulk[ARRAY_SIZE(supply)];
 	struct regulator		*reg_hdmi_en;
 	struct exynos_drm_clk		phy_clk;
+	struct drm_bridge		*bridge;
 };
 
 static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
@@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
 	drm_connector_register(connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
-	return 0;
+	if (hdata->bridge) {
+		encoder->bridge = hdata->bridge;
+		hdata->bridge->encoder = encoder;
+		ret = drm_bridge_attach(encoder->dev, hdata->bridge);
+		if (ret)
+			DRM_ERROR("Failed to attach bridge\n");
+	}
+
+	return ret;
 }
 
 static bool hdmi_mode_fixup(struct drm_encoder *encoder,
@@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk *clk, bool enable)
 		hdmiphy_disable(hdata);
 }
 
+static int hdmi_bridge_init(struct hdmi_context *hdata)
+{
+	struct device *dev = hdata->dev;
+	struct device_node *ep, *np;
+
+	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
+	if (!ep)
+		return 0;
+
+	np = of_graph_get_remote_port_parent(ep);
+	of_node_put(ep);
+	if (!np) {
+		DRM_ERROR("failed to get remote port parent");
+		return -EINVAL;
+	}
+
+	hdata->bridge = of_drm_find_bridge(np);
+	of_node_put(np);
+
+	if (!hdata->bridge)
+		return -EPROBE_DEFER;
+
+	return 0;
+}
+
 static int hdmi_resources_init(struct hdmi_context *hdata)
 {
 	struct device *dev = hdata->dev;
@@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
 
 	hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
 
-	if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
-		return 0;
+	if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
+		if (IS_ERR(hdata->reg_hdmi_en))
+			return PTR_ERR(hdata->reg_hdmi_en);
 
-	if (IS_ERR(hdata->reg_hdmi_en))
-		return PTR_ERR(hdata->reg_hdmi_en);
-
-	ret = regulator_enable(hdata->reg_hdmi_en);
-	if (ret)
-		DRM_ERROR("failed to enable hdmi-en regulator\n");
+		ret = regulator_enable(hdata->reg_hdmi_en);
+		if (ret) {
+			DRM_ERROR("failed to enable hdmi-en regulator\n");
+			return ret;
+		}
+	}
 
-	return ret;
+	return hdmi_bridge_init(hdata);
 }
 
 static struct of_device_id hdmi_match_types[] = {
-- 
2.7.4

Re: [PATCH 4/7] drm/exynos/hdmi: add bridge support

[Inki Dae]

2017년 01월 20일 15:52에 Andrzej Hajda 이(가) 쓴 글:
> In some platforms there is attached another device to the end of HDMI.
> The patch adds support for it.

Andrzej, can you clarify what bridge device can be attached and actually is now attached to the end of HDMI?
And I wonder if we have the device tree file which defines the bridge device in mainline. Seems we have no the device tree file.

If nothing, then this patch should be merged with real use case.

Thanks.

> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_hdmi.c | 56 +++++++++++++++++++++++++++++-------
>  1 file changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index a73b192..41fb894 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -35,6 +35,7 @@
>  #include <linux/io.h>
>  #include <linux/of_address.h>
>  #include <linux/of_device.h>
> +#include <linux/of_graph.h>
>  #include <linux/hdmi.h>
>  #include <linux/component.h>
>  #include <linux/mfd/syscon.h>
> @@ -133,6 +134,7 @@ struct hdmi_context {
>  	struct regulator_bulk_data	regul_bulk[ARRAY_SIZE(supply)];
>  	struct regulator		*reg_hdmi_en;
>  	struct exynos_drm_clk		phy_clk;
> +	struct drm_bridge		*bridge;
>  };
>  
>  static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
> @@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
>  	drm_connector_register(connector);
>  	drm_mode_connector_attach_encoder(connector, encoder);
>  
> -	return 0;
> +	if (hdata->bridge) {
> +		encoder->bridge = hdata->bridge;
> +		hdata->bridge->encoder = encoder;
> +		ret = drm_bridge_attach(encoder->dev, hdata->bridge);
> +		if (ret)
> +			DRM_ERROR("Failed to attach bridge\n");
> +	}
> +
> +	return ret;
>  }
>  
>  static bool hdmi_mode_fixup(struct drm_encoder *encoder,
> @@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk *clk, bool enable)
>  		hdmiphy_disable(hdata);
>  }
>  
> +static int hdmi_bridge_init(struct hdmi_context *hdata)
> +{
> +	struct device *dev = hdata->dev;
> +	struct device_node *ep, *np;
> +
> +	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> +	if (!ep)
> +		return 0;
> +
> +	np = of_graph_get_remote_port_parent(ep);
> +	of_node_put(ep);
> +	if (!np) {
> +		DRM_ERROR("failed to get remote port parent");
> +		return -EINVAL;
> +	}
> +
> +	hdata->bridge = of_drm_find_bridge(np);
> +	of_node_put(np);
> +
> +	if (!hdata->bridge)
> +		return -EPROBE_DEFER;
> +
> +	return 0;
> +}
> +
>  static int hdmi_resources_init(struct hdmi_context *hdata)
>  {
>  	struct device *dev = hdata->dev;
> @@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>  
>  	hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
>  
> -	if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
> -		return 0;
> +	if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
> +		if (IS_ERR(hdata->reg_hdmi_en))
> +			return PTR_ERR(hdata->reg_hdmi_en);
>  
> -	if (IS_ERR(hdata->reg_hdmi_en))
> -		return PTR_ERR(hdata->reg_hdmi_en);
> -
> -	ret = regulator_enable(hdata->reg_hdmi_en);
> -	if (ret)
> -		DRM_ERROR("failed to enable hdmi-en regulator\n");
> +		ret = regulator_enable(hdata->reg_hdmi_en);
> +		if (ret) {
> +			DRM_ERROR("failed to enable hdmi-en regulator\n");
> +			return ret;
> +		}
> +	}
>  
> -	return ret;
> +	return hdmi_bridge_init(hdata);
>  }
>  
>  static struct of_device_id hdmi_match_types[] = {
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/7] drm/exynos/hdmi: add bridge support

[Inki Dae]

2017년 01월 20일 15:52에 Andrzej Hajda 이(가) 쓴 글:
> In some platforms there is attached another device to the end of HDMI.
> The patch adds support for it.

Andrzej, can you clarify what bridge device can be attached and actually is now attached to the end of HDMI?
And I wonder if we have the device tree file which defines the bridge device in mainline. Seems we have no the device tree file.

If nothing, then this patch should be merged with real use case.

Thanks.

> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_hdmi.c | 56 +++++++++++++++++++++++++++++-------
>  1 file changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index a73b192..41fb894 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -35,6 +35,7 @@
>  #include <linux/io.h>
>  #include <linux/of_address.h>
>  #include <linux/of_device.h>
> +#include <linux/of_graph.h>
>  #include <linux/hdmi.h>
>  #include <linux/component.h>
>  #include <linux/mfd/syscon.h>
> @@ -133,6 +134,7 @@ struct hdmi_context {
>  	struct regulator_bulk_data	regul_bulk[ARRAY_SIZE(supply)];
>  	struct regulator		*reg_hdmi_en;
>  	struct exynos_drm_clk		phy_clk;
> +	struct drm_bridge		*bridge;
>  };
>  
>  static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
> @@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
>  	drm_connector_register(connector);
>  	drm_mode_connector_attach_encoder(connector, encoder);
>  
> -	return 0;
> +	if (hdata->bridge) {
> +		encoder->bridge = hdata->bridge;
> +		hdata->bridge->encoder = encoder;
> +		ret = drm_bridge_attach(encoder->dev, hdata->bridge);
> +		if (ret)
> +			DRM_ERROR("Failed to attach bridge\n");
> +	}
> +
> +	return ret;
>  }
>  
>  static bool hdmi_mode_fixup(struct drm_encoder *encoder,
> @@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk *clk, bool enable)
>  		hdmiphy_disable(hdata);
>  }
>  
> +static int hdmi_bridge_init(struct hdmi_context *hdata)
> +{
> +	struct device *dev = hdata->dev;
> +	struct device_node *ep, *np;
> +
> +	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> +	if (!ep)
> +		return 0;
> +
> +	np = of_graph_get_remote_port_parent(ep);
> +	of_node_put(ep);
> +	if (!np) {
> +		DRM_ERROR("failed to get remote port parent");
> +		return -EINVAL;
> +	}
> +
> +	hdata->bridge = of_drm_find_bridge(np);
> +	of_node_put(np);
> +
> +	if (!hdata->bridge)
> +		return -EPROBE_DEFER;
> +
> +	return 0;
> +}
> +
>  static int hdmi_resources_init(struct hdmi_context *hdata)
>  {
>  	struct device *dev = hdata->dev;
> @@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>  
>  	hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
>  
> -	if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
> -		return 0;
> +	if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
> +		if (IS_ERR(hdata->reg_hdmi_en))
> +			return PTR_ERR(hdata->reg_hdmi_en);
>  
> -	if (IS_ERR(hdata->reg_hdmi_en))
> -		return PTR_ERR(hdata->reg_hdmi_en);
> -
> -	ret = regulator_enable(hdata->reg_hdmi_en);
> -	if (ret)
> -		DRM_ERROR("failed to enable hdmi-en regulator\n");
> +		ret = regulator_enable(hdata->reg_hdmi_en);
> +		if (ret) {
> +			DRM_ERROR("failed to enable hdmi-en regulator\n");
> +			return ret;
> +		}
> +	}
>  
> -	return ret;
> +	return hdmi_bridge_init(hdata);
>  }
>  
>  static struct of_device_id hdmi_match_types[] = {
> 

Re: [PATCH 4/7] drm/exynos/hdmi: add bridge support

[Andrzej Hajda]

On 01.02.2017 08:31, Inki Dae wrote:
>
> 2017년 01월 20일 15:52에 Andrzej Hajda 이(가) 쓴 글:
>> In some platforms there is attached another device to the end of HDMI.
>> The patch adds support for it.
> Andrzej, can you clarify what bridge device can be attached and actually is now attached to the end of HDMI?
> And I wonder if we have the device tree file which defines the bridge device in mainline. Seems we have no the device tree file.
>
> If nothing, then this patch should be merged with real use case.

SiI8620 MHL bridge in tm2/tm2e.

Regards
Andrzej

>
> Thanks.
>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_hdmi.c | 56 +++++++++++++++++++++++++++++-------
>>  1 file changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index a73b192..41fb894 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -35,6 +35,7 @@
>>  #include <linux/io.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of_device.h>
>> +#include <linux/of_graph.h>
>>  #include <linux/hdmi.h>
>>  #include <linux/component.h>
>>  #include <linux/mfd/syscon.h>
>> @@ -133,6 +134,7 @@ struct hdmi_context {
>>  	struct regulator_bulk_data	regul_bulk[ARRAY_SIZE(supply)];
>>  	struct regulator		*reg_hdmi_en;
>>  	struct exynos_drm_clk		phy_clk;
>> +	struct drm_bridge		*bridge;
>>  };
>>  
>>  static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
>> @@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
>>  	drm_connector_register(connector);
>>  	drm_mode_connector_attach_encoder(connector, encoder);
>>  
>> -	return 0;
>> +	if (hdata->bridge) {
>> +		encoder->bridge = hdata->bridge;
>> +		hdata->bridge->encoder = encoder;
>> +		ret = drm_bridge_attach(encoder->dev, hdata->bridge);
>> +		if (ret)
>> +			DRM_ERROR("Failed to attach bridge\n");
>> +	}
>> +
>> +	return ret;
>>  }
>>  
>>  static bool hdmi_mode_fixup(struct drm_encoder *encoder,
>> @@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk *clk, bool enable)
>>  		hdmiphy_disable(hdata);
>>  }
>>  
>> +static int hdmi_bridge_init(struct hdmi_context *hdata)
>> +{
>> +	struct device *dev = hdata->dev;
>> +	struct device_node *ep, *np;
>> +
>> +	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
>> +	if (!ep)
>> +		return 0;
>> +
>> +	np = of_graph_get_remote_port_parent(ep);
>> +	of_node_put(ep);
>> +	if (!np) {
>> +		DRM_ERROR("failed to get remote port parent");
>> +		return -EINVAL;
>> +	}
>> +
>> +	hdata->bridge = of_drm_find_bridge(np);
>> +	of_node_put(np);
>> +
>> +	if (!hdata->bridge)
>> +		return -EPROBE_DEFER;
>> +
>> +	return 0;
>> +}
>> +
>>  static int hdmi_resources_init(struct hdmi_context *hdata)
>>  {
>>  	struct device *dev = hdata->dev;
>> @@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>>  
>>  	hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
>>  
>> -	if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
>> -		return 0;
>> +	if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
>> +		if (IS_ERR(hdata->reg_hdmi_en))
>> +			return PTR_ERR(hdata->reg_hdmi_en);
>>  
>> -	if (IS_ERR(hdata->reg_hdmi_en))
>> -		return PTR_ERR(hdata->reg_hdmi_en);
>> -
>> -	ret = regulator_enable(hdata->reg_hdmi_en);
>> -	if (ret)
>> -		DRM_ERROR("failed to enable hdmi-en regulator\n");
>> +		ret = regulator_enable(hdata->reg_hdmi_en);
>> +		if (ret) {
>> +			DRM_ERROR("failed to enable hdmi-en regulator\n");
>> +			return ret;
>> +		}
>> +	}
>>  
>> -	return ret;
>> +	return hdmi_bridge_init(hdata);
>>  }
>>  
>>  static struct of_device_id hdmi_match_types[] = {
>>
>

Re: [PATCH 4/7] drm/exynos/hdmi: add bridge support

[Inki Dae]

2017년 02월 01일 16:34에 Andrzej Hajda 이(가) 쓴 글:
> On 01.02.2017 08:31, Inki Dae wrote:
>>
>> 2017년 01월 20일 15:52에 Andrzej Hajda 이(가) 쓴 글:
>>> In some platforms there is attached another device to the end of HDMI.
>>> The patch adds support for it.
>> Andrzej, can you clarify what bridge device can be attached and actually is now attached to the end of HDMI?
>> And I wonder if we have the device tree file which defines the bridge device in mainline. Seems we have no the device tree file.
>>
>> If nothing, then this patch should be merged with real use case.
> 
> SiI8620 MHL bridge in tm2/tm2e.

Did you post dt patch to add the properties related to SiI8620 MHL bridge device?

I see below patch series,
[PATCH 00/24] drm/bridge/sii8620: add Ultra HD modes support

but I couldn't find dt patch for it. And specifying SiI8620 MHL bridge device as description would be needed.

Thanks.

> 
> Regards
> Andrzej
> 
>>
>> Thanks.
>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_hdmi.c | 56 +++++++++++++++++++++++++++++-------
>>>  1 file changed, 46 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> index a73b192..41fb894 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> @@ -35,6 +35,7 @@
>>>  #include <linux/io.h>
>>>  #include <linux/of_address.h>
>>>  #include <linux/of_device.h>
>>> +#include <linux/of_graph.h>
>>>  #include <linux/hdmi.h>
>>>  #include <linux/component.h>
>>>  #include <linux/mfd/syscon.h>
>>> @@ -133,6 +134,7 @@ struct hdmi_context {
>>>  	struct regulator_bulk_data	regul_bulk[ARRAY_SIZE(supply)];
>>>  	struct regulator		*reg_hdmi_en;
>>>  	struct exynos_drm_clk		phy_clk;
>>> +	struct drm_bridge		*bridge;
>>>  };
>>>  
>>>  static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
>>> @@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
>>>  	drm_connector_register(connector);
>>>  	drm_mode_connector_attach_encoder(connector, encoder);
>>>  
>>> -	return 0;
>>> +	if (hdata->bridge) {
>>> +		encoder->bridge = hdata->bridge;
>>> +		hdata->bridge->encoder = encoder;
>>> +		ret = drm_bridge_attach(encoder->dev, hdata->bridge);
>>> +		if (ret)
>>> +			DRM_ERROR("Failed to attach bridge\n");
>>> +	}
>>> +
>>> +	return ret;
>>>  }
>>>  
>>>  static bool hdmi_mode_fixup(struct drm_encoder *encoder,
>>> @@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk *clk, bool enable)
>>>  		hdmiphy_disable(hdata);
>>>  }
>>>  
>>> +static int hdmi_bridge_init(struct hdmi_context *hdata)
>>> +{
>>> +	struct device *dev = hdata->dev;
>>> +	struct device_node *ep, *np;
>>> +
>>> +	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
>>> +	if (!ep)
>>> +		return 0;
>>> +
>>> +	np = of_graph_get_remote_port_parent(ep);
>>> +	of_node_put(ep);
>>> +	if (!np) {
>>> +		DRM_ERROR("failed to get remote port parent");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	hdata->bridge = of_drm_find_bridge(np);
>>> +	of_node_put(np);
>>> +
>>> +	if (!hdata->bridge)
>>> +		return -EPROBE_DEFER;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  static int hdmi_resources_init(struct hdmi_context *hdata)
>>>  {
>>>  	struct device *dev = hdata->dev;
>>> @@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>>>  
>>>  	hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
>>>  
>>> -	if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
>>> -		return 0;
>>> +	if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
>>> +		if (IS_ERR(hdata->reg_hdmi_en))
>>> +			return PTR_ERR(hdata->reg_hdmi_en);
>>>  
>>> -	if (IS_ERR(hdata->reg_hdmi_en))
>>> -		return PTR_ERR(hdata->reg_hdmi_en);
>>> -
>>> -	ret = regulator_enable(hdata->reg_hdmi_en);
>>> -	if (ret)
>>> -		DRM_ERROR("failed to enable hdmi-en regulator\n");
>>> +		ret = regulator_enable(hdata->reg_hdmi_en);
>>> +		if (ret) {
>>> +			DRM_ERROR("failed to enable hdmi-en regulator\n");
>>> +			return ret;
>>> +		}
>>> +	}
>>>  
>>> -	return ret;
>>> +	return hdmi_bridge_init(hdata);
>>>  }
>>>  
>>>  static struct of_device_id hdmi_match_types[] = {
>>>
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/7] drm/exynos/hdmi: add bridge support

[Inki Dae]

2017년 02월 01일 16:34에 Andrzej Hajda 이(가) 쓴 글:
> On 01.02.2017 08:31, Inki Dae wrote:
>>
>> 2017년 01월 20일 15:52에 Andrzej Hajda 이(가) 쓴 글:
>>> In some platforms there is attached another device to the end of HDMI.
>>> The patch adds support for it.
>> Andrzej, can you clarify what bridge device can be attached and actually is now attached to the end of HDMI?
>> And I wonder if we have the device tree file which defines the bridge device in mainline. Seems we have no the device tree file.
>>
>> If nothing, then this patch should be merged with real use case.
> 
> SiI8620 MHL bridge in tm2/tm2e.

Did you post dt patch to add the properties related to SiI8620 MHL bridge device?

I see below patch series,
[PATCH 00/24] drm/bridge/sii8620: add Ultra HD modes support

but I couldn't find dt patch for it. And specifying SiI8620 MHL bridge device as description would be needed.

Thanks.

> 
> Regards
> Andrzej
> 
>>
>> Thanks.
>>
>>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_hdmi.c | 56 +++++++++++++++++++++++++++++-------
>>>  1 file changed, 46 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> index a73b192..41fb894 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>> @@ -35,6 +35,7 @@
>>>  #include <linux/io.h>
>>>  #include <linux/of_address.h>
>>>  #include <linux/of_device.h>
>>> +#include <linux/of_graph.h>
>>>  #include <linux/hdmi.h>
>>>  #include <linux/component.h>
>>>  #include <linux/mfd/syscon.h>
>>> @@ -133,6 +134,7 @@ struct hdmi_context {
>>>  	struct regulator_bulk_data	regul_bulk[ARRAY_SIZE(supply)];
>>>  	struct regulator		*reg_hdmi_en;
>>>  	struct exynos_drm_clk		phy_clk;
>>> +	struct drm_bridge		*bridge;
>>>  };
>>>  
>>>  static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
>>> @@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
>>>  	drm_connector_register(connector);
>>>  	drm_mode_connector_attach_encoder(connector, encoder);
>>>  
>>> -	return 0;
>>> +	if (hdata->bridge) {
>>> +		encoder->bridge = hdata->bridge;
>>> +		hdata->bridge->encoder = encoder;
>>> +		ret = drm_bridge_attach(encoder->dev, hdata->bridge);
>>> +		if (ret)
>>> +			DRM_ERROR("Failed to attach bridge\n");
>>> +	}
>>> +
>>> +	return ret;
>>>  }
>>>  
>>>  static bool hdmi_mode_fixup(struct drm_encoder *encoder,
>>> @@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk *clk, bool enable)
>>>  		hdmiphy_disable(hdata);
>>>  }
>>>  
>>> +static int hdmi_bridge_init(struct hdmi_context *hdata)
>>> +{
>>> +	struct device *dev = hdata->dev;
>>> +	struct device_node *ep, *np;
>>> +
>>> +	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
>>> +	if (!ep)
>>> +		return 0;
>>> +
>>> +	np = of_graph_get_remote_port_parent(ep);
>>> +	of_node_put(ep);
>>> +	if (!np) {
>>> +		DRM_ERROR("failed to get remote port parent");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	hdata->bridge = of_drm_find_bridge(np);
>>> +	of_node_put(np);
>>> +
>>> +	if (!hdata->bridge)
>>> +		return -EPROBE_DEFER;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  static int hdmi_resources_init(struct hdmi_context *hdata)
>>>  {
>>>  	struct device *dev = hdata->dev;
>>> @@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>>>  
>>>  	hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
>>>  
>>> -	if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
>>> -		return 0;
>>> +	if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
>>> +		if (IS_ERR(hdata->reg_hdmi_en))
>>> +			return PTR_ERR(hdata->reg_hdmi_en);
>>>  
>>> -	if (IS_ERR(hdata->reg_hdmi_en))
>>> -		return PTR_ERR(hdata->reg_hdmi_en);
>>> -
>>> -	ret = regulator_enable(hdata->reg_hdmi_en);
>>> -	if (ret)
>>> -		DRM_ERROR("failed to enable hdmi-en regulator\n");
>>> +		ret = regulator_enable(hdata->reg_hdmi_en);
>>> +		if (ret) {
>>> +			DRM_ERROR("failed to enable hdmi-en regulator\n");
>>> +			return ret;
>>> +		}
>>> +	}
>>>  
>>> -	return ret;
>>> +	return hdmi_bridge_init(hdata);
>>>  }
>>>  
>>>  static struct of_device_id hdmi_match_types[] = {
>>>
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH 4/7] drm/exynos/hdmi: add bridge support

[Andrzej Hajda]

On 01.02.2017 08:44, Inki Dae wrote:
>
> 2017년 02월 01일 16:34에 Andrzej Hajda 이(가) 쓴 글:
>> On 01.02.2017 08:31, Inki Dae wrote:
>>> 2017년 01월 20일 15:52에 Andrzej Hajda 이(가) 쓴 글:
>>>> In some platforms there is attached another device to the end of HDMI.
>>>> The patch adds support for it.
>>> Andrzej, can you clarify what bridge device can be attached and actually is now attached to the end of HDMI?
>>> And I wonder if we have the device tree file which defines the bridge device in mainline. Seems we have no the device tree file.
>>>
>>> If nothing, then this patch should be merged with real use case.
>> SiI8620 MHL bridge in tm2/tm2e.
> Did you post dt patch to add the properties related to SiI8620 MHL bridge device?
>
> I see below patch series,
> [PATCH 00/24] drm/bridge/sii8620: add Ultra HD modes support
>
> but I couldn't find dt patch for it. And specifying SiI8620 MHL bridge device as description would be needed.
>
> Thanks.
>
>

Patch adding HDMI path to tm2(e) including SiI8620 node and OF bindings
between hdmi and SiI8620 [1].
As the bridge framework in lower modes was used only to eliminate
unsupported modes, support for bridge in exynos_hdmi driver was not
critical, HDMI path worked well and I forgot about it.
But for UltraHD modes it is required as SiI8620 uses bridge framework to
gather info about applied video mode to program registers accordingly.
I will add requested description to this patch.

[1]: http://www.spinics.net/lists/devicetree/msg157892.html

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

Re: [PATCH 4/7] drm/exynos/hdmi: add bridge support

[Andrzej Hajda]

On 01.02.2017 08:44, Inki Dae wrote:
>
> 2017년 02월 01일 16:34에 Andrzej Hajda 이(가) 쓴 글:
>> On 01.02.2017 08:31, Inki Dae wrote:
>>> 2017년 01월 20일 15:52에 Andrzej Hajda 이(가) 쓴 글:
>>>> In some platforms there is attached another device to the end of HDMI.
>>>> The patch adds support for it.
>>> Andrzej, can you clarify what bridge device can be attached and actually is now attached to the end of HDMI?
>>> And I wonder if we have the device tree file which defines the bridge device in mainline. Seems we have no the device tree file.
>>>
>>> If nothing, then this patch should be merged with real use case.
>> SiI8620 MHL bridge in tm2/tm2e.
> Did you post dt patch to add the properties related to SiI8620 MHL bridge device?
>
> I see below patch series,
> [PATCH 00/24] drm/bridge/sii8620: add Ultra HD modes support
>
> but I couldn't find dt patch for it. And specifying SiI8620 MHL bridge device as description would be needed.
>
> Thanks.
>
>

Patch adding HDMI path to tm2(e) including SiI8620 node and OF bindings
between hdmi and SiI8620 [1].
As the bridge framework in lower modes was used only to eliminate
unsupported modes, support for bridge in exynos_hdmi driver was not
critical, HDMI path worked well and I forgot about it.
But for UltraHD modes it is required as SiI8620 uses bridge framework to
gather info about applied video mode to program registers accordingly.
I will add requested description to this patch.

[1]: http://www.spinics.net/lists/devicetree/msg157892.html

Regards
Andrzej

Re: [PATCH 4/7] drm/exynos/hdmi: add bridge support

[Inki Dae]

2017년 02월 01일 17:12에 Andrzej Hajda 이(가) 쓴 글:
> On 01.02.2017 08:44, Inki Dae wrote:
>>
>> 2017년 02월 01일 16:34에 Andrzej Hajda 이(가) 쓴 글:
>>> On 01.02.2017 08:31, Inki Dae wrote:
>>>> 2017년 01월 20일 15:52에 Andrzej Hajda 이(가) 쓴 글:
>>>>> In some platforms there is attached another device to the end of HDMI.
>>>>> The patch adds support for it.
>>>> Andrzej, can you clarify what bridge device can be attached and actually is now attached to the end of HDMI?
>>>> And I wonder if we have the device tree file which defines the bridge device in mainline. Seems we have no the device tree file.
>>>>
>>>> If nothing, then this patch should be merged with real use case.
>>> SiI8620 MHL bridge in tm2/tm2e.
>> Did you post dt patch to add the properties related to SiI8620 MHL bridge device?
>>
>> I see below patch series,
>> [PATCH 00/24] drm/bridge/sii8620: add Ultra HD modes support
>>
>> but I couldn't find dt patch for it. And specifying SiI8620 MHL bridge device as description would be needed.
>>
>> Thanks.
>>
>>
> 
> Patch adding HDMI path to tm2(e) including SiI8620 node and OF bindings
> between hdmi and SiI8620 [1].

Oops, you already CCed me. I didn't check it. Sorry.

> As the bridge framework in lower modes was used only to eliminate
> unsupported modes, support for bridge in exynos_hdmi driver was not
> critical, HDMI path worked well and I forgot about it.
> But for UltraHD modes it is required as SiI8620 uses bridge framework to
> gather info about applied video mode to program registers accordingly.
> I will add requested description to this patch.

Thanks.

> 
> [1]: http://www.spinics.net/lists/devicetree/msg157892.html
> 
> Regards
> Andrzej
> 
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/7] drm/exynos/hdmi: add bridge support

[Inki Dae]

2017년 02월 01일 17:12에 Andrzej Hajda 이(가) 쓴 글:
> On 01.02.2017 08:44, Inki Dae wrote:
>>
>> 2017년 02월 01일 16:34에 Andrzej Hajda 이(가) 쓴 글:
>>> On 01.02.2017 08:31, Inki Dae wrote:
>>>> 2017년 01월 20일 15:52에 Andrzej Hajda 이(가) 쓴 글:
>>>>> In some platforms there is attached another device to the end of HDMI.
>>>>> The patch adds support for it.
>>>> Andrzej, can you clarify what bridge device can be attached and actually is now attached to the end of HDMI?
>>>> And I wonder if we have the device tree file which defines the bridge device in mainline. Seems we have no the device tree file.
>>>>
>>>> If nothing, then this patch should be merged with real use case.
>>> SiI8620 MHL bridge in tm2/tm2e.
>> Did you post dt patch to add the properties related to SiI8620 MHL bridge device?
>>
>> I see below patch series,
>> [PATCH 00/24] drm/bridge/sii8620: add Ultra HD modes support
>>
>> but I couldn't find dt patch for it. And specifying SiI8620 MHL bridge device as description would be needed.
>>
>> Thanks.
>>
>>
> 
> Patch adding HDMI path to tm2(e) including SiI8620 node and OF bindings
> between hdmi and SiI8620 [1].

Oops, you already CCed me. I didn't check it. Sorry.

> As the bridge framework in lower modes was used only to eliminate
> unsupported modes, support for bridge in exynos_hdmi driver was not
> critical, HDMI path worked well and I forgot about it.
> But for UltraHD modes it is required as SiI8620 uses bridge framework to
> gather info about applied video mode to program registers accordingly.
> I will add requested description to this patch.

Thanks.

> 
> [1]: http://www.spinics.net/lists/devicetree/msg157892.html
> 
> Regards
> Andrzej
> 
> 

[PATCH v3 4/7] drm/exynos/hdmi: add bridge support

[Andrzej Hajda]

On TM2/TM2e platforms HDMI output is connected to MHL bridge
SiI8620. To allow configure UltraHD modes on the bridge
and to eliminate unsupported modes this bridge should be
attached to drm_encoder implemented in exynos_hdmi.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 56 +++++++++++++++++++++++++++++-------
 1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index a73b192..41fb894 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -35,6 +35,7 @@
 #include <linux/io.h>
 #include <linux/of_address.h>
 #include <linux/of_device.h>
+#include <linux/of_graph.h>
 #include <linux/hdmi.h>
 #include <linux/component.h>
 #include <linux/mfd/syscon.h>
@@ -133,6 +134,7 @@ struct hdmi_context {
 	struct regulator_bulk_data	regul_bulk[ARRAY_SIZE(supply)];
 	struct regulator		*reg_hdmi_en;
 	struct exynos_drm_clk		phy_clk;
+	struct drm_bridge		*bridge;
 };
 
 static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
@@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
 	drm_connector_register(connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
-	return 0;
+	if (hdata->bridge) {
+		encoder->bridge = hdata->bridge;
+		hdata->bridge->encoder = encoder;
+		ret = drm_bridge_attach(encoder->dev, hdata->bridge);
+		if (ret)
+			DRM_ERROR("Failed to attach bridge\n");
+	}
+
+	return ret;
 }
 
 static bool hdmi_mode_fixup(struct drm_encoder *encoder,
@@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk *clk, bool enable)
 		hdmiphy_disable(hdata);
 }
 
+static int hdmi_bridge_init(struct hdmi_context *hdata)
+{
+	struct device *dev = hdata->dev;
+	struct device_node *ep, *np;
+
+	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
+	if (!ep)
+		return 0;
+
+	np = of_graph_get_remote_port_parent(ep);
+	of_node_put(ep);
+	if (!np) {
+		DRM_ERROR("failed to get remote port parent");
+		return -EINVAL;
+	}
+
+	hdata->bridge = of_drm_find_bridge(np);
+	of_node_put(np);
+
+	if (!hdata->bridge)
+		return -EPROBE_DEFER;
+
+	return 0;
+}
+
 static int hdmi_resources_init(struct hdmi_context *hdata)
 {
 	struct device *dev = hdata->dev;
@@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
 
 	hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
 
-	if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
-		return 0;
+	if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
+		if (IS_ERR(hdata->reg_hdmi_en))
+			return PTR_ERR(hdata->reg_hdmi_en);
 
-	if (IS_ERR(hdata->reg_hdmi_en))
-		return PTR_ERR(hdata->reg_hdmi_en);
-
-	ret = regulator_enable(hdata->reg_hdmi_en);
-	if (ret)
-		DRM_ERROR("failed to enable hdmi-en regulator\n");
+		ret = regulator_enable(hdata->reg_hdmi_en);
+		if (ret) {
+			DRM_ERROR("failed to enable hdmi-en regulator\n");
+			return ret;
+		}
+	}
 
-	return ret;
+	return hdmi_bridge_init(hdata);
 }
 
 static struct of_device_id hdmi_match_types[] = {
-- 
2.7.4

Re: [PATCH v3 4/7] drm/exynos/hdmi: add bridge support

[Inki Dae]

2017년 02월 01일 17:29에 Andrzej Hajda 이(가) 쓴 글:
> On TM2/TM2e platforms HDMI output is connected to MHL bridge
> SiI8620. To allow configure UltraHD modes on the bridge
> and to eliminate unsupported modes this bridge should be
> attached to drm_encoder implemented in exynos_hdmi.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_hdmi.c | 56 +++++++++++++++++++++++++++++-------
>  1 file changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index a73b192..41fb894 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -35,6 +35,7 @@
>  #include <linux/io.h>
>  #include <linux/of_address.h>
>  #include <linux/of_device.h>
> +#include <linux/of_graph.h>
>  #include <linux/hdmi.h>
>  #include <linux/component.h>
>  #include <linux/mfd/syscon.h>
> @@ -133,6 +134,7 @@ struct hdmi_context {
>  	struct regulator_bulk_data	regul_bulk[ARRAY_SIZE(supply)];
>  	struct regulator		*reg_hdmi_en;
>  	struct exynos_drm_clk		phy_clk;
> +	struct drm_bridge		*bridge;
>  };
>  
>  static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
> @@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
>  	drm_connector_register(connector);
>  	drm_mode_connector_attach_encoder(connector, encoder);
>  
> -	return 0;
> +	if (hdata->bridge) {
> +		encoder->bridge = hdata->bridge;
> +		hdata->bridge->encoder = encoder;
> +		ret = drm_bridge_attach(encoder->dev, hdata->bridge);

arguments of drm_bridge_attach function has been changed so fixed it - trivial thing.
Applied it including other patches.

Thanks,
Inki Dae

> +		if (ret)
> +			DRM_ERROR("Failed to attach bridge\n");
> +	}
> +
> +	return ret;
>  }
>  
>  static bool hdmi_mode_fixup(struct drm_encoder *encoder,
> @@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk *clk, bool enable)
>  		hdmiphy_disable(hdata);
>  }
>  
> +static int hdmi_bridge_init(struct hdmi_context *hdata)
> +{
> +	struct device *dev = hdata->dev;
> +	struct device_node *ep, *np;
> +
> +	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> +	if (!ep)
> +		return 0;
> +
> +	np = of_graph_get_remote_port_parent(ep);
> +	of_node_put(ep);
> +	if (!np) {
> +		DRM_ERROR("failed to get remote port parent");
> +		return -EINVAL;
> +	}
> +
> +	hdata->bridge = of_drm_find_bridge(np);
> +	of_node_put(np);
> +
> +	if (!hdata->bridge)
> +		return -EPROBE_DEFER;
> +
> +	return 0;
> +}
> +
>  static int hdmi_resources_init(struct hdmi_context *hdata)
>  {
>  	struct device *dev = hdata->dev;
> @@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>  
>  	hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
>  
> -	if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
> -		return 0;
> +	if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
> +		if (IS_ERR(hdata->reg_hdmi_en))
> +			return PTR_ERR(hdata->reg_hdmi_en);
>  
> -	if (IS_ERR(hdata->reg_hdmi_en))
> -		return PTR_ERR(hdata->reg_hdmi_en);
> -
> -	ret = regulator_enable(hdata->reg_hdmi_en);
> -	if (ret)
> -		DRM_ERROR("failed to enable hdmi-en regulator\n");
> +		ret = regulator_enable(hdata->reg_hdmi_en);
> +		if (ret) {
> +			DRM_ERROR("failed to enable hdmi-en regulator\n");
> +			return ret;
> +		}
> +	}
>  
> -	return ret;
> +	return hdmi_bridge_init(hdata);
>  }
>  
>  static struct of_device_id hdmi_match_types[] = {
> 

Re: [PATCH v3 4/7] drm/exynos/hdmi: add bridge support

[Inki Dae]

2017년 02월 03일 15:38에 Inki Dae 이(가) 쓴 글:
> 
> 
> 2017년 02월 01일 17:29에 Andrzej Hajda 이(가) 쓴 글:
>> On TM2/TM2e platforms HDMI output is connected to MHL bridge
>> SiI8620. To allow configure UltraHD modes on the bridge
>> and to eliminate unsupported modes this bridge should be
>> attached to drm_encoder implemented in exynos_hdmi.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_hdmi.c | 56 +++++++++++++++++++++++++++++-------
>>  1 file changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index a73b192..41fb894 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -35,6 +35,7 @@
>>  #include <linux/io.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of_device.h>
>> +#include <linux/of_graph.h>
>>  #include <linux/hdmi.h>
>>  #include <linux/component.h>
>>  #include <linux/mfd/syscon.h>
>> @@ -133,6 +134,7 @@ struct hdmi_context {
>>  	struct regulator_bulk_data	regul_bulk[ARRAY_SIZE(supply)];
>>  	struct regulator		*reg_hdmi_en;
>>  	struct exynos_drm_clk		phy_clk;
>> +	struct drm_bridge		*bridge;
>>  };
>>  
>>  static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
>> @@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
>>  	drm_connector_register(connector);
>>  	drm_mode_connector_attach_encoder(connector, encoder);
>>  
>> -	return 0;
>> +	if (hdata->bridge) {
>> +		encoder->bridge = hdata->bridge;
>> +		hdata->bridge->encoder = encoder;
>> +		ret = drm_bridge_attach(encoder->dev, hdata->bridge);
> 
> arguments of drm_bridge_attach function has been changed so fixed it - trivial thing.
> Applied it including other patches.

The argument was changed by below patch,
drm: bridge: Link encoder and bridge in core code

Thanks.

> 
> Thanks,
> Inki Dae
> 
>> +		if (ret)
>> +			DRM_ERROR("Failed to attach bridge\n");
>> +	}
>> +
>> +	return ret;
>>  }
>>  
>>  static bool hdmi_mode_fixup(struct drm_encoder *encoder,
>> @@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk *clk, bool enable)
>>  		hdmiphy_disable(hdata);
>>  }
>>  
>> +static int hdmi_bridge_init(struct hdmi_context *hdata)
>> +{
>> +	struct device *dev = hdata->dev;
>> +	struct device_node *ep, *np;
>> +
>> +	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
>> +	if (!ep)
>> +		return 0;
>> +
>> +	np = of_graph_get_remote_port_parent(ep);
>> +	of_node_put(ep);
>> +	if (!np) {
>> +		DRM_ERROR("failed to get remote port parent");
>> +		return -EINVAL;
>> +	}
>> +
>> +	hdata->bridge = of_drm_find_bridge(np);
>> +	of_node_put(np);
>> +
>> +	if (!hdata->bridge)
>> +		return -EPROBE_DEFER;
>> +
>> +	return 0;
>> +}
>> +
>>  static int hdmi_resources_init(struct hdmi_context *hdata)
>>  {
>>  	struct device *dev = hdata->dev;
>> @@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>>  
>>  	hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
>>  
>> -	if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
>> -		return 0;
>> +	if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
>> +		if (IS_ERR(hdata->reg_hdmi_en))
>> +			return PTR_ERR(hdata->reg_hdmi_en);
>>  
>> -	if (IS_ERR(hdata->reg_hdmi_en))
>> -		return PTR_ERR(hdata->reg_hdmi_en);
>> -
>> -	ret = regulator_enable(hdata->reg_hdmi_en);
>> -	if (ret)
>> -		DRM_ERROR("failed to enable hdmi-en regulator\n");
>> +		ret = regulator_enable(hdata->reg_hdmi_en);
>> +		if (ret) {
>> +			DRM_ERROR("failed to enable hdmi-en regulator\n");
>> +			return ret;
>> +		}
>> +	}
>>  
>> -	return ret;
>> +	return hdmi_bridge_init(hdata);
>>  }
>>  
>>  static struct of_device_id hdmi_match_types[] = {
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 4/7] drm/exynos/hdmi: add bridge support

[Inki Dae]

2017년 02월 03일 15:38에 Inki Dae 이(가) 쓴 글:
> 
> 
> 2017년 02월 01일 17:29에 Andrzej Hajda 이(가) 쓴 글:
>> On TM2/TM2e platforms HDMI output is connected to MHL bridge
>> SiI8620. To allow configure UltraHD modes on the bridge
>> and to eliminate unsupported modes this bridge should be
>> attached to drm_encoder implemented in exynos_hdmi.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_hdmi.c | 56 +++++++++++++++++++++++++++++-------
>>  1 file changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index a73b192..41fb894 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -35,6 +35,7 @@
>>  #include <linux/io.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of_device.h>
>> +#include <linux/of_graph.h>
>>  #include <linux/hdmi.h>
>>  #include <linux/component.h>
>>  #include <linux/mfd/syscon.h>
>> @@ -133,6 +134,7 @@ struct hdmi_context {
>>  	struct regulator_bulk_data	regul_bulk[ARRAY_SIZE(supply)];
>>  	struct regulator		*reg_hdmi_en;
>>  	struct exynos_drm_clk		phy_clk;
>> +	struct drm_bridge		*bridge;
>>  };
>>  
>>  static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
>> @@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder *encoder)
>>  	drm_connector_register(connector);
>>  	drm_mode_connector_attach_encoder(connector, encoder);
>>  
>> -	return 0;
>> +	if (hdata->bridge) {
>> +		encoder->bridge = hdata->bridge;
>> +		hdata->bridge->encoder = encoder;
>> +		ret = drm_bridge_attach(encoder->dev, hdata->bridge);
> 
> arguments of drm_bridge_attach function has been changed so fixed it - trivial thing.
> Applied it including other patches.

The argument was changed by below patch,
drm: bridge: Link encoder and bridge in core code

Thanks.

> 
> Thanks,
> Inki Dae
> 
>> +		if (ret)
>> +			DRM_ERROR("Failed to attach bridge\n");
>> +	}
>> +
>> +	return ret;
>>  }
>>  
>>  static bool hdmi_mode_fixup(struct drm_encoder *encoder,
>> @@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk *clk, bool enable)
>>  		hdmiphy_disable(hdata);
>>  }
>>  
>> +static int hdmi_bridge_init(struct hdmi_context *hdata)
>> +{
>> +	struct device *dev = hdata->dev;
>> +	struct device_node *ep, *np;
>> +
>> +	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
>> +	if (!ep)
>> +		return 0;
>> +
>> +	np = of_graph_get_remote_port_parent(ep);
>> +	of_node_put(ep);
>> +	if (!np) {
>> +		DRM_ERROR("failed to get remote port parent");
>> +		return -EINVAL;
>> +	}
>> +
>> +	hdata->bridge = of_drm_find_bridge(np);
>> +	of_node_put(np);
>> +
>> +	if (!hdata->bridge)
>> +		return -EPROBE_DEFER;
>> +
>> +	return 0;
>> +}
>> +
>>  static int hdmi_resources_init(struct hdmi_context *hdata)
>>  {
>>  	struct device *dev = hdata->dev;
>> @@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context *hdata)
>>  
>>  	hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
>>  
>> -	if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
>> -		return 0;
>> +	if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
>> +		if (IS_ERR(hdata->reg_hdmi_en))
>> +			return PTR_ERR(hdata->reg_hdmi_en);
>>  
>> -	if (IS_ERR(hdata->reg_hdmi_en))
>> -		return PTR_ERR(hdata->reg_hdmi_en);
>> -
>> -	ret = regulator_enable(hdata->reg_hdmi_en);
>> -	if (ret)
>> -		DRM_ERROR("failed to enable hdmi-en regulator\n");
>> +		ret = regulator_enable(hdata->reg_hdmi_en);
>> +		if (ret) {
>> +			DRM_ERROR("failed to enable hdmi-en regulator\n");
>> +			return ret;
>> +		}
>> +	}
>>  
>> -	return ret;
>> +	return hdmi_bridge_init(hdata);
>>  }
>>  
>>  static struct of_device_id hdmi_match_types[] = {
>>

[PATCH 5/7] drm/exynos/decon5433: add support for interlace modes

[Andrzej Hajda]

Some registers should be programmed differently in interlace mode.
Additionally IP does not signal stop state properly in interlaced
mode, so warning has been removed.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 45 +++++++++++++++++++--------
 include/video/exynos5433_decon.h              |  1 +
 2 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 042d6ef..46434ba9 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -146,6 +146,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 	struct drm_display_mode *m = &crtc->base.mode;
+	bool interlaced = false;
 	u32 val;
 
 	if (test_bit(BIT_SUSPENDED, &ctx->flags))
@@ -156,12 +157,16 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 		m->crtc_hsync_end = m->crtc_htotal - 92;
 		m->crtc_vsync_start = m->crtc_vdisplay + 1;
 		m->crtc_vsync_end = m->crtc_vsync_start + 1;
+		if (m->flags & DRM_MODE_FLAG_INTERLACE)
+			interlaced = true;
 	}
 
 	decon_setup_trigger(ctx);
 
 	/* lcd on and use command if */
 	val = VIDOUT_LCD_ON;
+	if (interlaced)
+		val |= VIDOUT_INTERLACE_EN_F;
 	if (ctx->out_type & IFTYPE_I80) {
 		val |= VIDOUT_COMMAND_IF;
 	} else {
@@ -170,15 +175,21 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 
 	writel(val, ctx->addr + DECON_VIDOUTCON0);
 
-	val = VIDTCON2_LINEVAL(m->vdisplay - 1) |
-		VIDTCON2_HOZVAL(m->hdisplay - 1);
+	if (interlaced)
+		val = VIDTCON2_LINEVAL(m->vdisplay / 2 - 1) |
+			VIDTCON2_HOZVAL(m->hdisplay - 1);
+	else
+		val = VIDTCON2_LINEVAL(m->vdisplay - 1) |
+			VIDTCON2_HOZVAL(m->hdisplay - 1);
 	writel(val, ctx->addr + DECON_VIDTCON2);
 
 	if (!(ctx->out_type & IFTYPE_I80)) {
-		val = VIDTCON00_VBPD_F(
-				m->crtc_vtotal - m->crtc_vsync_end - 1) |
-			VIDTCON00_VFPD_F(
-				m->crtc_vsync_start - m->crtc_vdisplay - 1);
+		int vbp = m->crtc_vtotal - m->crtc_vsync_end;
+		int vfp = m->crtc_vsync_start - m->crtc_vdisplay;
+
+		if (interlaced)
+			vbp = vbp / 2 - 1;
+		val = VIDTCON00_VBPD_F(vbp - 1) | VIDTCON00_VFPD_F(vfp - 1);
 		writel(val, ctx->addr + DECON_VIDTCON00);
 
 		val = VIDTCON01_VSPW_F(
@@ -293,12 +304,22 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
-	val = COORDINATE_X(state->crtc.x) | COORDINATE_Y(state->crtc.y);
-	writel(val, ctx->addr + DECON_VIDOSDxA(win));
+	if (crtc->base.mode.flags & DRM_MODE_FLAG_INTERLACE) {
+		val = COORDINATE_X(state->crtc.x) |
+			COORDINATE_Y(state->crtc.y / 2);
+		writel(val, ctx->addr + DECON_VIDOSDxA(win));
+
+		val = COORDINATE_X(state->crtc.x + state->crtc.w - 1) |
+			COORDINATE_Y((state->crtc.y + state->crtc.h) / 2 - 1);
+		writel(val, ctx->addr + DECON_VIDOSDxB(win));
+	} else {
+		val = COORDINATE_X(state->crtc.x) | COORDINATE_Y(state->crtc.y);
+		writel(val, ctx->addr + DECON_VIDOSDxA(win));
 
-	val = COORDINATE_X(state->crtc.x + state->crtc.w - 1) |
-		COORDINATE_Y(state->crtc.y + state->crtc.h - 1);
-	writel(val, ctx->addr + DECON_VIDOSDxB(win));
+		val = COORDINATE_X(state->crtc.x + state->crtc.w - 1) |
+				COORDINATE_Y(state->crtc.y + state->crtc.h - 1);
+		writel(val, ctx->addr + DECON_VIDOSDxB(win));
+	}
 
 	val = VIDOSD_Wx_ALPHA_R_F(0x0) | VIDOSD_Wx_ALPHA_G_F(0x0) |
 		VIDOSD_Wx_ALPHA_B_F(0x0);
@@ -370,8 +391,6 @@ static void decon_swreset(struct decon_context *ctx)
 		udelay(10);
 	}
 
-	WARN(tries == 0, "failed to disable DECON\n");
-
 	writel(VIDCON0_SWRESET, ctx->addr + DECON_VIDCON0);
 	for (tries = 2000; tries; --tries) {
 		if (~readl(ctx->addr + DECON_VIDCON0) & VIDCON0_SWRESET)
diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h
index 0098a52..b26511a 100644
--- a/include/video/exynos5433_decon.h
+++ b/include/video/exynos5433_decon.h
@@ -89,6 +89,7 @@
 #define VIDCON0_ENVID_F			(1 << 0)
 
 /* VIDOUTCON0 */
+#define VIDOUT_INTERLACE_EN_F		(1 << 28)
 #define VIDOUT_LCD_ON			(1 << 24)
 #define VIDOUT_IF_F_MASK		(0x3 << 20)
 #define VIDOUT_RGB_IF			(0x0 << 20)
-- 
2.7.4

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

[PATCH 5/7] drm/exynos/decon5433: add support for interlace modes

[Andrzej Hajda]

Some registers should be programmed differently in interlace mode.
Additionally IP does not signal stop state properly in interlaced
mode, so warning has been removed.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 45 +++++++++++++++++++--------
 include/video/exynos5433_decon.h              |  1 +
 2 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 042d6ef..46434ba9 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -146,6 +146,7 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 	struct drm_display_mode *m = &crtc->base.mode;
+	bool interlaced = false;
 	u32 val;
 
 	if (test_bit(BIT_SUSPENDED, &ctx->flags))
@@ -156,12 +157,16 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 		m->crtc_hsync_end = m->crtc_htotal - 92;
 		m->crtc_vsync_start = m->crtc_vdisplay + 1;
 		m->crtc_vsync_end = m->crtc_vsync_start + 1;
+		if (m->flags & DRM_MODE_FLAG_INTERLACE)
+			interlaced = true;
 	}
 
 	decon_setup_trigger(ctx);
 
 	/* lcd on and use command if */
 	val = VIDOUT_LCD_ON;
+	if (interlaced)
+		val |= VIDOUT_INTERLACE_EN_F;
 	if (ctx->out_type & IFTYPE_I80) {
 		val |= VIDOUT_COMMAND_IF;
 	} else {
@@ -170,15 +175,21 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 
 	writel(val, ctx->addr + DECON_VIDOUTCON0);
 
-	val = VIDTCON2_LINEVAL(m->vdisplay - 1) |
-		VIDTCON2_HOZVAL(m->hdisplay - 1);
+	if (interlaced)
+		val = VIDTCON2_LINEVAL(m->vdisplay / 2 - 1) |
+			VIDTCON2_HOZVAL(m->hdisplay - 1);
+	else
+		val = VIDTCON2_LINEVAL(m->vdisplay - 1) |
+			VIDTCON2_HOZVAL(m->hdisplay - 1);
 	writel(val, ctx->addr + DECON_VIDTCON2);
 
 	if (!(ctx->out_type & IFTYPE_I80)) {
-		val = VIDTCON00_VBPD_F(
-				m->crtc_vtotal - m->crtc_vsync_end - 1) |
-			VIDTCON00_VFPD_F(
-				m->crtc_vsync_start - m->crtc_vdisplay - 1);
+		int vbp = m->crtc_vtotal - m->crtc_vsync_end;
+		int vfp = m->crtc_vsync_start - m->crtc_vdisplay;
+
+		if (interlaced)
+			vbp = vbp / 2 - 1;
+		val = VIDTCON00_VBPD_F(vbp - 1) | VIDTCON00_VFPD_F(vfp - 1);
 		writel(val, ctx->addr + DECON_VIDTCON00);
 
 		val = VIDTCON01_VSPW_F(
@@ -293,12 +304,22 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	if (test_bit(BIT_SUSPENDED, &ctx->flags))
 		return;
 
-	val = COORDINATE_X(state->crtc.x) | COORDINATE_Y(state->crtc.y);
-	writel(val, ctx->addr + DECON_VIDOSDxA(win));
+	if (crtc->base.mode.flags & DRM_MODE_FLAG_INTERLACE) {
+		val = COORDINATE_X(state->crtc.x) |
+			COORDINATE_Y(state->crtc.y / 2);
+		writel(val, ctx->addr + DECON_VIDOSDxA(win));
+
+		val = COORDINATE_X(state->crtc.x + state->crtc.w - 1) |
+			COORDINATE_Y((state->crtc.y + state->crtc.h) / 2 - 1);
+		writel(val, ctx->addr + DECON_VIDOSDxB(win));
+	} else {
+		val = COORDINATE_X(state->crtc.x) | COORDINATE_Y(state->crtc.y);
+		writel(val, ctx->addr + DECON_VIDOSDxA(win));
 
-	val = COORDINATE_X(state->crtc.x + state->crtc.w - 1) |
-		COORDINATE_Y(state->crtc.y + state->crtc.h - 1);
-	writel(val, ctx->addr + DECON_VIDOSDxB(win));
+		val = COORDINATE_X(state->crtc.x + state->crtc.w - 1) |
+				COORDINATE_Y(state->crtc.y + state->crtc.h - 1);
+		writel(val, ctx->addr + DECON_VIDOSDxB(win));
+	}
 
 	val = VIDOSD_Wx_ALPHA_R_F(0x0) | VIDOSD_Wx_ALPHA_G_F(0x0) |
 		VIDOSD_Wx_ALPHA_B_F(0x0);
@@ -370,8 +391,6 @@ static void decon_swreset(struct decon_context *ctx)
 		udelay(10);
 	}
 
-	WARN(tries == 0, "failed to disable DECON\n");
-
 	writel(VIDCON0_SWRESET, ctx->addr + DECON_VIDCON0);
 	for (tries = 2000; tries; --tries) {
 		if (~readl(ctx->addr + DECON_VIDCON0) & VIDCON0_SWRESET)
diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h
index 0098a52..b26511a 100644
--- a/include/video/exynos5433_decon.h
+++ b/include/video/exynos5433_decon.h
@@ -89,6 +89,7 @@
 #define VIDCON0_ENVID_F			(1 << 0)
 
 /* VIDOUTCON0 */
+#define VIDOUT_INTERLACE_EN_F		(1 << 28)
 #define VIDOUT_LCD_ON			(1 << 24)
 #define VIDOUT_IF_F_MASK		(0x3 << 20)
 #define VIDOUT_RGB_IF			(0x0 << 20)
-- 
2.7.4

[PATCH 6/7] drm/exynos/decon5433: signal vblank only on odd fields

[Andrzej Hajda]

In case of interlace mode irq is generated for odd and even fields, but
vblank should be signaled only for the last emitted field.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 7 +++++++
 include/video/exynos5433_decon.h              | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 46434ba9..ad8b93a 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -591,6 +591,13 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)
 
 	if (val) {
 		writel(val, ctx->addr + DECON_VIDINTCON1);
+		if (ctx->out_type & IFTYPE_HDMI) {
+			val = readl(ctx->addr + DECON_VIDOUTCON0);
+			val &= VIDOUT_INTERLACE_EN_F | VIDOUT_INTERLACE_FIELD_F;
+			if (val ==
+			    (VIDOUT_INTERLACE_EN_F | VIDOUT_INTERLACE_FIELD_F))
+				return IRQ_HANDLED;
+		}
 		drm_crtc_handle_vblank(&ctx->crtc->base);
 	}
 
diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h
index b26511a..ef8e2a8 100644
--- a/include/video/exynos5433_decon.h
+++ b/include/video/exynos5433_decon.h
@@ -89,6 +89,7 @@
 #define VIDCON0_ENVID_F			(1 << 0)
 
 /* VIDOUTCON0 */
+#define VIDOUT_INTERLACE_FIELD_F	(1 << 29)
 #define VIDOUT_INTERLACE_EN_F		(1 << 28)
 #define VIDOUT_LCD_ON			(1 << 24)
 #define VIDOUT_IF_F_MASK		(0x3 << 20)
-- 
2.7.4

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

[PATCH 6/7] drm/exynos/decon5433: signal vblank only on odd fields

[Andrzej Hajda]

In case of interlace mode irq is generated for odd and even fields, but
vblank should be signaled only for the last emitted field.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 7 +++++++
 include/video/exynos5433_decon.h              | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 46434ba9..ad8b93a 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -591,6 +591,13 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)
 
 	if (val) {
 		writel(val, ctx->addr + DECON_VIDINTCON1);
+		if (ctx->out_type & IFTYPE_HDMI) {
+			val = readl(ctx->addr + DECON_VIDOUTCON0);
+			val &= VIDOUT_INTERLACE_EN_F | VIDOUT_INTERLACE_FIELD_F;
+			if (val ==
+			    (VIDOUT_INTERLACE_EN_F | VIDOUT_INTERLACE_FIELD_F))
+				return IRQ_HANDLED;
+		}
 		drm_crtc_handle_vblank(&ctx->crtc->base);
 	}
 
diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h
index b26511a..ef8e2a8 100644
--- a/include/video/exynos5433_decon.h
+++ b/include/video/exynos5433_decon.h
@@ -89,6 +89,7 @@
 #define VIDCON0_ENVID_F			(1 << 0)
 
 /* VIDOUTCON0 */
+#define VIDOUT_INTERLACE_FIELD_F	(1 << 29)
 #define VIDOUT_INTERLACE_EN_F		(1 << 28)
 #define VIDOUT_LCD_ON			(1 << 24)
 #define VIDOUT_IF_F_MASK		(0x3 << 20)
-- 
2.7.4

Re: [PATCH 6/7] drm/exynos/decon5433: signal vblank only on odd fields

[Ville Syrjälä]

On Fri, Jan 20, 2017 at 07:52:24AM +0100, Andrzej Hajda wrote:
> In case of interlace mode irq is generated for odd and even fields, but
> vblank should be signaled only for the last emitted field.

I'm pretty sure most drivers signal it for both fields. At least i915
does.

> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 7 +++++++
>  include/video/exynos5433_decon.h              | 1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> index 46434ba9..ad8b93a 100644
> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> @@ -591,6 +591,13 @@ static irqreturn_t decon_irq_handler(int irq, void *dev_id)
>  
>  	if (val) {
>  		writel(val, ctx->addr + DECON_VIDINTCON1);
> +		if (ctx->out_type & IFTYPE_HDMI) {
> +			val = readl(ctx->addr + DECON_VIDOUTCON0);
> +			val &= VIDOUT_INTERLACE_EN_F | VIDOUT_INTERLACE_FIELD_F;
> +			if (val ==
> +			    (VIDOUT_INTERLACE_EN_F | VIDOUT_INTERLACE_FIELD_F))
> +				return IRQ_HANDLED;
> +		}
>  		drm_crtc_handle_vblank(&ctx->crtc->base);
>  	}
>  
> diff --git a/include/video/exynos5433_decon.h b/include/video/exynos5433_decon.h
> index b26511a..ef8e2a8 100644
> --- a/include/video/exynos5433_decon.h
> +++ b/include/video/exynos5433_decon.h
> @@ -89,6 +89,7 @@
>  #define VIDCON0_ENVID_F			(1 << 0)
>  
>  /* VIDOUTCON0 */
> +#define VIDOUT_INTERLACE_FIELD_F	(1 << 29)
>  #define VIDOUT_INTERLACE_EN_F		(1 << 28)
>  #define VIDOUT_LCD_ON			(1 << 24)
>  #define VIDOUT_IF_F_MASK		(0x3 << 20)
> -- 
> 2.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 6/7] drm/exynos/decon5433: signal vblank only on odd fields

[Andrzej Hajda]

On 20.01.2017 14:55, Ville Syrjälä wrote:
> On Fri, Jan 20, 2017 at 07:52:24AM +0100, Andrzej Hajda wrote:
>> In case of interlace mode irq is generated for odd and even fields, but
>> vblank should be signaled only for the last emitted field.
> I'm pretty sure most drivers signal it for both fields. At least i915
> does.

The question is which behavior is correct? I have not found any clear
statement in the documentation, or drm core code.
I have guessed that since vblank event is used to signal end of scan-out
of buffer it should be called after scan-out of whole buffer - in case
of interlaced mode after scan-out of 2nd field.
Maybe my assumption is wrong, in such case this patch should be dropped
and mixer driver also should be fixed, but before doing that it would be
good to know for sure how it should be handled correctly.

Regards
Andrzej

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

Re: [PATCH 6/7] drm/exynos/decon5433: signal vblank only on odd fields

[Andrzej Hajda]

On 20.01.2017 14:55, Ville Syrjälä wrote:
> On Fri, Jan 20, 2017 at 07:52:24AM +0100, Andrzej Hajda wrote:
>> In case of interlace mode irq is generated for odd and even fields, but
>> vblank should be signaled only for the last emitted field.
> I'm pretty sure most drivers signal it for both fields. At least i915
> does.

The question is which behavior is correct? I have not found any clear
statement in the documentation, or drm core code.
I have guessed that since vblank event is used to signal end of scan-out
of buffer it should be called after scan-out of whole buffer - in case
of interlaced mode after scan-out of 2nd field.
Maybe my assumption is wrong, in such case this patch should be dropped
and mixer driver also should be fixed, but before doing that it would be
good to know for sure how it should be handled correctly.

Regards
Andrzej

Re: [PATCH 6/7] drm/exynos/decon5433: signal vblank only on odd fields

[Ville Syrjälä]

On Mon, Jan 23, 2017 at 10:15:16AM +0100, Andrzej Hajda wrote:
> On 20.01.2017 14:55, Ville Syrjälä wrote:
> > On Fri, Jan 20, 2017 at 07:52:24AM +0100, Andrzej Hajda wrote:
> >> In case of interlace mode irq is generated for odd and even fields, but
> >> vblank should be signaled only for the last emitted field.
> > I'm pretty sure most drivers signal it for both fields. At least i915
> > does.
> 
> The question is which behavior is correct? I have not found any clear
> statement in the documentation, or drm core code.

That's very typical for us unfortunately.

I would say what we should do what i915 does. It allows more flexibility
in how you use the hardware. Eg. then you can actually scan out
interlaced material to an interlaced display and not mess up the fields,
and you can also do 3:2 pulldown type of stuff. Or you can even just
stuff progressive frames down the pipe at field rate.

One problem with interlaced stuff is that we don't have any field
indication in the events, nor do we have a way to flip on a specific
field. I tried to specify the latter for the SETPLANE ioctl way
back when, but it didn't end up being implemented and now we would
need something different for atomic.

> I have guessed that since vblank event is used to signal end of scan-out
> of buffer it should be called after scan-out of whole buffer - in case
> of interlaced mode after scan-out of 2nd field.

Each field has a proper vertical blanking interval, so you'd just end up
totally wasting one of them.

> Maybe my assumption is wrong, in such case this patch should be dropped
> and mixer driver also should be fixed, but before doing that it would be
> good to know for sure how it should be handled correctly.
> 
> Regards
> Andrzej

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/7] drm/exynos/decon5433: signal vblank only on odd fields

[Ville Syrjälä]

On Mon, Jan 23, 2017 at 10:15:16AM +0100, Andrzej Hajda wrote:
> On 20.01.2017 14:55, Ville Syrjälä wrote:
> > On Fri, Jan 20, 2017 at 07:52:24AM +0100, Andrzej Hajda wrote:
> >> In case of interlace mode irq is generated for odd and even fields, but
> >> vblank should be signaled only for the last emitted field.
> > I'm pretty sure most drivers signal it for both fields. At least i915
> > does.
> 
> The question is which behavior is correct? I have not found any clear
> statement in the documentation, or drm core code.

That's very typical for us unfortunately.

I would say what we should do what i915 does. It allows more flexibility
in how you use the hardware. Eg. then you can actually scan out
interlaced material to an interlaced display and not mess up the fields,
and you can also do 3:2 pulldown type of stuff. Or you can even just
stuff progressive frames down the pipe at field rate.

One problem with interlaced stuff is that we don't have any field
indication in the events, nor do we have a way to flip on a specific
field. I tried to specify the latter for the SETPLANE ioctl way
back when, but it didn't end up being implemented and now we would
need something different for atomic.

> I have guessed that since vblank event is used to signal end of scan-out
> of buffer it should be called after scan-out of whole buffer - in case
> of interlaced mode after scan-out of 2nd field.

Each field has a proper vertical blanking interval, so you'd just end up
totally wasting one of them.

> Maybe my assumption is wrong, in such case this patch should be dropped
> and mixer driver also should be fixed, but before doing that it would be
> good to know for sure how it should be handled correctly.
> 
> Regards
> Andrzej

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 6/7] drm/exynos/decon5433: signal vblank only on odd fields

[Andrzej Hajda]

On 25.01.2017 15:06, Ville Syrjälä wrote:
> On Mon, Jan 23, 2017 at 10:15:16AM +0100, Andrzej Hajda wrote:
>> On 20.01.2017 14:55, Ville Syrjälä wrote:
>>> On Fri, Jan 20, 2017 at 07:52:24AM +0100, Andrzej Hajda wrote:
>>>> In case of interlace mode irq is generated for odd and even fields, but
>>>> vblank should be signaled only for the last emitted field.
>>> I'm pretty sure most drivers signal it for both fields. At least i915
>>> does.
>> The question is which behavior is correct? I have not found any clear
>> statement in the documentation, or drm core code.
> That's very typical for us unfortunately.
>
> I would say what we should do what i915 does. It allows more flexibility
> in how you use the hardware. Eg. then you can actually scan out
> interlaced material to an interlaced display and not mess up the fields,
> and you can also do 3:2 pulldown type of stuff. Or you can even just
> stuff progressive frames down the pipe at field rate.
>
> One problem with interlaced stuff is that we don't have any field
> indication in the events, nor do we have a way to flip on a specific
> field. I tried to specify the latter for the SETPLANE ioctl way
> back when, but it didn't end up being implemented and now we would
> need something different for atomic.
>
>> I have guessed that since vblank event is used to signal end of scan-out
>> of buffer it should be called after scan-out of whole buffer - in case
>> of interlaced mode after scan-out of 2nd field.
> Each field has a proper vertical blanking interval, so you'd just end up
> totally wasting one of them.

The problem in this particular case is that hardware does not allow to
change buffers between fields, or more precisely it updates its internal
registers after 2nd field - ie after reading full frame.
I am still investigating the issue, but it is possible this limitation
cannot be overcome.

Regards
Andrzej

>
>> Maybe my assumption is wrong, in such case this patch should be dropped
>> and mixer driver also should be fixed, but before doing that it would be
>> good to know for sure how it should be handled correctly.
>>
>> Regards
>> Andrzej

Re: [PATCH 6/7] drm/exynos/decon5433: signal vblank only on odd fields

[Ville Syrjälä]

On Thu, Jan 26, 2017 at 09:22:27AM +0100, Andrzej Hajda wrote:
> On 25.01.2017 15:06, Ville Syrjälä wrote:
> > On Mon, Jan 23, 2017 at 10:15:16AM +0100, Andrzej Hajda wrote:
> >> On 20.01.2017 14:55, Ville Syrjälä wrote:
> >>> On Fri, Jan 20, 2017 at 07:52:24AM +0100, Andrzej Hajda wrote:
> >>>> In case of interlace mode irq is generated for odd and even fields, but
> >>>> vblank should be signaled only for the last emitted field.
> >>> I'm pretty sure most drivers signal it for both fields. At least i915
> >>> does.
> >> The question is which behavior is correct? I have not found any clear
> >> statement in the documentation, or drm core code.
> > That's very typical for us unfortunately.
> >
> > I would say what we should do what i915 does. It allows more flexibility
> > in how you use the hardware. Eg. then you can actually scan out
> > interlaced material to an interlaced display and not mess up the fields,
> > and you can also do 3:2 pulldown type of stuff. Or you can even just
> > stuff progressive frames down the pipe at field rate.
> >
> > One problem with interlaced stuff is that we don't have any field
> > indication in the events, nor do we have a way to flip on a specific
> > field. I tried to specify the latter for the SETPLANE ioctl way
> > back when, but it didn't end up being implemented and now we would
> > need something different for atomic.
> >
> >> I have guessed that since vblank event is used to signal end of scan-out
> >> of buffer it should be called after scan-out of whole buffer - in case
> >> of interlaced mode after scan-out of 2nd field.
> > Each field has a proper vertical blanking interval, so you'd just end up
> > totally wasting one of them.
> 
> The problem in this particular case is that hardware does not allow to
> change buffers between fields, or more precisely it updates its internal
> registers after 2nd field - ie after reading full frame.

Oh. That's a rather odd piece of hw then. In that case it might indeed
be better to not signal vblank for the field that can't do the flip.

> I am still investigating the issue, but it is possible this limitation
> cannot be overcome.
> 
> Regards
> Andrzej
> 
> >
> >> Maybe my assumption is wrong, in such case this patch should be dropped
> >> and mixer driver also should be fixed, but before doing that it would be
> >> good to know for sure how it should be handled correctly.
> >>
> >> Regards
> >> Andrzej
> 

-- 
Ville Syrjälä
Intel OTC

[PATCH 7/7] arm64: dts: exynos: configure TV path clocks for Ultra HD modes

[Andrzej Hajda]

Ultra HD modes requires clock ticking at increased rate.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index f120d99..314d928 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -764,6 +764,9 @@
 			clock-names = "pclk", "aclk_decon", "aclk_smmu_decon0x",
 				      "aclk_xiu_decon0x", "pclk_smmu_decon0x",
 				      "sclk_decon_vclk", "sclk_decon_eclk";
+			assigned-clocks = <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>, <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>, <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>, <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
+			assigned-clock-parents = <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>, <0>, <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>, <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
+			assigned-clock-rates = <0>, <400000000>;
 			samsung,disp-sysreg = <&syscon_disp>;
 			interrupt-names = "fifo", "vsync", "lcd_sys";
 			interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>,
-- 
2.7.4

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

[PATCH 7/7] arm64: dts: exynos: configure TV path clocks for Ultra HD modes

[Andrzej Hajda]

Ultra HD modes requires clock ticking at increased rate.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index f120d99..314d928 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -764,6 +764,9 @@
 			clock-names = "pclk", "aclk_decon", "aclk_smmu_decon0x",
 				      "aclk_xiu_decon0x", "pclk_smmu_decon0x",
 				      "sclk_decon_vclk", "sclk_decon_eclk";
+			assigned-clocks = <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>, <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>, <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>, <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
+			assigned-clock-parents = <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>, <0>, <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>, <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
+			assigned-clock-rates = <0>, <400000000>;
 			samsung,disp-sysreg = <&syscon_disp>;
 			interrupt-names = "fifo", "vsync", "lcd_sys";
 			interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>,
-- 
2.7.4

Re: [PATCH 7/7] arm64: dts: exynos: configure TV path clocks for Ultra HD modes

[Krzysztof Kozlowski]

On Fri, Jan 20, 2017 at 07:52:25AM +0100, Andrzej Hajda wrote:
> Ultra HD modes requires clock ticking at increased rate.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> index f120d99..314d928 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> @@ -764,6 +764,9 @@
>  			clock-names = "pclk", "aclk_decon", "aclk_smmu_decon0x",
>  				      "aclk_xiu_decon0x", "pclk_smmu_decon0x",
>  				      "sclk_decon_vclk", "sclk_decon_eclk";
> +			assigned-clocks = <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>, <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>, <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>, <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
> +			assigned-clock-parents = <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>, <0>, <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>, <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
> +			assigned-clock-rates = <0>, <400000000>;

Lines got too long, please split them.

Best regards,
Krzysztof

>  			samsung,disp-sysreg = <&syscon_disp>;
>  			interrupt-names = "fifo", "vsync", "lcd_sys";
>  			interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>,
> -- 
> 2.7.4
> 

[PATCH v2 7/7] arm64: dts: exynos: configure TV path clocks for Ultra HD modes

[Andrzej Hajda]

Ultra HD modes requires clock ticking at increased rate.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index f120d99..4d28e93 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -764,6 +764,17 @@
 			clock-names = "pclk", "aclk_decon", "aclk_smmu_decon0x",
 				      "aclk_xiu_decon0x", "pclk_smmu_decon0x",
 				      "sclk_decon_vclk", "sclk_decon_eclk";
+			assigned-clocks =
+				<&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
+				<&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
+				<&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
+				<&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
+			assigned-clock-parents =
+				<&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
+				<0>,
+				<&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
+				<&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
+			assigned-clock-rates = <0>, <400000000>;
 			samsung,disp-sysreg = <&syscon_disp>;
 			interrupt-names = "fifo", "vsync", "lcd_sys";
 			interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>,
-- 
2.7.4

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

[PATCH v2 7/7] arm64: dts: exynos: configure TV path clocks for Ultra HD modes

[Andrzej Hajda]

Ultra HD modes requires clock ticking at increased rate.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index f120d99..4d28e93 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -764,6 +764,17 @@
 			clock-names = "pclk", "aclk_decon", "aclk_smmu_decon0x",
 				      "aclk_xiu_decon0x", "pclk_smmu_decon0x",
 				      "sclk_decon_vclk", "sclk_decon_eclk";
+			assigned-clocks =
+				<&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
+				<&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
+				<&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
+				<&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
+			assigned-clock-parents =
+				<&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
+				<0>,
+				<&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
+				<&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
+			assigned-clock-rates = <0>, <400000000>;
 			samsung,disp-sysreg = <&syscon_disp>;
 			interrupt-names = "fifo", "vsync", "lcd_sys";
 			interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>,
-- 
2.7.4

Re: [PATCH v2 7/7] arm64: dts: exynos: configure TV path clocks for Ultra HD modes

[Marek Szyprowski]

Hi Andrzej,

On 2017-01-23 08:56, Andrzej Hajda wrote:
> Ultra HD modes requires clock ticking at increased rate.
>
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>   arch/arm64/boot/dts/exynos/exynos5433.dtsi | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> index f120d99..4d28e93 100644
> --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi

I think that exynos5433-tm2-common.dtsi is a better place for such 
definitions.
They can be also moved to &cmu_disp node to match convention for the clocks
configuration used on particular board.

> @@ -764,6 +764,17 @@
>   			clock-names = "pclk", "aclk_decon", "aclk_smmu_decon0x",
>   				      "aclk_xiu_decon0x", "pclk_smmu_decon0x",
>   				      "sclk_decon_vclk", "sclk_decon_eclk";
> +			assigned-clocks =
> +				<&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
> +				<&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
> +				<&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
> +				<&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
> +			assigned-clock-parents =
> +				<&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
> +				<0>,
> +				<&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
> +				<&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
> +			assigned-clock-rates = <0>, <400000000>;
>   			samsung,disp-sysreg = <&syscon_disp>;
>   			interrupt-names = "fifo", "vsync", "lcd_sys";
>   			interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_HIGH>,

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

[PATCH v3 7/7] arm64: dts: exynos: configure TV path clocks for Ultra HD modes

[Andrzej Hajda]

Ultra HD modes requires clock ticking at increased rate.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
v2: long lines wrapped
v3: moved assigned clocks to cmu_disp node in tm2-common
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
index cd8847b..5f1e172 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
@@ -217,6 +217,18 @@
 	assigned-clock-parents = <&cmu_top CLK_FOUT_AUD_PLL>;
 };
 
+&cmu_disp {
+	assigned-clocks = <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
+			  <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
+			  <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
+			  <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
+	assigned-clock-parents = <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
+				 <0>,
+				 <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
+				 <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
+	assigned-clock-rates = <0>, <400000000>;
+};
+
 &cmu_fsys {
 	assigned-clocks = <&cmu_top CLK_MOUT_SCLK_USBDRD30>,
 		<&cmu_top CLK_MOUT_SCLK_USBHOST30>,
-- 
2.7.4

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

[PATCH v3 7/7] arm64: dts: exynos: configure TV path clocks for Ultra HD modes

[Andrzej Hajda]

Ultra HD modes requires clock ticking at increased rate.

Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
---
v2: long lines wrapped
v3: moved assigned clocks to cmu_disp node in tm2-common
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
index cd8847b..5f1e172 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
@@ -217,6 +217,18 @@
 	assigned-clock-parents = <&cmu_top CLK_FOUT_AUD_PLL>;
 };
 
+&cmu_disp {
+	assigned-clocks = <&cmu_mif CLK_MOUT_SCLK_DECON_TV_ECLK_A>,
+			  <&cmu_mif CLK_DIV_SCLK_DECON_TV_ECLK>,
+			  <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>,
+			  <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK>;
+	assigned-clock-parents = <&cmu_mif CLK_MOUT_BUS_PLL_DIV2>,
+				 <0>,
+				 <&cmu_mif CLK_SCLK_DECON_TV_ECLK_DISP>,
+				 <&cmu_disp CLK_MOUT_SCLK_DECON_TV_ECLK_USER>;
+	assigned-clock-rates = <0>, <400000000>;
+};
+
 &cmu_fsys {
 	assigned-clocks = <&cmu_top CLK_MOUT_SCLK_USBDRD30>,
 		<&cmu_top CLK_MOUT_SCLK_USBHOST30>,
-- 
2.7.4

Re: [PATCH v3 7/7] arm64: dts: exynos: configure TV path clocks for Ultra HD modes

[Krzysztof Kozlowski]

On Mon, Jan 23, 2017 at 11:05:49AM +0100, Andrzej Hajda wrote:
> Ultra HD modes requires clock ticking at increased rate.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
> v2: long lines wrapped
> v3: moved assigned clocks to cmu_disp node in tm2-common
> ---
>  arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 

Thanks, applied.

Best regards,
Krzysztof

Re: [PATCH 0/7] drm/exynos: add Ultra HD and interlace modes support to Exynos5433

[Krzysztof Kozlowski]

On Fri, Jan 20, 2017 at 07:52:18AM +0100, Andrzej Hajda wrote:
> Hi Inki,
> 
> This patchset adds support for UltraHD and intrelace modes on DECON and HDMI
> drivers of Exynos5433 chipset. To fully support it on TM2(e) platforms MHL
> patches are alse required which will be posted in separate patchset.
> 
> Krzysztof, DTS patch do not depend on the previous patches, so there is
> no need to synchronize branches.

Thanks for noticing. Appreciated!

Best regards,
Krzysztof