[PATCH v2] drm/rockchip: Refactor the component match logic.

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v2] drm/rockchip: Refactor the component match logic.
@ 2017-03-14 10:45   ` Jeffy Chen
  0 siblings, 0 replies; 10+ messages in thread
From: Jeffy Chen @ 2017-03-14 10:45 UTC (permalink / raw)
  To: Mark Yao
  Cc: Douglas Anderson, Guenter Roeck, Brian Norris, Sean Paul,
	Tomasz Figa, Jeffy Chen, Heiko Stuebner, linux-kernel, dri-devel,
	linux-rockchip, David Airlie, linux-arm-kernel

Currently we are adding all components from the dts, if one of their
drivers been disabled, we would not be able to bring up others.

Refactor component match logic, follow exynos drm.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>

---

 drivers/gpu/drm/rockchip/Kconfig                |  10 +-
 drivers/gpu/drm/rockchip/Makefile               |  16 +--
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c |   9 +-
 drivers/gpu/drm/rockchip/cdn-dp-core.c          |   8 +-
 drivers/gpu/drm/rockchip/dw-mipi-dsi.c          |   8 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c     |  10 +-
 drivers/gpu/drm/rockchip/inno_hdmi.c            |  10 +-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c     | 162 +++++++++++++++++-------
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h     |  16 +++
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c     |   8 +-
 10 files changed, 149 insertions(+), 108 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 0e4eb84..50c41c0 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -13,7 +13,7 @@ config DRM_ROCKCHIP
 	  IP found on the SoC.
 
 config ROCKCHIP_ANALOGIX_DP
-	tristate "Rockchip specific extensions for Analogix DP driver"
+	bool "Rockchip specific extensions for Analogix DP driver"
 	depends on DRM_ROCKCHIP
 	select DRM_ANALOGIX_DP
 	help
@@ -22,7 +22,7 @@ config ROCKCHIP_ANALOGIX_DP
 	  on RK3288 based SoC, you should selet this option.
 
 config ROCKCHIP_CDN_DP
-        tristate "Rockchip cdn DP"
+        bool "Rockchip cdn DP"
         depends on DRM_ROCKCHIP
 	depends on EXTCON
 	select SND_SOC_HDMI_CODEC if SND_SOC
@@ -33,7 +33,7 @@ config ROCKCHIP_CDN_DP
 	  option.
 
 config ROCKCHIP_DW_HDMI
-        tristate "Rockchip specific extensions for Synopsys DW HDMI"
+        bool "Rockchip specific extensions for Synopsys DW HDMI"
         depends on DRM_ROCKCHIP
         select DRM_DW_HDMI
         help
@@ -43,7 +43,7 @@ config ROCKCHIP_DW_HDMI
 	  option.
 
 config ROCKCHIP_DW_MIPI_DSI
-	tristate "Rockchip specific extensions for Synopsys DW MIPI DSI"
+	bool "Rockchip specific extensions for Synopsys DW MIPI DSI"
 	depends on DRM_ROCKCHIP
 	select DRM_MIPI_DSI
 	help
@@ -53,7 +53,7 @@ config ROCKCHIP_DW_MIPI_DSI
 	 option.
 
 config ROCKCHIP_INNO_HDMI
-	tristate "Rockchip specific extensions for Innosilicon HDMI"
+	bool "Rockchip specific extensions for Innosilicon HDMI"
 	depends on DRM_ROCKCHIP
 	help
 	  This selects support for Rockchip SoC specific extensions
diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
index c931e2a..fa8dc9d 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -3,14 +3,14 @@
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
 rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
-		rockchip_drm_gem.o rockchip_drm_psr.o rockchip_drm_vop.o
+		rockchip_drm_gem.o rockchip_drm_psr.o \
+		rockchip_drm_vop.o rockchip_vop_reg.o
 rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
 
-obj-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
-obj-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp.o
-cdn-dp-objs := cdn-dp-core.o cdn-dp-reg.o
-obj-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
-obj-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
-obj-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
+rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
+rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
+rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
+rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
+rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
 
-obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o rockchip_vop_reg.o
+obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 8548e82..91ebe5c 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -507,7 +507,7 @@ static const struct of_device_id rockchip_dp_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, rockchip_dp_dt_ids);
 
-static struct platform_driver rockchip_dp_driver = {
+struct platform_driver rockchip_dp_driver = {
 	.probe = rockchip_dp_probe,
 	.remove = rockchip_dp_remove,
 	.driver = {
@@ -516,10 +516,3 @@ static struct platform_driver rockchip_dp_driver = {
 		   .of_match_table = of_match_ptr(rockchip_dp_dt_ids),
 	},
 };
-
-module_platform_driver(rockchip_dp_driver);
-
-MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
-MODULE_AUTHOR("Jeff chen <jeff.chen@rock-chips.com>");
-MODULE_DESCRIPTION("Rockchip Specific Analogix-DP Driver Extension");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index fd79a70..e1796d0 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -1243,7 +1243,7 @@ static const struct dev_pm_ops cdn_dp_pm_ops = {
 				cdn_dp_resume)
 };
 
-static struct platform_driver cdn_dp_driver = {
+struct platform_driver cdn_dp_driver = {
 	.probe = cdn_dp_probe,
 	.remove = cdn_dp_remove,
 	.shutdown = cdn_dp_shutdown,
@@ -1254,9 +1254,3 @@ static struct platform_driver cdn_dp_driver = {
 		   .pm = &cdn_dp_pm_ops,
 	},
 };
-
-module_platform_driver(cdn_dp_driver);
-
-MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
-MODULE_DESCRIPTION("cdn DP Driver");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index d9aa382..65a474f 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -1165,7 +1165,7 @@ static int dw_mipi_dsi_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver dw_mipi_dsi_driver = {
+struct platform_driver dw_mipi_dsi_driver = {
 	.probe		= dw_mipi_dsi_probe,
 	.remove		= dw_mipi_dsi_remove,
 	.driver		= {
@@ -1173,9 +1173,3 @@ static struct platform_driver dw_mipi_dsi_driver = {
 		.name	= DRIVER_NAME,
 	},
 };
-module_platform_driver(dw_mipi_dsi_driver);
-
-MODULE_DESCRIPTION("ROCKCHIP MIPI DSI host controller driver");
-MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:" DRIVER_NAME);
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index a6d4a02..976ea79 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -326,7 +326,7 @@ static int dw_hdmi_rockchip_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
+struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
 	.probe  = dw_hdmi_rockchip_probe,
 	.remove = dw_hdmi_rockchip_remove,
 	.driver = {
@@ -334,11 +334,3 @@ static struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
 		.of_match_table = dw_hdmi_rockchip_dt_ids,
 	},
 };
-
-module_platform_driver(dw_hdmi_rockchip_pltfm_driver);
-
-MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com>");
-MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
-MODULE_DESCRIPTION("Rockchip Specific DW-HDMI Driver Extension");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:dwhdmi-rockchip");
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 006260d..7d9b75e 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -923,7 +923,7 @@ static const struct of_device_id inno_hdmi_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, inno_hdmi_dt_ids);
 
-static struct platform_driver inno_hdmi_driver = {
+struct platform_driver inno_hdmi_driver = {
 	.probe  = inno_hdmi_probe,
 	.remove = inno_hdmi_remove,
 	.driver = {
@@ -931,11 +931,3 @@ static struct platform_driver inno_hdmi_driver = {
 		.of_match_table = inno_hdmi_dt_ids,
 	},
 };
-
-module_platform_driver(inno_hdmi_driver);
-
-MODULE_AUTHOR("Zheng Yang <zhengyang@rock-chips.com>");
-MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
-MODULE_DESCRIPTION("Rockchip Specific INNO-HDMI Driver");
-MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:innohdmi-rockchip");
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index b360e62..b6bccfc 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -356,34 +356,52 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
 				rockchip_drm_sys_resume)
 };
 
-static int compare_of(struct device *dev, void *data)
-{
-	struct device_node *np = data;
+static struct platform_driver *rockchip_drm_comp_drvs[] = {
+	&vop_platform_driver,
+#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
+	&rockchip_dp_driver,
+#endif
+#ifdef CONFIG_ROCKCHIP_CDN_DP
+	&cdn_dp_driver,
+#endif
+#ifdef CONFIG_ROCKCHIP_DW_HDMI
+	&dw_hdmi_rockchip_pltfm_driver,
+#endif
+#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
+	&dw_mipi_dsi_driver,
+#endif
+#ifdef CONFIG_ROCKCHIP_INNO_HDMI
+	&inno_hdmi_driver,
+#endif
+};
 
-	return dev->of_node == np;
+static int compare_dev(struct device *dev, void *data)
+{
+	return dev == (struct device *)data;
 }
 
-static void rockchip_add_endpoints(struct device *dev,
-				   struct component_match **match,
-				   struct device_node *port)
+static struct component_match *rockchip_drm_match_add(struct device *dev)
 {
-	struct device_node *ep, *remote;
+	struct component_match *match = NULL;
+	int i;
 
-	for_each_child_of_node(port, ep) {
-		remote = of_graph_get_remote_port_parent(ep);
-		if (!remote || !of_device_is_available(remote)) {
-			of_node_put(remote);
-			continue;
-		} else if (!of_device_is_available(remote->parent)) {
-			dev_warn(dev, "parent device of %s is not available\n",
-				 remote->full_name);
-			of_node_put(remote);
-			continue;
-		}
+	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
+		struct platform_driver *drv = rockchip_drm_comp_drvs[i];
+		struct device *p = NULL, *d;
 
-		drm_of_component_match_add(dev, match, compare_of, remote);
-		of_node_put(remote);
+		do {
+			d = bus_find_device(&platform_bus_type, p, &drv->driver,
+					    (void *)platform_bus_type.match);
+			put_device(p);
+			p = d;
+
+			if (!d)
+				break;
+			component_match_add(dev, &match, compare_dev, d);
+		} while (true);
 	}
+
+	return match ?: ERR_PTR(-ENODEV);
 }
 
 static const struct component_master_ops rockchip_drm_ops = {
@@ -391,21 +409,16 @@ static const struct component_master_ops rockchip_drm_ops = {
 	.unbind = rockchip_drm_unbind,
 };
 
-static int rockchip_drm_platform_probe(struct platform_device *pdev)
+static int rockchip_drm_platform_of_probe(struct device *dev)
 {
-	struct device *dev = &pdev->dev;
-	struct component_match *match = NULL;
 	struct device_node *np = dev->of_node;
 	struct device_node *port;
+	bool found = false;
 	int i;
 
 	if (!np)
 		return -ENODEV;
-	/*
-	 * Bind the crtc ports first, so that
-	 * drm_of_find_possible_crtcs called from encoder .bind callbacks
-	 * works as expected.
-	 */
+
 	for (i = 0;; i++) {
 		struct device_node *iommu;
 
@@ -429,9 +442,9 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
 			is_support_iommu = false;
 		}
 
+		found = true;
+
 		of_node_put(iommu);
-		drm_of_component_match_add(dev, &match, compare_of,
-					   port->parent);
 		of_node_put(port);
 	}
 
@@ -440,27 +453,27 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	if (!match) {
+	if (!found) {
 		dev_err(dev, "No available vop found for display-subsystem.\n");
 		return -ENODEV;
 	}
-	/*
-	 * For each bound crtc, bind the encoders attached to its
-	 * remote endpoint.
-	 */
-	for (i = 0;; i++) {
-		port = of_parse_phandle(np, "ports", i);
-		if (!port)
-			break;
 
-		if (!of_device_is_available(port->parent)) {
-			of_node_put(port);
-			continue;
-		}
+	return 0;
+}
 
-		rockchip_add_endpoints(dev, &match, port);
-		of_node_put(port);
-	}
+static int rockchip_drm_platform_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct component_match *match = NULL;
+	int ret;
+
+	ret = rockchip_drm_platform_of_probe(dev);
+	if (ret)
+		return ret;
+
+	match = rockchip_drm_match_add(dev);
+	if (IS_ERR(match))
+		return PTR_ERR(match);
 
 	return component_master_add_with_match(dev, &rockchip_drm_ops, match);
 }
@@ -488,7 +501,60 @@ static struct platform_driver rockchip_drm_platform_driver = {
 	},
 };
 
-module_platform_driver(rockchip_drm_platform_driver);
+static void rockchip_drm_unregister_drivers(void)
+{
+	int i;
+
+	for (i = ARRAY_SIZE(rockchip_drm_comp_drvs) - 1; i >= 0; i--)
+		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
+}
+
+static int rockchip_drm_register_drivers(void)
+{
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
+		ret = platform_driver_register(rockchip_drm_comp_drvs[i]);
+		if (ret)
+			goto err_unreg;
+	}
+
+	return 0;
+
+err_unreg:
+	for (; i--; )
+		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
+	return ret;
+}
+
+static int __init rockchip_drm_init(void)
+{
+	int ret;
+
+	ret = rockchip_drm_register_drivers();
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&rockchip_drm_platform_driver);
+	if (ret)
+		goto err_unreg_drivers;
+
+	return 0;
+
+err_unreg_drivers:
+	rockchip_drm_unregister_drivers();
+	return ret;
+}
+
+static void __exit rockchip_drm_fini(void)
+{
+	platform_driver_unregister(&rockchip_drm_platform_driver);
+
+	rockchip_drm_unregister_drivers();
+}
+
+module_init(rockchip_drm_init);
+module_exit(rockchip_drm_fini);
 
 MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
 MODULE_DESCRIPTION("ROCKCHIP DRM Driver");
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index adc3930..91812bd 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -79,4 +79,20 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
 				unsigned int mstimeout);
 
+#ifdef CONFIG_ROCKCHIP_CDN_DP
+extern struct platform_driver cdn_dp_driver;
+#endif
+#ifdef CONFIG_ROCKCHIP_DW_HDMI
+extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
+#endif
+#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
+extern struct platform_driver dw_mipi_dsi_driver;
+#endif
+#ifdef CONFIG_ROCKCHIP_INNO_HDMI
+extern struct platform_driver inno_hdmi_driver;
+#endif
+#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
+extern struct platform_driver rockchip_dp_driver;
+#endif
+extern struct platform_driver vop_platform_driver;
 #endif /* _ROCKCHIP_DRM_DRV_H_ */
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 91fbc7b..0da4444 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -404,7 +404,7 @@ static int vop_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver vop_platform_driver = {
+struct platform_driver vop_platform_driver = {
 	.probe = vop_probe,
 	.remove = vop_remove,
 	.driver = {
@@ -412,9 +412,3 @@ static struct platform_driver vop_platform_driver = {
 		.of_match_table = of_match_ptr(vop_driver_dt_match),
 	},
 };
-
-module_platform_driver(vop_platform_driver);
-
-MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
-MODULE_DESCRIPTION("ROCKCHIP VOP Driver");
-MODULE_LICENSE("GPL v2");
-- 
2.1.4

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

* [PATCH v2] drm/rockchip: Refactor the component match logic.
@ 2017-03-14 10:45   ` Jeffy Chen
  0 siblings, 0 replies; 10+ messages in thread
From: Jeffy Chen @ 2017-03-14 10:45 UTC (permalink / raw)
  To: linux-arm-kernel

Currently we are adding all components from the dts, if one of their
drivers been disabled, we would not be able to bring up others.

Refactor component match logic, follow exynos drm.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>

