All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] DRM: ARC: add HDMI 2.0 TX encoder support
@ 2020-04-14 23:29 ` Eugeniy Paltsev
  0 siblings, 0 replies; 21+ messages in thread
From: Eugeniy Paltsev @ 2020-04-14 23:29 UTC (permalink / raw)
  To: dri-devel, Alexey Brodkin
  Cc: linux-snps-arc, linux-kernel, David Airlie, Daniel Vetter,
	Maarten Lankhorst, Maxime Ripard, devicetree, Rob Herring,
	Sam Ravnborg, Eugeniy Paltsev

Changes v1->v2:
 * use DT Schema format please (.yaml files) for DT bindings

Changes v2->v3:
 * Add missing 'interrupts' property in DT bindings
 * Drop dummy 'dw_hdmi_bridge_mode_valid'
 * Change bracing format of 'of_device_id' structure
 * Change compatible string to "snps,arc-dw-hdmi-hsdk"
   Now DT binding file is snps,arc-dw-hdmi.yaml and compatible is
   "snps,arc-dw-hdmi-<soc/board>"
 * Minor fixes

Eugeniy Paltsev (2):
  DRM: ARC: add HDMI 2.0 TX encoder support
  dt-bindings: Document the Synopsys ARC HDMI TX bindings

 .../display/bridge/snps,arc-dw-hdmi.yaml      | 136 ++++++++++++++++++
 MAINTAINERS                                   |   6 +
 drivers/gpu/drm/Makefile                      |   2 +-
 drivers/gpu/drm/arc/Kconfig                   |   7 +
 drivers/gpu/drm/arc/Makefile                  |   1 +
 drivers/gpu/drm/arc/arc-dw-hdmi.c             | 116 +++++++++++++++
 6 files changed, 267 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
 create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c

-- 
2.21.1


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

* [PATCH v3 0/2] DRM: ARC: add HDMI 2.0 TX encoder support
@ 2020-04-14 23:29 ` Eugeniy Paltsev
  0 siblings, 0 replies; 21+ messages in thread
From: Eugeniy Paltsev @ 2020-04-14 23:29 UTC (permalink / raw)
  To: dri-devel, Alexey Brodkin
  Cc: devicetree, David Airlie, Eugeniy Paltsev, Maarten Lankhorst,
	linux-kernel, Maxime Ripard, Rob Herring, Daniel Vetter,
	linux-snps-arc, Sam Ravnborg

Changes v1->v2:
 * use DT Schema format please (.yaml files) for DT bindings

Changes v2->v3:
 * Add missing 'interrupts' property in DT bindings
 * Drop dummy 'dw_hdmi_bridge_mode_valid'
 * Change bracing format of 'of_device_id' structure
 * Change compatible string to "snps,arc-dw-hdmi-hsdk"
   Now DT binding file is snps,arc-dw-hdmi.yaml and compatible is
   "snps,arc-dw-hdmi-<soc/board>"
 * Minor fixes

Eugeniy Paltsev (2):
  DRM: ARC: add HDMI 2.0 TX encoder support
  dt-bindings: Document the Synopsys ARC HDMI TX bindings

 .../display/bridge/snps,arc-dw-hdmi.yaml      | 136 ++++++++++++++++++
 MAINTAINERS                                   |   6 +
 drivers/gpu/drm/Makefile                      |   2 +-
 drivers/gpu/drm/arc/Kconfig                   |   7 +
 drivers/gpu/drm/arc/Makefile                  |   1 +
 drivers/gpu/drm/arc/arc-dw-hdmi.c             | 116 +++++++++++++++
 6 files changed, 267 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
 create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c

-- 
2.21.1


_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

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

* [PATCH v3 0/2] DRM: ARC: add HDMI 2.0 TX encoder support
@ 2020-04-14 23:29 ` Eugeniy Paltsev
  0 siblings, 0 replies; 21+ messages in thread
From: Eugeniy Paltsev @ 2020-04-14 23:29 UTC (permalink / raw)
  To: dri-devel, Alexey Brodkin
  Cc: devicetree, David Airlie, Eugeniy Paltsev, linux-kernel,
	Rob Herring, linux-snps-arc, Sam Ravnborg

Changes v1->v2:
 * use DT Schema format please (.yaml files) for DT bindings

Changes v2->v3:
 * Add missing 'interrupts' property in DT bindings
 * Drop dummy 'dw_hdmi_bridge_mode_valid'
 * Change bracing format of 'of_device_id' structure
 * Change compatible string to "snps,arc-dw-hdmi-hsdk"
   Now DT binding file is snps,arc-dw-hdmi.yaml and compatible is
   "snps,arc-dw-hdmi-<soc/board>"
 * Minor fixes

Eugeniy Paltsev (2):
  DRM: ARC: add HDMI 2.0 TX encoder support
  dt-bindings: Document the Synopsys ARC HDMI TX bindings

 .../display/bridge/snps,arc-dw-hdmi.yaml      | 136 ++++++++++++++++++
 MAINTAINERS                                   |   6 +
 drivers/gpu/drm/Makefile                      |   2 +-
 drivers/gpu/drm/arc/Kconfig                   |   7 +
 drivers/gpu/drm/arc/Makefile                  |   1 +
 drivers/gpu/drm/arc/arc-dw-hdmi.c             | 116 +++++++++++++++
 6 files changed, 267 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
 create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c

-- 
2.21.1

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

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

* [PATCH v3 1/2] DRM: ARC: add HDMI 2.0 TX encoder support
  2020-04-14 23:29 ` Eugeniy Paltsev
  (?)
@ 2020-04-14 23:29   ` Eugeniy Paltsev
  -1 siblings, 0 replies; 21+ messages in thread
From: Eugeniy Paltsev @ 2020-04-14 23:29 UTC (permalink / raw)
  To: dri-devel, Alexey Brodkin
  Cc: linux-snps-arc, linux-kernel, David Airlie, Daniel Vetter,
	Maarten Lankhorst, Maxime Ripard, devicetree, Rob Herring,
	Sam Ravnborg, Eugeniy Paltsev

The Synopsys ARC SoCs (like HSDK4xD) include on-chip DesignWare HDMI
encoders. Support them with a platform driver to provide platform glue
data to the dw-hdmi driver.

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 MAINTAINERS                       |   6 ++
 drivers/gpu/drm/Makefile          |   2 +-
 drivers/gpu/drm/arc/Kconfig       |   7 ++
 drivers/gpu/drm/arc/Makefile      |   1 +
 drivers/gpu/drm/arc/arc-dw-hdmi.c | 116 ++++++++++++++++++++++++++++++
 5 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a6fbdf354d34..2aaed1190370 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1258,6 +1258,12 @@ S:	Supported
 F:	drivers/gpu/drm/arc/
 F:	Documentation/devicetree/bindings/display/snps,arcpgu.txt
 
+ARC DW HDMI DRIVER
+M:	Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
+S:	Supported
+F:	drivers/gpu/drm/arc/arc-dw-hdmi.c
+F:	Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
+
 ARCNET NETWORK LAYER
 M:	Michael Grzeschik <m.grzeschik@pengutronix.de>
 L:	netdev@vger.kernel.org
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 6493088a0fdd..5b0bcf7f45cd 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -109,7 +109,7 @@ obj-y			+= panel/
 obj-y			+= bridge/
 obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
 obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
-obj-$(CONFIG_DRM_ARCPGU)+= arc/
+obj-y			+= arc/
 obj-y			+= hisilicon/
 obj-$(CONFIG_DRM_ZTE)	+= zte/
 obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig
index e8f3d63e0b91..baec9d2a4fba 100644
--- a/drivers/gpu/drm/arc/Kconfig
+++ b/drivers/gpu/drm/arc/Kconfig
@@ -8,3 +8,10 @@ config DRM_ARCPGU
 	  Choose this option if you have an ARC PGU controller.
 
 	  If M is selected the module will be called arcpgu.
+
+config DRM_ARC_DW_HDMI
+	tristate "ARC DW HDMI"
+	depends on DRM && OF
+	select DRM_DW_HDMI
+	help
+	  Synopsys DW HDMI driver for various ARC development boards
diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
index c7028b7427b3..7a156d8c2c3c 100644
--- a/drivers/gpu/drm/arc/Makefile
+++ b/drivers/gpu/drm/arc/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
 obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
+obj-$(CONFIG_DRM_ARC_DW_HDMI) += arc-dw-hdmi.o
diff --git a/drivers/gpu/drm/arc/arc-dw-hdmi.c b/drivers/gpu/drm/arc/arc-dw-hdmi.c
new file mode 100644
index 000000000000..46a6ee09b302
--- /dev/null
+++ b/drivers/gpu/drm/arc/arc-dw-hdmi.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Synopsys DW HDMI driver for various ARC development boards
+//
+// Copyright (C) 2020 Synopsys
+// Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
+
+#include <linux/component.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <drm/bridge/dw_hdmi.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_edid.h>
+#include <drm/drm_encoder_slave.h>
+#include <drm/drm_of.h>
+
+static const struct dw_hdmi_mpll_config snps_hdmi_mpll_cfg[] = {
+	{
+		27000000, {
+			{ 0x00B3, 0x0000 },
+			{ 0x00B3, 0x0000 },
+			{ 0x00B3, 0x0000 }
+		},
+	}, {
+		74250000, {
+			{ 0x0072, 0x0001},
+			{ 0x0072, 0x0001},
+			{ 0x0072, 0x0001}
+		},
+	}, {
+		148500000, {
+			{ 0x0051, 0x0002},
+			{ 0x0051, 0x0002},
+			{ 0x0051, 0x0002}
+		},
+	}, {
+		~0UL, {
+			{ 0x00B3, 0x0000 },
+			{ 0x00B3, 0x0000 },
+			{ 0x00B3, 0x0000 },
+		},
+	}
+};
+
+static const struct dw_hdmi_curr_ctrl snps_hdmi_cur_ctr[] = {
+	/* pixelclk    bpp8    bpp10   bpp12 */
+	{ 27000000,  { 0x0000, 0x0000, 0x0000 }, },
+	{ 74250000,  { 0x0008, 0x0008, 0x0008 }, },
+	{ 148500000, { 0x001b, 0x001b, 0x001b }, },
+	{ ~0UL,      { 0x0000, 0x0000, 0x0000 }, }
+};
+
+
+static const struct dw_hdmi_phy_config snps_hdmi_phy_config[] = {
+	/* pixelclk   symbol  term    vlev */
+	{ 27000000,   0x8009, 0x0004, 0x0232},
+	{ 74250000,   0x8009, 0x0004, 0x0232},
+	{ 148500000,  0x8009, 0x0004, 0x0232},
+	{ ~0UL,       0x8009, 0x0004, 0x0232}
+};
+
+static struct dw_hdmi_plat_data snps_dw_hdmi_drv_data = {
+	.mpll_cfg   = snps_hdmi_mpll_cfg,
+	.cur_ctr    = snps_hdmi_cur_ctr,
+	.phy_config = snps_hdmi_phy_config,
+};
+
+static const struct of_device_id snps_dw_hdmi_dt_ids[] = {
+	{ .compatible = "snps,arc-dw-hdmi-hsdk", .data = &snps_dw_hdmi_drv_data },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, snps_dw_hdmi_dt_ids);
+
+static int snps_dw_hdmi_probe(struct platform_device *pdev)
+{
+	const struct dw_hdmi_plat_data *plat_data;
+	const struct of_device_id *match;
+	struct dw_hdmi *hdmi;
+
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	match = of_match_node(snps_dw_hdmi_dt_ids, pdev->dev.of_node);
+	plat_data = match->data;
+
+	hdmi = dw_hdmi_probe(pdev, plat_data);
+	if (IS_ERR(hdmi))
+		return PTR_ERR(hdmi);
+
+	platform_set_drvdata(pdev, hdmi);
+
+	return 0;
+}
+
+static int snps_dw_hdmi_remove(struct platform_device *pdev)
+{
+	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
+
+	dw_hdmi_remove(hdmi);
+
+	return 0;
+}
+
+static struct platform_driver snps_dw_hdmi_platform_driver = {
+	.probe  = snps_dw_hdmi_probe,
+	.remove = snps_dw_hdmi_remove,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = snps_dw_hdmi_dt_ids,
+	},
+};
+module_platform_driver(snps_dw_hdmi_platform_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("ARC specific DW-HDMI driver extension");
+MODULE_AUTHOR("Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>");
-- 
2.21.1


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

* [PATCH v3 1/2] DRM: ARC: add HDMI 2.0 TX encoder support
@ 2020-04-14 23:29   ` Eugeniy Paltsev
  0 siblings, 0 replies; 21+ messages in thread
From: Eugeniy Paltsev @ 2020-04-14 23:29 UTC (permalink / raw)
  To: dri-devel, Alexey Brodkin
  Cc: devicetree, David Airlie, Eugeniy Paltsev, Maarten Lankhorst,
	linux-kernel, Maxime Ripard, Rob Herring, Daniel Vetter,
	linux-snps-arc, Sam Ravnborg

The Synopsys ARC SoCs (like HSDK4xD) include on-chip DesignWare HDMI
encoders. Support them with a platform driver to provide platform glue
data to the dw-hdmi driver.

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 MAINTAINERS                       |   6 ++
 drivers/gpu/drm/Makefile          |   2 +-
 drivers/gpu/drm/arc/Kconfig       |   7 ++
 drivers/gpu/drm/arc/Makefile      |   1 +
 drivers/gpu/drm/arc/arc-dw-hdmi.c | 116 ++++++++++++++++++++++++++++++
 5 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a6fbdf354d34..2aaed1190370 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1258,6 +1258,12 @@ S:	Supported
 F:	drivers/gpu/drm/arc/
 F:	Documentation/devicetree/bindings/display/snps,arcpgu.txt
 
+ARC DW HDMI DRIVER
+M:	Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
+S:	Supported
+F:	drivers/gpu/drm/arc/arc-dw-hdmi.c
+F:	Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
+
 ARCNET NETWORK LAYER
 M:	Michael Grzeschik <m.grzeschik@pengutronix.de>
 L:	netdev@vger.kernel.org
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 6493088a0fdd..5b0bcf7f45cd 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -109,7 +109,7 @@ obj-y			+= panel/
 obj-y			+= bridge/
 obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
 obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
-obj-$(CONFIG_DRM_ARCPGU)+= arc/
+obj-y			+= arc/
 obj-y			+= hisilicon/
 obj-$(CONFIG_DRM_ZTE)	+= zte/
 obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig
index e8f3d63e0b91..baec9d2a4fba 100644
--- a/drivers/gpu/drm/arc/Kconfig
+++ b/drivers/gpu/drm/arc/Kconfig
@@ -8,3 +8,10 @@ config DRM_ARCPGU
 	  Choose this option if you have an ARC PGU controller.
 
 	  If M is selected the module will be called arcpgu.
+
+config DRM_ARC_DW_HDMI
+	tristate "ARC DW HDMI"
+	depends on DRM && OF
+	select DRM_DW_HDMI
+	help
+	  Synopsys DW HDMI driver for various ARC development boards
diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
index c7028b7427b3..7a156d8c2c3c 100644
--- a/drivers/gpu/drm/arc/Makefile
+++ b/drivers/gpu/drm/arc/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
 obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
+obj-$(CONFIG_DRM_ARC_DW_HDMI) += arc-dw-hdmi.o
diff --git a/drivers/gpu/drm/arc/arc-dw-hdmi.c b/drivers/gpu/drm/arc/arc-dw-hdmi.c
new file mode 100644
index 000000000000..46a6ee09b302
--- /dev/null
+++ b/drivers/gpu/drm/arc/arc-dw-hdmi.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Synopsys DW HDMI driver for various ARC development boards
+//
+// Copyright (C) 2020 Synopsys
+// Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
+
+#include <linux/component.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <drm/bridge/dw_hdmi.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_edid.h>
+#include <drm/drm_encoder_slave.h>
+#include <drm/drm_of.h>
+
+static const struct dw_hdmi_mpll_config snps_hdmi_mpll_cfg[] = {
+	{
+		27000000, {
+			{ 0x00B3, 0x0000 },
+			{ 0x00B3, 0x0000 },
+			{ 0x00B3, 0x0000 }
+		},
+	}, {
+		74250000, {
+			{ 0x0072, 0x0001},
+			{ 0x0072, 0x0001},
+			{ 0x0072, 0x0001}
+		},
+	}, {
+		148500000, {
+			{ 0x0051, 0x0002},
+			{ 0x0051, 0x0002},
+			{ 0x0051, 0x0002}
+		},
+	}, {
+		~0UL, {
+			{ 0x00B3, 0x0000 },
+			{ 0x00B3, 0x0000 },
+			{ 0x00B3, 0x0000 },
+		},
+	}
+};
+
+static const struct dw_hdmi_curr_ctrl snps_hdmi_cur_ctr[] = {
+	/* pixelclk    bpp8    bpp10   bpp12 */
+	{ 27000000,  { 0x0000, 0x0000, 0x0000 }, },
+	{ 74250000,  { 0x0008, 0x0008, 0x0008 }, },
+	{ 148500000, { 0x001b, 0x001b, 0x001b }, },
+	{ ~0UL,      { 0x0000, 0x0000, 0x0000 }, }
+};
+
+
+static const struct dw_hdmi_phy_config snps_hdmi_phy_config[] = {
+	/* pixelclk   symbol  term    vlev */
+	{ 27000000,   0x8009, 0x0004, 0x0232},
+	{ 74250000,   0x8009, 0x0004, 0x0232},
+	{ 148500000,  0x8009, 0x0004, 0x0232},
+	{ ~0UL,       0x8009, 0x0004, 0x0232}
+};
+
+static struct dw_hdmi_plat_data snps_dw_hdmi_drv_data = {
+	.mpll_cfg   = snps_hdmi_mpll_cfg,
+	.cur_ctr    = snps_hdmi_cur_ctr,
+	.phy_config = snps_hdmi_phy_config,
+};
+
+static const struct of_device_id snps_dw_hdmi_dt_ids[] = {
+	{ .compatible = "snps,arc-dw-hdmi-hsdk", .data = &snps_dw_hdmi_drv_data },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, snps_dw_hdmi_dt_ids);
+
+static int snps_dw_hdmi_probe(struct platform_device *pdev)
+{
+	const struct dw_hdmi_plat_data *plat_data;
+	const struct of_device_id *match;
+	struct dw_hdmi *hdmi;
+
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	match = of_match_node(snps_dw_hdmi_dt_ids, pdev->dev.of_node);
+	plat_data = match->data;
+
+	hdmi = dw_hdmi_probe(pdev, plat_data);
+	if (IS_ERR(hdmi))
+		return PTR_ERR(hdmi);
+
+	platform_set_drvdata(pdev, hdmi);
+
+	return 0;
+}
+
+static int snps_dw_hdmi_remove(struct platform_device *pdev)
+{
+	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
+
+	dw_hdmi_remove(hdmi);
+
+	return 0;
+}
+
+static struct platform_driver snps_dw_hdmi_platform_driver = {
+	.probe  = snps_dw_hdmi_probe,
+	.remove = snps_dw_hdmi_remove,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = snps_dw_hdmi_dt_ids,
+	},
+};
+module_platform_driver(snps_dw_hdmi_platform_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("ARC specific DW-HDMI driver extension");
+MODULE_AUTHOR("Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>");
-- 
2.21.1


_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

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

* [PATCH v3 1/2] DRM: ARC: add HDMI 2.0 TX encoder support
@ 2020-04-14 23:29   ` Eugeniy Paltsev
  0 siblings, 0 replies; 21+ messages in thread
From: Eugeniy Paltsev @ 2020-04-14 23:29 UTC (permalink / raw)
  To: dri-devel, Alexey Brodkin
  Cc: devicetree, David Airlie, Eugeniy Paltsev, linux-kernel,
	Rob Herring, linux-snps-arc, Sam Ravnborg

The Synopsys ARC SoCs (like HSDK4xD) include on-chip DesignWare HDMI
encoders. Support them with a platform driver to provide platform glue
data to the dw-hdmi driver.

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 MAINTAINERS                       |   6 ++
 drivers/gpu/drm/Makefile          |   2 +-
 drivers/gpu/drm/arc/Kconfig       |   7 ++
 drivers/gpu/drm/arc/Makefile      |   1 +
 drivers/gpu/drm/arc/arc-dw-hdmi.c | 116 ++++++++++++++++++++++++++++++
 5 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a6fbdf354d34..2aaed1190370 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1258,6 +1258,12 @@ S:	Supported
 F:	drivers/gpu/drm/arc/
 F:	Documentation/devicetree/bindings/display/snps,arcpgu.txt
 
+ARC DW HDMI DRIVER
+M:	Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
+S:	Supported
+F:	drivers/gpu/drm/arc/arc-dw-hdmi.c
+F:	Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
+
 ARCNET NETWORK LAYER
 M:	Michael Grzeschik <m.grzeschik@pengutronix.de>
 L:	netdev@vger.kernel.org
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 6493088a0fdd..5b0bcf7f45cd 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -109,7 +109,7 @@ obj-y			+= panel/
 obj-y			+= bridge/
 obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
 obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
-obj-$(CONFIG_DRM_ARCPGU)+= arc/
+obj-y			+= arc/
 obj-y			+= hisilicon/
 obj-$(CONFIG_DRM_ZTE)	+= zte/
 obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig
index e8f3d63e0b91..baec9d2a4fba 100644
--- a/drivers/gpu/drm/arc/Kconfig
+++ b/drivers/gpu/drm/arc/Kconfig
@@ -8,3 +8,10 @@ config DRM_ARCPGU
 	  Choose this option if you have an ARC PGU controller.
 
 	  If M is selected the module will be called arcpgu.
+
+config DRM_ARC_DW_HDMI
+	tristate "ARC DW HDMI"
+	depends on DRM && OF
+	select DRM_DW_HDMI
+	help
+	  Synopsys DW HDMI driver for various ARC development boards
diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
index c7028b7427b3..7a156d8c2c3c 100644
--- a/drivers/gpu/drm/arc/Makefile
+++ b/drivers/gpu/drm/arc/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
 obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
+obj-$(CONFIG_DRM_ARC_DW_HDMI) += arc-dw-hdmi.o
diff --git a/drivers/gpu/drm/arc/arc-dw-hdmi.c b/drivers/gpu/drm/arc/arc-dw-hdmi.c
new file mode 100644
index 000000000000..46a6ee09b302
--- /dev/null
+++ b/drivers/gpu/drm/arc/arc-dw-hdmi.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Synopsys DW HDMI driver for various ARC development boards
+//
+// Copyright (C) 2020 Synopsys
+// Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
+
+#include <linux/component.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <drm/bridge/dw_hdmi.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_edid.h>
+#include <drm/drm_encoder_slave.h>
+#include <drm/drm_of.h>
+
+static const struct dw_hdmi_mpll_config snps_hdmi_mpll_cfg[] = {
+	{
+		27000000, {
+			{ 0x00B3, 0x0000 },
+			{ 0x00B3, 0x0000 },
+			{ 0x00B3, 0x0000 }
+		},
+	}, {
+		74250000, {
+			{ 0x0072, 0x0001},
+			{ 0x0072, 0x0001},
+			{ 0x0072, 0x0001}
+		},
+	}, {
+		148500000, {
+			{ 0x0051, 0x0002},
+			{ 0x0051, 0x0002},
+			{ 0x0051, 0x0002}
+		},
+	}, {
+		~0UL, {
+			{ 0x00B3, 0x0000 },
+			{ 0x00B3, 0x0000 },
+			{ 0x00B3, 0x0000 },
+		},
+	}
+};
+
+static const struct dw_hdmi_curr_ctrl snps_hdmi_cur_ctr[] = {
+	/* pixelclk    bpp8    bpp10   bpp12 */
+	{ 27000000,  { 0x0000, 0x0000, 0x0000 }, },
+	{ 74250000,  { 0x0008, 0x0008, 0x0008 }, },
+	{ 148500000, { 0x001b, 0x001b, 0x001b }, },
+	{ ~0UL,      { 0x0000, 0x0000, 0x0000 }, }
+};
+
+
+static const struct dw_hdmi_phy_config snps_hdmi_phy_config[] = {
+	/* pixelclk   symbol  term    vlev */
+	{ 27000000,   0x8009, 0x0004, 0x0232},
+	{ 74250000,   0x8009, 0x0004, 0x0232},
+	{ 148500000,  0x8009, 0x0004, 0x0232},
+	{ ~0UL,       0x8009, 0x0004, 0x0232}
+};
+
+static struct dw_hdmi_plat_data snps_dw_hdmi_drv_data = {
+	.mpll_cfg   = snps_hdmi_mpll_cfg,
+	.cur_ctr    = snps_hdmi_cur_ctr,
+	.phy_config = snps_hdmi_phy_config,
+};
+
+static const struct of_device_id snps_dw_hdmi_dt_ids[] = {
+	{ .compatible = "snps,arc-dw-hdmi-hsdk", .data = &snps_dw_hdmi_drv_data },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, snps_dw_hdmi_dt_ids);
+
+static int snps_dw_hdmi_probe(struct platform_device *pdev)
+{
+	const struct dw_hdmi_plat_data *plat_data;
+	const struct of_device_id *match;
+	struct dw_hdmi *hdmi;
+
+	if (!pdev->dev.of_node)
+		return -ENODEV;
+
+	match = of_match_node(snps_dw_hdmi_dt_ids, pdev->dev.of_node);
+	plat_data = match->data;
+
+	hdmi = dw_hdmi_probe(pdev, plat_data);
+	if (IS_ERR(hdmi))
+		return PTR_ERR(hdmi);
+
+	platform_set_drvdata(pdev, hdmi);
+
+	return 0;
+}
+
+static int snps_dw_hdmi_remove(struct platform_device *pdev)
+{
+	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
+
+	dw_hdmi_remove(hdmi);
+
+	return 0;
+}
+
+static struct platform_driver snps_dw_hdmi_platform_driver = {
+	.probe  = snps_dw_hdmi_probe,
+	.remove = snps_dw_hdmi_remove,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = snps_dw_hdmi_dt_ids,
+	},
+};
+module_platform_driver(snps_dw_hdmi_platform_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("ARC specific DW-HDMI driver extension");
+MODULE_AUTHOR("Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>");
-- 
2.21.1

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

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

* [PATCH v3 2/2] dt-bindings: Document the Synopsys ARC HDMI TX bindings
  2020-04-14 23:29 ` Eugeniy Paltsev
  (?)
@ 2020-04-14 23:29   ` Eugeniy Paltsev
  -1 siblings, 0 replies; 21+ messages in thread
From: Eugeniy Paltsev @ 2020-04-14 23:29 UTC (permalink / raw)
  To: dri-devel, Alexey Brodkin
  Cc: linux-snps-arc, linux-kernel, David Airlie, Daniel Vetter,
	Maarten Lankhorst, Maxime Ripard, devicetree, Rob Herring,
	Sam Ravnborg, Eugeniy Paltsev

This patch adds documentation of device tree bindings for the Synopsys
HDMI 2.0 TX encoder driver for ARC SoCs.

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 .../display/bridge/snps,arc-dw-hdmi.yaml      | 136 ++++++++++++++++++
 1 file changed, 136 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml b/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
new file mode 100644
index 000000000000..9b2fdfecd5b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
@@ -0,0 +1,136 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/snps,arc-dw-hdmi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare HDMI 2.0 TX encoder driver
+
+maintainers:
+  - Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
+
+description: |
+  The HDMI transmitter is a Synopsys DesignWare HDMI 2.0 TX controller IP
+  with a companion of Synopsys DesignWare HDMI 2.0 TX PHY IP.
+
+  These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
+  Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
+  with the following device-specific properties.
+
+properties:
+  compatible:
+    const: snps,arc-dw-hdmi-hsdk
+
+  reg:
+    maxItems: 1
+    description: |
+      Memory mapped base address and length of the DWC HDMI TX registers.
+
+  clocks:
+    items:
+      - description: The bus clock for AHB / APB
+      - description: The internal register configuration clock
+
+  clock-names:
+    items:
+      - const: iahb
+      - const: isfr
+
+  interrupts:
+    maxItems: 1
+    description: Reference to the DWC HDMI TX interrupt
+
+  reg-io-width:
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - enum: [1, 4]
+        description: |
+          Width of the registers specified by the reg property. The
+          value is expressed in bytes and must be equal to 1 or 4 if specified.
+          The register width defaults to 1 if the property is not present.
+
+  ports:
+    type: object
+    description: |
+      A ports node with endpoint definitions as defined in
+      Documentation/devicetree/bindings/media/video-interfaces.txt
+
+    properties:
+      "#address-cells":
+        const: 1
+
+      "#size-cells":
+        const: 0
+
+      port@0:
+        type: object
+        description: |
+          Video input endpoints of the controller.
+          Usually it is associated with ARC PGU.
+
+      port@1:
+        type: object
+        description: |
+          Output endpoints of the controller. HDMI connector.
+
+    required:
+      - "#address-cells"
+      - "#size-cells"
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    hdmi@10000 {
+        compatible = "snps,arc-dw-hdmi-hsdk";
+        reg = <0x10000 0x10000>;
+        reg-io-width = <4>;
+        interrupts = <14>;
+        clocks = <&apbclk>, <&hdmi_pix_clk>;
+        clock-names = "iahb", "isfr";
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                hdmi_enc_input: endpoint {
+                    remote-endpoint = <&pgu_output>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+                hdmi_enc_out: endpoint {
+                    remote-endpoint = <&hdmi_con>;
+                };
+            };
+        };
+    };
+
+    hdmi-out {
+        port {
+            hdmi_con: endpoint {
+                remote-endpoint = <&hdmi_enc_out>;
+            };
+        };
+    };
+
+    pgu {
+        port_o: port {
+            pgu_output: endpoint {
+                remote-endpoint = <&hdmi_enc_input>;
+            };
+        };
+    };
-- 
2.21.1


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

* [PATCH v3 2/2] dt-bindings: Document the Synopsys ARC HDMI TX bindings
@ 2020-04-14 23:29   ` Eugeniy Paltsev
  0 siblings, 0 replies; 21+ messages in thread
From: Eugeniy Paltsev @ 2020-04-14 23:29 UTC (permalink / raw)
  To: dri-devel, Alexey Brodkin
  Cc: devicetree, David Airlie, Eugeniy Paltsev, Maarten Lankhorst,
	linux-kernel, Maxime Ripard, Rob Herring, Daniel Vetter,
	linux-snps-arc, Sam Ravnborg

This patch adds documentation of device tree bindings for the Synopsys
HDMI 2.0 TX encoder driver for ARC SoCs.

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 .../display/bridge/snps,arc-dw-hdmi.yaml      | 136 ++++++++++++++++++
 1 file changed, 136 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml b/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
new file mode 100644
index 000000000000..9b2fdfecd5b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
@@ -0,0 +1,136 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/snps,arc-dw-hdmi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare HDMI 2.0 TX encoder driver
+
+maintainers:
+  - Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
+
+description: |
+  The HDMI transmitter is a Synopsys DesignWare HDMI 2.0 TX controller IP
+  with a companion of Synopsys DesignWare HDMI 2.0 TX PHY IP.
+
+  These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
+  Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
+  with the following device-specific properties.
+
+properties:
+  compatible:
+    const: snps,arc-dw-hdmi-hsdk
+
+  reg:
+    maxItems: 1
+    description: |
+      Memory mapped base address and length of the DWC HDMI TX registers.
+
+  clocks:
+    items:
+      - description: The bus clock for AHB / APB
+      - description: The internal register configuration clock
+
+  clock-names:
+    items:
+      - const: iahb
+      - const: isfr
+
+  interrupts:
+    maxItems: 1
+    description: Reference to the DWC HDMI TX interrupt
+
+  reg-io-width:
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - enum: [1, 4]
+        description: |
+          Width of the registers specified by the reg property. The
+          value is expressed in bytes and must be equal to 1 or 4 if specified.
+          The register width defaults to 1 if the property is not present.
+
+  ports:
+    type: object
+    description: |
+      A ports node with endpoint definitions as defined in
+      Documentation/devicetree/bindings/media/video-interfaces.txt
+
+    properties:
+      "#address-cells":
+        const: 1
+
+      "#size-cells":
+        const: 0
+
+      port@0:
+        type: object
+        description: |
+          Video input endpoints of the controller.
+          Usually it is associated with ARC PGU.
+
+      port@1:
+        type: object
+        description: |
+          Output endpoints of the controller. HDMI connector.
+
+    required:
+      - "#address-cells"
+      - "#size-cells"
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    hdmi@10000 {
+        compatible = "snps,arc-dw-hdmi-hsdk";
+        reg = <0x10000 0x10000>;
+        reg-io-width = <4>;
+        interrupts = <14>;
+        clocks = <&apbclk>, <&hdmi_pix_clk>;
+        clock-names = "iahb", "isfr";
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                hdmi_enc_input: endpoint {
+                    remote-endpoint = <&pgu_output>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+                hdmi_enc_out: endpoint {
+                    remote-endpoint = <&hdmi_con>;
+                };
+            };
+        };
+    };
+
+    hdmi-out {
+        port {
+            hdmi_con: endpoint {
+                remote-endpoint = <&hdmi_enc_out>;
+            };
+        };
+    };
+
+    pgu {
+        port_o: port {
+            pgu_output: endpoint {
+                remote-endpoint = <&hdmi_enc_input>;
+            };
+        };
+    };
-- 
2.21.1


_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

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