---

 drivers/gpu/drm/rockchip/Kconfig                |  10 +-
 drivers/gpu/drm/rockchip/Makefile               |  16 +--
 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c |   9 +-
 drivers/gpu/drm/rockchip/cdn-dp-core.c          |   8 +-
 drivers/gpu/drm/rockchip/dw-mipi-dsi.c          |   8 +-
 drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c     |  10 +-
 drivers/gpu/drm/rockchip/inno_hdmi.c            |  10 +-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c     | 162 +++++++++++++++++-------
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h     |  16 +++
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c     |   8 +-
 10 files changed, 149 insertions(+), 108 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 0e4eb84..50c41c0 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -13,7 +13,7 @@ config DRM_ROCKCHIP
 	  IP found on the SoC.
 
 config ROCKCHIP_ANALOGIX_DP
-	tristate "Rockchip specific extensions for Analogix DP driver"
+	bool "Rockchip specific extensions for Analogix DP driver"
 	depends on DRM_ROCKCHIP
 	select DRM_ANALOGIX_DP
 	help
@@ -22,7 +22,7 @@ config ROCKCHIP_ANALOGIX_DP
 	  on RK3288 based SoC, you should selet this option.
 
 config ROCKCHIP_CDN_DP
-        tristate "Rockchip cdn DP"
+        bool "Rockchip cdn DP"
         depends on DRM_ROCKCHIP
 	depends on EXTCON
 	select SND_SOC_HDMI_CODEC if SND_SOC
@@ -33,7 +33,7 @@ config ROCKCHIP_CDN_DP
 	  option.
 
 config ROCKCHIP_DW_HDMI
-        tristate "Rockchip specific extensions for Synopsys DW HDMI"
+        bool "Rockchip specific extensions for Synopsys DW HDMI"
         depends on DRM_ROCKCHIP
         select DRM_DW_HDMI
         help
@@ -43,7 +43,7 @@ config ROCKCHIP_DW_HDMI
 	  option.
 
 config ROCKCHIP_DW_MIPI_DSI
-	tristate "Rockchip specific extensions for Synopsys DW MIPI DSI"
+	bool "Rockchip specific extensions for Synopsys DW MIPI DSI"
 	depends on DRM_ROCKCHIP
 	select DRM_MIPI_DSI
 	help
@@ -53,7 +53,7 @@ config ROCKCHIP_DW_MIPI_DSI
 	 option.
 
 config ROCKCHIP_INNO_HDMI
-	tristate "Rockchip specific extensions for Innosilicon HDMI"
+	bool "Rockchip specific extensions for Innosilicon HDMI"
 	depends on DRM_ROCKCHIP
 	help
 	  This selects support for Rockchip SoC specific extensions
diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
index c931e2a..fa8dc9d 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -3,14 +3,14 @@
 # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
 
 rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
-		rockchip_drm_gem.o rockchip_drm_psr.o rockchip_drm_vop.o
+		rockchip_drm_gem.o rockchip_drm_psr.o \
+		rockchip_drm_vop.o rockchip_vop_reg.o
 rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
 
-obj-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
-obj-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp.o
-cdn-dp-objs := cdn-dp-core.o cdn-dp-reg.o
-obj-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
-obj-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
-obj-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
+rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
+rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
+rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
+rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
+rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
 
-obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o rockchip_vop_reg.o
+obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o
diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 8548e82..91ebe5c 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -507,7 +507,7 @@ static const struct of_device_id rockchip_dp_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, rockchip_dp_dt_ids);
 
-static struct platform_driver rockchip_dp_driver = {
+struct platform_driver rockchip_dp_driver = {
 	.probe = rockchip_dp_probe,
 	.remove = rockchip_dp_remove,
 	.driver = {
@@ -516,10 +516,3 @@ static struct platform_driver rockchip_dp_driver = {
 		   .of_match_table = of_match_ptr(rockchip_dp_dt_ids),
 	},
 };
-
-module_platform_driver(rockchip_dp_driver);
-
-MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
-MODULE_AUTHOR("Jeff chen <jeff.chen@rock-chips.com>");
-MODULE_DESCRIPTION("Rockchip Specific Analogix-DP Driver Extension");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index fd79a70..e1796d0 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -1243,7 +1243,7 @@ static const struct dev_pm_ops cdn_dp_pm_ops = {
 				cdn_dp_resume)
 };
 
-static struct platform_driver cdn_dp_driver = {
+struct platform_driver cdn_dp_driver = {
 	.probe = cdn_dp_probe,
 	.remove = cdn_dp_remove,
 	.shutdown = cdn_dp_shutdown,
@@ -1254,9 +1254,3 @@ static struct platform_driver cdn_dp_driver = {
 		   .pm = &cdn_dp_pm_ops,
 	},
 };
-
-module_platform_driver(cdn_dp_driver);
-
-MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
-MODULE_DESCRIPTION("cdn DP Driver");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
index d9aa382..65a474f 100644
--- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
@@ -1165,7 +1165,7 @@ static int dw_mipi_dsi_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver dw_mipi_dsi_driver = {
+struct platform_driver dw_mipi_dsi_driver = {
 	.probe		= dw_mipi_dsi_probe,
 	.remove		= dw_mipi_dsi_remove,
 	.driver		= {
@@ -1173,9 +1173,3 @@ static struct platform_driver dw_mipi_dsi_driver = {
 		.name	= DRIVER_NAME,
 	},
 };
-module_platform_driver(dw_mipi_dsi_driver);
-
-MODULE_DESCRIPTION("ROCKCHIP MIPI DSI host controller driver");
-MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:" DRIVER_NAME);
diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
index a6d4a02..976ea79 100644
--- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
+++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
@@ -326,7 +326,7 @@ static int dw_hdmi_rockchip_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
+struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
 	.probe  = dw_hdmi_rockchip_probe,
 	.remove = dw_hdmi_rockchip_remove,
 	.driver = {
@@ -334,11 +334,3 @@ static struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
 		.of_match_table = dw_hdmi_rockchip_dt_ids,
 	},
 };
-
-module_platform_driver(dw_hdmi_rockchip_pltfm_driver);
-
-MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com>");
-MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
-MODULE_DESCRIPTION("Rockchip Specific DW-HDMI Driver Extension");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:dwhdmi-rockchip");
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 006260d..7d9b75e 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -923,7 +923,7 @@ static const struct of_device_id inno_hdmi_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, inno_hdmi_dt_ids);
 
-static struct platform_driver inno_hdmi_driver = {
+struct platform_driver inno_hdmi_driver = {
 	.probe  = inno_hdmi_probe,
 	.remove = inno_hdmi_remove,
 	.driver = {
@@ -931,11 +931,3 @@ static struct platform_driver inno_hdmi_driver = {
 		.of_match_table = inno_hdmi_dt_ids,
 	},
 };
-
-module_platform_driver(inno_hdmi_driver);
-
-MODULE_AUTHOR("Zheng Yang <zhengyang@rock-chips.com>");
-MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
-MODULE_DESCRIPTION("Rockchip Specific INNO-HDMI Driver");
-MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:innohdmi-rockchip");
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index b360e62..b6bccfc 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -356,34 +356,52 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
 				rockchip_drm_sys_resume)
 };
 
-static int compare_of(struct device *dev, void *data)
-{
-	struct device_node *np = data;
+static struct platform_driver *rockchip_drm_comp_drvs[] = {
+	&vop_platform_driver,
+#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
+	&rockchip_dp_driver,
+#endif
+#ifdef CONFIG_ROCKCHIP_CDN_DP
+	&cdn_dp_driver,
+#endif
+#ifdef CONFIG_ROCKCHIP_DW_HDMI
+	&dw_hdmi_rockchip_pltfm_driver,
+#endif
+#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
+	&dw_mipi_dsi_driver,
+#endif
+#ifdef CONFIG_ROCKCHIP_INNO_HDMI
+	&inno_hdmi_driver,
+#endif
+};
 
-	return dev->of_node == np;
+static int compare_dev(struct device *dev, void *data)
+{
+	return dev == (struct device *)data;
 }
 
-static void rockchip_add_endpoints(struct device *dev,
-				   struct component_match **match,
-				   struct device_node *port)
+static struct component_match *rockchip_drm_match_add(struct device *dev)
 {
-	struct device_node *ep, *remote;
+	struct component_match *match = NULL;
+	int i;
 
-	for_each_child_of_node(port, ep) {
-		remote = of_graph_get_remote_port_parent(ep);
-		if (!remote || !of_device_is_available(remote)) {
-			of_node_put(remote);
-			continue;
-		} else if (!of_device_is_available(remote->parent)) {
-			dev_warn(dev, "parent device of %s is not available\n",
-				 remote->full_name);
-			of_node_put(remote);
-			continue;
-		}
+	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
+		struct platform_driver *drv = rockchip_drm_comp_drvs[i];
+		struct device *p = NULL, *d;
 
-		drm_of_component_match_add(dev, match, compare_of, remote);
-		of_node_put(remote);
+		do {
+			d = bus_find_device(&platform_bus_type, p, &drv->driver,
+					    (void *)platform_bus_type.match);
+			put_device(p);
+			p = d;
+
+			if (!d)
+				break;
+			component_match_add(dev, &match, compare_dev, d);
+		} while (true);
 	}
+
+	return match ?: ERR_PTR(-ENODEV);
 }
 
 static const struct component_master_ops rockchip_drm_ops = {
@@ -391,21 +409,16 @@ static const struct component_master_ops rockchip_drm_ops = {
 	.unbind = rockchip_drm_unbind,
 };
 
-static int rockchip_drm_platform_probe(struct platform_device *pdev)
+static int rockchip_drm_platform_of_probe(struct device *dev)
 {
-	struct device *dev = &pdev->dev;
-	struct component_match *match = NULL;
 	struct device_node *np = dev->of_node;
 	struct device_node *port;
+	bool found = false;
 	int i;
 
 	if (!np)
 		return -ENODEV;
-	/*
-	 * Bind the crtc ports first, so that
-	 * drm_of_find_possible_crtcs called from encoder .bind callbacks
-	 * works as expected.
-	 */
+
 	for (i = 0;; i++) {
 		struct device_node *iommu;
 
@@ -429,9 +442,9 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
 			is_support_iommu = false;
 		}
 
+		found = true;
+
 		of_node_put(iommu);
-		drm_of_component_match_add(dev, &match, compare_of,
-					   port->parent);
 		of_node_put(port);
 	}
 
@@ -440,27 +453,27 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	if (!match) {
+	if (!found) {
 		dev_err(dev, "No available vop found for display-subsystem.\n");
 		return -ENODEV;
 	}
-	/*
-	 * For each bound crtc, bind the encoders attached to its
-	 * remote endpoint.
-	 */
-	for (i = 0;; i++) {
-		port = of_parse_phandle(np, "ports", i);
-		if (!port)
-			break;
 
-		if (!of_device_is_available(port->parent)) {
-			of_node_put(port);
-			continue;
-		}
+	return 0;
+}
 
-		rockchip_add_endpoints(dev, &match, port);
-		of_node_put(port);
-	}
+static int rockchip_drm_platform_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct component_match *match = NULL;
+	int ret;
+
+	ret = rockchip_drm_platform_of_probe(dev);
+	if (ret)
+		return ret;
+
+	match = rockchip_drm_match_add(dev);
+	if (IS_ERR(match))
+		return PTR_ERR(match);
 
 	return component_master_add_with_match(dev, &rockchip_drm_ops, match);
 }
@@ -488,7 +501,60 @@ static struct platform_driver rockchip_drm_platform_driver = {
 	},
 };
 
-module_platform_driver(rockchip_drm_platform_driver);
+static void rockchip_drm_unregister_drivers(void)
+{
+	int i;
+
+	for (i = ARRAY_SIZE(rockchip_drm_comp_drvs) - 1; i >= 0; i--)
+		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
+}
+
+static int rockchip_drm_register_drivers(void)
+{
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
+		ret = platform_driver_register(rockchip_drm_comp_drvs[i]);
+		if (ret)
+			goto err_unreg;
+	}
+
+	return 0;
+
+err_unreg:
+	for (; i--; )
+		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
+	return ret;
+}
+
+static int __init rockchip_drm_init(void)
+{
+	int ret;
+
+	ret = rockchip_drm_register_drivers();
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&rockchip_drm_platform_driver);
+	if (ret)
+		goto err_unreg_drivers;
+
+	return 0;
+
+err_unreg_drivers:
+	rockchip_drm_unregister_drivers();
+	return ret;
+}
+
+static void __exit rockchip_drm_fini(void)
+{
+	platform_driver_unregister(&rockchip_drm_platform_driver);
+
+	rockchip_drm_unregister_drivers();
+}
+
+module_init(rockchip_drm_init);
+module_exit(rockchip_drm_fini);
 
 MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
 MODULE_DESCRIPTION("ROCKCHIP DRM Driver");
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index adc3930..91812bd 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -79,4 +79,20 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
 				unsigned int mstimeout);
 
+#ifdef CONFIG_ROCKCHIP_CDN_DP
+extern struct platform_driver cdn_dp_driver;
+#endif
+#ifdef CONFIG_ROCKCHIP_DW_HDMI
+extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
+#endif
+#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
+extern struct platform_driver dw_mipi_dsi_driver;
+#endif
+#ifdef CONFIG_ROCKCHIP_INNO_HDMI
+extern struct platform_driver inno_hdmi_driver;
+#endif
+#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
+extern struct platform_driver rockchip_dp_driver;
+#endif
+extern struct platform_driver vop_platform_driver;
 #endif /* _ROCKCHIP_DRM_DRV_H_ */
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 91fbc7b..0da4444 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -404,7 +404,7 @@ static int vop_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver vop_platform_driver = {
+struct platform_driver vop_platform_driver = {
 	.probe = vop_probe,
 	.remove = vop_remove,
 	.driver = {
@@ -412,9 +412,3 @@ static struct platform_driver vop_platform_driver = {
 		.of_match_table = of_match_ptr(vop_driver_dt_match),
 	},
 };
-
-module_platform_driver(vop_platform_driver);
-
-MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
-MODULE_DESCRIPTION("ROCKCHIP VOP Driver");
-MODULE_LICENSE("GPL v2");
-- 
2.1.4

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

* Re: [PATCH v2] drm/rockchip: Refactor the component match logic.
  2017-03-14 10:45   ` Jeffy Chen
  (?)
@ 2017-03-14 12:05     ` Andrzej Hajda
  -1 siblings, 0 replies; 10+ messages in thread
From: Andrzej Hajda @ 2017-03-14 12:05 UTC (permalink / raw)
  To: Jeffy Chen, Mark Yao
  Cc: linux-kernel, Brian Norris, Douglas Anderson, dri-devel,
	Tomasz Figa, linux-rockchip, Guenter Roeck, linux-arm-kernel

Hi Jeffy,

On 14.03.2017 11:45, Jeffy Chen wrote:
> Currently we are adding all components from the dts, if one of their
> drivers been disabled, we would not be able to bring up others.
> 
> Refactor component match logic, follow exynos drm.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>

Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

Below few nitpicks.

(...)

> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index b360e62..b6bccfc 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -356,34 +356,52 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
>  				rockchip_drm_sys_resume)
>  };
>  
> -static int compare_of(struct device *dev, void *data)
> -{
> -	struct device_node *np = data;
> +static struct platform_driver *rockchip_drm_comp_drvs[] = {
> +	&vop_platform_driver,
> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
> +	&rockchip_dp_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_CDN_DP
> +	&cdn_dp_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
> +	&dw_hdmi_rockchip_pltfm_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
> +	&dw_mipi_dsi_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
> +	&inno_hdmi_driver,
> +#endif
> +};

To avoid #ifdefs, you can use DRV_PTR [1], but it depends what do you
hate more? ifdefs or macro magic :)

[1]:
http://lxr.free-electrons.com/source/drivers/gpu/drm/exynos/exynos_drm_drv.c#L472

>  
> -	return dev->of_node == np;
> +static int compare_dev(struct device *dev, void *data)
> +{
> +	return dev == (struct device *)data;
>  }
>  
> -static void rockchip_add_endpoints(struct device *dev,
> -				   struct component_match **match,
> -				   struct device_node *port)
> +static struct component_match *rockchip_drm_match_add(struct device *dev)
>  {
> -	struct device_node *ep, *remote;
> +	struct component_match *match = NULL;
> +	int i;
>  
> -	for_each_child_of_node(port, ep) {
> -		remote = of_graph_get_remote_port_parent(ep);
> -		if (!remote || !of_device_is_available(remote)) {
> -			of_node_put(remote);
> -			continue;
> -		} else if (!of_device_is_available(remote->parent)) {
> -			dev_warn(dev, "parent device of %s is not available\n",
> -				 remote->full_name);
> -			of_node_put(remote);
> -			continue;
> -		}
> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
> +		struct platform_driver *drv = rockchip_drm_comp_drvs[i];
> +		struct device *p = NULL, *d;
>  
> -		drm_of_component_match_add(dev, match, compare_of, remote);
> -		of_node_put(remote);
> +		do {
> +			d = bus_find_device(&platform_bus_type, p, &drv->driver,
> +					    (void *)platform_bus_type.match);
> +			put_device(p);
> +			p = d;
> +
> +			if (!d)
> +				break;
> +			component_match_add(dev, &match, compare_dev, d);
> +		} while (true);
>  	}
> +
> +	return match ?: ERR_PTR(-ENODEV);
>  }
>  
>  static const struct component_master_ops rockchip_drm_ops = {
> @@ -391,21 +409,16 @@ static const struct component_master_ops rockchip_drm_ops = {
>  	.unbind = rockchip_drm_unbind,
>  };
>  
> -static int rockchip_drm_platform_probe(struct platform_device *pdev)
> +static int rockchip_drm_platform_of_probe(struct device *dev)
>  {
> -	struct device *dev = &pdev->dev;
> -	struct component_match *match = NULL;
>  	struct device_node *np = dev->of_node;
>  	struct device_node *port;
> +	bool found = false;
>  	int i;
>  
>  	if (!np)
>  		return -ENODEV;
> -	/*
> -	 * Bind the crtc ports first, so that
> -	 * drm_of_find_possible_crtcs called from encoder .bind callbacks
> -	 * works as expected.
> -	 */
> +
>  	for (i = 0;; i++) {
>  		struct device_node *iommu;
>  
> @@ -429,9 +442,9 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>  			is_support_iommu = false;
>  		}
>  
> +		found = true;
> +
>  		of_node_put(iommu);
> -		drm_of_component_match_add(dev, &match, compare_of,
> -					   port->parent);
>  		of_node_put(port);
>  	}
>  
> @@ -440,27 +453,27 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> -	if (!match) {
> +	if (!found) {
>  		dev_err(dev, "No available vop found for display-subsystem.\n");
>  		return -ENODEV;
>  	}
> -	/*
> -	 * For each bound crtc, bind the encoders attached to its
> -	 * remote endpoint.
> -	 */
> -	for (i = 0;; i++) {
> -		port = of_parse_phandle(np, "ports", i);
> -		if (!port)
> -			break;
>  
> -		if (!of_device_is_available(port->parent)) {
> -			of_node_put(port);
> -			continue;
> -		}
> +	return 0;
> +}
>  
> -		rockchip_add_endpoints(dev, &match, port);
> -		of_node_put(port);
> -	}
> +static int rockchip_drm_platform_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct component_match *match = NULL;
> +	int ret;
> +
> +	ret = rockchip_drm_platform_of_probe(dev);
> +	if (ret)
> +		return ret;
> +
> +	match = rockchip_drm_match_add(dev);
> +	if (IS_ERR(match))
> +		return PTR_ERR(match);
>  
>  	return component_master_add_with_match(dev, &rockchip_drm_ops, match);
>  }
> @@ -488,7 +501,60 @@ static struct platform_driver rockchip_drm_platform_driver = {
>  	},
>  };
>  
> -module_platform_driver(rockchip_drm_platform_driver);
> +static void rockchip_drm_unregister_drivers(void)
> +{
> +	int i;
> +
> +	for (i = ARRAY_SIZE(rockchip_drm_comp_drvs) - 1; i >= 0; i--)
> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
> +}
> +
> +static int rockchip_drm_register_drivers(void)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
> +		ret = platform_driver_register(rockchip_drm_comp_drvs[i]);
> +		if (ret)
> +			goto err_unreg;
> +	}
> +
> +	return 0;
> +
> +err_unreg:
> +	for (; i--; )
> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);

Strange placement of decremental in for, maybe sth like this would be
better:
	while (i-- > 0)
		platform_driver_unregister(rockchip_drm_comp_drvs[i]);


> +	return ret;
> +}
> +
> +static int __init rockchip_drm_init(void)
> +{
> +	int ret;
> +
> +	ret = rockchip_drm_register_drivers();
> +	if (ret)
> +		return ret;
> +
> +	ret = platform_driver_register(&rockchip_drm_platform_driver);
> +	if (ret)
> +		goto err_unreg_drivers;
> +
> +	return 0;
> +
> +err_unreg_drivers:
> +	rockchip_drm_unregister_drivers();
> +	return ret;
> +}
> +
> +static void __exit rockchip_drm_fini(void)
> +{
> +	platform_driver_unregister(&rockchip_drm_platform_driver);
> +
> +	rockchip_drm_unregister_drivers();
> +}
> +
> +module_init(rockchip_drm_init);
> +module_exit(rockchip_drm_fini);
>  
>  MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
>  MODULE_DESCRIPTION("ROCKCHIP DRM Driver");
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index adc3930..91812bd 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -79,4 +79,20 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>  int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
>  				unsigned int mstimeout);
>  
> +#ifdef CONFIG_ROCKCHIP_CDN_DP
> +extern struct platform_driver cdn_dp_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
> +extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
> +extern struct platform_driver dw_mipi_dsi_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
> +extern struct platform_driver inno_hdmi_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
> +extern struct platform_driver rockchip_dp_driver;
> +#endif
> +extern struct platform_driver vop_platform_driver;
>  #endif /* _ROCKCHIP_DRM_DRV_H_ */

I think gcc should be fine if you remove ifdefs. Unused extern
declarations are ignored, unless you use some paranoic static checker,
aren't they? But this is also matter of taste.


Regards
Andrzej


> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 91fbc7b..0da4444 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -404,7 +404,7 @@ static int vop_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct platform_driver vop_platform_driver = {
> +struct platform_driver vop_platform_driver = {
>  	.probe = vop_probe,
>  	.remove = vop_remove,
>  	.driver = {
> @@ -412,9 +412,3 @@ static struct platform_driver vop_platform_driver = {
>  		.of_match_table = of_match_ptr(vop_driver_dt_match),
>  	},
>  };
> -
> -module_platform_driver(vop_platform_driver);
> -
> -MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
> -MODULE_DESCRIPTION("ROCKCHIP VOP Driver");
> -MODULE_LICENSE("GPL v2");
> 

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

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

* Re: [PATCH v2] drm/rockchip: Refactor the component match logic.
@ 2017-03-14 12:05     ` Andrzej Hajda
  0 siblings, 0 replies; 10+ messages in thread
From: Andrzej Hajda @ 2017-03-14 12:05 UTC (permalink / raw)
  To: Jeffy Chen, Mark Yao
  Cc: Douglas Anderson, Guenter Roeck, Brian Norris, Sean Paul,
	Tomasz Figa, Heiko Stuebner, linux-kernel, dri-devel,
	linux-rockchip, David Airlie, linux-arm-kernel

Hi Jeffy,

On 14.03.2017 11:45, Jeffy Chen wrote:
> Currently we are adding all components from the dts, if one of their
> drivers been disabled, we would not be able to bring up others.
> 
> Refactor component match logic, follow exynos drm.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>

Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

Below few nitpicks.

(...)

> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index b360e62..b6bccfc 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -356,34 +356,52 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
>  				rockchip_drm_sys_resume)
>  };
>  
> -static int compare_of(struct device *dev, void *data)
> -{
> -	struct device_node *np = data;
> +static struct platform_driver *rockchip_drm_comp_drvs[] = {
> +	&vop_platform_driver,
> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
> +	&rockchip_dp_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_CDN_DP
> +	&cdn_dp_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
> +	&dw_hdmi_rockchip_pltfm_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
> +	&dw_mipi_dsi_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
> +	&inno_hdmi_driver,
> +#endif
> +};

To avoid #ifdefs, you can use DRV_PTR [1], but it depends what do you
hate more? ifdefs or macro magic :)

[1]:
http://lxr.free-electrons.com/source/drivers/gpu/drm/exynos/exynos_drm_drv.c#L472

>  
> -	return dev->of_node == np;
> +static int compare_dev(struct device *dev, void *data)
> +{
> +	return dev == (struct device *)data;
>  }
>  
> -static void rockchip_add_endpoints(struct device *dev,
> -				   struct component_match **match,
> -				   struct device_node *port)
> +static struct component_match *rockchip_drm_match_add(struct device *dev)
>  {
> -	struct device_node *ep, *remote;
> +	struct component_match *match = NULL;
> +	int i;
>  
> -	for_each_child_of_node(port, ep) {
> -		remote = of_graph_get_remote_port_parent(ep);
> -		if (!remote || !of_device_is_available(remote)) {
> -			of_node_put(remote);
> -			continue;
> -		} else if (!of_device_is_available(remote->parent)) {
> -			dev_warn(dev, "parent device of %s is not available\n",
> -				 remote->full_name);
> -			of_node_put(remote);
> -			continue;
> -		}
> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
> +		struct platform_driver *drv = rockchip_drm_comp_drvs[i];
> +		struct device *p = NULL, *d;
>  
> -		drm_of_component_match_add(dev, match, compare_of, remote);
> -		of_node_put(remote);
> +		do {
> +			d = bus_find_device(&platform_bus_type, p, &drv->driver,
> +					    (void *)platform_bus_type.match);
> +			put_device(p);
> +			p = d;
> +
> +			if (!d)
> +				break;
> +			component_match_add(dev, &match, compare_dev, d);
> +		} while (true);
>  	}
> +
> +	return match ?: ERR_PTR(-ENODEV);
>  }
>  
>  static const struct component_master_ops rockchip_drm_ops = {
> @@ -391,21 +409,16 @@ static const struct component_master_ops rockchip_drm_ops = {
>  	.unbind = rockchip_drm_unbind,
>  };
>  
> -static int rockchip_drm_platform_probe(struct platform_device *pdev)
> +static int rockchip_drm_platform_of_probe(struct device *dev)
>  {
> -	struct device *dev = &pdev->dev;
> -	struct component_match *match = NULL;
>  	struct device_node *np = dev->of_node;
>  	struct device_node *port;
> +	bool found = false;
>  	int i;
>  
>  	if (!np)
>  		return -ENODEV;
> -	/*
> -	 * Bind the crtc ports first, so that
> -	 * drm_of_find_possible_crtcs called from encoder .bind callbacks
> -	 * works as expected.
> -	 */
> +
>  	for (i = 0;; i++) {
>  		struct device_node *iommu;
>  
> @@ -429,9 +442,9 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>  			is_support_iommu = false;
>  		}
>  
> +		found = true;
> +
>  		of_node_put(iommu);
> -		drm_of_component_match_add(dev, &match, compare_of,
> -					   port->parent);
>  		of_node_put(port);
>  	}
>  
> @@ -440,27 +453,27 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> -	if (!match) {
> +	if (!found) {
>  		dev_err(dev, "No available vop found for display-subsystem.\n");
>  		return -ENODEV;
>  	}
> -	/*
> -	 * For each bound crtc, bind the encoders attached to its
> -	 * remote endpoint.
> -	 */
> -	for (i = 0;; i++) {
> -		port = of_parse_phandle(np, "ports", i);
> -		if (!port)
> -			break;
>  
> -		if (!of_device_is_available(port->parent)) {
> -			of_node_put(port);
> -			continue;
> -		}
> +	return 0;
> +}
>  
> -		rockchip_add_endpoints(dev, &match, port);
> -		of_node_put(port);
> -	}
> +static int rockchip_drm_platform_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct component_match *match = NULL;
> +	int ret;
> +
> +	ret = rockchip_drm_platform_of_probe(dev);
> +	if (ret)
> +		return ret;
> +
> +	match = rockchip_drm_match_add(dev);
> +	if (IS_ERR(match))
> +		return PTR_ERR(match);
>  
>  	return component_master_add_with_match(dev, &rockchip_drm_ops, match);
>  }
> @@ -488,7 +501,60 @@ static struct platform_driver rockchip_drm_platform_driver = {
>  	},
>  };
>  
> -module_platform_driver(rockchip_drm_platform_driver);
> +static void rockchip_drm_unregister_drivers(void)
> +{
> +	int i;
> +
> +	for (i = ARRAY_SIZE(rockchip_drm_comp_drvs) - 1; i >= 0; i--)
> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
> +}
> +
> +static int rockchip_drm_register_drivers(void)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
> +		ret = platform_driver_register(rockchip_drm_comp_drvs[i]);
> +		if (ret)
> +			goto err_unreg;
> +	}
> +
> +	return 0;
> +
> +err_unreg:
> +	for (; i--; )
> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);

Strange placement of decremental in for, maybe sth like this would be
better:
	while (i-- > 0)
		platform_driver_unregister(rockchip_drm_comp_drvs[i]);


> +	return ret;
> +}
> +
> +static int __init rockchip_drm_init(void)
> +{
> +	int ret;
> +
> +	ret = rockchip_drm_register_drivers();
> +	if (ret)
> +		return ret;
> +
> +	ret = platform_driver_register(&rockchip_drm_platform_driver);
> +	if (ret)
> +		goto err_unreg_drivers;
> +
> +	return 0;
> +
> +err_unreg_drivers:
> +	rockchip_drm_unregister_drivers();
> +	return ret;
> +}
> +
> +static void __exit rockchip_drm_fini(void)
> +{
> +	platform_driver_unregister(&rockchip_drm_platform_driver);
> +
> +	rockchip_drm_unregister_drivers();
> +}
> +
> +module_init(rockchip_drm_init);
> +module_exit(rockchip_drm_fini);
>  
>  MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
>  MODULE_DESCRIPTION("ROCKCHIP DRM Driver");
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index adc3930..91812bd 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -79,4 +79,20 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>  int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
>  				unsigned int mstimeout);
>  
> +#ifdef CONFIG_ROCKCHIP_CDN_DP
> +extern struct platform_driver cdn_dp_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
> +extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
> +extern struct platform_driver dw_mipi_dsi_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
> +extern struct platform_driver inno_hdmi_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
> +extern struct platform_driver rockchip_dp_driver;
> +#endif
> +extern struct platform_driver vop_platform_driver;
>  #endif /* _ROCKCHIP_DRM_DRV_H_ */

I think gcc should be fine if you remove ifdefs. Unused extern
declarations are ignored, unless you use some paranoic static checker,
aren't they? But this is also matter of taste.


Regards
Andrzej


> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 91fbc7b..0da4444 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -404,7 +404,7 @@ static int vop_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct platform_driver vop_platform_driver = {
> +struct platform_driver vop_platform_driver = {
>  	.probe = vop_probe,
>  	.remove = vop_remove,
>  	.driver = {
> @@ -412,9 +412,3 @@ static struct platform_driver vop_platform_driver = {
>  		.of_match_table = of_match_ptr(vop_driver_dt_match),
>  	},
>  };
> -
> -module_platform_driver(vop_platform_driver);
> -
> -MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
> -MODULE_DESCRIPTION("ROCKCHIP VOP Driver");
> -MODULE_LICENSE("GPL v2");
> 

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

* [PATCH v2] drm/rockchip: Refactor the component match logic.
@ 2017-03-14 12:05     ` Andrzej Hajda
  0 siblings, 0 replies; 10+ messages in thread
From: Andrzej Hajda @ 2017-03-14 12:05 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Jeffy,

On 14.03.2017 11:45, Jeffy Chen wrote:
> Currently we are adding all components from the dts, if one of their
> drivers been disabled, we would not be able to bring up others.
> 
> Refactor component match logic, follow exynos drm.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>

Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

Below few nitpicks.

(...)

> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index b360e62..b6bccfc 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -356,34 +356,52 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
>  				rockchip_drm_sys_resume)
>  };
>  
> -static int compare_of(struct device *dev, void *data)
> -{
> -	struct device_node *np = data;
> +static struct platform_driver *rockchip_drm_comp_drvs[] = {
> +	&vop_platform_driver,
> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
> +	&rockchip_dp_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_CDN_DP
> +	&cdn_dp_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
> +	&dw_hdmi_rockchip_pltfm_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
> +	&dw_mipi_dsi_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
> +	&inno_hdmi_driver,
> +#endif
> +};

To avoid #ifdefs, you can use DRV_PTR [1], but it depends what do you
hate more? ifdefs or macro magic :)