* [PATCH v3 2/2] dt-bindings: Document the Synopsys ARC HDMI TX bindings
@ 2020-04-14 23:29   ` Eugeniy Paltsev
  0 siblings, 0 replies; 21+ messages in thread
From: Eugeniy Paltsev @ 2020-04-14 23:29 UTC (permalink / raw)
  To: dri-devel, Alexey Brodkin
  Cc: devicetree, David Airlie, Eugeniy Paltsev, linux-kernel,
	Rob Herring, linux-snps-arc, Sam Ravnborg

This patch adds documentation of device tree bindings for the Synopsys
HDMI 2.0 TX encoder driver for ARC SoCs.

Acked-by: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 .../display/bridge/snps,arc-dw-hdmi.yaml      | 136 ++++++++++++++++++
 1 file changed, 136 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml

diff --git a/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml b/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
new file mode 100644
index 000000000000..9b2fdfecd5b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
@@ -0,0 +1,136 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/snps,arc-dw-hdmi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare HDMI 2.0 TX encoder driver
+
+maintainers:
+  - Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
+
+description: |
+  The HDMI transmitter is a Synopsys DesignWare HDMI 2.0 TX controller IP
+  with a companion of Synopsys DesignWare HDMI 2.0 TX PHY IP.
+
+  These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
+  Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
+  with the following device-specific properties.
+
+properties:
+  compatible:
+    const: snps,arc-dw-hdmi-hsdk
+
+  reg:
+    maxItems: 1
+    description: |
+      Memory mapped base address and length of the DWC HDMI TX registers.
+
+  clocks:
+    items:
+      - description: The bus clock for AHB / APB
+      - description: The internal register configuration clock
+
+  clock-names:
+    items:
+      - const: iahb
+      - const: isfr
+
+  interrupts:
+    maxItems: 1
+    description: Reference to the DWC HDMI TX interrupt
+
+  reg-io-width:
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - enum: [1, 4]
+        description: |
+          Width of the registers specified by the reg property. The
+          value is expressed in bytes and must be equal to 1 or 4 if specified.
+          The register width defaults to 1 if the property is not present.
+
+  ports:
+    type: object
+    description: |
+      A ports node with endpoint definitions as defined in
+      Documentation/devicetree/bindings/media/video-interfaces.txt
+
+    properties:
+      "#address-cells":
+        const: 1
+
+      "#size-cells":
+        const: 0
+
+      port@0:
+        type: object
+        description: |
+          Video input endpoints of the controller.
+          Usually it is associated with ARC PGU.
+
+      port@1:
+        type: object
+        description: |
+          Output endpoints of the controller. HDMI connector.
+
+    required:
+      - "#address-cells"
+      - "#size-cells"
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    hdmi@10000 {
+        compatible = "snps,arc-dw-hdmi-hsdk";
+        reg = <0x10000 0x10000>;
+        reg-io-width = <4>;
+        interrupts = <14>;
+        clocks = <&apbclk>, <&hdmi_pix_clk>;
+        clock-names = "iahb", "isfr";
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                hdmi_enc_input: endpoint {
+                    remote-endpoint = <&pgu_output>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+                hdmi_enc_out: endpoint {
+                    remote-endpoint = <&hdmi_con>;
+                };
+            };
+        };
+    };
+
+    hdmi-out {
+        port {
+            hdmi_con: endpoint {
+                remote-endpoint = <&hdmi_enc_out>;
+            };
+        };
+    };
+
+    pgu {
+        port_o: port {
+            pgu_output: endpoint {
+                remote-endpoint = <&hdmi_enc_input>;
+            };
+        };
+    };
-- 
2.21.1

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

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

* Re: [PATCH v3 1/2] DRM: ARC: add HDMI 2.0 TX encoder support
  2020-04-14 23:29   ` Eugeniy Paltsev
  (?)
@ 2020-04-15 17:33     ` Daniel Vetter
  -1 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2020-04-15 17:33 UTC (permalink / raw)
  To: Eugeniy Paltsev
  Cc: dri-devel, Alexey Brodkin, linux-snps-arc, linux-kernel,
	David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	devicetree, Rob Herring, Sam Ravnborg

On Wed, Apr 15, 2020 at 02:29:28AM +0300, Eugeniy Paltsev wrote:
> The Synopsys ARC SoCs (like HSDK4xD) include on-chip DesignWare HDMI
> encoders. Support them with a platform driver to provide platform glue
> data to the dw-hdmi driver.
> 
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> ---
>  MAINTAINERS                       |   6 ++
>  drivers/gpu/drm/Makefile          |   2 +-
>  drivers/gpu/drm/arc/Kconfig       |   7 ++
>  drivers/gpu/drm/arc/Makefile      |   1 +
>  drivers/gpu/drm/arc/arc-dw-hdmi.c | 116 ++++++++++++++++++++++++++++++
>  5 files changed, 131 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a6fbdf354d34..2aaed1190370 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1258,6 +1258,12 @@ S:	Supported
>  F:	drivers/gpu/drm/arc/
>  F:	Documentation/devicetree/bindings/display/snps,arcpgu.txt
>  
> +ARC DW HDMI DRIVER
> +M:	Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> +S:	Supported
> +F:	drivers/gpu/drm/arc/arc-dw-hdmi.c
> +F:	Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> +
>  ARCNET NETWORK LAYER
>  M:	Michael Grzeschik <m.grzeschik@pengutronix.de>
>  L:	netdev@vger.kernel.org
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 6493088a0fdd..5b0bcf7f45cd 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -109,7 +109,7 @@ obj-y			+= panel/
>  obj-y			+= bridge/
>  obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
>  obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
> -obj-$(CONFIG_DRM_ARCPGU)+= arc/
> +obj-y			+= arc/
>  obj-y			+= hisilicon/
>  obj-$(CONFIG_DRM_ZTE)	+= zte/
>  obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
> diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig
> index e8f3d63e0b91..baec9d2a4fba 100644
> --- a/drivers/gpu/drm/arc/Kconfig
> +++ b/drivers/gpu/drm/arc/Kconfig
> @@ -8,3 +8,10 @@ config DRM_ARCPGU
>  	  Choose this option if you have an ARC PGU controller.
>  
>  	  If M is selected the module will be called arcpgu.
> +
> +config DRM_ARC_DW_HDMI
> +	tristate "ARC DW HDMI"
> +	depends on DRM && OF
> +	select DRM_DW_HDMI
> +	help
> +	  Synopsys DW HDMI driver for various ARC development boards
> diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
> index c7028b7427b3..7a156d8c2c3c 100644
> --- a/drivers/gpu/drm/arc/Makefile
> +++ b/drivers/gpu/drm/arc/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
>  obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
> +obj-$(CONFIG_DRM_ARC_DW_HDMI) += arc-dw-hdmi.o
> diff --git a/drivers/gpu/drm/arc/arc-dw-hdmi.c b/drivers/gpu/drm/arc/arc-dw-hdmi.c
> new file mode 100644
> index 000000000000..46a6ee09b302
> --- /dev/null
> +++ b/drivers/gpu/drm/arc/arc-dw-hdmi.c
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//
> +// Synopsys DW HDMI driver for various ARC development boards
> +//
> +// Copyright (C) 2020 Synopsys
> +// Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> +
> +#include <linux/component.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <drm/bridge/dw_hdmi.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_edid.h>
> +#include <drm/drm_encoder_slave.h>
> +#include <drm/drm_of.h>
> +
> +static const struct dw_hdmi_mpll_config snps_hdmi_mpll_cfg[] = {
> +	{
> +		27000000, {
> +			{ 0x00B3, 0x0000 },
> +			{ 0x00B3, 0x0000 },
> +			{ 0x00B3, 0x0000 }
> +		},
> +	}, {
> +		74250000, {
> +			{ 0x0072, 0x0001},
> +			{ 0x0072, 0x0001},
> +			{ 0x0072, 0x0001}
> +		},
> +	}, {
> +		148500000, {
> +			{ 0x0051, 0x0002},
> +			{ 0x0051, 0x0002},
> +			{ 0x0051, 0x0002}
> +		},
> +	}, {
> +		~0UL, {
> +			{ 0x00B3, 0x0000 },
> +			{ 0x00B3, 0x0000 },
> +			{ 0x00B3, 0x0000 },
> +		},
> +	}
> +};
> +
> +static const struct dw_hdmi_curr_ctrl snps_hdmi_cur_ctr[] = {
> +	/* pixelclk    bpp8    bpp10   bpp12 */
> +	{ 27000000,  { 0x0000, 0x0000, 0x0000 }, },
> +	{ 74250000,  { 0x0008, 0x0008, 0x0008 }, },
> +	{ 148500000, { 0x001b, 0x001b, 0x001b }, },
> +	{ ~0UL,      { 0x0000, 0x0000, 0x0000 }, }
> +};
> +
> +
> +static const struct dw_hdmi_phy_config snps_hdmi_phy_config[] = {
> +	/* pixelclk   symbol  term    vlev */
> +	{ 27000000,   0x8009, 0x0004, 0x0232},
> +	{ 74250000,   0x8009, 0x0004, 0x0232},
> +	{ 148500000,  0x8009, 0x0004, 0x0232},
> +	{ ~0UL,       0x8009, 0x0004, 0x0232}
> +};
> +
> +static struct dw_hdmi_plat_data snps_dw_hdmi_drv_data = {
> +	.mpll_cfg   = snps_hdmi_mpll_cfg,
> +	.cur_ctr    = snps_hdmi_cur_ctr,
> +	.phy_config = snps_hdmi_phy_config,
> +};
> +
> +static const struct of_device_id snps_dw_hdmi_dt_ids[] = {
> +	{ .compatible = "snps,arc-dw-hdmi-hsdk", .data = &snps_dw_hdmi_drv_data },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, snps_dw_hdmi_dt_ids);
> +
> +static int snps_dw_hdmi_probe(struct platform_device *pdev)
> +{
> +	const struct dw_hdmi_plat_data *plat_data;
> +	const struct of_device_id *match;
> +	struct dw_hdmi *hdmi;
> +
> +	if (!pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	match = of_match_node(snps_dw_hdmi_dt_ids, pdev->dev.of_node);
> +	plat_data = match->data;
> +
> +	hdmi = dw_hdmi_probe(pdev, plat_data);

So this is kinda not how bridge drivers are supposed to be done nowadays,
direct calling into the driver was the old way, and dw-hdmi still works
like that. Modern way is roughly
- bridge drivers bind automatically to any bridge they support
- bridge drivers publish a bridge with drm_bridge_add()
- the driver using the bridge fishes out with dt magic using
  of_drm_find_bridge() or another of the related of_ functions

I know a bit late, just spotted this because you brought your series here
up in my arc cleanup series, but can you pls look into adjusting
accordingly?

I shouldn't take more than moving this binding here into the dw-hdmi
driver, and switching arc itself over to the of_drm_find_bridge() call.
That way we could slowly work to transform old bridge drivers like dw-hdmi
to the new way, instead of adding more cases that will never get
converted.

Other upside is that arc stays a neat&tiny driver :-)

Thanks, Daniel

> +	if (IS_ERR(hdmi))
> +		return PTR_ERR(hdmi);
> +
> +	platform_set_drvdata(pdev, hdmi);
> +
> +	return 0;
> +}
> +
> +static int snps_dw_hdmi_remove(struct platform_device *pdev)
> +{
> +	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
> +
> +	dw_hdmi_remove(hdmi);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver snps_dw_hdmi_platform_driver = {
> +	.probe  = snps_dw_hdmi_probe,
> +	.remove = snps_dw_hdmi_remove,
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.of_match_table = snps_dw_hdmi_dt_ids,
> +	},
> +};
> +module_platform_driver(snps_dw_hdmi_platform_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("ARC specific DW-HDMI driver extension");
> +MODULE_AUTHOR("Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>");
> -- 
> 2.21.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH v3 1/2] DRM: ARC: add HDMI 2.0 TX encoder support
@ 2020-04-15 17:33     ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2020-04-15 17:33 UTC (permalink / raw)
  To: Eugeniy Paltsev
  Cc: devicetree, David Airlie, Alexey Brodkin, Maarten Lankhorst,
	linux-kernel, dri-devel, Rob Herring, Maxime Ripard,
	Daniel Vetter, linux-snps-arc, Sam Ravnborg

On Wed, Apr 15, 2020 at 02:29:28AM +0300, Eugeniy Paltsev wrote:
> The Synopsys ARC SoCs (like HSDK4xD) include on-chip DesignWare HDMI
> encoders. Support them with a platform driver to provide platform glue
> data to the dw-hdmi driver.
> 
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> ---
>  MAINTAINERS                       |   6 ++
>  drivers/gpu/drm/Makefile          |   2 +-
>  drivers/gpu/drm/arc/Kconfig       |   7 ++
>  drivers/gpu/drm/arc/Makefile      |   1 +
>  drivers/gpu/drm/arc/arc-dw-hdmi.c | 116 ++++++++++++++++++++++++++++++
>  5 files changed, 131 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a6fbdf354d34..2aaed1190370 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1258,6 +1258,12 @@ S:	Supported
>  F:	drivers/gpu/drm/arc/
>  F:	Documentation/devicetree/bindings/display/snps,arcpgu.txt
>  
> +ARC DW HDMI DRIVER
> +M:	Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> +S:	Supported
> +F:	drivers/gpu/drm/arc/arc-dw-hdmi.c
> +F:	Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> +
>  ARCNET NETWORK LAYER
>  M:	Michael Grzeschik <m.grzeschik@pengutronix.de>
>  L:	netdev@vger.kernel.org
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 6493088a0fdd..5b0bcf7f45cd 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -109,7 +109,7 @@ obj-y			+= panel/
>  obj-y			+= bridge/
>  obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
>  obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
> -obj-$(CONFIG_DRM_ARCPGU)+= arc/
> +obj-y			+= arc/
>  obj-y			+= hisilicon/
>  obj-$(CONFIG_DRM_ZTE)	+= zte/
>  obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
> diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig
> index e8f3d63e0b91..baec9d2a4fba 100644
> --- a/drivers/gpu/drm/arc/Kconfig
> +++ b/drivers/gpu/drm/arc/Kconfig
> @@ -8,3 +8,10 @@ config DRM_ARCPGU
>  	  Choose this option if you have an ARC PGU controller.
>  
>  	  If M is selected the module will be called arcpgu.
> +
> +config DRM_ARC_DW_HDMI
> +	tristate "ARC DW HDMI"
> +	depends on DRM && OF
> +	select DRM_DW_HDMI
> +	help
> +	  Synopsys DW HDMI driver for various ARC development boards
> diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
> index c7028b7427b3..7a156d8c2c3c 100644
> --- a/drivers/gpu/drm/arc/Makefile
> +++ b/drivers/gpu/drm/arc/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
>  obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
> +obj-$(CONFIG_DRM_ARC_DW_HDMI) += arc-dw-hdmi.o
> diff --git a/drivers/gpu/drm/arc/arc-dw-hdmi.c b/drivers/gpu/drm/arc/arc-dw-hdmi.c
> new file mode 100644
> index 000000000000..46a6ee09b302
> --- /dev/null
> +++ b/drivers/gpu/drm/arc/arc-dw-hdmi.c
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//
> +// Synopsys DW HDMI driver for various ARC development boards
> +//
> +// Copyright (C) 2020 Synopsys
> +// Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> +
> +#include <linux/component.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <drm/bridge/dw_hdmi.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_edid.h>
> +#include <drm/drm_encoder_slave.h>
> +#include <drm/drm_of.h>
> +
> +static const struct dw_hdmi_mpll_config snps_hdmi_mpll_cfg[] = {
> +	{
> +		27000000, {
> +			{ 0x00B3, 0x0000 },
> +			{ 0x00B3, 0x0000 },
> +			{ 0x00B3, 0x0000 }
> +		},
> +	}, {
> +		74250000, {
> +			{ 0x0072, 0x0001},
> +			{ 0x0072, 0x0001},
> +			{ 0x0072, 0x0001}
> +		},
> +	}, {
> +		148500000, {
> +			{ 0x0051, 0x0002},
> +			{ 0x0051, 0x0002},
> +			{ 0x0051, 0x0002}
> +		},
> +	}, {
> +		~0UL, {
> +			{ 0x00B3, 0x0000 },
> +			{ 0x00B3, 0x0000 },
> +			{ 0x00B3, 0x0000 },
> +		},
> +	}
> +};
> +
> +static const struct dw_hdmi_curr_ctrl snps_hdmi_cur_ctr[] = {
> +	/* pixelclk    bpp8    bpp10   bpp12 */
> +	{ 27000000,  { 0x0000, 0x0000, 0x0000 }, },
> +	{ 74250000,  { 0x0008, 0x0008, 0x0008 }, },
> +	{ 148500000, { 0x001b, 0x001b, 0x001b }, },
> +	{ ~0UL,      { 0x0000, 0x0000, 0x0000 }, }
> +};
> +
> +
> +static const struct dw_hdmi_phy_config snps_hdmi_phy_config[] = {
> +	/* pixelclk   symbol  term    vlev */
> +	{ 27000000,   0x8009, 0x0004, 0x0232},
> +	{ 74250000,   0x8009, 0x0004, 0x0232},
> +	{ 148500000,  0x8009, 0x0004, 0x0232},
> +	{ ~0UL,       0x8009, 0x0004, 0x0232}
> +};
> +
> +static struct dw_hdmi_plat_data snps_dw_hdmi_drv_data = {
> +	.mpll_cfg   = snps_hdmi_mpll_cfg,
> +	.cur_ctr    = snps_hdmi_cur_ctr,
> +	.phy_config = snps_hdmi_phy_config,
> +};
> +
> +static const struct of_device_id snps_dw_hdmi_dt_ids[] = {
> +	{ .compatible = "snps,arc-dw-hdmi-hsdk", .data = &snps_dw_hdmi_drv_data },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, snps_dw_hdmi_dt_ids);
> +
> +static int snps_dw_hdmi_probe(struct platform_device *pdev)
> +{
> +	const struct dw_hdmi_plat_data *plat_data;
> +	const struct of_device_id *match;
> +	struct dw_hdmi *hdmi;
> +
> +	if (!pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	match = of_match_node(snps_dw_hdmi_dt_ids, pdev->dev.of_node);
> +	plat_data = match->data;
> +
> +	hdmi = dw_hdmi_probe(pdev, plat_data);

So this is kinda not how bridge drivers are supposed to be done nowadays,
direct calling into the driver was the old way, and dw-hdmi still works
like that. Modern way is roughly
- bridge drivers bind automatically to any bridge they support
- bridge drivers publish a bridge with drm_bridge_add()
- the driver using the bridge fishes out with dt magic using
  of_drm_find_bridge() or another of the related of_ functions

I know a bit late, just spotted this because you brought your series here
up in my arc cleanup series, but can you pls look into adjusting
accordingly?

I shouldn't take more than moving this binding here into the dw-hdmi
driver, and switching arc itself over to the of_drm_find_bridge() call.
That way we could slowly work to transform old bridge drivers like dw-hdmi
to the new way, instead of adding more cases that will never get
converted.

Other upside is that arc stays a neat&tiny driver :-)

Thanks, Daniel

> +	if (IS_ERR(hdmi))
> +		return PTR_ERR(hdmi);
> +
> +	platform_set_drvdata(pdev, hdmi);
> +
> +	return 0;
> +}
> +
> +static int snps_dw_hdmi_remove(struct platform_device *pdev)
> +{
> +	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
> +
> +	dw_hdmi_remove(hdmi);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver snps_dw_hdmi_platform_driver = {
> +	.probe  = snps_dw_hdmi_probe,
> +	.remove = snps_dw_hdmi_remove,
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.of_match_table = snps_dw_hdmi_dt_ids,
> +	},
> +};
> +module_platform_driver(snps_dw_hdmi_platform_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("ARC specific DW-HDMI driver extension");
> +MODULE_AUTHOR("Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>");
> -- 
> 2.21.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

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

* Re: [PATCH v3 1/2] DRM: ARC: add HDMI 2.0 TX encoder support
@ 2020-04-15 17:33     ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2020-04-15 17:33 UTC (permalink / raw)
  To: Eugeniy Paltsev
  Cc: devicetree, David Airlie, Alexey Brodkin, linux-kernel,
	dri-devel, Rob Herring, linux-snps-arc, Sam Ravnborg

On Wed, Apr 15, 2020 at 02:29:28AM +0300, Eugeniy Paltsev wrote:
> The Synopsys ARC SoCs (like HSDK4xD) include on-chip DesignWare HDMI
> encoders. Support them with a platform driver to provide platform glue
> data to the dw-hdmi driver.
> 
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> ---
>  MAINTAINERS                       |   6 ++
>  drivers/gpu/drm/Makefile          |   2 +-
>  drivers/gpu/drm/arc/Kconfig       |   7 ++
>  drivers/gpu/drm/arc/Makefile      |   1 +
>  drivers/gpu/drm/arc/arc-dw-hdmi.c | 116 ++++++++++++++++++++++++++++++
>  5 files changed, 131 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a6fbdf354d34..2aaed1190370 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1258,6 +1258,12 @@ S:	Supported
>  F:	drivers/gpu/drm/arc/
>  F:	Documentation/devicetree/bindings/display/snps,arcpgu.txt
>  
> +ARC DW HDMI DRIVER
> +M:	Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> +S:	Supported
> +F:	drivers/gpu/drm/arc/arc-dw-hdmi.c
> +F:	Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> +
>  ARCNET NETWORK LAYER
>  M:	Michael Grzeschik <m.grzeschik@pengutronix.de>
>  L:	netdev@vger.kernel.org
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 6493088a0fdd..5b0bcf7f45cd 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -109,7 +109,7 @@ obj-y			+= panel/
>  obj-y			+= bridge/
>  obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
>  obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
> -obj-$(CONFIG_DRM_ARCPGU)+= arc/
> +obj-y			+= arc/
>  obj-y			+= hisilicon/
>  obj-$(CONFIG_DRM_ZTE)	+= zte/
>  obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
> diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig
> index e8f3d63e0b91..baec9d2a4fba 100644
> --- a/drivers/gpu/drm/arc/Kconfig
> +++ b/drivers/gpu/drm/arc/Kconfig
> @@ -8,3 +8,10 @@ config DRM_ARCPGU
>  	  Choose this option if you have an ARC PGU controller.
>  
>  	  If M is selected the module will be called arcpgu.
> +
> +config DRM_ARC_DW_HDMI
> +	tristate "ARC DW HDMI"
> +	depends on DRM && OF
> +	select DRM_DW_HDMI
> +	help
> +	  Synopsys DW HDMI driver for various ARC development boards
> diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
> index c7028b7427b3..7a156d8c2c3c 100644
> --- a/drivers/gpu/drm/arc/Makefile
> +++ b/drivers/gpu/drm/arc/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
>  obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
> +obj-$(CONFIG_DRM_ARC_DW_HDMI) += arc-dw-hdmi.o
> diff --git a/drivers/gpu/drm/arc/arc-dw-hdmi.c b/drivers/gpu/drm/arc/arc-dw-hdmi.c
> new file mode 100644
> index 000000000000..46a6ee09b302
> --- /dev/null
> +++ b/drivers/gpu/drm/arc/arc-dw-hdmi.c
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +//
> +// Synopsys DW HDMI driver for various ARC development boards
> +//
> +// Copyright (C) 2020 Synopsys
> +// Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> +
> +#include <linux/component.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <drm/bridge/dw_hdmi.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_edid.h>
> +#include <drm/drm_encoder_slave.h>
> +#include <drm/drm_of.h>
> +
> +static const struct dw_hdmi_mpll_config snps_hdmi_mpll_cfg[] = {
> +	{
> +		27000000, {
> +			{ 0x00B3, 0x0000 },
> +			{ 0x00B3, 0x0000 },
> +			{ 0x00B3, 0x0000 }
> +		},
> +	}, {
> +		74250000, {
> +			{ 0x0072, 0x0001},
> +			{ 0x0072, 0x0001},
> +			{ 0x0072, 0x0001}
> +		},
> +	}, {
> +		148500000, {
> +			{ 0x0051, 0x0002},
> +			{ 0x0051, 0x0002},
> +			{ 0x0051, 0x0002}
> +		},
> +	}, {
> +		~0UL, {
> +			{ 0x00B3, 0x0000 },
> +			{ 0x00B3, 0x0000 },
> +			{ 0x00B3, 0x0000 },
> +		},
> +	}
> +};
> +
> +static const struct dw_hdmi_curr_ctrl snps_hdmi_cur_ctr[] = {
> +	/* pixelclk    bpp8    bpp10   bpp12 */
> +	{ 27000000,  { 0x0000, 0x0000, 0x0000 }, },
> +	{ 74250000,  { 0x0008, 0x0008, 0x0008 }, },
> +	{ 148500000, { 0x001b, 0x001b, 0x001b }, },
> +	{ ~0UL,      { 0x0000, 0x0000, 0x0000 }, }
> +};
> +
> +
> +static const struct dw_hdmi_phy_config snps_hdmi_phy_config[] = {
> +	/* pixelclk   symbol  term    vlev */
> +	{ 27000000,   0x8009, 0x0004, 0x0232},
> +	{ 74250000,   0x8009, 0x0004, 0x0232},
> +	{ 148500000,  0x8009, 0x0004, 0x0232},
> +	{ ~0UL,       0x8009, 0x0004, 0x0232}
> +};
> +
> +static struct dw_hdmi_plat_data snps_dw_hdmi_drv_data = {
> +	.mpll_cfg   = snps_hdmi_mpll_cfg,
> +	.cur_ctr    = snps_hdmi_cur_ctr,
> +	.phy_config = snps_hdmi_phy_config,
> +};
> +
> +static const struct of_device_id snps_dw_hdmi_dt_ids[] = {
> +	{ .compatible = "snps,arc-dw-hdmi-hsdk", .data = &snps_dw_hdmi_drv_data },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, snps_dw_hdmi_dt_ids);
> +
> +static int snps_dw_hdmi_probe(struct platform_device *pdev)
> +{
> +	const struct dw_hdmi_plat_data *plat_data;
> +	const struct of_device_id *match;
> +	struct dw_hdmi *hdmi;
> +
> +	if (!pdev->dev.of_node)
> +		return -ENODEV;
> +
> +	match = of_match_node(snps_dw_hdmi_dt_ids, pdev->dev.of_node);
> +	plat_data = match->data;
> +
> +	hdmi = dw_hdmi_probe(pdev, plat_data);

So this is kinda not how bridge drivers are supposed to be done nowadays,
direct calling into the driver was the old way, and dw-hdmi still works
like that. Modern way is roughly
- bridge drivers bind automatically to any bridge they support
- bridge drivers publish a bridge with drm_bridge_add()
- the driver using the bridge fishes out with dt magic using
  of_drm_find_bridge() or another of the related of_ functions

I know a bit late, just spotted this because you brought your series here
up in my arc cleanup series, but can you pls look into adjusting
accordingly?

I shouldn't take more than moving this binding here into the dw-hdmi
driver, and switching arc itself over to the of_drm_find_bridge() call.
That way we could slowly work to transform old bridge drivers like dw-hdmi
to the new way, instead of adding more cases that will never get
converted.

Other upside is that arc stays a neat&tiny driver :-)

Thanks, Daniel

> +	if (IS_ERR(hdmi))
> +		return PTR_ERR(hdmi);
> +
> +	platform_set_drvdata(pdev, hdmi);
> +
> +	return 0;
> +}
> +
> +static int snps_dw_hdmi_remove(struct platform_device *pdev)
> +{
> +	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
> +
> +	dw_hdmi_remove(hdmi);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver snps_dw_hdmi_platform_driver = {
> +	.probe  = snps_dw_hdmi_probe,
> +	.remove = snps_dw_hdmi_remove,
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.of_match_table = snps_dw_hdmi_dt_ids,
> +	},
> +};
> +module_platform_driver(snps_dw_hdmi_platform_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("ARC specific DW-HDMI driver extension");
> +MODULE_AUTHOR("Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>");
> -- 
> 2.21.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v3 2/2] dt-bindings: Document the Synopsys ARC HDMI TX bindings
  2020-04-14 23:29   ` Eugeniy Paltsev
  (?)