[1]:
http://lxr.free-electrons.com/source/drivers/gpu/drm/exynos/exynos_drm_drv.c#L472

>  
> -	return dev->of_node == np;
> +static int compare_dev(struct device *dev, void *data)
> +{
> +	return dev == (struct device *)data;
>  }
>  
> -static void rockchip_add_endpoints(struct device *dev,
> -				   struct component_match **match,
> -				   struct device_node *port)
> +static struct component_match *rockchip_drm_match_add(struct device *dev)
>  {
> -	struct device_node *ep, *remote;
> +	struct component_match *match = NULL;
> +	int i;
>  
> -	for_each_child_of_node(port, ep) {
> -		remote = of_graph_get_remote_port_parent(ep);
> -		if (!remote || !of_device_is_available(remote)) {
> -			of_node_put(remote);
> -			continue;
> -		} else if (!of_device_is_available(remote->parent)) {
> -			dev_warn(dev, "parent device of %s is not available\n",
> -				 remote->full_name);
> -			of_node_put(remote);
> -			continue;
> -		}
> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
> +		struct platform_driver *drv = rockchip_drm_comp_drvs[i];
> +		struct device *p = NULL, *d;
>  
> -		drm_of_component_match_add(dev, match, compare_of, remote);
> -		of_node_put(remote);
> +		do {
> +			d = bus_find_device(&platform_bus_type, p, &drv->driver,
> +					    (void *)platform_bus_type.match);
> +			put_device(p);
> +			p = d;
> +
> +			if (!d)
> +				break;
> +			component_match_add(dev, &match, compare_dev, d);
> +		} while (true);
>  	}
> +
> +	return match ?: ERR_PTR(-ENODEV);
>  }
>  
>  static const struct component_master_ops rockchip_drm_ops = {
> @@ -391,21 +409,16 @@ static const struct component_master_ops rockchip_drm_ops = {
>  	.unbind = rockchip_drm_unbind,
>  };
>  
> -static int rockchip_drm_platform_probe(struct platform_device *pdev)
> +static int rockchip_drm_platform_of_probe(struct device *dev)
>  {
> -	struct device *dev = &pdev->dev;
> -	struct component_match *match = NULL;
>  	struct device_node *np = dev->of_node;
>  	struct device_node *port;
> +	bool found = false;
>  	int i;
>  
>  	if (!np)
>  		return -ENODEV;
> -	/*
> -	 * Bind the crtc ports first, so that
> -	 * drm_of_find_possible_crtcs called from encoder .bind callbacks
> -	 * works as expected.
> -	 */
> +
>  	for (i = 0;; i++) {
>  		struct device_node *iommu;
>  
> @@ -429,9 +442,9 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>  			is_support_iommu = false;
>  		}
>  
> +		found = true;
> +
>  		of_node_put(iommu);
> -		drm_of_component_match_add(dev, &match, compare_of,
> -					   port->parent);
>  		of_node_put(port);
>  	}
>  
> @@ -440,27 +453,27 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> -	if (!match) {
> +	if (!found) {
>  		dev_err(dev, "No available vop found for display-subsystem.\n");
>  		return -ENODEV;
>  	}
> -	/*
> -	 * For each bound crtc, bind the encoders attached to its
> -	 * remote endpoint.
> -	 */
> -	for (i = 0;; i++) {
> -		port = of_parse_phandle(np, "ports", i);
> -		if (!port)
> -			break;
>  
> -		if (!of_device_is_available(port->parent)) {
> -			of_node_put(port);
> -			continue;
> -		}
> +	return 0;
> +}
>  
> -		rockchip_add_endpoints(dev, &match, port);
> -		of_node_put(port);
> -	}
> +static int rockchip_drm_platform_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct component_match *match = NULL;
> +	int ret;
> +
> +	ret = rockchip_drm_platform_of_probe(dev);
> +	if (ret)
> +		return ret;
> +
> +	match = rockchip_drm_match_add(dev);
> +	if (IS_ERR(match))
> +		return PTR_ERR(match);
>  
>  	return component_master_add_with_match(dev, &rockchip_drm_ops, match);
>  }
> @@ -488,7 +501,60 @@ static struct platform_driver rockchip_drm_platform_driver = {
>  	},
>  };
>  
> -module_platform_driver(rockchip_drm_platform_driver);
> +static void rockchip_drm_unregister_drivers(void)
> +{
> +	int i;
> +
> +	for (i = ARRAY_SIZE(rockchip_drm_comp_drvs) - 1; i >= 0; i--)
> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
> +}
> +
> +static int rockchip_drm_register_drivers(void)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
> +		ret = platform_driver_register(rockchip_drm_comp_drvs[i]);
> +		if (ret)
> +			goto err_unreg;
> +	}
> +
> +	return 0;
> +
> +err_unreg:
> +	for (; i--; )
> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);

Strange placement of decremental in for, maybe sth like this would be
better:
	while (i-- > 0)
		platform_driver_unregister(rockchip_drm_comp_drvs[i]);


> +	return ret;
> +}
> +
> +static int __init rockchip_drm_init(void)
> +{
> +	int ret;
> +
> +	ret = rockchip_drm_register_drivers();
> +	if (ret)
> +		return ret;
> +
> +	ret = platform_driver_register(&rockchip_drm_platform_driver);
> +	if (ret)
> +		goto err_unreg_drivers;
> +
> +	return 0;
> +
> +err_unreg_drivers:
> +	rockchip_drm_unregister_drivers();
> +	return ret;
> +}
> +
> +static void __exit rockchip_drm_fini(void)
> +{
> +	platform_driver_unregister(&rockchip_drm_platform_driver);
> +
> +	rockchip_drm_unregister_drivers();
> +}
> +
> +module_init(rockchip_drm_init);
> +module_exit(rockchip_drm_fini);
>  
>  MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
>  MODULE_DESCRIPTION("ROCKCHIP DRM Driver");
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index adc3930..91812bd 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -79,4 +79,20 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>  int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
>  				unsigned int mstimeout);
>  
> +#ifdef CONFIG_ROCKCHIP_CDN_DP
> +extern struct platform_driver cdn_dp_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
> +extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
> +extern struct platform_driver dw_mipi_dsi_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
> +extern struct platform_driver inno_hdmi_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
> +extern struct platform_driver rockchip_dp_driver;
> +#endif
> +extern struct platform_driver vop_platform_driver;
>  #endif /* _ROCKCHIP_DRM_DRV_H_ */

I think gcc should be fine if you remove ifdefs. Unused extern
declarations are ignored, unless you use some paranoic static checker,
aren't they? But this is also matter of taste.


Regards
Andrzej


> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 91fbc7b..0da4444 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -404,7 +404,7 @@ static int vop_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct platform_driver vop_platform_driver = {
> +struct platform_driver vop_platform_driver = {
>  	.probe = vop_probe,
>  	.remove = vop_remove,
>  	.driver = {
> @@ -412,9 +412,3 @@ static struct platform_driver vop_platform_driver = {
>  		.of_match_table = of_match_ptr(vop_driver_dt_match),
>  	},
>  };
> -
> -module_platform_driver(vop_platform_driver);
> -
> -MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
> -MODULE_DESCRIPTION("ROCKCHIP VOP Driver");
> -MODULE_LICENSE("GPL v2");
> 

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

* Re: [PATCH v2] drm/rockchip: Refactor the component match logic.
  2017-03-14 12:05     ` Andrzej Hajda
@ 2017-03-15 10:25       ` jeffy
  -1 siblings, 0 replies; 10+ messages in thread
From: jeffy @ 2017-03-15 10:25 UTC (permalink / raw)
  To: Andrzej Hajda, Mark Yao
  Cc: Douglas Anderson, Guenter Roeck, Brian Norris, Sean Paul,
	Tomasz Figa, Heiko Stuebner, linux-kernel, dri-devel,
	linux-rockchip, David Airlie, linux-arm-kernel

Hi Andrzej,


On 03/14/2017 08:05 PM, Andrzej Hajda wrote:
> Hi Jeffy,
>
> On 14.03.2017 11:45, Jeffy Chen wrote:
>> Currently we are adding all components from the dts, if one of their
>> drivers been disabled, we would not be able to bring up others.
>>
>> Refactor component match logic, follow exynos drm.
>>
>> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
>
> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
>
> Below few nitpicks.
>
> (...)
>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> index b360e62..b6bccfc 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> @@ -356,34 +356,52 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
>>   				rockchip_drm_sys_resume)
>>   };
>>
>> -static int compare_of(struct device *dev, void *data)
>> -{
>> -	struct device_node *np = data;
>> +static struct platform_driver *rockchip_drm_comp_drvs[] = {
>> +	&vop_platform_driver,
>> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
>> +	&rockchip_dp_driver,
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_CDN_DP
>> +	&cdn_dp_driver,
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
>> +	&dw_hdmi_rockchip_pltfm_driver,
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
>> +	&dw_mipi_dsi_driver,
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
>> +	&inno_hdmi_driver,
>> +#endif
>> +};
>
> To avoid #ifdefs, you can use DRV_PTR [1], but it depends what do you
> hate more? ifdefs or macro magic :)
>
> [1]:
> http://lxr.free-electrons.com/source/drivers/gpu/drm/exynos/exynos_drm_drv.c#L472
>
Done.
>>
>> -	return dev->of_node == np;
>> +static int compare_dev(struct device *dev, void *data)
>> +{
>> +	return dev == (struct device *)data;
>>   }
>>
>> -static void rockchip_add_endpoints(struct device *dev,
>> -				   struct component_match **match,
>> -				   struct device_node *port)
>> +static struct component_match *rockchip_drm_match_add(struct device *dev)
>>   {
>> -	struct device_node *ep, *remote;
>> +	struct component_match *match = NULL;
>> +	int i;
>>
>> -	for_each_child_of_node(port, ep) {
>> -		remote = of_graph_get_remote_port_parent(ep);
>> -		if (!remote || !of_device_is_available(remote)) {
>> -			of_node_put(remote);
>> -			continue;
>> -		} else if (!of_device_is_available(remote->parent)) {
>> -			dev_warn(dev, "parent device of %s is not available\n",
>> -				 remote->full_name);
>> -			of_node_put(remote);
>> -			continue;
>> -		}
>> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
>> +		struct platform_driver *drv = rockchip_drm_comp_drvs[i];
>> +		struct device *p = NULL, *d;
>>
>> -		drm_of_component_match_add(dev, match, compare_of, remote);
>> -		of_node_put(remote);
>> +		do {
>> +			d = bus_find_device(&platform_bus_type, p, &drv->driver,
>> +					    (void *)platform_bus_type.match);
>> +			put_device(p);
>> +			p = d;
>> +
>> +			if (!d)
>> +				break;
>> +			component_match_add(dev, &match, compare_dev, d);
>> +		} while (true);
>>   	}
>> +
>> +	return match ?: ERR_PTR(-ENODEV);
>>   }
>>
>>   static const struct component_master_ops rockchip_drm_ops = {
>> @@ -391,21 +409,16 @@ static const struct component_master_ops rockchip_drm_ops = {
>>   	.unbind = rockchip_drm_unbind,
>>   };
>>
>> -static int rockchip_drm_platform_probe(struct platform_device *pdev)
>> +static int rockchip_drm_platform_of_probe(struct device *dev)
>>   {
>> -	struct device *dev = &pdev->dev;
>> -	struct component_match *match = NULL;
>>   	struct device_node *np = dev->of_node;
>>   	struct device_node *port;
>> +	bool found = false;
>>   	int i;
>>
>>   	if (!np)
>>   		return -ENODEV;
>> -	/*
>> -	 * Bind the crtc ports first, so that
>> -	 * drm_of_find_possible_crtcs called from encoder .bind callbacks
>> -	 * works as expected.
>> -	 */
>> +
>>   	for (i = 0;; i++) {
>>   		struct device_node *iommu;
>>
>> @@ -429,9 +442,9 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>>   			is_support_iommu = false;
>>   		}
>>
>> +		found = true;
>> +
>>   		of_node_put(iommu);
>> -		drm_of_component_match_add(dev, &match, compare_of,
>> -					   port->parent);
>>   		of_node_put(port);
>>   	}
>>
>> @@ -440,27 +453,27 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>>   		return -ENODEV;
>>   	}
>>
>> -	if (!match) {
>> +	if (!found) {
>>   		dev_err(dev, "No available vop found for display-subsystem.\n");
>>   		return -ENODEV;
>>   	}
>> -	/*
>> -	 * For each bound crtc, bind the encoders attached to its
>> -	 * remote endpoint.
>> -	 */
>> -	for (i = 0;; i++) {
>> -		port = of_parse_phandle(np, "ports", i);
>> -		if (!port)
>> -			break;
>>
>> -		if (!of_device_is_available(port->parent)) {
>> -			of_node_put(port);
>> -			continue;
>> -		}
>> +	return 0;
>> +}
>>
>> -		rockchip_add_endpoints(dev, &match, port);
>> -		of_node_put(port);
>> -	}
>> +static int rockchip_drm_platform_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct component_match *match = NULL;
>> +	int ret;
>> +
>> +	ret = rockchip_drm_platform_of_probe(dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	match = rockchip_drm_match_add(dev);
>> +	if (IS_ERR(match))
>> +		return PTR_ERR(match);
>>
>>   	return component_master_add_with_match(dev, &rockchip_drm_ops, match);
>>   }
>> @@ -488,7 +501,60 @@ static struct platform_driver rockchip_drm_platform_driver = {
>>   	},
>>   };
>>
>> -module_platform_driver(rockchip_drm_platform_driver);
>> +static void rockchip_drm_unregister_drivers(void)
>> +{
>> +	int i;
>> +
>> +	for (i = ARRAY_SIZE(rockchip_drm_comp_drvs) - 1; i >= 0; i--)
>> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
>> +}
>> +
>> +static int rockchip_drm_register_drivers(void)
>> +{
>> +	int i, ret;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
>> +		ret = platform_driver_register(rockchip_drm_comp_drvs[i]);
>> +		if (ret)
>> +			goto err_unreg;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_unreg:
>> +	for (; i--; )
>> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
>
> Strange placement of decremental in for, maybe sth like this would be
> better:
> 	while (i-- > 0)
> 		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
>
Yes, this looks better.
>
>> +	return ret;
>> +}
>> +
>> +static int __init rockchip_drm_init(void)
>> +{
>> +	int ret;
>> +
>> +	ret = rockchip_drm_register_drivers();
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = platform_driver_register(&rockchip_drm_platform_driver);
>> +	if (ret)
>> +		goto err_unreg_drivers;
>> +
>> +	return 0;
>> +
>> +err_unreg_drivers:
>> +	rockchip_drm_unregister_drivers();
>> +	return ret;
>> +}
>> +
>> +static void __exit rockchip_drm_fini(void)
>> +{
>> +	platform_driver_unregister(&rockchip_drm_platform_driver);
>> +
>> +	rockchip_drm_unregister_drivers();
>> +}
>> +
>> +module_init(rockchip_drm_init);
>> +module_exit(rockchip_drm_fini);
>>
>>   MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
>>   MODULE_DESCRIPTION("ROCKCHIP DRM Driver");
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> index adc3930..91812bd 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> @@ -79,4 +79,20 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>>   int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
>>   				unsigned int mstimeout);
>>
>> +#ifdef CONFIG_ROCKCHIP_CDN_DP
>> +extern struct platform_driver cdn_dp_driver;
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
>> +extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
>> +extern struct platform_driver dw_mipi_dsi_driver;
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
>> +extern struct platform_driver inno_hdmi_driver;
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
>> +extern struct platform_driver rockchip_dp_driver;
>> +#endif
>> +extern struct platform_driver vop_platform_driver;
>>   #endif /* _ROCKCHIP_DRM_DRV_H_ */
>
> I think gcc should be fine if you remove ifdefs. Unused extern
> declarations are ignored, unless you use some paranoic static checker,
> aren't they? But this is also matter of taste.