@ 2020-04-20 21:24     ` Rob Herring
  -1 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2020-04-20 21:24 UTC (permalink / raw)
  To: Eugeniy Paltsev
  Cc: dri-devel, Alexey Brodkin, linux-snps-arc, linux-kernel,
	David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	devicetree, Sam Ravnborg

On Wed, Apr 15, 2020 at 02:29:29AM +0300, Eugeniy Paltsev wrote:
> This patch adds documentation of device tree bindings for the Synopsys
> HDMI 2.0 TX encoder driver for ARC SoCs.

You're going to need to base this on top of Laurent's conversion of 
dw_hdmi.txt to schema.

> 
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> ---
>  .../display/bridge/snps,arc-dw-hdmi.yaml      | 136 ++++++++++++++++++
>  1 file changed, 136 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml b/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> new file mode 100644
> index 000000000000..9b2fdfecd5b3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> @@ -0,0 +1,136 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings please:

(GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/snps,arc-dw-hdmi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DesignWare HDMI 2.0 TX encoder driver

Bindings are for h/w blocks, not drivers.

> +
> +maintainers:
> +  - Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> +
> +description: |
> +  The HDMI transmitter is a Synopsys DesignWare HDMI 2.0 TX controller IP
> +  with a companion of Synopsys DesignWare HDMI 2.0 TX PHY IP.

Sounds like 2 blocks?

> +
> +  These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
> +  Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
> +  with the following device-specific properties.
> +
> +properties:
> +  compatible:
> +    const: snps,arc-dw-hdmi-hsdk
> +
> +  reg:
> +    maxItems: 1
> +    description: |
> +      Memory mapped base address and length of the DWC HDMI TX registers.

Can drop.

> +
> +  clocks:
> +    items:
> +      - description: The bus clock for AHB / APB
> +      - description: The internal register configuration clock
> +
> +  clock-names:
> +    items:
> +      - const: iahb
> +      - const: isfr
> +
> +  interrupts:
> +    maxItems: 1
> +    description: Reference to the DWC HDMI TX interrupt

Can drop.

> +
> +  reg-io-width:
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +      - enum: [1, 4]
> +        description: |
> +          Width of the registers specified by the reg property. The
> +          value is expressed in bytes and must be equal to 1 or 4 if specified.
> +          The register width defaults to 1 if the property is not present.

default: 1

The description is pretty much a plain text version of the constraints, 
so all but the first sentence can be dropped.

> +
> +  ports:
> +    type: object
> +    description: |
> +      A ports node with endpoint definitions as defined in
> +      Documentation/devicetree/bindings/media/video-interfaces.txt

Can drop. That's all 'ports'.

> +
> +    properties:
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +      port@0:
> +        type: object
> +        description: |
> +          Video input endpoints of the controller.
> +          Usually it is associated with ARC PGU.
> +
> +      port@1:
> +        type: object
> +        description: |
> +          Output endpoints of the controller. HDMI connector.
> +
> +    required:
> +      - "#address-cells"
> +      - "#size-cells"
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    hdmi@10000 {
> +        compatible = "snps,arc-dw-hdmi-hsdk";
> +        reg = <0x10000 0x10000>;
> +        reg-io-width = <4>;
> +        interrupts = <14>;
> +        clocks = <&apbclk>, <&hdmi_pix_clk>;
> +        clock-names = "iahb", "isfr";
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +                hdmi_enc_input: endpoint {
> +                    remote-endpoint = <&pgu_output>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +                hdmi_enc_out: endpoint {
> +                    remote-endpoint = <&hdmi_con>;
> +                };
> +            };
> +        };
> +    };
> +
> +    hdmi-out {
> +        port {
> +            hdmi_con: endpoint {
> +                remote-endpoint = <&hdmi_enc_out>;
> +            };
> +        };
> +    };
> +
> +    pgu {
> +        port_o: port {
> +            pgu_output: endpoint {
> +                remote-endpoint = <&hdmi_enc_input>;
> +            };
> +        };
> +    };
> -- 
> 2.21.1
> 

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

* Re: [PATCH v3 2/2] dt-bindings: Document the Synopsys ARC HDMI TX bindings
@ 2020-04-20 21:24     ` Rob Herring
  0 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2020-04-20 21:24 UTC (permalink / raw)
  To: Eugeniy Paltsev
  Cc: devicetree, David Airlie, Alexey Brodkin, Maarten Lankhorst,
	linux-kernel, dri-devel, Maxime Ripard, Daniel Vetter,
	linux-snps-arc, Sam Ravnborg

On Wed, Apr 15, 2020 at 02:29:29AM +0300, Eugeniy Paltsev wrote:
> This patch adds documentation of device tree bindings for the Synopsys
> HDMI 2.0 TX encoder driver for ARC SoCs.

You're going to need to base this on top of Laurent's conversion of 
dw_hdmi.txt to schema.

> 
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> ---
>  .../display/bridge/snps,arc-dw-hdmi.yaml      | 136 ++++++++++++++++++
>  1 file changed, 136 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml b/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> new file mode 100644
> index 000000000000..9b2fdfecd5b3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> @@ -0,0 +1,136 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings please:

(GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/snps,arc-dw-hdmi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DesignWare HDMI 2.0 TX encoder driver

Bindings are for h/w blocks, not drivers.

> +
> +maintainers:
> +  - Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> +
> +description: |
> +  The HDMI transmitter is a Synopsys DesignWare HDMI 2.0 TX controller IP
> +  with a companion of Synopsys DesignWare HDMI 2.0 TX PHY IP.

Sounds like 2 blocks?

> +
> +  These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
> +  Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
> +  with the following device-specific properties.
> +
> +properties:
> +  compatible:
> +    const: snps,arc-dw-hdmi-hsdk
> +
> +  reg:
> +    maxItems: 1
> +    description: |
> +      Memory mapped base address and length of the DWC HDMI TX registers.

Can drop.

> +
> +  clocks:
> +    items:
> +      - description: The bus clock for AHB / APB
> +      - description: The internal register configuration clock
> +
> +  clock-names:
> +    items:
> +      - const: iahb
> +      - const: isfr
> +
> +  interrupts:
> +    maxItems: 1
> +    description: Reference to the DWC HDMI TX interrupt

Can drop.

> +
> +  reg-io-width:
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +      - enum: [1, 4]
> +        description: |
> +          Width of the registers specified by the reg property. The
> +          value is expressed in bytes and must be equal to 1 or 4 if specified.
> +          The register width defaults to 1 if the property is not present.

default: 1

The description is pretty much a plain text version of the constraints, 
so all but the first sentence can be dropped.

> +
> +  ports:
> +    type: object
> +    description: |
> +      A ports node with endpoint definitions as defined in
> +      Documentation/devicetree/bindings/media/video-interfaces.txt

Can drop. That's all 'ports'.

> +
> +    properties:
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +      port@0:
> +        type: object
> +        description: |
> +          Video input endpoints of the controller.
> +          Usually it is associated with ARC PGU.
> +
> +      port@1:
> +        type: object
> +        description: |
> +          Output endpoints of the controller. HDMI connector.
> +
> +    required:
> +      - "#address-cells"
> +      - "#size-cells"
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    hdmi@10000 {
> +        compatible = "snps,arc-dw-hdmi-hsdk";
> +        reg = <0x10000 0x10000>;
> +        reg-io-width = <4>;
> +        interrupts = <14>;
> +        clocks = <&apbclk>, <&hdmi_pix_clk>;
> +        clock-names = "iahb", "isfr";
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +                hdmi_enc_input: endpoint {
> +                    remote-endpoint = <&pgu_output>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +                hdmi_enc_out: endpoint {
> +                    remote-endpoint = <&hdmi_con>;
> +                };
> +            };
> +        };
> +    };
> +
> +    hdmi-out {
> +        port {
> +            hdmi_con: endpoint {
> +                remote-endpoint = <&hdmi_enc_out>;
> +            };
> +        };
> +    };
> +
> +    pgu {
> +        port_o: port {
> +            pgu_output: endpoint {
> +                remote-endpoint = <&hdmi_enc_input>;
> +            };
> +        };
> +    };
> -- 
> 2.21.1
> 

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

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

* Re: [PATCH v3 2/2] dt-bindings: Document the Synopsys ARC HDMI TX bindings
@ 2020-04-20 21:24     ` Rob Herring
  0 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2020-04-20 21:24 UTC (permalink / raw)
  To: Eugeniy Paltsev
  Cc: devicetree, David Airlie, Alexey Brodkin, linux-kernel,
	dri-devel, linux-snps-arc, Sam Ravnborg

On Wed, Apr 15, 2020 at 02:29:29AM +0300, Eugeniy Paltsev wrote:
> This patch adds documentation of device tree bindings for the Synopsys
> HDMI 2.0 TX encoder driver for ARC SoCs.

You're going to need to base this on top of Laurent's conversion of 
dw_hdmi.txt to schema.

> 
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> ---
>  .../display/bridge/snps,arc-dw-hdmi.yaml      | 136 ++++++++++++++++++
>  1 file changed, 136 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml b/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> new file mode 100644
> index 000000000000..9b2fdfecd5b3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> @@ -0,0 +1,136 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings please:

(GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/snps,arc-dw-hdmi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DesignWare HDMI 2.0 TX encoder driver

Bindings are for h/w blocks, not drivers.

> +
> +maintainers:
> +  - Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> +
> +description: |
> +  The HDMI transmitter is a Synopsys DesignWare HDMI 2.0 TX controller IP
> +  with a companion of Synopsys DesignWare HDMI 2.0 TX PHY IP.

Sounds like 2 blocks?

> +
> +  These DT bindings follow the Synopsys DWC HDMI TX bindings defined in
> +  Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt
> +  with the following device-specific properties.
> +
> +properties:
> +  compatible:
> +    const: snps,arc-dw-hdmi-hsdk
> +
> +  reg:
> +    maxItems: 1
> +    description: |
> +      Memory mapped base address and length of the DWC HDMI TX registers.

Can drop.

> +
> +  clocks:
> +    items:
> +      - description: The bus clock for AHB / APB
> +      - description: The internal register configuration clock
> +
> +  clock-names:
> +    items:
> +      - const: iahb
> +      - const: isfr
> +
> +  interrupts:
> +    maxItems: 1
> +    description: Reference to the DWC HDMI TX interrupt

Can drop.

> +
> +  reg-io-width:
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +      - enum: [1, 4]
> +        description: |
> +          Width of the registers specified by the reg property. The
> +          value is expressed in bytes and must be equal to 1 or 4 if specified.
> +          The register width defaults to 1 if the property is not present.

default: 1

The description is pretty much a plain text version of the constraints, 
so all but the first sentence can be dropped.

> +
> +  ports:
> +    type: object
> +    description: |
> +      A ports node with endpoint definitions as defined in
> +      Documentation/devicetree/bindings/media/video-interfaces.txt

Can drop. That's all 'ports'.

> +
> +    properties:
> +      "#address-cells":
> +        const: 1
> +
> +      "#size-cells":
> +        const: 0
> +
> +      port@0:
> +        type: object
> +        description: |
> +          Video input endpoints of the controller.
> +          Usually it is associated with ARC PGU.
> +
> +      port@1:
> +        type: object
> +        description: |
> +          Output endpoints of the controller. HDMI connector.
> +
> +    required:
> +      - "#address-cells"
> +      - "#size-cells"
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    hdmi@10000 {
> +        compatible = "snps,arc-dw-hdmi-hsdk";
> +        reg = <0x10000 0x10000>;
> +        reg-io-width = <4>;
> +        interrupts = <14>;
> +        clocks = <&apbclk>, <&hdmi_pix_clk>;
> +        clock-names = "iahb", "isfr";
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +                hdmi_enc_input: endpoint {
> +                    remote-endpoint = <&pgu_output>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +                hdmi_enc_out: endpoint {
> +                    remote-endpoint = <&hdmi_con>;
> +                };
> +            };
> +        };
> +    };
> +
> +    hdmi-out {
> +        port {
> +            hdmi_con: endpoint {
> +                remote-endpoint = <&hdmi_enc_out>;
> +            };
> +        };
> +    };
> +
> +    pgu {
> +        port_o: port {
> +            pgu_output: endpoint {
> +                remote-endpoint = <&hdmi_enc_input>;
> +            };
> +        };
> +    };
> -- 
> 2.21.1
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v3 1/2] DRM: ARC: add HDMI 2.0 TX encoder support
  2020-04-15 17:33     ` Daniel Vetter
  (?)
@ 2020-04-21 15:55       ` Neil Armstrong
  -1 siblings, 0 replies; 21+ messages in thread
From: Neil Armstrong @ 2020-04-21 15:55 UTC (permalink / raw)
  To: Eugeniy Paltsev, dri-devel, Alexey Brodkin, linux-snps-arc,
	linux-kernel, David Airlie, Maarten Lankhorst, Maxime Ripard,
	devicetree, Rob Herring, Sam Ravnborg

On 15/04/2020 19:33, Daniel Vetter wrote:
> On Wed, Apr 15, 2020 at 02:29:28AM +0300, Eugeniy Paltsev wrote:
>> The Synopsys ARC SoCs (like HSDK4xD) include on-chip DesignWare HDMI
>> encoders. Support them with a platform driver to provide platform glue
>> data to the dw-hdmi driver.
>>
>> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
>> ---
>>  MAINTAINERS                       |   6 ++
>>  drivers/gpu/drm/Makefile          |   2 +-
>>  drivers/gpu/drm/arc/Kconfig       |   7 ++
>>  drivers/gpu/drm/arc/Makefile      |   1 +
>>  drivers/gpu/drm/arc/arc-dw-hdmi.c | 116 ++++++++++++++++++++++++++++++
>>  5 files changed, 131 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a6fbdf354d34..2aaed1190370 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1258,6 +1258,12 @@ S:	Supported
>>  F:	drivers/gpu/drm/arc/
>>  F:	Documentation/devicetree/bindings/display/snps,arcpgu.txt
>>  
>> +ARC DW HDMI DRIVER
>> +M:	Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
>> +S:	Supported
>> +F:	drivers/gpu/drm/arc/arc-dw-hdmi.c
>> +F:	Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
>> +
>>  ARCNET NETWORK LAYER
>>  M:	Michael Grzeschik <m.grzeschik@pengutronix.de>
>>  L:	netdev@vger.kernel.org
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 6493088a0fdd..5b0bcf7f45cd 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -109,7 +109,7 @@ obj-y			+= panel/
>>  obj-y			+= bridge/
>>  obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
>>  obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
>> -obj-$(CONFIG_DRM_ARCPGU)+= arc/
>> +obj-y			+= arc/
>>  obj-y			+= hisilicon/
>>  obj-$(CONFIG_DRM_ZTE)	+= zte/
>>  obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
>> diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig
>> index e8f3d63e0b91..baec9d2a4fba 100644
>> --- a/drivers/gpu/drm/arc/Kconfig
>> +++ b/drivers/gpu/drm/arc/Kconfig
>> @@ -8,3 +8,10 @@ config DRM_ARCPGU
>>  	  Choose this option if you have an ARC PGU controller.
>>  
>>  	  If M is selected the module will be called arcpgu.
>> +
>> +config DRM_ARC_DW_HDMI
>> +	tristate "ARC DW HDMI"
>> +	depends on DRM && OF
>> +	select DRM_DW_HDMI
>> +	help
>> +	  Synopsys DW HDMI driver for various ARC development boards
>> diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
>> index c7028b7427b3..7a156d8c2c3c 100644
>> --- a/drivers/gpu/drm/arc/Makefile
>> +++ b/drivers/gpu/drm/arc/Makefile
>> @@ -1,3 +1,4 @@
>>  # SPDX-License-Identifier: GPL-2.0-only
>>  arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
>>  obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
>> +obj-$(CONFIG_DRM_ARC_DW_HDMI) += arc-dw-hdmi.o
>> diff --git a/drivers/gpu/drm/arc/arc-dw-hdmi.c b/drivers/gpu/drm/arc/arc-dw-hdmi.c
>> new file mode 100644
>> index 000000000000..46a6ee09b302
>> --- /dev/null
>> +++ b/drivers/gpu/drm/arc/arc-dw-hdmi.c
>> @@ -0,0 +1,116 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +//
>> +// Synopsys DW HDMI driver for various ARC development boards
>> +//
>> +// Copyright (C) 2020 Synopsys
>> +// Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
>> +
>> +#include <linux/component.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <drm/bridge/dw_hdmi.h>
>> +#include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_edid.h>
>> +#include <drm/drm_encoder_slave.h>
>> +#include <drm/drm_of.h>
>> +
>> +static const struct dw_hdmi_mpll_config snps_hdmi_mpll_cfg[] = {
>> +	{
>> +		27000000, {
>> +			{ 0x00B3, 0x0000 },
>> +			{ 0x00B3, 0x0000 },
>> +			{ 0x00B3, 0x0000 }
>> +		},
>> +	}, {
>> +		74250000, {
>> +			{ 0x0072, 0x0001},
>> +			{ 0x0072, 0x0001},
>> +			{ 0x0072, 0x0001}
>> +		},
>> +	}, {
>> +		148500000, {
>> +			{ 0x0051, 0x0002},
>> +			{ 0x0051, 0x0002},
>> +			{ 0x0051, 0x0002}
>> +		},
>> +	}, {
>> +		~0UL, {
>> +			{ 0x00B3, 0x0000 },
>> +			{ 0x00B3, 0x0000 },
>> +			{ 0x00B3, 0x0000 },
>> +		},
>> +	}
>> +};
>> +
>> +static const struct dw_hdmi_curr_ctrl snps_hdmi_cur_ctr[] = {
>> +	/* pixelclk    bpp8    bpp10   bpp12 */
>> +	{ 27000000,  { 0x0000, 0x0000, 0x0000 }, },
>> +	{ 74250000,  { 0x0008, 0x0008, 0x0008 }, },
>> +	{ 148500000, { 0x001b, 0x001b, 0x001b }, },
>> +	{ ~0UL,      { 0x0000, 0x0000, 0x0000 }, }
>> +};
>> +
>> +
>> +static const struct dw_hdmi_phy_config snps_hdmi_phy_config[] = {
>> +	/* pixelclk   symbol  term    vlev */
>> +	{ 27000000,   0x8009, 0x0004, 0x0232},
>> +	{ 74250000,   0x8009, 0x0004, 0x0232},
>> +	{ 148500000,  0x8009, 0x0004, 0x0232},
>> +	{ ~0UL,       0x8009, 0x0004, 0x0232}
>> +};
>> +
>> +static struct dw_hdmi_plat_data snps_dw_hdmi_drv_data = {
>> +	.mpll_cfg   = snps_hdmi_mpll_cfg,
>> +	.cur_ctr    = snps_hdmi_cur_ctr,
>> +	.phy_config = snps_hdmi_phy_config,
>> +};
>> +
>> +static const struct of_device_id snps_dw_hdmi_dt_ids[] = {
>> +	{ .compatible = "snps,arc-dw-hdmi-hsdk", .data = &snps_dw_hdmi_drv_data },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, snps_dw_hdmi_dt_ids);
>> +
>> +static int snps_dw_hdmi_probe(struct platform_device *pdev)
>> +{
>> +	const struct dw_hdmi_plat_data *plat_data;
>> +	const struct of_device_id *match;
>> +	struct dw_hdmi *hdmi;
>> +
>> +	if (!pdev->dev.of_node)
>> +		return -ENODEV;
>> +
>> +	match = of_match_node(snps_dw_hdmi_dt_ids, pdev->dev.of_node);
>> +	plat_data = match->data;
>> +
>> +	hdmi = dw_hdmi_probe(pdev, plat_data);
> 
> So this is kinda not how bridge drivers are supposed to be done nowadays,
> direct calling into the driver was the old way, and dw-hdmi still works
> like that. Modern way is roughly
> - bridge drivers bind automatically to any bridge they support
> - bridge drivers publish a bridge with drm_bridge_add()
> - the driver using the bridge fishes out with dt magic using
>   of_drm_find_bridge() or another of the related of_ functions

dw-hdmi is an IP, with some platform specific code and arrays to make it work
on very different systems, thus we can't use this scheme everywhere....

Some platforms (like r-car) uses the "right" model because the IP is integrated
as-is with the default PHY and as an independent IP on the system.

It's definitely not the case on Rockchip/Amlogic/Allwinner systems,
and even worse on Amlogic system having a glue on top of the IP, and a
custom PHY instead of the Synopsys PHY.

Thus it would be great this would be the case on a Synopsys SoC... but like
other platforms they have platform specific parameters.

All this has been discussed and reviewed a few years ago, I would
personally prefer "fishing out a bridge using dt magic" instead having
1k glue code around the IP.

Neil

> 
> I know a bit late, just spotted this because you brought your series here
> up in my arc cleanup series, but can you pls look into adjusting
> accordingly?
> 
> I shouldn't take more than moving this binding here into the dw-hdmi
> driver, and switching arc itself over to the of_drm_find_bridge() call.
> That way we could slowly work to transform old bridge drivers like dw-hdmi
> to the new way, instead of adding more cases that will never get
> converted.
> 
> Other upside is that arc stays a neat&tiny driver :-)
> 
> Thanks, Daniel
> 
>> +	if (IS_ERR(hdmi))
>> +		return PTR_ERR(hdmi);
>> +
>> +	platform_set_drvdata(pdev, hdmi);
>> +
>> +	return 0;
>> +}
>> +
>> +static int snps_dw_hdmi_remove(struct platform_device *pdev)
>> +{
>> +	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
>> +
>> +	dw_hdmi_remove(hdmi);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver snps_dw_hdmi_platform_driver = {
>> +	.probe  = snps_dw_hdmi_probe,
>> +	.remove = snps_dw_hdmi_remove,
>> +	.driver = {
>> +		.name = KBUILD_MODNAME,
>> +		.of_match_table = snps_dw_hdmi_dt_ids,
>> +	},
>> +};
>> +module_platform_driver(snps_dw_hdmi_platform_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("ARC specific DW-HDMI driver extension");
>> +MODULE_AUTHOR("Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>");
>> -- 
>> 2.21.1
>>
> 


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

* Re: [PATCH v3 1/2] DRM: ARC: add HDMI 2.0 TX encoder support
@ 2020-04-21 15:55       ` Neil Armstrong
  0 siblings, 0 replies; 21+ messages in thread
From: Neil Armstrong @ 2020-04-21 15:55 UTC (permalink / raw)
  To: Eugeniy Paltsev, dri-devel, Alexey Brodkin, linux-snps-arc,
	linux-kernel, David Airlie, Maarten Lankhorst, Maxime Ripard,
	devicetree, Rob Herring, Sam Ravnborg