Ok.
>
>
> Regards
> Andrzej
>
>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> index 91fbc7b..0da4444 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> @@ -404,7 +404,7 @@ static int vop_remove(struct platform_device *pdev)
>>   	return 0;
>>   }
>>
>> -static struct platform_driver vop_platform_driver = {
>> +struct platform_driver vop_platform_driver = {
>>   	.probe = vop_probe,
>>   	.remove = vop_remove,
>>   	.driver = {
>> @@ -412,9 +412,3 @@ static struct platform_driver vop_platform_driver = {
>>   		.of_match_table = of_match_ptr(vop_driver_dt_match),
>>   	},
>>   };
>> -
>> -module_platform_driver(vop_platform_driver);
>> -
>> -MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
>> -MODULE_DESCRIPTION("ROCKCHIP VOP Driver");
>> -MODULE_LICENSE("GPL v2");
>>
>
>
>
>
Thanx for your comments~

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

* [PATCH v2] drm/rockchip: Refactor the component match logic.
@ 2017-03-15 10:25       ` jeffy
  0 siblings, 0 replies; 10+ messages in thread
From: jeffy @ 2017-03-15 10:25 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Andrzej,


On 03/14/2017 08:05 PM, Andrzej Hajda wrote:
> Hi Jeffy,
>
> On 14.03.2017 11:45, Jeffy Chen wrote:
>> Currently we are adding all components from the dts, if one of their
>> drivers been disabled, we would not be able to bring up others.
>>
>> Refactor component match logic, follow exynos drm.
>>
>> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
>
> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
>
> Below few nitpicks.
>
> (...)
>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> index b360e62..b6bccfc 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> @@ -356,34 +356,52 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
>>   				rockchip_drm_sys_resume)
>>   };
>>
>> -static int compare_of(struct device *dev, void *data)
>> -{
>> -	struct device_node *np = data;
>> +static struct platform_driver *rockchip_drm_comp_drvs[] = {
>> +	&vop_platform_driver,
>> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
>> +	&rockchip_dp_driver,
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_CDN_DP
>> +	&cdn_dp_driver,
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
>> +	&dw_hdmi_rockchip_pltfm_driver,
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
>> +	&dw_mipi_dsi_driver,
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
>> +	&inno_hdmi_driver,
>> +#endif
>> +};
>
> To avoid #ifdefs, you can use DRV_PTR [1], but it depends what do you
> hate more? ifdefs or macro magic :)
>
> [1]:
> http://lxr.free-electrons.com/source/drivers/gpu/drm/exynos/exynos_drm_drv.c#L472
>
Done.
>>
>> -	return dev->of_node == np;
>> +static int compare_dev(struct device *dev, void *data)
>> +{
>> +	return dev == (struct device *)data;
>>   }
>>
>> -static void rockchip_add_endpoints(struct device *dev,
>> -				   struct component_match **match,
>> -				   struct device_node *port)
>> +static struct component_match *rockchip_drm_match_add(struct device *dev)
>>   {
>> -	struct device_node *ep, *remote;
>> +	struct component_match *match = NULL;
>> +	int i;
>>
>> -	for_each_child_of_node(port, ep) {
>> -		remote = of_graph_get_remote_port_parent(ep);
>> -		if (!remote || !of_device_is_available(remote)) {
>> -			of_node_put(remote);
>> -			continue;
>> -		} else if (!of_device_is_available(remote->parent)) {
>> -			dev_warn(dev, "parent device of %s is not available\n",
>> -				 remote->full_name);
>> -			of_node_put(remote);
>> -			continue;
>> -		}
>> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
>> +		struct platform_driver *drv = rockchip_drm_comp_drvs[i];
>> +		struct device *p = NULL, *d;
>>
>> -		drm_of_component_match_add(dev, match, compare_of, remote);
>> -		of_node_put(remote);
>> +		do {
>> +			d = bus_find_device(&platform_bus_type, p, &drv->driver,
>> +					    (void *)platform_bus_type.match);
>> +			put_device(p);
>> +			p = d;
>> +
>> +			if (!d)
>> +				break;
>> +			component_match_add(dev, &match, compare_dev, d);
>> +		} while (true);
>>   	}
>> +
>> +	return match ?: ERR_PTR(-ENODEV);
>>   }
>>
>>   static const struct component_master_ops rockchip_drm_ops = {
>> @@ -391,21 +409,16 @@ static const struct component_master_ops rockchip_drm_ops = {
>>   	.unbind = rockchip_drm_unbind,
>>   };
>>
>> -static int rockchip_drm_platform_probe(struct platform_device *pdev)
>> +static int rockchip_drm_platform_of_probe(struct device *dev)
>>   {
>> -	struct device *dev = &pdev->dev;
>> -	struct component_match *match = NULL;
>>   	struct device_node *np = dev->of_node;
>>   	struct device_node *port;
>> +	bool found = false;
>>   	int i;
>>
>>   	if (!np)
>>   		return -ENODEV;
>> -	/*
>> -	 * Bind the crtc ports first, so that
>> -	 * drm_of_find_possible_crtcs called from encoder .bind callbacks
>> -	 * works as expected.
>> -	 */
>> +
>>   	for (i = 0;; i++) {
>>   		struct device_node *iommu;
>>
>> @@ -429,9 +442,9 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>>   			is_support_iommu = false;
>>   		}
>>
>> +		found = true;
>> +
>>   		of_node_put(iommu);
>> -		drm_of_component_match_add(dev, &match, compare_of,
>> -					   port->parent);
>>   		of_node_put(port);
>>   	}
>>
>> @@ -440,27 +453,27 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>>   		return -ENODEV;
>>   	}
>>
>> -	if (!match) {
>> +	if (!found) {
>>   		dev_err(dev, "No available vop found for display-subsystem.\n");
>>   		return -ENODEV;
>>   	}
>> -	/*
>> -	 * For each bound crtc, bind the encoders attached to its
>> -	 * remote endpoint.
>> -	 */
>> -	for (i = 0;; i++) {
>> -		port = of_parse_phandle(np, "ports", i);
>> -		if (!port)
>> -			break;
>>
>> -		if (!of_device_is_available(port->parent)) {
>> -			of_node_put(port);
>> -			continue;
>> -		}
>> +	return 0;
>> +}
>>
>> -		rockchip_add_endpoints(dev, &match, port);
>> -		of_node_put(port);
>> -	}
>> +static int rockchip_drm_platform_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct component_match *match = NULL;
>> +	int ret;
>> +
>> +	ret = rockchip_drm_platform_of_probe(dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	match = rockchip_drm_match_add(dev);
>> +	if (IS_ERR(match))
>> +		return PTR_ERR(match);
>>
>>   	return component_master_add_with_match(dev, &rockchip_drm_ops, match);
>>   }
>> @@ -488,7 +501,60 @@ static struct platform_driver rockchip_drm_platform_driver = {
>>   	},
>>   };
>>
>> -module_platform_driver(rockchip_drm_platform_driver);
>> +static void rockchip_drm_unregister_drivers(void)
>> +{
>> +	int i;
>> +
>> +	for (i = ARRAY_SIZE(rockchip_drm_comp_drvs) - 1; i >= 0; i--)
>> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
>> +}
>> +
>> +static int rockchip_drm_register_drivers(void)
>> +{
>> +	int i, ret;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
>> +		ret = platform_driver_register(rockchip_drm_comp_drvs[i]);
>> +		if (ret)
>> +			goto err_unreg;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_unreg:
>> +	for (; i--; )
>> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
>
> Strange placement of decremental in for, maybe sth like this would be
> better:
> 	while (i-- > 0)
> 		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
>
Yes, this looks better.
>
>> +	return ret;
>> +}
>> +
>> +static int __init rockchip_drm_init(void)
>> +{
>> +	int ret;
>> +
>> +	ret = rockchip_drm_register_drivers();
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = platform_driver_register(&rockchip_drm_platform_driver);
>> +	if (ret)
>> +		goto err_unreg_drivers;
>> +
>> +	return 0;
>> +
>> +err_unreg_drivers:
>> +	rockchip_drm_unregister_drivers();
>> +	return ret;
>> +}
>> +
>> +static void __exit rockchip_drm_fini(void)
>> +{
>> +	platform_driver_unregister(&rockchip_drm_platform_driver);
>> +
>> +	rockchip_drm_unregister_drivers();
>> +}
>> +
>> +module_init(rockchip_drm_init);
>> +module_exit(rockchip_drm_fini);
>>
>>   MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
>>   MODULE_DESCRIPTION("ROCKCHIP DRM Driver");
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> index adc3930..91812bd 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> @@ -79,4 +79,20 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>>   int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
>>   				unsigned int mstimeout);
>>
>> +#ifdef CONFIG_ROCKCHIP_CDN_DP
>> +extern struct platform_driver cdn_dp_driver;
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
>> +extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
>> +extern struct platform_driver dw_mipi_dsi_driver;
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
>> +extern struct platform_driver inno_hdmi_driver;
>> +#endif
>> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
>> +extern struct platform_driver rockchip_dp_driver;
>> +#endif
>> +extern struct platform_driver vop_platform_driver;
>>   #endif /* _ROCKCHIP_DRM_DRV_H_ */
>
> I think gcc should be fine if you remove ifdefs. Unused extern
> declarations are ignored, unless you use some paranoic static checker,
> aren't they? But this is also matter of taste.

Ok.
>
>
> Regards
> Andrzej
>
>
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> index 91fbc7b..0da4444 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
>> @@ -404,7 +404,7 @@ static int vop_remove(struct platform_device *pdev)
>>   	return 0;
>>   }
>>
>> -static struct platform_driver vop_platform_driver = {
>> +struct platform_driver vop_platform_driver = {
>>   	.probe = vop_probe,
>>   	.remove = vop_remove,
>>   	.driver = {
>> @@ -412,9 +412,3 @@ static struct platform_driver vop_platform_driver = {
>>   		.of_match_table = of_match_ptr(vop_driver_dt_match),
>>   	},
>>   };
>> -
>> -module_platform_driver(vop_platform_driver);
>> -
>> -MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
>> -MODULE_DESCRIPTION("ROCKCHIP VOP Driver");
>> -MODULE_LICENSE("GPL v2");
>>
>
>
>
>
Thanx for your comments~

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

* Re: [PATCH v2] drm/rockchip: Refactor the component match logic.
  2017-03-14 10:45   ` Jeffy Chen
  (?)
@ 2017-03-14 15:37     ` Sean Paul
  -1 siblings, 0 replies; 10+ messages in thread
From: Sean Paul @ 2017-03-14 15:37 UTC (permalink / raw)
  To: Jeffy Chen
  Cc: linux-kernel, Brian Norris, Douglas Anderson, dri-devel,
	Tomasz Figa, linux-rockchip, Guenter Roeck, linux-arm-kernel

On Tue, Mar 14, 2017 at 06:45:49PM +0800, Jeffy Chen wrote:
> Currently we are adding all components from the dts, if one of their
> drivers been disabled, we would not be able to bring up others.
> 
> Refactor component match logic, follow exynos drm.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>

Thanks for revising the patch, this looks reasonable to me. 

Now that there are 2 drivers using this method of discovery, it's probably
worth looking at whether we can abstract some of this into the midlayer to
share code as well as provide helpers for other drivers.

If Mark doesn't have any objections, I'll apply this to drm-misc in a few days.

Sean

> 
> ---
> 
>  drivers/gpu/drm/rockchip/Kconfig                |  10 +-
>  drivers/gpu/drm/rockchip/Makefile               |  16 +--
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c |   9 +-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c          |   8 +-
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c          |   8 +-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c     |  10 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c            |  10 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c     | 162 +++++++++++++++++-------
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h     |  16 +++
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c     |   8 +-
>  10 files changed, 149 insertions(+), 108 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
> index 0e4eb84..50c41c0 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -13,7 +13,7 @@ config DRM_ROCKCHIP
>  	  IP found on the SoC.
>  
>  config ROCKCHIP_ANALOGIX_DP
> -	tristate "Rockchip specific extensions for Analogix DP driver"
> +	bool "Rockchip specific extensions for Analogix DP driver"
>  	depends on DRM_ROCKCHIP
>  	select DRM_ANALOGIX_DP
>  	help
> @@ -22,7 +22,7 @@ config ROCKCHIP_ANALOGIX_DP
>  	  on RK3288 based SoC, you should selet this option.
>  
>  config ROCKCHIP_CDN_DP
> -        tristate "Rockchip cdn DP"
> +        bool "Rockchip cdn DP"
>          depends on DRM_ROCKCHIP
>  	depends on EXTCON
>  	select SND_SOC_HDMI_CODEC if SND_SOC
> @@ -33,7 +33,7 @@ config ROCKCHIP_CDN_DP
>  	  option.
>  
>  config ROCKCHIP_DW_HDMI
> -        tristate "Rockchip specific extensions for Synopsys DW HDMI"
> +        bool "Rockchip specific extensions for Synopsys DW HDMI"
>          depends on DRM_ROCKCHIP
>          select DRM_DW_HDMI
>          help
> @@ -43,7 +43,7 @@ config ROCKCHIP_DW_HDMI
>  	  option.
>  
>  config ROCKCHIP_DW_MIPI_DSI
> -	tristate "Rockchip specific extensions for Synopsys DW MIPI DSI"
> +	bool "Rockchip specific extensions for Synopsys DW MIPI DSI"
>  	depends on DRM_ROCKCHIP
>  	select DRM_MIPI_DSI
>  	help
> @@ -53,7 +53,7 @@ config ROCKCHIP_DW_MIPI_DSI
>  	 option.
>  
>  config ROCKCHIP_INNO_HDMI
> -	tristate "Rockchip specific extensions for Innosilicon HDMI"
> +	bool "Rockchip specific extensions for Innosilicon HDMI"
>  	depends on DRM_ROCKCHIP
>  	help
>  	  This selects support for Rockchip SoC specific extensions
> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
> index c931e2a..fa8dc9d 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -3,14 +3,14 @@
>  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>  
>  rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
> -		rockchip_drm_gem.o rockchip_drm_psr.o rockchip_drm_vop.o
> +		rockchip_drm_gem.o rockchip_drm_psr.o \
> +		rockchip_drm_vop.o rockchip_vop_reg.o
>  rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>  
> -obj-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> -obj-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp.o
> -cdn-dp-objs := cdn-dp-core.o cdn-dp-reg.o
> -obj-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
> -obj-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
> -obj-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
>  
> -obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o rockchip_vop_reg.o
> +obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 8548e82..91ebe5c 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -507,7 +507,7 @@ static const struct of_device_id rockchip_dp_dt_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(of, rockchip_dp_dt_ids);
>  
> -static struct platform_driver rockchip_dp_driver = {
> +struct platform_driver rockchip_dp_driver = {
>  	.probe = rockchip_dp_probe,
>  	.remove = rockchip_dp_remove,
>  	.driver = {
> @@ -516,10 +516,3 @@ static struct platform_driver rockchip_dp_driver = {
>  		   .of_match_table = of_match_ptr(rockchip_dp_dt_ids),
>  	},
>  };
> -
> -module_platform_driver(rockchip_dp_driver);
> -
> -MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
> -MODULE_AUTHOR("Jeff chen <jeff.chen@rock-chips.com>");
> -MODULE_DESCRIPTION("Rockchip Specific Analogix-DP Driver Extension");
> -MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index fd79a70..e1796d0 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -1243,7 +1243,7 @@ static const struct dev_pm_ops cdn_dp_pm_ops = {
>  				cdn_dp_resume)
>  };
>  
> -static struct platform_driver cdn_dp_driver = {
> +struct platform_driver cdn_dp_driver = {
>  	.probe = cdn_dp_probe,
>  	.remove = cdn_dp_remove,
>  	.shutdown = cdn_dp_shutdown,
> @@ -1254,9 +1254,3 @@ static struct platform_driver cdn_dp_driver = {
>  		   .pm = &cdn_dp_pm_ops,
>  	},
>  };
> -
> -module_platform_driver(cdn_dp_driver);
> -
> -MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
> -MODULE_DESCRIPTION("cdn DP Driver");
> -MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index d9aa382..65a474f 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -1165,7 +1165,7 @@ static int dw_mipi_dsi_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct platform_driver dw_mipi_dsi_driver = {
> +struct platform_driver dw_mipi_dsi_driver = {
>  	.probe		= dw_mipi_dsi_probe,
>  	.remove		= dw_mipi_dsi_remove,
>  	.driver		= {
> @@ -1173,9 +1173,3 @@ static struct platform_driver dw_mipi_dsi_driver = {
>  		.name	= DRIVER_NAME,
>  	},
>  };
> -module_platform_driver(dw_mipi_dsi_driver);
> -
> -MODULE_DESCRIPTION("ROCKCHIP MIPI DSI host controller driver");
> -MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:" DRIVER_NAME);
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index a6d4a02..976ea79 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -326,7 +326,7 @@ static int dw_hdmi_rockchip_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
> +struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
>  	.probe  = dw_hdmi_rockchip_probe,
>  	.remove = dw_hdmi_rockchip_remove,
>  	.driver = {
> @@ -334,11 +334,3 @@ static struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
>  		.of_match_table = dw_hdmi_rockchip_dt_ids,
>  	},
>  };
> -
> -module_platform_driver(dw_hdmi_rockchip_pltfm_driver);
> -
> -MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com>");
> -MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
> -MODULE_DESCRIPTION("Rockchip Specific DW-HDMI Driver Extension");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:dwhdmi-rockchip");
> diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
> index 006260d..7d9b75e 100644
> --- a/drivers/gpu/drm/rockchip/inno_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
> @@ -923,7 +923,7 @@ static const struct of_device_id inno_hdmi_dt_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(of, inno_hdmi_dt_ids);
>  
> -static struct platform_driver inno_hdmi_driver = {
> +struct platform_driver inno_hdmi_driver = {
>  	.probe  = inno_hdmi_probe,
>  	.remove = inno_hdmi_remove,
>  	.driver = {
> @@ -931,11 +931,3 @@ static struct platform_driver inno_hdmi_driver = {
>  		.of_match_table = inno_hdmi_dt_ids,
>  	},
>  };
> -
> -module_platform_driver(inno_hdmi_driver);
> -
> -MODULE_AUTHOR("Zheng Yang <zhengyang@rock-chips.com>");
> -MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
> -MODULE_DESCRIPTION("Rockchip Specific INNO-HDMI Driver");
> -MODULE_LICENSE("GPL v2");
> -MODULE_ALIAS("platform:innohdmi-rockchip");
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index b360e62..b6bccfc 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -356,34 +356,52 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
>  				rockchip_drm_sys_resume)
>  };
>  
> -static int compare_of(struct device *dev, void *data)
> -{
> -	struct device_node *np = data;
> +static struct platform_driver *rockchip_drm_comp_drvs[] = {
> +	&vop_platform_driver,
> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
> +	&rockchip_dp_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_CDN_DP
> +	&cdn_dp_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
> +	&dw_hdmi_rockchip_pltfm_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
> +	&dw_mipi_dsi_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
> +	&inno_hdmi_driver,
> +#endif
> +};
>  
> -	return dev->of_node == np;
> +static int compare_dev(struct device *dev, void *data)
> +{
> +	return dev == (struct device *)data;
>  }
>  
> -static void rockchip_add_endpoints(struct device *dev,
> -				   struct component_match **match,
> -				   struct device_node *port)
> +static struct component_match *rockchip_drm_match_add(struct device *dev)
>  {
> -	struct device_node *ep, *remote;
> +	struct component_match *match = NULL;
> +	int i;
>  
> -	for_each_child_of_node(port, ep) {
> -		remote = of_graph_get_remote_port_parent(ep);
> -		if (!remote || !of_device_is_available(remote)) {
> -			of_node_put(remote);
> -			continue;
> -		} else if (!of_device_is_available(remote->parent)) {
> -			dev_warn(dev, "parent device of %s is not available\n",
> -				 remote->full_name);
> -			of_node_put(remote);
> -			continue;
> -		}
> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
> +		struct platform_driver *drv = rockchip_drm_comp_drvs[i];
> +		struct device *p = NULL, *d;
>  
> -		drm_of_component_match_add(dev, match, compare_of, remote);
> -		of_node_put(remote);
> +		do {
> +			d = bus_find_device(&platform_bus_type, p, &drv->driver,
> +					    (void *)platform_bus_type.match);
> +			put_device(p);
> +			p = d;
> +
> +			if (!d)
> +				break;
> +			component_match_add(dev, &match, compare_dev, d);
> +		} while (true);
>  	}
> +
> +	return match ?: ERR_PTR(-ENODEV);
>  }
>  
>  static const struct component_master_ops rockchip_drm_ops = {
> @@ -391,21 +409,16 @@ static const struct component_master_ops rockchip_drm_ops = {
>  	.unbind = rockchip_drm_unbind,
>  };
>  
> -static int rockchip_drm_platform_probe(struct platform_device *pdev)
> +static int rockchip_drm_platform_of_probe(struct device *dev)
>  {
> -	struct device *dev = &pdev->dev;
> -	struct component_match *match = NULL;
>  	struct device_node *np = dev->of_node;
>  	struct device_node *port;
> +	bool found = false;
>  	int i;
>  
>  	if (!np)
>  		return -ENODEV;
> -	/*
> -	 * Bind the crtc ports first, so that
> -	 * drm_of_find_possible_crtcs called from encoder .bind callbacks
> -	 * works as expected.
> -	 */
> +
>  	for (i = 0;; i++) {
>  		struct device_node *iommu;
>  
> @@ -429,9 +442,9 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>  			is_support_iommu = false;
>  		}
>  
> +		found = true;
> +
>  		of_node_put(iommu);
> -		drm_of_component_match_add(dev, &match, compare_of,
> -					   port->parent);
>  		of_node_put(port);
>  	}
>  
> @@ -440,27 +453,27 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> -	if (!match) {
> +	if (!found) {
>  		dev_err(dev, "No available vop found for display-subsystem.\n");
>  		return -ENODEV;
>  	}
> -	/*
> -	 * For each bound crtc, bind the encoders attached to its
> -	 * remote endpoint.
> -	 */
> -	for (i = 0;; i++) {
> -		port = of_parse_phandle(np, "ports", i);
> -		if (!port)
> -			break;
>  
> -		if (!of_device_is_available(port->parent)) {
> -			of_node_put(port);
> -			continue;
> -		}
> +	return 0;
> +}
>  
> -		rockchip_add_endpoints(dev, &match, port);
> -		of_node_put(port);
> -	}
> +static int rockchip_drm_platform_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct component_match *match = NULL;
> +	int ret;
> +
> +	ret = rockchip_drm_platform_of_probe(dev);
> +	if (ret)
> +		return ret;
> +
> +	match = rockchip_drm_match_add(dev);
> +	if (IS_ERR(match))
> +		return PTR_ERR(match);
>  
>  	return component_master_add_with_match(dev, &rockchip_drm_ops, match);
>  }
> @@ -488,7 +501,60 @@ static struct platform_driver rockchip_drm_platform_driver = {
>  	},
>  };
>  
> -module_platform_driver(rockchip_drm_platform_driver);
> +static void rockchip_drm_unregister_drivers(void)
> +{
> +	int i;
> +
> +	for (i = ARRAY_SIZE(rockchip_drm_comp_drvs) - 1; i >= 0; i--)
> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
> +}
> +
> +static int rockchip_drm_register_drivers(void)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
> +		ret = platform_driver_register(rockchip_drm_comp_drvs[i]);
> +		if (ret)
> +			goto err_unreg;
> +	}
> +
> +	return 0;
> +
> +err_unreg:
> +	for (; i--; )
> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
> +	return ret;
> +}
> +
> +static int __init rockchip_drm_init(void)
> +{
> +	int ret;
> +
> +	ret = rockchip_drm_register_drivers();
> +	if (ret)
> +		return ret;
> +
> +	ret = platform_driver_register(&rockchip_drm_platform_driver);
> +	if (ret)
> +		goto err_unreg_drivers;
> +
> +	return 0;
> +
> +err_unreg_drivers:
> +	rockchip_drm_unregister_drivers();
> +	return ret;
> +}
> +
> +static void __exit rockchip_drm_fini(void)
> +{
> +	platform_driver_unregister(&rockchip_drm_platform_driver);
> +
> +	rockchip_drm_unregister_drivers();
> +}
> +
> +module_init(rockchip_drm_init);
> +module_exit(rockchip_drm_fini);
>  
>  MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
>  MODULE_DESCRIPTION("ROCKCHIP DRM Driver");
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index adc3930..91812bd 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -79,4 +79,20 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>  int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
>  				unsigned int mstimeout);
>  
> +#ifdef CONFIG_ROCKCHIP_CDN_DP
> +extern struct platform_driver cdn_dp_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
> +extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
> +extern struct platform_driver dw_mipi_dsi_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
> +extern struct platform_driver inno_hdmi_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
> +extern struct platform_driver rockchip_dp_driver;
> +#endif
> +extern struct platform_driver vop_platform_driver;
>  #endif /* _ROCKCHIP_DRM_DRV_H_ */
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 91fbc7b..0da4444 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -404,7 +404,7 @@ static int vop_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct platform_driver vop_platform_driver = {
> +struct platform_driver vop_platform_driver = {
>  	.probe = vop_probe,
>  	.remove = vop_remove,
>  	.driver = {
> @@ -412,9 +412,3 @@ static struct platform_driver vop_platform_driver = {
>  		.of_match_table = of_match_ptr(vop_driver_dt_match),
>  	},
>  };
> -
> -module_platform_driver(vop_platform_driver);
> -
> -MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
> -MODULE_DESCRIPTION("ROCKCHIP VOP Driver");
> -MODULE_LICENSE("GPL v2");
> -- 
> 2.1.4
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v2] drm/rockchip: Refactor the component match logic.
@ 2017-03-14 15:37     ` Sean Paul
  0 siblings, 0 replies; 10+ messages in thread
From: Sean Paul @ 2017-03-14 15:37 UTC (permalink / raw)
  To: Jeffy Chen
  Cc: Mark Yao, Douglas Anderson, Guenter Roeck, Brian Norris,
	Sean Paul, Tomasz Figa, Heiko Stuebner, linux-kernel, dri-devel,
	linux-rockchip, David Airlie, linux-arm-kernel

On Tue, Mar 14, 2017 at 06:45:49PM +0800, Jeffy Chen wrote:
> Currently we are adding all components from the dts, if one of their
> drivers been disabled, we would not be able to bring up others.
> 
> Refactor component match logic, follow exynos drm.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>

Thanks for revising the patch, this looks reasonable to me. 

Now that there are 2 drivers using this method of discovery, it's probably
worth looking at whether we can abstract some of this into the midlayer to
share code as well as provide helpers for other drivers.

If Mark doesn't have any objections, I'll apply this to drm-misc in a few days.

Sean

> 
> ---
> 
>  drivers/gpu/drm/rockchip/Kconfig                |  10 +-
>  drivers/gpu/drm/rockchip/Makefile               |  16 +--
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c |   9 +-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c          |   8 +-
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c          |   8 +-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c     |  10 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c            |  10 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c     | 162 +++++++++++++++++-------
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h     |  16 +++
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c     |   8 +-
>  10 files changed, 149 insertions(+), 108 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
> index 0e4eb84..50c41c0 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -13,7 +13,7 @@ config DRM_ROCKCHIP
>  	  IP found on the SoC.
>  
>  config ROCKCHIP_ANALOGIX_DP
> -	tristate "Rockchip specific extensions for Analogix DP driver"
> +	bool "Rockchip specific extensions for Analogix DP driver"
>  	depends on DRM_ROCKCHIP
>  	select DRM_ANALOGIX_DP
>  	help
> @@ -22,7 +22,7 @@ config ROCKCHIP_ANALOGIX_DP
>  	  on RK3288 based SoC, you should selet this option.
>  
>  config ROCKCHIP_CDN_DP
> -        tristate "Rockchip cdn DP"
> +        bool "Rockchip cdn DP"
>          depends on DRM_ROCKCHIP
>  	depends on EXTCON
>  	select SND_SOC_HDMI_CODEC if SND_SOC
> @@ -33,7 +33,7 @@ config ROCKCHIP_CDN_DP
>  	  option.
>  
>  config ROCKCHIP_DW_HDMI
> -        tristate "Rockchip specific extensions for Synopsys DW HDMI"
> +        bool "Rockchip specific extensions for Synopsys DW HDMI"
>          depends on DRM_ROCKCHIP
>          select DRM_DW_HDMI
>          help
> @@ -43,7 +43,7 @@ config ROCKCHIP_DW_HDMI
>  	  option.
>  
>  config ROCKCHIP_DW_MIPI_DSI
> -	tristate "Rockchip specific extensions for Synopsys DW MIPI DSI"
> +	bool "Rockchip specific extensions for Synopsys DW MIPI DSI"
>  	depends on DRM_ROCKCHIP
>  	select DRM_MIPI_DSI
>  	help
> @@ -53,7 +53,7 @@ config ROCKCHIP_DW_MIPI_DSI
>  	 option.
>  
>  config ROCKCHIP_INNO_HDMI
> -	tristate "Rockchip specific extensions for Innosilicon HDMI"
> +	bool "Rockchip specific extensions for Innosilicon HDMI"
>  	depends on DRM_ROCKCHIP
>  	help
>  	  This selects support for Rockchip SoC specific extensions
> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
> index c931e2a..fa8dc9d 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -3,14 +3,14 @@
>  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>  
>  rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
> -		rockchip_drm_gem.o rockchip_drm_psr.o rockchip_drm_vop.o
> +		rockchip_drm_gem.o rockchip_drm_psr.o \
> +		rockchip_drm_vop.o rockchip_vop_reg.o
>  rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>  
> -obj-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> -obj-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp.o
> -cdn-dp-objs := cdn-dp-core.o cdn-dp-reg.o
> -obj-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
> -obj-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
> -obj-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
>  
> -obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o rockchip_vop_reg.o
> +obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 8548e82..91ebe5c 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -507,7 +507,7 @@ static const struct of_device_id rockchip_dp_dt_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(of, rockchip_dp_dt_ids);
>  
> -static struct platform_driver rockchip_dp_driver = {
> +struct platform_driver rockchip_dp_driver = {
>  	.probe = rockchip_dp_probe,
>  	.remove = rockchip_dp_remove,
>  	.driver = {
> @@ -516,10 +516,3 @@ static struct platform_driver rockchip_dp_driver = {
>  		   .of_match_table = of_match_ptr(rockchip_dp_dt_ids),
>  	},
>  };
> -
> -module_platform_driver(rockchip_dp_driver);
> -
> -MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
> -MODULE_AUTHOR("Jeff chen <jeff.chen@rock-chips.com>");
> -MODULE_DESCRIPTION("Rockchip Specific Analogix-DP Driver Extension");
> -MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index fd79a70..e1796d0 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -1243,7 +1243,7 @@ static const struct dev_pm_ops cdn_dp_pm_ops = {
>  				cdn_dp_resume)
>  };
>  
> -static struct platform_driver cdn_dp_driver = {
> +struct platform_driver cdn_dp_driver = {
>  	.probe = cdn_dp_probe,
>  	.remove = cdn_dp_remove,
>  	.shutdown = cdn_dp_shutdown,
> @@ -1254,9 +1254,3 @@ static struct platform_driver cdn_dp_driver = {
>  		   .pm = &cdn_dp_pm_ops,
>  	},
>  };
> -
> -module_platform_driver(cdn_dp_driver);
> -
> -MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
> -MODULE_DESCRIPTION("cdn DP Driver");
> -MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index d9aa382..65a474f 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -1165,7 +1165,7 @@ static int dw_mipi_dsi_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct platform_driver dw_mipi_dsi_driver = {
> +struct platform_driver dw_mipi_dsi_driver = {
>  	.probe		= dw_mipi_dsi_probe,
>  	.remove		= dw_mipi_dsi_remove,
>  	.driver		= {
> @@ -1173,9 +1173,3 @@ static struct platform_driver dw_mipi_dsi_driver = {
>  		.name	= DRIVER_NAME,
>  	},
>  };
> -module_platform_driver(dw_mipi_dsi_driver);
> -
> -MODULE_DESCRIPTION("ROCKCHIP MIPI DSI host controller driver");
> -MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:" DRIVER_NAME);
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index a6d4a02..976ea79 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -326,7 +326,7 @@ static int dw_hdmi_rockchip_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
> +struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
>  	.probe  = dw_hdmi_rockchip_probe,
>  	.remove = dw_hdmi_rockchip_remove,
>  	.driver = {
> @@ -334,11 +334,3 @@ static struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
>  		.of_match_table = dw_hdmi_rockchip_dt_ids,
>  	},
>  };
> -
> -module_platform_driver(dw_hdmi_rockchip_pltfm_driver);
> -
> -MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com>");
> -MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
> -MODULE_DESCRIPTION("Rockchip Specific DW-HDMI Driver Extension");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:dwhdmi-rockchip");
> diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
> index 006260d..7d9b75e 100644
> --- a/drivers/gpu/drm/rockchip/inno_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
> @@ -923,7 +923,7 @@ static const struct of_device_id inno_hdmi_dt_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(of, inno_hdmi_dt_ids);
>  
> -static struct platform_driver inno_hdmi_driver = {
> +struct platform_driver inno_hdmi_driver = {
>  	.probe  = inno_hdmi_probe,
>  	.remove = inno_hdmi_remove,
>  	.driver = {
> @@ -931,11 +931,3 @@ static struct platform_driver inno_hdmi_driver = {
>  		.of_match_table = inno_hdmi_dt_ids,
>  	},
>  };
> -
> -module_platform_driver(inno_hdmi_driver);
> -
> -MODULE_AUTHOR("Zheng Yang <zhengyang@rock-chips.com>");
> -MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
> -MODULE_DESCRIPTION("Rockchip Specific INNO-HDMI Driver");
> -MODULE_LICENSE("GPL v2");
> -MODULE_ALIAS("platform:innohdmi-rockchip");
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index b360e62..b6bccfc 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -356,34 +356,52 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
>  				rockchip_drm_sys_resume)
>  };
>  
> -static int compare_of(struct device *dev, void *data)
> -{
> -	struct device_node *np = data;
> +static struct platform_driver *rockchip_drm_comp_drvs[] = {
> +	&vop_platform_driver,
> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
> +	&rockchip_dp_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_CDN_DP
> +	&cdn_dp_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
> +	&dw_hdmi_rockchip_pltfm_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
> +	&dw_mipi_dsi_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
> +	&inno_hdmi_driver,
> +#endif
> +};
>  
> -	return dev->of_node == np;
> +static int compare_dev(struct device *dev, void *data)
> +{
> +	return dev == (struct device *)data;
>  }
>  
> -static void rockchip_add_endpoints(struct device *dev,
> -				   struct component_match **match,
> -				   struct device_node *port)
> +static struct component_match *rockchip_drm_match_add(struct device *dev)
>  {
> -	struct device_node *ep, *remote;
> +	struct component_match *match = NULL;
> +	int i;
>  
> -	for_each_child_of_node(port, ep) {
> -		remote = of_graph_get_remote_port_parent(ep);
> -		if (!remote || !of_device_is_available(remote)) {
> -			of_node_put(remote);
> -			continue;
> -		} else if (!of_device_is_available(remote->parent)) {
> -			dev_warn(dev, "parent device of %s is not available\n",
> -				 remote->full_name);
> -			of_node_put(remote);
> -			continue;
> -		}
> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
> +		struct platform_driver *drv = rockchip_drm_comp_drvs[i];
> +		struct device *p = NULL, *d;
>  
> -		drm_of_component_match_add(dev, match, compare_of, remote);
> -		of_node_put(remote);
> +		do {
> +			d = bus_find_device(&platform_bus_type, p, &drv->driver,
> +					    (void *)platform_bus_type.match);
> +			put_device(p);
> +			p = d;
> +
> +			if (!d)
> +				break;
> +			component_match_add(dev, &match, compare_dev, d);
> +		} while (true);
>  	}
> +
> +	return match ?: ERR_PTR(-ENODEV);
>  }
>  
>  static const struct component_master_ops rockchip_drm_ops = {
> @@ -391,21 +409,16 @@ static const struct component_master_ops rockchip_drm_ops = {
>  	.unbind = rockchip_drm_unbind,
>  };
>  
> -static int rockchip_drm_platform_probe(struct platform_device *pdev)
> +static int rockchip_drm_platform_of_probe(struct device *dev)
>  {
> -	struct device *dev = &pdev->dev;
> -	struct component_match *match = NULL;
>  	struct device_node *np = dev->of_node;
>  	struct device_node *port;
> +	bool found = false;
>  	int i;
>  
>  	if (!np)
>  		return -ENODEV;
> -	/*
> -	 * Bind the crtc ports first, so that
> -	 * drm_of_find_possible_crtcs called from encoder .bind callbacks
> -	 * works as expected.
> -	 */
> +
>  	for (i = 0;; i++) {
>  		struct device_node *iommu;
>  
> @@ -429,9 +442,9 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>  			is_support_iommu = false;
>  		}
>  
> +		found = true;
> +
>  		of_node_put(iommu);
> -		drm_of_component_match_add(dev, &match, compare_of,
> -					   port->parent);
>  		of_node_put(port);
>  	}
>  
> @@ -440,27 +453,27 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> -	if (!match) {
> +	if (!found) {
>  		dev_err(dev, "No available vop found for display-subsystem.\n");
>  		return -ENODEV;
>  	}
> -	/*
> -	 * For each bound crtc, bind the encoders attached to its
> -	 * remote endpoint.
> -	 */
> -	for (i = 0;; i++) {
> -		port = of_parse_phandle(np, "ports", i);
> -		if (!port)
> -			break;
>  
> -		if (!of_device_is_available(port->parent)) {
> -			of_node_put(port);
> -			continue;
> -		}
> +	return 0;
> +}
>  
> -		rockchip_add_endpoints(dev, &match, port);
> -		of_node_put(port);
> -	}
> +static int rockchip_drm_platform_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct component_match *match = NULL;
> +	int ret;
> +
> +	ret = rockchip_drm_platform_of_probe(dev);
> +	if (ret)
> +		return ret;
> +
> +	match = rockchip_drm_match_add(dev);
> +	if (IS_ERR(match))
> +		return PTR_ERR(match);
>  
>  	return component_master_add_with_match(dev, &rockchip_drm_ops, match);
>  }
> @@ -488,7 +501,60 @@ static struct platform_driver rockchip_drm_platform_driver = {
>  	},
>  };
>  
> -module_platform_driver(rockchip_drm_platform_driver);
> +static void rockchip_drm_unregister_drivers(void)
> +{
> +	int i;
> +
> +	for (i = ARRAY_SIZE(rockchip_drm_comp_drvs) - 1; i >= 0; i--)
> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
> +}
> +
> +static int rockchip_drm_register_drivers(void)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
> +		ret = platform_driver_register(rockchip_drm_comp_drvs[i]);
> +		if (ret)
> +			goto err_unreg;
> +	}
> +
> +	return 0;
> +
> +err_unreg:
> +	for (; i--; )
> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
> +	return ret;
> +}
> +
> +static int __init rockchip_drm_init(void)
> +{
> +	int ret;
> +
> +	ret = rockchip_drm_register_drivers();
> +	if (ret)
> +		return ret;
> +
> +	ret = platform_driver_register(&rockchip_drm_platform_driver);
> +	if (ret)
> +		goto err_unreg_drivers;
> +
> +	return 0;
> +
> +err_unreg_drivers:
> +	rockchip_drm_unregister_drivers();
> +	return ret;
> +}
> +
> +static void __exit rockchip_drm_fini(void)
> +{
> +	platform_driver_unregister(&rockchip_drm_platform_driver);
> +
> +	rockchip_drm_unregister_drivers();
> +}
> +
> +module_init(rockchip_drm_init);
> +module_exit(rockchip_drm_fini);
>  
>  MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
>  MODULE_DESCRIPTION("ROCKCHIP DRM Driver");
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index adc3930..91812bd 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -79,4 +79,20 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>  int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
>  				unsigned int mstimeout);
>  
> +#ifdef CONFIG_ROCKCHIP_CDN_DP
> +extern struct platform_driver cdn_dp_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
> +extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
> +extern struct platform_driver dw_mipi_dsi_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
> +extern struct platform_driver inno_hdmi_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
> +extern struct platform_driver rockchip_dp_driver;
> +#endif
> +extern struct platform_driver vop_platform_driver;
>  #endif /* _ROCKCHIP_DRM_DRV_H_ */
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 91fbc7b..0da4444 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -404,7 +404,7 @@ static int vop_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct platform_driver vop_platform_driver = {
> +struct platform_driver vop_platform_driver = {
>  	.probe = vop_probe,
>  	.remove = vop_remove,
>  	.driver = {
> @@ -412,9 +412,3 @@ static struct platform_driver vop_platform_driver = {
>  		.of_match_table = of_match_ptr(vop_driver_dt_match),
>  	},
>  };
> -
> -module_platform_driver(vop_platform_driver);
> -
> -MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
> -MODULE_DESCRIPTION("ROCKCHIP VOP Driver");
> -MODULE_LICENSE("GPL v2");
> -- 
> 2.1.4
> 

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

* [PATCH v2] drm/rockchip: Refactor the component match logic.
@ 2017-03-14 15:37     ` Sean Paul
  0 siblings, 0 replies; 10+ messages in thread
From: Sean Paul @ 2017-03-14 15:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Mar 14, 2017 at 06:45:49PM +0800, Jeffy Chen wrote:
> Currently we are adding all components from the dts, if one of their
> drivers been disabled, we would not be able to bring up others.
> 
> Refactor component match logic, follow exynos drm.
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>

Thanks for revising the patch, this looks reasonable to me. 

Now that there are 2 drivers using this method of discovery, it's probably
worth looking at whether we can abstract some of this into the midlayer to
share code as well as provide helpers for other drivers.

If Mark doesn't have any objections, I'll apply this to drm-misc in a few days.

Sean

> 
> ---
> 
>  drivers/gpu/drm/rockchip/Kconfig                |  10 +-
>  drivers/gpu/drm/rockchip/Makefile               |  16 +--
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c |   9 +-
>  drivers/gpu/drm/rockchip/cdn-dp-core.c          |   8 +-
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c          |   8 +-
>  drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c     |  10 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c            |  10 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c     | 162 +++++++++++++++++-------
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h     |  16 +++
>  drivers/gpu/drm/rockchip/rockchip_vop_reg.c     |   8 +-
>  10 files changed, 149 insertions(+), 108 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
> index 0e4eb84..50c41c0 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -13,7 +13,7 @@ config DRM_ROCKCHIP
>  	  IP found on the SoC.
>  
>  config ROCKCHIP_ANALOGIX_DP
> -	tristate "Rockchip specific extensions for Analogix DP driver"
> +	bool "Rockchip specific extensions for Analogix DP driver"
>  	depends on DRM_ROCKCHIP
>  	select DRM_ANALOGIX_DP
>  	help
> @@ -22,7 +22,7 @@ config ROCKCHIP_ANALOGIX_DP
>  	  on RK3288 based SoC, you should selet this option.
>  
>  config ROCKCHIP_CDN_DP
> -        tristate "Rockchip cdn DP"
> +        bool "Rockchip cdn DP"
>          depends on DRM_ROCKCHIP
>  	depends on EXTCON
>  	select SND_SOC_HDMI_CODEC if SND_SOC
> @@ -33,7 +33,7 @@ config ROCKCHIP_CDN_DP
>  	  option.
>  
>  config ROCKCHIP_DW_HDMI
> -        tristate "Rockchip specific extensions for Synopsys DW HDMI"
> +        bool "Rockchip specific extensions for Synopsys DW HDMI"
>          depends on DRM_ROCKCHIP
>          select DRM_DW_HDMI
>          help
> @@ -43,7 +43,7 @@ config ROCKCHIP_DW_HDMI
>  	  option.
>  
>  config ROCKCHIP_DW_MIPI_DSI
> -	tristate "Rockchip specific extensions for Synopsys DW MIPI DSI"
> +	bool "Rockchip specific extensions for Synopsys DW MIPI DSI"
>  	depends on DRM_ROCKCHIP
>  	select DRM_MIPI_DSI
>  	help
> @@ -53,7 +53,7 @@ config ROCKCHIP_DW_MIPI_DSI
>  	 option.
>  
>  config ROCKCHIP_INNO_HDMI
> -	tristate "Rockchip specific extensions for Innosilicon HDMI"
> +	bool "Rockchip specific extensions for Innosilicon HDMI"
>  	depends on DRM_ROCKCHIP
>  	help
>  	  This selects support for Rockchip SoC specific extensions
> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
> index c931e2a..fa8dc9d 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -3,14 +3,14 @@
>  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>  
>  rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
> -		rockchip_drm_gem.o rockchip_drm_psr.o rockchip_drm_vop.o
> +		rockchip_drm_gem.o rockchip_drm_psr.o \
> +		rockchip_drm_vop.o rockchip_vop_reg.o
>  rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>  
> -obj-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> -obj-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp.o
> -cdn-dp-objs := cdn-dp-core.o cdn-dp-reg.o
> -obj-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
> -obj-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
> -obj-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
>  
> -obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o rockchip_vop_reg.o
> +obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 8548e82..91ebe5c 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -507,7 +507,7 @@ static const struct of_device_id rockchip_dp_dt_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(of, rockchip_dp_dt_ids);
>  
> -static struct platform_driver rockchip_dp_driver = {
> +struct platform_driver rockchip_dp_driver = {
>  	.probe = rockchip_dp_probe,
>  	.remove = rockchip_dp_remove,
>  	.driver = {
> @@ -516,10 +516,3 @@ static struct platform_driver rockchip_dp_driver = {
>  		   .of_match_table = of_match_ptr(rockchip_dp_dt_ids),
>  	},
>  };
> -
> -module_platform_driver(rockchip_dp_driver);
> -
> -MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
> -MODULE_AUTHOR("Jeff chen <jeff.chen@rock-chips.com>");
> -MODULE_DESCRIPTION("Rockchip Specific Analogix-DP Driver Extension");
> -MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index fd79a70..e1796d0 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -1243,7 +1243,7 @@ static const struct dev_pm_ops cdn_dp_pm_ops = {
>  				cdn_dp_resume)
>  };
>  
> -static struct platform_driver cdn_dp_driver = {
> +struct platform_driver cdn_dp_driver = {
>  	.probe = cdn_dp_probe,
>  	.remove = cdn_dp_remove,
>  	.shutdown = cdn_dp_shutdown,
> @@ -1254,9 +1254,3 @@ static struct platform_driver cdn_dp_driver = {
>  		   .pm = &cdn_dp_pm_ops,
>  	},
>  };
> -
> -module_platform_driver(cdn_dp_driver);
> -
> -MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
> -MODULE_DESCRIPTION("cdn DP Driver");
> -MODULE_LICENSE("GPL v2");
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index d9aa382..65a474f 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -1165,7 +1165,7 @@ static int dw_mipi_dsi_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct platform_driver dw_mipi_dsi_driver = {
> +struct platform_driver dw_mipi_dsi_driver = {
>  	.probe		= dw_mipi_dsi_probe,
>  	.remove		= dw_mipi_dsi_remove,
>  	.driver		= {
> @@ -1173,9 +1173,3 @@ static struct platform_driver dw_mipi_dsi_driver = {
>  		.name	= DRIVER_NAME,
>  	},
>  };
> -module_platform_driver(dw_mipi_dsi_driver);
> -
> -MODULE_DESCRIPTION("ROCKCHIP MIPI DSI host controller driver");
> -MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:" DRIVER_NAME);
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> index a6d4a02..976ea79 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> @@ -326,7 +326,7 @@ static int dw_hdmi_rockchip_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
> +struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
>  	.probe  = dw_hdmi_rockchip_probe,
>  	.remove = dw_hdmi_rockchip_remove,
>  	.driver = {
> @@ -334,11 +334,3 @@ static struct platform_driver dw_hdmi_rockchip_pltfm_driver = {
>  		.of_match_table = dw_hdmi_rockchip_dt_ids,
>  	},
>  };
> -
> -module_platform_driver(dw_hdmi_rockchip_pltfm_driver);
> -
> -MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com>");
> -MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
> -MODULE_DESCRIPTION("Rockchip Specific DW-HDMI Driver Extension");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS("platform:dwhdmi-rockchip");
> diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
> index 006260d..7d9b75e 100644
> --- a/drivers/gpu/drm/rockchip/inno_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
> @@ -923,7 +923,7 @@ static const struct of_device_id inno_hdmi_dt_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(of, inno_hdmi_dt_ids);
>  
> -static struct platform_driver inno_hdmi_driver = {
> +struct platform_driver inno_hdmi_driver = {
>  	.probe  = inno_hdmi_probe,
>  	.remove = inno_hdmi_remove,
>  	.driver = {
> @@ -931,11 +931,3 @@ static struct platform_driver inno_hdmi_driver = {
>  		.of_match_table = inno_hdmi_dt_ids,
>  	},
>  };
> -
> -module_platform_driver(inno_hdmi_driver);
> -
> -MODULE_AUTHOR("Zheng Yang <zhengyang@rock-chips.com>");
> -MODULE_AUTHOR("Yakir Yang <ykk@rock-chips.com>");
> -MODULE_DESCRIPTION("Rockchip Specific INNO-HDMI Driver");
> -MODULE_LICENSE("GPL v2");
> -MODULE_ALIAS("platform:innohdmi-rockchip");
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index b360e62..b6bccfc 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -356,34 +356,52 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
>  				rockchip_drm_sys_resume)
>  };
>  
> -static int compare_of(struct device *dev, void *data)
> -{
> -	struct device_node *np = data;
> +static struct platform_driver *rockchip_drm_comp_drvs[] = {
> +	&vop_platform_driver,
> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
> +	&rockchip_dp_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_CDN_DP
> +	&cdn_dp_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
> +	&dw_hdmi_rockchip_pltfm_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
> +	&dw_mipi_dsi_driver,
> +#endif
> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
> +	&inno_hdmi_driver,
> +#endif
> +};
>  
> -	return dev->of_node == np;
> +static int compare_dev(struct device *dev, void *data)
> +{
> +	return dev == (struct device *)data;
>  }
>  
> -static void rockchip_add_endpoints(struct device *dev,
> -				   struct component_match **match,
> -				   struct device_node *port)
> +static struct component_match *rockchip_drm_match_add(struct device *dev)
>  {
> -	struct device_node *ep, *remote;
> +	struct component_match *match = NULL;
> +	int i;
>  
> -	for_each_child_of_node(port, ep) {
> -		remote = of_graph_get_remote_port_parent(ep);
> -		if (!remote || !of_device_is_available(remote)) {
> -			of_node_put(remote);
> -			continue;
> -		} else if (!of_device_is_available(remote->parent)) {
> -			dev_warn(dev, "parent device of %s is not available\n",
> -				 remote->full_name);
> -			of_node_put(remote);
> -			continue;
> -		}
> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
> +		struct platform_driver *drv = rockchip_drm_comp_drvs[i];
> +		struct device *p = NULL, *d;
>  
> -		drm_of_component_match_add(dev, match, compare_of, remote);
> -		of_node_put(remote);
> +		do {
> +			d = bus_find_device(&platform_bus_type, p, &drv->driver,
> +					    (void *)platform_bus_type.match);
> +			put_device(p);
> +			p = d;
> +
> +			if (!d)
> +				break;
> +			component_match_add(dev, &match, compare_dev, d);
> +		} while (true);
>  	}
> +
> +	return match ?: ERR_PTR(-ENODEV);
>  }
>  
>  static const struct component_master_ops rockchip_drm_ops = {
> @@ -391,21 +409,16 @@ static const struct component_master_ops rockchip_drm_ops = {
>  	.unbind = rockchip_drm_unbind,
>  };
>  
> -static int rockchip_drm_platform_probe(struct platform_device *pdev)
> +static int rockchip_drm_platform_of_probe(struct device *dev)
>  {
> -	struct device *dev = &pdev->dev;
> -	struct component_match *match = NULL;
>  	struct device_node *np = dev->of_node;
>  	struct device_node *port;
> +	bool found = false;
>  	int i;
>  
>  	if (!np)
>  		return -ENODEV;
> -	/*
> -	 * Bind the crtc ports first, so that
> -	 * drm_of_find_possible_crtcs called from encoder .bind callbacks
> -	 * works as expected.
> -	 */
> +
>  	for (i = 0;; i++) {
>  		struct device_node *iommu;
>  
> @@ -429,9 +442,9 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>  			is_support_iommu = false;
>  		}
>  
> +		found = true;
> +
>  		of_node_put(iommu);
> -		drm_of_component_match_add(dev, &match, compare_of,
> -					   port->parent);
>  		of_node_put(port);
>  	}
>  
> @@ -440,27 +453,27 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> -	if (!match) {
> +	if (!found) {
>  		dev_err(dev, "No available vop found for display-subsystem.\n");
>  		return -ENODEV;
>  	}
> -	/*
> -	 * For each bound crtc, bind the encoders attached to its
> -	 * remote endpoint.
> -	 */
> -	for (i = 0;; i++) {
> -		port = of_parse_phandle(np, "ports", i);
> -		if (!port)
> -			break;
>  
> -		if (!of_device_is_available(port->parent)) {
> -			of_node_put(port);
> -			continue;
> -		}
> +	return 0;
> +}
>  
> -		rockchip_add_endpoints(dev, &match, port);
> -		of_node_put(port);
> -	}
> +static int rockchip_drm_platform_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct component_match *match = NULL;
> +	int ret;
> +
> +	ret = rockchip_drm_platform_of_probe(dev);
> +	if (ret)
> +		return ret;
> +
> +	match = rockchip_drm_match_add(dev);
> +	if (IS_ERR(match))
> +		return PTR_ERR(match);
>  
>  	return component_master_add_with_match(dev, &rockchip_drm_ops, match);
>  }
> @@ -488,7 +501,60 @@ static struct platform_driver rockchip_drm_platform_driver = {
>  	},
>  };
>  
> -module_platform_driver(rockchip_drm_platform_driver);
> +static void rockchip_drm_unregister_drivers(void)
> +{
> +	int i;
> +
> +	for (i = ARRAY_SIZE(rockchip_drm_comp_drvs) - 1; i >= 0; i--)
> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
> +}
> +
> +static int rockchip_drm_register_drivers(void)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(rockchip_drm_comp_drvs); i++) {
> +		ret = platform_driver_register(rockchip_drm_comp_drvs[i]);
> +		if (ret)
> +			goto err_unreg;
> +	}
> +
> +	return 0;
> +
> +err_unreg:
> +	for (; i--; )
> +		platform_driver_unregister(rockchip_drm_comp_drvs[i]);
> +	return ret;
> +}
> +
> +static int __init rockchip_drm_init(void)
> +{
> +	int ret;
> +
> +	ret = rockchip_drm_register_drivers();
> +	if (ret)
> +		return ret;
> +
> +	ret = platform_driver_register(&rockchip_drm_platform_driver);
> +	if (ret)
> +		goto err_unreg_drivers;
> +
> +	return 0;
> +
> +err_unreg_drivers:
> +	rockchip_drm_unregister_drivers();
> +	return ret;
> +}
> +
> +static void __exit rockchip_drm_fini(void)
> +{
> +	platform_driver_unregister(&rockchip_drm_platform_driver);
> +
> +	rockchip_drm_unregister_drivers();
> +}
> +
> +module_init(rockchip_drm_init);
> +module_exit(rockchip_drm_fini);
>  
>  MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
>  MODULE_DESCRIPTION("ROCKCHIP DRM Driver");
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index adc3930..91812bd 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -79,4 +79,20 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>  int rockchip_drm_wait_line_flag(struct drm_crtc *crtc, unsigned int line_num,
>  				unsigned int mstimeout);
>  
> +#ifdef CONFIG_ROCKCHIP_CDN_DP
> +extern struct platform_driver cdn_dp_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_HDMI
> +extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_DW_MIPI_DSI
> +extern struct platform_driver dw_mipi_dsi_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_INNO_HDMI
> +extern struct platform_driver inno_hdmi_driver;
> +#endif
> +#ifdef CONFIG_ROCKCHIP_ANALOGIX_DP
> +extern struct platform_driver rockchip_dp_driver;
> +#endif
> +extern struct platform_driver vop_platform_driver;
>  #endif /* _ROCKCHIP_DRM_DRV_H_ */
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index 91fbc7b..0da4444 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -404,7 +404,7 @@ static int vop_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static struct platform_driver vop_platform_driver = {
> +struct platform_driver vop_platform_driver = {
>  	.probe = vop_probe,
>  	.remove = vop_remove,
>  	.driver = {
> @@ -412,9 +412,3 @@ static struct platform_driver vop_platform_driver = {
>  		.of_match_table = of_match_ptr(vop_driver_dt_match),
>  	},
>  };
> -
> -module_platform_driver(vop_platform_driver);
> -
> -MODULE_AUTHOR("Mark Yao <mark.yao@rock-chips.com>");
> -MODULE_DESCRIPTION("ROCKCHIP VOP Driver");
> -MODULE_LICENSE("GPL v2");
> -- 
> 2.1.4
> 

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

end of thread, other threads:[~2017-03-15 10:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20170314120534eucas1p20601e44f6552db2b4b3f65d8ab6b4720@eucas1p2.samsung.com>
2017-03-14 10:45 ` [PATCH v2] drm/rockchip: Refactor the component match logic Jeffy Chen
2017-03-14 10:45   ` Jeffy Chen
2017-03-14 12:05   ` Andrzej Hajda
2017-03-14 12:05     ` Andrzej Hajda
2017-03-14 12:05     ` Andrzej Hajda
2017-03-15 10:25     ` jeffy
2017-03-15 10:25       ` jeffy
2017-03-14 15:37   ` Sean Paul
2017-03-14 15:37     ` Sean Paul
2017-03-14 15:37     ` Sean Paul

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.