On 15/04/2020 19:33, Daniel Vetter wrote:
> On Wed, Apr 15, 2020 at 02:29:28AM +0300, Eugeniy Paltsev wrote:
>> The Synopsys ARC SoCs (like HSDK4xD) include on-chip DesignWare HDMI
>> encoders. Support them with a platform driver to provide platform glue
>> data to the dw-hdmi driver.
>>
>> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
>> ---
>>  MAINTAINERS                       |   6 ++
>>  drivers/gpu/drm/Makefile          |   2 +-
>>  drivers/gpu/drm/arc/Kconfig       |   7 ++
>>  drivers/gpu/drm/arc/Makefile      |   1 +
>>  drivers/gpu/drm/arc/arc-dw-hdmi.c | 116 ++++++++++++++++++++++++++++++
>>  5 files changed, 131 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a6fbdf354d34..2aaed1190370 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1258,6 +1258,12 @@ S:	Supported
>>  F:	drivers/gpu/drm/arc/
>>  F:	Documentation/devicetree/bindings/display/snps,arcpgu.txt
>>  
>> +ARC DW HDMI DRIVER
>> +M:	Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
>> +S:	Supported
>> +F:	drivers/gpu/drm/arc/arc-dw-hdmi.c
>> +F:	Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
>> +
>>  ARCNET NETWORK LAYER
>>  M:	Michael Grzeschik <m.grzeschik@pengutronix.de>
>>  L:	netdev@vger.kernel.org
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 6493088a0fdd..5b0bcf7f45cd 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -109,7 +109,7 @@ obj-y			+= panel/
>>  obj-y			+= bridge/
>>  obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
>>  obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
>> -obj-$(CONFIG_DRM_ARCPGU)+= arc/
>> +obj-y			+= arc/
>>  obj-y			+= hisilicon/
>>  obj-$(CONFIG_DRM_ZTE)	+= zte/
>>  obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
>> diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig
>> index e8f3d63e0b91..baec9d2a4fba 100644
>> --- a/drivers/gpu/drm/arc/Kconfig
>> +++ b/drivers/gpu/drm/arc/Kconfig
>> @@ -8,3 +8,10 @@ config DRM_ARCPGU
>>  	  Choose this option if you have an ARC PGU controller.
>>  
>>  	  If M is selected the module will be called arcpgu.
>> +
>> +config DRM_ARC_DW_HDMI
>> +	tristate "ARC DW HDMI"
>> +	depends on DRM && OF
>> +	select DRM_DW_HDMI
>> +	help
>> +	  Synopsys DW HDMI driver for various ARC development boards
>> diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
>> index c7028b7427b3..7a156d8c2c3c 100644
>> --- a/drivers/gpu/drm/arc/Makefile
>> +++ b/drivers/gpu/drm/arc/Makefile
>> @@ -1,3 +1,4 @@
>>  # SPDX-License-Identifier: GPL-2.0-only
>>  arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
>>  obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
>> +obj-$(CONFIG_DRM_ARC_DW_HDMI) += arc-dw-hdmi.o
>> diff --git a/drivers/gpu/drm/arc/arc-dw-hdmi.c b/drivers/gpu/drm/arc/arc-dw-hdmi.c
>> new file mode 100644
>> index 000000000000..46a6ee09b302
>> --- /dev/null
>> +++ b/drivers/gpu/drm/arc/arc-dw-hdmi.c
>> @@ -0,0 +1,116 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +//
>> +// Synopsys DW HDMI driver for various ARC development boards
>> +//
>> +// Copyright (C) 2020 Synopsys
>> +// Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
>> +
>> +#include <linux/component.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <drm/bridge/dw_hdmi.h>
>> +#include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_edid.h>
>> +#include <drm/drm_encoder_slave.h>
>> +#include <drm/drm_of.h>
>> +
>> +static const struct dw_hdmi_mpll_config snps_hdmi_mpll_cfg[] = {
>> +	{
>> +		27000000, {
>> +			{ 0x00B3, 0x0000 },
>> +			{ 0x00B3, 0x0000 },
>> +			{ 0x00B3, 0x0000 }
>> +		},
>> +	}, {
>> +		74250000, {
>> +			{ 0x0072, 0x0001},
>> +			{ 0x0072, 0x0001},
>> +			{ 0x0072, 0x0001}
>> +		},
>> +	}, {
>> +		148500000, {
>> +			{ 0x0051, 0x0002},
>> +			{ 0x0051, 0x0002},
>> +			{ 0x0051, 0x0002}
>> +		},
>> +	}, {
>> +		~0UL, {
>> +			{ 0x00B3, 0x0000 },
>> +			{ 0x00B3, 0x0000 },
>> +			{ 0x00B3, 0x0000 },
>> +		},
>> +	}
>> +};
>> +
>> +static const struct dw_hdmi_curr_ctrl snps_hdmi_cur_ctr[] = {
>> +	/* pixelclk    bpp8    bpp10   bpp12 */
>> +	{ 27000000,  { 0x0000, 0x0000, 0x0000 }, },
>> +	{ 74250000,  { 0x0008, 0x0008, 0x0008 }, },
>> +	{ 148500000, { 0x001b, 0x001b, 0x001b }, },
>> +	{ ~0UL,      { 0x0000, 0x0000, 0x0000 }, }
>> +};
>> +
>> +
>> +static const struct dw_hdmi_phy_config snps_hdmi_phy_config[] = {
>> +	/* pixelclk   symbol  term    vlev */
>> +	{ 27000000,   0x8009, 0x0004, 0x0232},
>> +	{ 74250000,   0x8009, 0x0004, 0x0232},
>> +	{ 148500000,  0x8009, 0x0004, 0x0232},
>> +	{ ~0UL,       0x8009, 0x0004, 0x0232}
>> +};
>> +
>> +static struct dw_hdmi_plat_data snps_dw_hdmi_drv_data = {
>> +	.mpll_cfg   = snps_hdmi_mpll_cfg,
>> +	.cur_ctr    = snps_hdmi_cur_ctr,
>> +	.phy_config = snps_hdmi_phy_config,
>> +};
>> +
>> +static const struct of_device_id snps_dw_hdmi_dt_ids[] = {
>> +	{ .compatible = "snps,arc-dw-hdmi-hsdk", .data = &snps_dw_hdmi_drv_data },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, snps_dw_hdmi_dt_ids);
>> +
>> +static int snps_dw_hdmi_probe(struct platform_device *pdev)
>> +{
>> +	const struct dw_hdmi_plat_data *plat_data;
>> +	const struct of_device_id *match;
>> +	struct dw_hdmi *hdmi;
>> +
>> +	if (!pdev->dev.of_node)
>> +		return -ENODEV;
>> +
>> +	match = of_match_node(snps_dw_hdmi_dt_ids, pdev->dev.of_node);
>> +	plat_data = match->data;
>> +
>> +	hdmi = dw_hdmi_probe(pdev, plat_data);
> 
> So this is kinda not how bridge drivers are supposed to be done nowadays,
> direct calling into the driver was the old way, and dw-hdmi still works
> like that. Modern way is roughly
> - bridge drivers bind automatically to any bridge they support
> - bridge drivers publish a bridge with drm_bridge_add()
> - the driver using the bridge fishes out with dt magic using
>   of_drm_find_bridge() or another of the related of_ functions

dw-hdmi is an IP, with some platform specific code and arrays to make it work
on very different systems, thus we can't use this scheme everywhere....

Some platforms (like r-car) uses the "right" model because the IP is integrated
as-is with the default PHY and as an independent IP on the system.

It's definitely not the case on Rockchip/Amlogic/Allwinner systems,
and even worse on Amlogic system having a glue on top of the IP, and a
custom PHY instead of the Synopsys PHY.

Thus it would be great this would be the case on a Synopsys SoC... but like
other platforms they have platform specific parameters.

All this has been discussed and reviewed a few years ago, I would
personally prefer "fishing out a bridge using dt magic" instead having
1k glue code around the IP.

Neil

> 
> I know a bit late, just spotted this because you brought your series here
> up in my arc cleanup series, but can you pls look into adjusting
> accordingly?
> 
> I shouldn't take more than moving this binding here into the dw-hdmi
> driver, and switching arc itself over to the of_drm_find_bridge() call.
> That way we could slowly work to transform old bridge drivers like dw-hdmi
> to the new way, instead of adding more cases that will never get
> converted.
> 
> Other upside is that arc stays a neat&tiny driver :-)
> 
> Thanks, Daniel
> 
>> +	if (IS_ERR(hdmi))
>> +		return PTR_ERR(hdmi);
>> +
>> +	platform_set_drvdata(pdev, hdmi);
>> +
>> +	return 0;
>> +}
>> +
>> +static int snps_dw_hdmi_remove(struct platform_device *pdev)
>> +{
>> +	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
>> +
>> +	dw_hdmi_remove(hdmi);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver snps_dw_hdmi_platform_driver = {
>> +	.probe  = snps_dw_hdmi_probe,
>> +	.remove = snps_dw_hdmi_remove,
>> +	.driver = {
>> +		.name = KBUILD_MODNAME,
>> +		.of_match_table = snps_dw_hdmi_dt_ids,
>> +	},
>> +};
>> +module_platform_driver(snps_dw_hdmi_platform_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("ARC specific DW-HDMI driver extension");
>> +MODULE_AUTHOR("Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>");
>> -- 
>> 2.21.1
>>
> 


_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

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

* Re: [PATCH v3 1/2] DRM: ARC: add HDMI 2.0 TX encoder support
@ 2020-04-21 15:55       ` Neil Armstrong
  0 siblings, 0 replies; 21+ messages in thread
From: Neil Armstrong @ 2020-04-21 15:55 UTC (permalink / raw)
  To: Eugeniy Paltsev, dri-devel, Alexey Brodkin, linux-snps-arc,
	linux-kernel, David Airlie, Maarten Lankhorst, Maxime Ripard,
	devicetree, Rob Herring, Sam Ravnborg

On 15/04/2020 19:33, Daniel Vetter wrote:
> On Wed, Apr 15, 2020 at 02:29:28AM +0300, Eugeniy Paltsev wrote:
>> The Synopsys ARC SoCs (like HSDK4xD) include on-chip DesignWare HDMI
>> encoders. Support them with a platform driver to provide platform glue
>> data to the dw-hdmi driver.
>>
>> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
>> ---
>>  MAINTAINERS                       |   6 ++
>>  drivers/gpu/drm/Makefile          |   2 +-
>>  drivers/gpu/drm/arc/Kconfig       |   7 ++
>>  drivers/gpu/drm/arc/Makefile      |   1 +
>>  drivers/gpu/drm/arc/arc-dw-hdmi.c | 116 ++++++++++++++++++++++++++++++
>>  5 files changed, 131 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a6fbdf354d34..2aaed1190370 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1258,6 +1258,12 @@ S:	Supported
>>  F:	drivers/gpu/drm/arc/
>>  F:	Documentation/devicetree/bindings/display/snps,arcpgu.txt
>>  
>> +ARC DW HDMI DRIVER
>> +M:	Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
>> +S:	Supported
>> +F:	drivers/gpu/drm/arc/arc-dw-hdmi.c
>> +F:	Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
>> +
>>  ARCNET NETWORK LAYER
>>  M:	Michael Grzeschik <m.grzeschik@pengutronix.de>
>>  L:	netdev@vger.kernel.org
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 6493088a0fdd..5b0bcf7f45cd 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -109,7 +109,7 @@ obj-y			+= panel/
>>  obj-y			+= bridge/
>>  obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
>>  obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
>> -obj-$(CONFIG_DRM_ARCPGU)+= arc/
>> +obj-y			+= arc/
>>  obj-y			+= hisilicon/
>>  obj-$(CONFIG_DRM_ZTE)	+= zte/
>>  obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
>> diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig
>> index e8f3d63e0b91..baec9d2a4fba 100644
>> --- a/drivers/gpu/drm/arc/Kconfig
>> +++ b/drivers/gpu/drm/arc/Kconfig
>> @@ -8,3 +8,10 @@ config DRM_ARCPGU
>>  	  Choose this option if you have an ARC PGU controller.
>>  
>>  	  If M is selected the module will be called arcpgu.
>> +
>> +config DRM_ARC_DW_HDMI
>> +	tristate "ARC DW HDMI"
>> +	depends on DRM && OF
>> +	select DRM_DW_HDMI
>> +	help
>> +	  Synopsys DW HDMI driver for various ARC development boards
>> diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
>> index c7028b7427b3..7a156d8c2c3c 100644
>> --- a/drivers/gpu/drm/arc/Makefile
>> +++ b/drivers/gpu/drm/arc/Makefile
>> @@ -1,3 +1,4 @@
>>  # SPDX-License-Identifier: GPL-2.0-only
>>  arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
>>  obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
>> +obj-$(CONFIG_DRM_ARC_DW_HDMI) += arc-dw-hdmi.o
>> diff --git a/drivers/gpu/drm/arc/arc-dw-hdmi.c b/drivers/gpu/drm/arc/arc-dw-hdmi.c
>> new file mode 100644
>> index 000000000000..46a6ee09b302
>> --- /dev/null
>> +++ b/drivers/gpu/drm/arc/arc-dw-hdmi.c
>> @@ -0,0 +1,116 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +//
>> +// Synopsys DW HDMI driver for various ARC development boards
>> +//
>> +// Copyright (C) 2020 Synopsys
>> +// Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
>> +
>> +#include <linux/component.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <drm/bridge/dw_hdmi.h>
>> +#include <drm/drm_crtc_helper.h>
>> +#include <drm/drm_edid.h>
>> +#include <drm/drm_encoder_slave.h>
>> +#include <drm/drm_of.h>
>> +
>> +static const struct dw_hdmi_mpll_config snps_hdmi_mpll_cfg[] = {
>> +	{
>> +		27000000, {
>> +			{ 0x00B3, 0x0000 },
>> +			{ 0x00B3, 0x0000 },
>> +			{ 0x00B3, 0x0000 }
>> +		},
>> +	}, {
>> +		74250000, {
>> +			{ 0x0072, 0x0001},
>> +			{ 0x0072, 0x0001},
>> +			{ 0x0072, 0x0001}
>> +		},
>> +	}, {
>> +		148500000, {
>> +			{ 0x0051, 0x0002},
>> +			{ 0x0051, 0x0002},
>> +			{ 0x0051, 0x0002}
>> +		},
>> +	}, {
>> +		~0UL, {
>> +			{ 0x00B3, 0x0000 },
>> +			{ 0x00B3, 0x0000 },
>> +			{ 0x00B3, 0x0000 },
>> +		},
>> +	}
>> +};
>> +
>> +static const struct dw_hdmi_curr_ctrl snps_hdmi_cur_ctr[] = {
>> +	/* pixelclk    bpp8    bpp10   bpp12 */
>> +	{ 27000000,  { 0x0000, 0x0000, 0x0000 }, },
>> +	{ 74250000,  { 0x0008, 0x0008, 0x0008 }, },
>> +	{ 148500000, { 0x001b, 0x001b, 0x001b }, },
>> +	{ ~0UL,      { 0x0000, 0x0000, 0x0000 }, }
>> +};
>> +
>> +
>> +static const struct dw_hdmi_phy_config snps_hdmi_phy_config[] = {
>> +	/* pixelclk   symbol  term    vlev */
>> +	{ 27000000,   0x8009, 0x0004, 0x0232},
>> +	{ 74250000,   0x8009, 0x0004, 0x0232},
>> +	{ 148500000,  0x8009, 0x0004, 0x0232},
>> +	{ ~0UL,       0x8009, 0x0004, 0x0232}
>> +};
>> +
>> +static struct dw_hdmi_plat_data snps_dw_hdmi_drv_data = {
>> +	.mpll_cfg   = snps_hdmi_mpll_cfg,
>> +	.cur_ctr    = snps_hdmi_cur_ctr,
>> +	.phy_config = snps_hdmi_phy_config,
>> +};
>> +
>> +static const struct of_device_id snps_dw_hdmi_dt_ids[] = {
>> +	{ .compatible = "snps,arc-dw-hdmi-hsdk", .data = &snps_dw_hdmi_drv_data },
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, snps_dw_hdmi_dt_ids);
>> +
>> +static int snps_dw_hdmi_probe(struct platform_device *pdev)
>> +{
>> +	const struct dw_hdmi_plat_data *plat_data;
>> +	const struct of_device_id *match;
>> +	struct dw_hdmi *hdmi;
>> +
>> +	if (!pdev->dev.of_node)
>> +		return -ENODEV;
>> +
>> +	match = of_match_node(snps_dw_hdmi_dt_ids, pdev->dev.of_node);
>> +	plat_data = match->data;
>> +
>> +	hdmi = dw_hdmi_probe(pdev, plat_data);
> 
> So this is kinda not how bridge drivers are supposed to be done nowadays,
> direct calling into the driver was the old way, and dw-hdmi still works
> like that. Modern way is roughly
> - bridge drivers bind automatically to any bridge they support
> - bridge drivers publish a bridge with drm_bridge_add()
> - the driver using the bridge fishes out with dt magic using
>   of_drm_find_bridge() or another of the related of_ functions

dw-hdmi is an IP, with some platform specific code and arrays to make it work
on very different systems, thus we can't use this scheme everywhere....

Some platforms (like r-car) uses the "right" model because the IP is integrated
as-is with the default PHY and as an independent IP on the system.

It's definitely not the case on Rockchip/Amlogic/Allwinner systems,
and even worse on Amlogic system having a glue on top of the IP, and a
custom PHY instead of the Synopsys PHY.

Thus it would be great this would be the case on a Synopsys SoC... but like
other platforms they have platform specific parameters.

All this has been discussed and reviewed a few years ago, I would
personally prefer "fishing out a bridge using dt magic" instead having
1k glue code around the IP.

Neil

> 
> I know a bit late, just spotted this because you brought your series here
> up in my arc cleanup series, but can you pls look into adjusting
> accordingly?
> 
> I shouldn't take more than moving this binding here into the dw-hdmi
> driver, and switching arc itself over to the of_drm_find_bridge() call.
> That way we could slowly work to transform old bridge drivers like dw-hdmi
> to the new way, instead of adding more cases that will never get
> converted.
> 
> Other upside is that arc stays a neat&tiny driver :-)
> 
> Thanks, Daniel
> 
>> +	if (IS_ERR(hdmi))
>> +		return PTR_ERR(hdmi);
>> +
>> +	platform_set_drvdata(pdev, hdmi);
>> +
>> +	return 0;
>> +}
>> +
>> +static int snps_dw_hdmi_remove(struct platform_device *pdev)
>> +{
>> +	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
>> +
>> +	dw_hdmi_remove(hdmi);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver snps_dw_hdmi_platform_driver = {
>> +	.probe  = snps_dw_hdmi_probe,
>> +	.remove = snps_dw_hdmi_remove,
>> +	.driver = {
>> +		.name = KBUILD_MODNAME,
>> +		.of_match_table = snps_dw_hdmi_dt_ids,
>> +	},
>> +};
>> +module_platform_driver(snps_dw_hdmi_platform_driver);
>> +
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("ARC specific DW-HDMI driver extension");
>> +MODULE_AUTHOR("Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>");
>> -- 
>> 2.21.1
>>
> 

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

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

* Re: [PATCH v3 1/2] DRM: ARC: add HDMI 2.0 TX encoder support
  2020-04-21 15:55       ` Neil Armstrong
  (?)
@ 2020-04-28 10:19         ` Daniel Vetter
  -1 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2020-04-28 10:19 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Eugeniy Paltsev, dri-devel, Alexey Brodkin, linux-snps-arc,
	linux-kernel, David Airlie, Maarten Lankhorst, Maxime Ripard,
	devicetree, Rob Herring, Sam Ravnborg

On Tue, Apr 21, 2020 at 05:55:38PM +0200, Neil Armstrong wrote:
> On 15/04/2020 19:33, Daniel Vetter wrote:
> > On Wed, Apr 15, 2020 at 02:29:28AM +0300, Eugeniy Paltsev wrote:
> >> The Synopsys ARC SoCs (like HSDK4xD) include on-chip DesignWare HDMI
> >> encoders. Support them with a platform driver to provide platform glue
> >> data to the dw-hdmi driver.
> >>
> >> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> >> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> >> ---
> >>  MAINTAINERS                       |   6 ++
> >>  drivers/gpu/drm/Makefile          |   2 +-
> >>  drivers/gpu/drm/arc/Kconfig       |   7 ++
> >>  drivers/gpu/drm/arc/Makefile      |   1 +
> >>  drivers/gpu/drm/arc/arc-dw-hdmi.c | 116 ++++++++++++++++++++++++++++++
> >>  5 files changed, 131 insertions(+), 1 deletion(-)
> >>  create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index a6fbdf354d34..2aaed1190370 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -1258,6 +1258,12 @@ S:	Supported
> >>  F:	drivers/gpu/drm/arc/
> >>  F:	Documentation/devicetree/bindings/display/snps,arcpgu.txt
> >>  
> >> +ARC DW HDMI DRIVER
> >> +M:	Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> >> +S:	Supported
> >> +F:	drivers/gpu/drm/arc/arc-dw-hdmi.c
> >> +F:	Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> >> +
> >>  ARCNET NETWORK LAYER
> >>  M:	Michael Grzeschik <m.grzeschik@pengutronix.de>
> >>  L:	netdev@vger.kernel.org
> >> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> >> index 6493088a0fdd..5b0bcf7f45cd 100644
> >> --- a/drivers/gpu/drm/Makefile
> >> +++ b/drivers/gpu/drm/Makefile
> >> @@ -109,7 +109,7 @@ obj-y			+= panel/
> >>  obj-y			+= bridge/
> >>  obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
> >>  obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
> >> -obj-$(CONFIG_DRM_ARCPGU)+= arc/
> >> +obj-y			+= arc/
> >>  obj-y			+= hisilicon/
> >>  obj-$(CONFIG_DRM_ZTE)	+= zte/
> >>  obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
> >> diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig
> >> index e8f3d63e0b91..baec9d2a4fba 100644
> >> --- a/drivers/gpu/drm/arc/Kconfig
> >> +++ b/drivers/gpu/drm/arc/Kconfig
> >> @@ -8,3 +8,10 @@ config DRM_ARCPGU
> >>  	  Choose this option if you have an ARC PGU controller.
> >>  
> >>  	  If M is selected the module will be called arcpgu.
> >> +
> >> +config DRM_ARC_DW_HDMI
> >> +	tristate "ARC DW HDMI"
> >> +	depends on DRM && OF
> >> +	select DRM_DW_HDMI
> >> +	help
> >> +	  Synopsys DW HDMI driver for various ARC development boards
> >> diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
> >> index c7028b7427b3..7a156d8c2c3c 100644
> >> --- a/drivers/gpu/drm/arc/Makefile
> >> +++ b/drivers/gpu/drm/arc/Makefile
> >> @@ -1,3 +1,4 @@
> >>  # SPDX-License-Identifier: GPL-2.0-only
> >>  arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
> >>  obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
> >> +obj-$(CONFIG_DRM_ARC_DW_HDMI) += arc-dw-hdmi.o
> >> diff --git a/drivers/gpu/drm/arc/arc-dw-hdmi.c b/drivers/gpu/drm/arc/arc-dw-hdmi.c
> >> new file mode 100644
> >> index 000000000000..46a6ee09b302
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/arc/arc-dw-hdmi.c
> >> @@ -0,0 +1,116 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +//
> >> +// Synopsys DW HDMI driver for various ARC development boards
> >> +//
> >> +// Copyright (C) 2020 Synopsys
> >> +// Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> >> +
> >> +#include <linux/component.h>
> >> +#include <linux/module.h>
> >> +#include <linux/platform_device.h>
> >> +#include <drm/bridge/dw_hdmi.h>
> >> +#include <drm/drm_crtc_helper.h>
> >> +#include <drm/drm_edid.h>
> >> +#include <drm/drm_encoder_slave.h>
> >> +#include <drm/drm_of.h>
> >> +
> >> +static const struct dw_hdmi_mpll_config snps_hdmi_mpll_cfg[] = {
> >> +	{
> >> +		27000000, {
> >> +			{ 0x00B3, 0x0000 },
> >> +			{ 0x00B3, 0x0000 },
> >> +			{ 0x00B3, 0x0000 }
> >> +		},
> >> +	}, {
> >> +		74250000, {
> >> +			{ 0x0072, 0x0001},
> >> +			{ 0x0072, 0x0001},
> >> +			{ 0x0072, 0x0001}
> >> +		},
> >> +	}, {
> >> +		148500000, {
> >> +			{ 0x0051, 0x0002},
> >> +			{ 0x0051, 0x0002},
> >> +			{ 0x0051, 0x0002}
> >> +		},
> >> +	}, {
> >> +		~0UL, {
> >> +			{ 0x00B3, 0x0000 },
> >> +			{ 0x00B3, 0x0000 },
> >> +			{ 0x00B3, 0x0000 },
> >> +		},
> >> +	}
> >> +};
> >> +
> >> +static const struct dw_hdmi_curr_ctrl snps_hdmi_cur_ctr[] = {
> >> +	/* pixelclk    bpp8    bpp10   bpp12 */
> >> +	{ 27000000,  { 0x0000, 0x0000, 0x0000 }, },
> >> +	{ 74250000,  { 0x0008, 0x0008, 0x0008 }, },
> >> +	{ 148500000, { 0x001b, 0x001b, 0x001b }, },
> >> +	{ ~0UL,      { 0x0000, 0x0000, 0x0000 }, }
> >> +};
> >> +
> >> +
> >> +static const struct dw_hdmi_phy_config snps_hdmi_phy_config[] = {
> >> +	/* pixelclk   symbol  term    vlev */
> >> +	{ 27000000,   0x8009, 0x0004, 0x0232},
> >> +	{ 74250000,   0x8009, 0x0004, 0x0232},
> >> +	{ 148500000,  0x8009, 0x0004, 0x0232},
> >> +	{ ~0UL,       0x8009, 0x0004, 0x0232}
> >> +};
> >> +
> >> +static struct dw_hdmi_plat_data snps_dw_hdmi_drv_data = {
> >> +	.mpll_cfg   = snps_hdmi_mpll_cfg,
> >> +	.cur_ctr    = snps_hdmi_cur_ctr,
> >> +	.phy_config = snps_hdmi_phy_config,
> >> +};
> >> +
> >> +static const struct of_device_id snps_dw_hdmi_dt_ids[] = {
> >> +	{ .compatible = "snps,arc-dw-hdmi-hsdk", .data = &snps_dw_hdmi_drv_data },
> >> +	{ /* sentinel */ }
> >> +};
> >> +MODULE_DEVICE_TABLE(of, snps_dw_hdmi_dt_ids);
> >> +
> >> +static int snps_dw_hdmi_probe(struct platform_device *pdev)
> >> +{
> >> +	const struct dw_hdmi_plat_data *plat_data;
> >> +	const struct of_device_id *match;
> >> +	struct dw_hdmi *hdmi;
> >> +
> >> +	if (!pdev->dev.of_node)
> >> +		return -ENODEV;
> >> +
> >> +	match = of_match_node(snps_dw_hdmi_dt_ids, pdev->dev.of_node);
> >> +	plat_data = match->data;
> >> +
> >> +	hdmi = dw_hdmi_probe(pdev, plat_data);
> > 
> > So this is kinda not how bridge drivers are supposed to be done nowadays,
> > direct calling into the driver was the old way, and dw-hdmi still works
> > like that. Modern way is roughly
> > - bridge drivers bind automatically to any bridge they support
> > - bridge drivers publish a bridge with drm_bridge_add()
> > - the driver using the bridge fishes out with dt magic using
> >   of_drm_find_bridge() or another of the related of_ functions
> 
> dw-hdmi is an IP, with some platform specific code and arrays to make it work
> on very different systems, thus we can't use this scheme everywhere....
> 
> Some platforms (like r-car) uses the "right" model because the IP is integrated
> as-is with the default PHY and as an independent IP on the system.
> 
> It's definitely not the case on Rockchip/Amlogic/Allwinner systems,
> and even worse on Amlogic system having a glue on top of the IP, and a
> custom PHY instead of the Synopsys PHY.
> 
> Thus it would be great this would be the case on a Synopsys SoC... but like
> other platforms they have platform specific parameters.
> 
> All this has been discussed and reviewed a few years ago, I would
> personally prefer "fishing out a bridge using dt magic" instead having
> 1k glue code around the IP.

I'm not opposed to the per-instance glue code, that seems required. I'm
kinda questioning where it is.

Current design is that the glue code is in the drm_device driver side of
things, with the drm_bridge somewhat awkward in-between. That doesn't seem
very clean.

I think more suitable for drm_bridge would be to push all the glue behind
the drm_bridge (maybe into separate files in a drm/bridge/dw-hdmi/
directory), and the drm_device driver side simply grabs a drm_bridge and
that's it. Then the drm_bridge abstraction lines up with the code
organization again. Atm it's a bit a confusing state of things.

Plan B would be to give up dw-hdmi being a drm_bridge, and rework dw-hdmi
to be some kind of helper library to build a driver for a specific dw-hdmi
instance. I think that would also be a clean design.

But right now we kinda have a bit of both, so not really abstracted, but
also not really a clean helper without midlayer either, and that doesn't
look great.
-Daniel

> 
> Neil
> 
> > 
> > I know a bit late, just spotted this because you brought your series here
> > up in my arc cleanup series, but can you pls look into adjusting
> > accordingly?
> > 
> > I shouldn't take more than moving this binding here into the dw-hdmi
> > driver, and switching arc itself over to the of_drm_find_bridge() call.
> > That way we could slowly work to transform old bridge drivers like dw-hdmi
> > to the new way, instead of adding more cases that will never get
> > converted.
> > 
> > Other upside is that arc stays a neat&tiny driver :-)
> > 
> > Thanks, Daniel
> > 
> >> +	if (IS_ERR(hdmi))
> >> +		return PTR_ERR(hdmi);
> >> +
> >> +	platform_set_drvdata(pdev, hdmi);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int snps_dw_hdmi_remove(struct platform_device *pdev)
> >> +{
> >> +	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
> >> +
> >> +	dw_hdmi_remove(hdmi);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static struct platform_driver snps_dw_hdmi_platform_driver = {
> >> +	.probe  = snps_dw_hdmi_probe,
> >> +	.remove = snps_dw_hdmi_remove,
> >> +	.driver = {
> >> +		.name = KBUILD_MODNAME,
> >> +		.of_match_table = snps_dw_hdmi_dt_ids,
> >> +	},
> >> +};
> >> +module_platform_driver(snps_dw_hdmi_platform_driver);
> >> +
> >> +MODULE_LICENSE("GPL v2");
> >> +MODULE_DESCRIPTION("ARC specific DW-HDMI driver extension");
> >> +MODULE_AUTHOR("Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>");
> >> -- 
> >> 2.21.1
> >>
> > 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCH v3 1/2] DRM: ARC: add HDMI 2.0 TX encoder support
@ 2020-04-28 10:19         ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2020-04-28 10:19 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: devicetree, David Airlie, Sam Ravnborg, Alexey Brodkin,
	Maarten Lankhorst, linux-kernel, dri-devel, Rob Herring,
	Maxime Ripard, linux-snps-arc, Eugeniy Paltsev

On Tue, Apr 21, 2020 at 05:55:38PM +0200, Neil Armstrong wrote:
> On 15/04/2020 19:33, Daniel Vetter wrote:
> > On Wed, Apr 15, 2020 at 02:29:28AM +0300, Eugeniy Paltsev wrote:
> >> The Synopsys ARC SoCs (like HSDK4xD) include on-chip DesignWare HDMI
> >> encoders. Support them with a platform driver to provide platform glue
> >> data to the dw-hdmi driver.
> >>
> >> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> >> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> >> ---
> >>  MAINTAINERS                       |   6 ++
> >>  drivers/gpu/drm/Makefile          |   2 +-
> >>  drivers/gpu/drm/arc/Kconfig       |   7 ++
> >>  drivers/gpu/drm/arc/Makefile      |   1 +
> >>  drivers/gpu/drm/arc/arc-dw-hdmi.c | 116 ++++++++++++++++++++++++++++++
> >>  5 files changed, 131 insertions(+), 1 deletion(-)
> >>  create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index a6fbdf354d34..2aaed1190370 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -1258,6 +1258,12 @@ S:	Supported
> >>  F:	drivers/gpu/drm/arc/
> >>  F:	Documentation/devicetree/bindings/display/snps,arcpgu.txt
> >>  
> >> +ARC DW HDMI DRIVER
> >> +M:	Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> >> +S:	Supported
> >> +F:	drivers/gpu/drm/arc/arc-dw-hdmi.c
> >> +F:	Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> >> +
> >>  ARCNET NETWORK LAYER
> >>  M:	Michael Grzeschik <m.grzeschik@pengutronix.de>
> >>  L:	netdev@vger.kernel.org
> >> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> >> index 6493088a0fdd..5b0bcf7f45cd 100644
> >> --- a/drivers/gpu/drm/Makefile
> >> +++ b/drivers/gpu/drm/Makefile
> >> @@ -109,7 +109,7 @@ obj-y			+= panel/
> >>  obj-y			+= bridge/
> >>  obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
> >>  obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
> >> -obj-$(CONFIG_DRM_ARCPGU)+= arc/
> >> +obj-y			+= arc/
> >>  obj-y			+= hisilicon/
> >>  obj-$(CONFIG_DRM_ZTE)	+= zte/
> >>  obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
> >> diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig
> >> index e8f3d63e0b91..baec9d2a4fba 100644
> >> --- a/drivers/gpu/drm/arc/Kconfig
> >> +++ b/drivers/gpu/drm/arc/Kconfig
> >> @@ -8,3 +8,10 @@ config DRM_ARCPGU
> >>  	  Choose this option if you have an ARC PGU controller.
> >>  
> >>  	  If M is selected the module will be called arcpgu.
> >> +
> >> +config DRM_ARC_DW_HDMI
> >> +	tristate "ARC DW HDMI"
> >> +	depends on DRM && OF
> >> +	select DRM_DW_HDMI
> >> +	help
> >> +	  Synopsys DW HDMI driver for various ARC development boards
> >> diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
> >> index c7028b7427b3..7a156d8c2c3c 100644
> >> --- a/drivers/gpu/drm/arc/Makefile
> >> +++ b/drivers/gpu/drm/arc/Makefile
> >> @@ -1,3 +1,4 @@
> >>  # SPDX-License-Identifier: GPL-2.0-only
> >>  arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
> >>  obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
> >> +obj-$(CONFIG_DRM_ARC_DW_HDMI) += arc-dw-hdmi.o
> >> diff --git a/drivers/gpu/drm/arc/arc-dw-hdmi.c b/drivers/gpu/drm/arc/arc-dw-hdmi.c
> >> new file mode 100644
> >> index 000000000000..46a6ee09b302
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/arc/arc-dw-hdmi.c
> >> @@ -0,0 +1,116 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +//
> >> +// Synopsys DW HDMI driver for various ARC development boards
> >> +//
> >> +// Copyright (C) 2020 Synopsys
> >> +// Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> >> +
> >> +#include <linux/component.h>
> >> +#include <linux/module.h>
> >> +#include <linux/platform_device.h>
> >> +#include <drm/bridge/dw_hdmi.h>
> >> +#include <drm/drm_crtc_helper.h>
> >> +#include <drm/drm_edid.h>
> >> +#include <drm/drm_encoder_slave.h>
> >> +#include <drm/drm_of.h>
> >> +
> >> +static const struct dw_hdmi_mpll_config snps_hdmi_mpll_cfg[] = {
> >> +	{
> >> +		27000000, {
> >> +			{ 0x00B3, 0x0000 },
> >> +			{ 0x00B3, 0x0000 },
> >> +			{ 0x00B3, 0x0000 }
> >> +		},
> >> +	}, {
> >> +		74250000, {
> >> +			{ 0x0072, 0x0001},
> >> +			{ 0x0072, 0x0001},
> >> +			{ 0x0072, 0x0001}
> >> +		},
> >> +	}, {
> >> +		148500000, {
> >> +			{ 0x0051, 0x0002},
> >> +			{ 0x0051, 0x0002},
> >> +			{ 0x0051, 0x0002}
> >> +		},
> >> +	}, {
> >> +		~0UL, {
> >> +			{ 0x00B3, 0x0000 },
> >> +			{ 0x00B3, 0x0000 },
> >> +			{ 0x00B3, 0x0000 },
> >> +		},
> >> +	}
> >> +};
> >> +
> >> +static const struct dw_hdmi_curr_ctrl snps_hdmi_cur_ctr[] = {
> >> +	/* pixelclk    bpp8    bpp10   bpp12 */
> >> +	{ 27000000,  { 0x0000, 0x0000, 0x0000 }, },
> >> +	{ 74250000,  { 0x0008, 0x0008, 0x0008 }, },
> >> +	{ 148500000, { 0x001b, 0x001b, 0x001b }, },
> >> +	{ ~0UL,      { 0x0000, 0x0000, 0x0000 }, }
> >> +};
> >> +
> >> +
> >> +static const struct dw_hdmi_phy_config snps_hdmi_phy_config[] = {
> >> +	/* pixelclk   symbol  term    vlev */
> >> +	{ 27000000,   0x8009, 0x0004, 0x0232},
> >> +	{ 74250000,   0x8009, 0x0004, 0x0232},
> >> +	{ 148500000,  0x8009, 0x0004, 0x0232},
> >> +	{ ~0UL,       0x8009, 0x0004, 0x0232}
> >> +};
> >> +
> >> +static struct dw_hdmi_plat_data snps_dw_hdmi_drv_data = {
> >> +	.mpll_cfg   = snps_hdmi_mpll_cfg,
> >> +	.cur_ctr    = snps_hdmi_cur_ctr,
> >> +	.phy_config = snps_hdmi_phy_config,
> >> +};
> >> +
> >> +static const struct of_device_id snps_dw_hdmi_dt_ids[] = {
> >> +	{ .compatible = "snps,arc-dw-hdmi-hsdk", .data = &snps_dw_hdmi_drv_data },
> >> +	{ /* sentinel */ }
> >> +};
> >> +MODULE_DEVICE_TABLE(of, snps_dw_hdmi_dt_ids);
> >> +
> >> +static int snps_dw_hdmi_probe(struct platform_device *pdev)
> >> +{
> >> +	const struct dw_hdmi_plat_data *plat_data;
> >> +	const struct of_device_id *match;
> >> +	struct dw_hdmi *hdmi;
> >> +
> >> +	if (!pdev->dev.of_node)
> >> +		return -ENODEV;
> >> +
> >> +	match = of_match_node(snps_dw_hdmi_dt_ids, pdev->dev.of_node);
> >> +	plat_data = match->data;
> >> +
> >> +	hdmi = dw_hdmi_probe(pdev, plat_data);
> > 
> > So this is kinda not how bridge drivers are supposed to be done nowadays,
> > direct calling into the driver was the old way, and dw-hdmi still works
> > like that. Modern way is roughly
> > - bridge drivers bind automatically to any bridge they support
> > - bridge drivers publish a bridge with drm_bridge_add()
> > - the driver using the bridge fishes out with dt magic using
> >   of_drm_find_bridge() or another of the related of_ functions
> 
> dw-hdmi is an IP, with some platform specific code and arrays to make it work
> on very different systems, thus we can't use this scheme everywhere....
> 
> Some platforms (like r-car) uses the "right" model because the IP is integrated
> as-is with the default PHY and as an independent IP on the system.
> 
> It's definitely not the case on Rockchip/Amlogic/Allwinner systems,
> and even worse on Amlogic system having a glue on top of the IP, and a
> custom PHY instead of the Synopsys PHY.
> 
> Thus it would be great this would be the case on a Synopsys SoC... but like
> other platforms they have platform specific parameters.
> 
> All this has been discussed and reviewed a few years ago, I would
> personally prefer "fishing out a bridge using dt magic" instead having
> 1k glue code around the IP.

I'm not opposed to the per-instance glue code, that seems required. I'm
kinda questioning where it is.

Current design is that the glue code is in the drm_device driver side of
things, with the drm_bridge somewhat awkward in-between. That doesn't seem
very clean.

I think more suitable for drm_bridge would be to push all the glue behind
the drm_bridge (maybe into separate files in a drm/bridge/dw-hdmi/
directory), and the drm_device driver side simply grabs a drm_bridge and
that's it. Then the drm_bridge abstraction lines up with the code
organization again. Atm it's a bit a confusing state of things.

Plan B would be to give up dw-hdmi being a drm_bridge, and rework dw-hdmi
to be some kind of helper library to build a driver for a specific dw-hdmi
instance. I think that would also be a clean design.

But right now we kinda have a bit of both, so not really abstracted, but
also not really a clean helper without midlayer either, and that doesn't
look great.
-Daniel

> 
> Neil
> 
> > 
> > I know a bit late, just spotted this because you brought your series here
> > up in my arc cleanup series, but can you pls look into adjusting
> > accordingly?
> > 
> > I shouldn't take more than moving this binding here into the dw-hdmi
> > driver, and switching arc itself over to the of_drm_find_bridge() call.
> > That way we could slowly work to transform old bridge drivers like dw-hdmi
> > to the new way, instead of adding more cases that will never get
> > converted.
> > 
> > Other upside is that arc stays a neat&tiny driver :-)
> > 
> > Thanks, Daniel
> > 
> >> +	if (IS_ERR(hdmi))
> >> +		return PTR_ERR(hdmi);
> >> +
> >> +	platform_set_drvdata(pdev, hdmi);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int snps_dw_hdmi_remove(struct platform_device *pdev)
> >> +{
> >> +	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
> >> +
> >> +	dw_hdmi_remove(hdmi);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static struct platform_driver snps_dw_hdmi_platform_driver = {
> >> +	.probe  = snps_dw_hdmi_probe,
> >> +	.remove = snps_dw_hdmi_remove,
> >> +	.driver = {
> >> +		.name = KBUILD_MODNAME,
> >> +		.of_match_table = snps_dw_hdmi_dt_ids,
> >> +	},
> >> +};
> >> +module_platform_driver(snps_dw_hdmi_platform_driver);
> >> +
> >> +MODULE_LICENSE("GPL v2");
> >> +MODULE_DESCRIPTION("ARC specific DW-HDMI driver extension");
> >> +MODULE_AUTHOR("Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>");
> >> -- 
> >> 2.21.1
> >>
> > 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

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

* Re: [PATCH v3 1/2] DRM: ARC: add HDMI 2.0 TX encoder support
@ 2020-04-28 10:19         ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2020-04-28 10:19 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: devicetree, David Airlie, Sam Ravnborg, Alexey Brodkin,
	linux-kernel, dri-devel, Rob Herring, linux-snps-arc,
	Eugeniy Paltsev

On Tue, Apr 21, 2020 at 05:55:38PM +0200, Neil Armstrong wrote:
> On 15/04/2020 19:33, Daniel Vetter wrote:
> > On Wed, Apr 15, 2020 at 02:29:28AM +0300, Eugeniy Paltsev wrote:
> >> The Synopsys ARC SoCs (like HSDK4xD) include on-chip DesignWare HDMI
> >> encoders. Support them with a platform driver to provide platform glue
> >> data to the dw-hdmi driver.
> >>
> >> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> >> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> >> ---
> >>  MAINTAINERS                       |   6 ++
> >>  drivers/gpu/drm/Makefile          |   2 +-
> >>  drivers/gpu/drm/arc/Kconfig       |   7 ++
> >>  drivers/gpu/drm/arc/Makefile      |   1 +
> >>  drivers/gpu/drm/arc/arc-dw-hdmi.c | 116 ++++++++++++++++++++++++++++++
> >>  5 files changed, 131 insertions(+), 1 deletion(-)
> >>  create mode 100644 drivers/gpu/drm/arc/arc-dw-hdmi.c
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index a6fbdf354d34..2aaed1190370 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -1258,6 +1258,12 @@ S:	Supported
> >>  F:	drivers/gpu/drm/arc/
> >>  F:	Documentation/devicetree/bindings/display/snps,arcpgu.txt
> >>  
> >> +ARC DW HDMI DRIVER
> >> +M:	Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> >> +S:	Supported
> >> +F:	drivers/gpu/drm/arc/arc-dw-hdmi.c
> >> +F:	Documentation/devicetree/bindings/display/bridge/snps,arc-dw-hdmi.yaml
> >> +
> >>  ARCNET NETWORK LAYER
> >>  M:	Michael Grzeschik <m.grzeschik@pengutronix.de>
> >>  L:	netdev@vger.kernel.org
> >> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> >> index 6493088a0fdd..5b0bcf7f45cd 100644
> >> --- a/drivers/gpu/drm/Makefile
> >> +++ b/drivers/gpu/drm/Makefile
> >> @@ -109,7 +109,7 @@ obj-y			+= panel/
> >>  obj-y			+= bridge/
> >>  obj-$(CONFIG_DRM_FSL_DCU) += fsl-dcu/
> >>  obj-$(CONFIG_DRM_ETNAVIV) += etnaviv/
> >> -obj-$(CONFIG_DRM_ARCPGU)+= arc/
> >> +obj-y			+= arc/
> >>  obj-y			+= hisilicon/
> >>  obj-$(CONFIG_DRM_ZTE)	+= zte/
> >>  obj-$(CONFIG_DRM_MXSFB)	+= mxsfb/
> >> diff --git a/drivers/gpu/drm/arc/Kconfig b/drivers/gpu/drm/arc/Kconfig
> >> index e8f3d63e0b91..baec9d2a4fba 100644
> >> --- a/drivers/gpu/drm/arc/Kconfig
> >> +++ b/drivers/gpu/drm/arc/Kconfig
> >> @@ -8,3 +8,10 @@ config DRM_ARCPGU
> >>  	  Choose this option if you have an ARC PGU controller.
> >>  
> >>  	  If M is selected the module will be called arcpgu.
> >> +
> >> +config DRM_ARC_DW_HDMI
> >> +	tristate "ARC DW HDMI"
> >> +	depends on DRM && OF
> >> +	select DRM_DW_HDMI
> >> +	help
> >> +	  Synopsys DW HDMI driver for various ARC development boards
> >> diff --git a/drivers/gpu/drm/arc/Makefile b/drivers/gpu/drm/arc/Makefile
> >> index c7028b7427b3..7a156d8c2c3c 100644
> >> --- a/drivers/gpu/drm/arc/Makefile
> >> +++ b/drivers/gpu/drm/arc/Makefile
> >> @@ -1,3 +1,4 @@
> >>  # SPDX-License-Identifier: GPL-2.0-only
> >>  arcpgu-y := arcpgu_crtc.o arcpgu_hdmi.o arcpgu_sim.o arcpgu_drv.o
> >>  obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
> >> +obj-$(CONFIG_DRM_ARC_DW_HDMI) += arc-dw-hdmi.o
> >> diff --git a/drivers/gpu/drm/arc/arc-dw-hdmi.c b/drivers/gpu/drm/arc/arc-dw-hdmi.c
> >> new file mode 100644
> >> index 000000000000..46a6ee09b302
> >> --- /dev/null
> >> +++ b/drivers/gpu/drm/arc/arc-dw-hdmi.c
> >> @@ -0,0 +1,116 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +//
> >> +// Synopsys DW HDMI driver for various ARC development boards
> >> +//
> >> +// Copyright (C) 2020 Synopsys
> >> +// Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> >> +
> >> +#include <linux/component.h>
> >> +#include <linux/module.h>
> >> +#include <linux/platform_device.h>
> >> +#include <drm/bridge/dw_hdmi.h>
> >> +#include <drm/drm_crtc_helper.h>
> >> +#include <drm/drm_edid.h>
> >> +#include <drm/drm_encoder_slave.h>
> >> +#include <drm/drm_of.h>
> >> +
> >> +static const struct dw_hdmi_mpll_config snps_hdmi_mpll_cfg[] = {
> >> +	{
> >> +		27000000, {
> >> +			{ 0x00B3, 0x0000 },
> >> +			{ 0x00B3, 0x0000 },
> >> +			{ 0x00B3, 0x0000 }
> >> +		},
> >> +	}, {
> >> +		74250000, {
> >> +			{ 0x0072, 0x0001},
> >> +			{ 0x0072, 0x0001},
> >> +			{ 0x0072, 0x0001}
> >> +		},
> >> +	}, {
> >> +		148500000, {
> >> +			{ 0x0051, 0x0002},
> >> +			{ 0x0051, 0x0002},
> >> +			{ 0x0051, 0x0002}
> >> +		},
> >> +	}, {
> >> +		~0UL, {
> >> +			{ 0x00B3, 0x0000 },
> >> +			{ 0x00B3, 0x0000 },
> >> +			{ 0x00B3, 0x0000 },
> >> +		},
> >> +	}
> >> +};
> >> +
> >> +static const struct dw_hdmi_curr_ctrl snps_hdmi_cur_ctr[] = {
> >> +	/* pixelclk    bpp8    bpp10   bpp12 */
> >> +	{ 27000000,  { 0x0000, 0x0000, 0x0000 }, },
> >> +	{ 74250000,  { 0x0008, 0x0008, 0x0008 }, },
> >> +	{ 148500000, { 0x001b, 0x001b, 0x001b }, },
> >> +	{ ~0UL,      { 0x0000, 0x0000, 0x0000 }, }
> >> +};
> >> +
> >> +
> >> +static const struct dw_hdmi_phy_config snps_hdmi_phy_config[] = {
> >> +	/* pixelclk   symbol  term    vlev */
> >> +	{ 27000000,   0x8009, 0x0004, 0x0232},
> >> +	{ 74250000,   0x8009, 0x0004, 0x0232},
> >> +	{ 148500000,  0x8009, 0x0004, 0x0232},
> >> +	{ ~0UL,       0x8009, 0x0004, 0x0232}
> >> +};
> >> +
> >> +static struct dw_hdmi_plat_data snps_dw_hdmi_drv_data = {
> >> +	.mpll_cfg   = snps_hdmi_mpll_cfg,
> >> +	.cur_ctr    = snps_hdmi_cur_ctr,
> >> +	.phy_config = snps_hdmi_phy_config,
> >> +};
> >> +
> >> +static const struct of_device_id snps_dw_hdmi_dt_ids[] = {
> >> +	{ .compatible = "snps,arc-dw-hdmi-hsdk", .data = &snps_dw_hdmi_drv_data },
> >> +	{ /* sentinel */ }
> >> +};
> >> +MODULE_DEVICE_TABLE(of, snps_dw_hdmi_dt_ids);
> >> +
> >> +static int snps_dw_hdmi_probe(struct platform_device *pdev)
> >> +{
> >> +	const struct dw_hdmi_plat_data *plat_data;
> >> +	const struct of_device_id *match;
> >> +	struct dw_hdmi *hdmi;
> >> +
> >> +	if (!pdev->dev.of_node)
> >> +		return -ENODEV;
> >> +
> >> +	match = of_match_node(snps_dw_hdmi_dt_ids, pdev->dev.of_node);
> >> +	plat_data = match->data;
> >> +
> >> +	hdmi = dw_hdmi_probe(pdev, plat_data);
> > 
> > So this is kinda not how bridge drivers are supposed to be done nowadays,
> > direct calling into the driver was the old way, and dw-hdmi still works
> > like that. Modern way is roughly
> > - bridge drivers bind automatically to any bridge they support
> > - bridge drivers publish a bridge with drm_bridge_add()
> > - the driver using the bridge fishes out with dt magic using
> >   of_drm_find_bridge() or another of the related of_ functions
> 
> dw-hdmi is an IP, with some platform specific code and arrays to make it work
> on very different systems, thus we can't use this scheme everywhere....
> 
> Some platforms (like r-car) uses the "right" model because the IP is integrated
> as-is with the default PHY and as an independent IP on the system.
> 
> It's definitely not the case on Rockchip/Amlogic/Allwinner systems,
> and even worse on Amlogic system having a glue on top of the IP, and a
> custom PHY instead of the Synopsys PHY.
> 
> Thus it would be great this would be the case on a Synopsys SoC... but like
> other platforms they have platform specific parameters.
> 
> All this has been discussed and reviewed a few years ago, I would
> personally prefer "fishing out a bridge using dt magic" instead having
> 1k glue code around the IP.

I'm not opposed to the per-instance glue code, that seems required. I'm
kinda questioning where it is.

Current design is that the glue code is in the drm_device driver side of
things, with the drm_bridge somewhat awkward in-between. That doesn't seem
very clean.

I think more suitable for drm_bridge would be to push all the glue behind
the drm_bridge (maybe into separate files in a drm/bridge/dw-hdmi/
directory), and the drm_device driver side simply grabs a drm_bridge and
that's it. Then the drm_bridge abstraction lines up with the code
organization again. Atm it's a bit a confusing state of things.

Plan B would be to give up dw-hdmi being a drm_bridge, and rework dw-hdmi
to be some kind of helper library to build a driver for a specific dw-hdmi
instance. I think that would also be a clean design.

But right now we kinda have a bit of both, so not really abstracted, but
also not really a clean helper without midlayer either, and that doesn't
look great.
-Daniel

> 
> Neil
> 
> > 
> > I know a bit late, just spotted this because you brought your series here
> > up in my arc cleanup series, but can you pls look into adjusting
> > accordingly?
> > 
> > I shouldn't take more than moving this binding here into the dw-hdmi
> > driver, and switching arc itself over to the of_drm_find_bridge() call.
> > That way we could slowly work to transform old bridge drivers like dw-hdmi
> > to the new way, instead of adding more cases that will never get
> > converted.
> > 
> > Other upside is that arc stays a neat&tiny driver :-)
> > 
> > Thanks, Daniel
> > 
> >> +	if (IS_ERR(hdmi))
> >> +		return PTR_ERR(hdmi);
> >> +
> >> +	platform_set_drvdata(pdev, hdmi);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int snps_dw_hdmi_remove(struct platform_device *pdev)
> >> +{
> >> +	struct dw_hdmi *hdmi = platform_get_drvdata(pdev);
> >> +
> >> +	dw_hdmi_remove(hdmi);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static struct platform_driver snps_dw_hdmi_platform_driver = {
> >> +	.probe  = snps_dw_hdmi_probe,
> >> +	.remove = snps_dw_hdmi_remove,
> >> +	.driver = {
> >> +		.name = KBUILD_MODNAME,
> >> +		.of_match_table = snps_dw_hdmi_dt_ids,
> >> +	},
> >> +};
> >> +module_platform_driver(snps_dw_hdmi_platform_driver);
> >> +
> >> +MODULE_LICENSE("GPL v2");
> >> +MODULE_DESCRIPTION("ARC specific DW-HDMI driver extension");
> >> +MODULE_AUTHOR("Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>");
> >> -- 
> >> 2.21.1
> >>
> > 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2020-04-28 10:19 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-14 23:29 [PATCH v3 0/2] DRM: ARC: add HDMI 2.0 TX encoder support Eugeniy Paltsev
2020-04-14 23:29 ` Eugeniy Paltsev
2020-04-14 23:29 ` Eugeniy Paltsev
2020-04-14 23:29 ` [PATCH v3 1/2] " Eugeniy Paltsev
2020-04-14 23:29   ` Eugeniy Paltsev
2020-04-14 23:29   ` Eugeniy Paltsev
2020-04-15 17:33   ` Daniel Vetter
2020-04-15 17:33     ` Daniel Vetter
2020-04-15 17:33     ` Daniel Vetter
2020-04-21 15:55     ` Neil Armstrong
2020-04-21 15:55       ` Neil Armstrong
2020-04-21 15:55       ` Neil Armstrong
2020-04-28 10:19       ` Daniel Vetter
2020-04-28 10:19         ` Daniel Vetter
2020-04-28 10:19         ` Daniel Vetter
2020-04-14 23:29 ` [PATCH v3 2/2] dt-bindings: Document the Synopsys ARC HDMI TX bindings Eugeniy Paltsev
2020-04-14 23:29   ` Eugeniy Paltsev
2020-04-14 23:29   ` Eugeniy Paltsev
2020-04-20 21:24   ` Rob Herring
2020-04-20 21:24     ` Rob Herring
2020-04-20 21:24     ` Rob Herring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.