[PATCH v5 1/2] devicetree/bindings: display: Add document for rockchip RGB output

[PATCH v5 1/2] devicetree/bindings: display: Add document for rockchip RGB output

[Sandy Huang]

This path add support rv1108 and px30 rgb output interface driver.

Signed-off-by: Sandy Huang <hjc@rock-chips.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1509522765-118759-1-git-send-email-hjc@rock-chips.com
---
 .../bindings/display/rockchip/rockchip-rgb.txt     | 70 ++++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt

diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
new file mode 100644
index 0000000..1f4909c
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
@@ -0,0 +1,70 @@
+Rockchip RV1108 RGB interface
+================================
+
+Required properties:
+- compatible: matching the soc type:
+	- "rockchip,px30-rgb";
+	- "rockchip,rv1108-rgb";
+
+Optional properties:
+- pinctrl-names: should be a "lcdc" entry or a "default" entry.
+- pinctrl-0: pin control group to be used for this interface.
+
+The rgb has two video ports described by:
+	Documentation/devicetree/bindings/media/video-interfaces.txt
+Their connections are modeled using the OF graph bindings specified in
+	Documentation/devicetree/bindings/graph.txt.
+
+- video port 0 for the VOP input, the remote endpoint maybe vopb/vopl/vop
+- video port 1 for either a panel or subsequent encoder
+
+the panel described by:
+	Documentation/devicetree/bindings/display/panel/simple-panel.txt
+Panel other required properties:
+- ports for remote rgb output.
+
+Example:
+
+panel: panel {
+	compatible = "auo,b101ean01";
+	enable-gpios = <&gpio7 21 GPIO_ACTIVE_HIGH>;
+
+	ports {
+		panel_in_rgb: endpoint {
+			remote-endpoint = <&rgb_out_panel>;
+		};
+	};
+};
+
+For Rockchip RV1108:
+
+	rgb: rgb {
+		compatible = "rockchip,rv1108-rgb";
+		pinctrl-names = "lcdc";
+		pinctrl-0 = <&lcdc_ctl>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			rgb_in: port@0 {
+				reg = <0>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				rgb_in_vop: endpoint {
+					remote-endpoint = <&vop_out_rgb>;
+				};
+			};
+
+			rgb_out: port@1 {
+				reg = <1>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				rgb_out_panel: endpoint {
+					remote-endpoint = <&panel_in_rgb>;
+				};
+			};
+		};
+	};
-- 
2.7.4

[PATCH v5 2/2] drm/rockchip: Add support for Rockchip Soc RGB output interface

[Sandy Huang]

Some Rockchip CRTCs, like rv1108 and px30, can directly output parallel
and serial RGB data to panel or conversion chip, so we add this driver
to probe encoder and connector.

Signed-off-by: Sandy Huang <hjc@rock-chips.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Mark Yao <mark.yao@rock-chips.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1509522851-128181-1-git-send-email-hjc@rock-chips.com
---
Changes in v5:
    1. add SPDX-License-Identifier tag
Changes in v4:
    1. add support px30;
Changes in v3:
    1. update for rgb-mode move to panel node.
Changes in v2:
    1. add error log when probe failed;
    2. update name_to_output_mode() according to sean's suggest;	
    3. Fix uninitialized use of ret.

 drivers/gpu/drm/rockchip/Kconfig            |  11 +
 drivers/gpu/drm/rockchip/Makefile           |   1 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c |   2 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   1 +
 drivers/gpu/drm/rockchip/rockchip_rgb.c     | 314 ++++++++++++++++++++++++++++
 5 files changed, 329 insertions(+)
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_rgb.c

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 0ccc762..e88eb71 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -8,6 +8,7 @@ config DRM_ROCKCHIP
 	select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
 	select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
 	select DRM_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
+	select DRM_RGB if ROCKCHIP_RGB
 	select SND_SOC_HDMI_CODEC if ROCKCHIP_CDN_DP && SND_SOC
 	help
 	  Choose this option if you have a Rockchip soc chipset.
@@ -66,4 +67,14 @@ config ROCKCHIP_LVDS
 	  Rockchip rk3288 SoC has LVDS TX Controller can be used, and it
 	  support LVDS, rgb, dual LVDS output mode. say Y to enable its
 	  driver.
+
+config ROCKCHIP_RGB
+	bool "Rockchip RGB support"
+	depends on DRM_ROCKCHIP
+	depends on PINCTRL
+	help
+	  Choose this option to enable support for Rockchip RGB output.
+	  Some Rockchip CRTCs, like rv1108, can directly output parallel
+	  and serial RGB format to panel or connect to a conversion chip.
+	  say Y to enable its driver.
 endif
diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
index a314e21..868263f 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -14,5 +14,6 @@ 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
 rockchipdrm-$(CONFIG_ROCKCHIP_LVDS) += rockchip_lvds.o
+rockchipdrm-$(CONFIG_ROCKCHIP_RGB) += rockchip_rgb.o
 
 obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 1d9c4a9..a9f35a7 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -432,6 +432,8 @@ static int __init rockchip_drm_init(void)
 				CONFIG_ROCKCHIP_LVDS);
 	ADD_ROCKCHIP_SUB_DRIVER(rockchip_dp_driver,
 				CONFIG_ROCKCHIP_ANALOGIX_DP);
+	ADD_ROCKCHIP_SUB_DRIVER(rockchip_rgb_driver,
+				CONFIG_ROCKCHIP_RGB);
 	ADD_ROCKCHIP_SUB_DRIVER(cdn_dp_driver, CONFIG_ROCKCHIP_CDN_DP);
 	ADD_ROCKCHIP_SUB_DRIVER(dw_hdmi_rockchip_pltfm_driver,
 				CONFIG_ROCKCHIP_DW_HDMI);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index d67ad0a..009cd34 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -70,5 +70,6 @@ extern struct platform_driver dw_mipi_dsi_driver;
 extern struct platform_driver inno_hdmi_driver;
 extern struct platform_driver rockchip_dp_driver;
 extern struct platform_driver rockchip_lvds_driver;
+extern struct platform_driver rockchip_rgb_driver;
 extern struct platform_driver vop_platform_driver;
 #endif /* _ROCKCHIP_DRM_DRV_H_ */
diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c b/drivers/gpu/drm/rockchip/rockchip_rgb.c
new file mode 100644
index 0000000..b7e226b
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
@@ -0,0 +1,314 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
+ * Author:
+ *      Sandy Huang <hjc@rock-chips.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_dp_helper.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_of.h>
+
+#include <linux/component.h>
+#include <linux/of_graph.h>
+
+#include "rockchip_drm_drv.h"
+#include "rockchip_drm_vop.h"
+
+#define connector_to_rgb(c) container_of(c, struct rockchip_rgb, connector)
+#define encoder_to_rgb(c) container_of(c, struct rockchip_rgb, encoder)
+
+struct rockchip_rgb {
+	struct device *dev;
+	struct drm_device *drm_dev;
+	struct drm_panel *panel;
+	struct drm_bridge *bridge;
+	struct drm_connector connector;
+	struct drm_encoder encoder;
+};
+
+static const struct drm_connector_funcs rockchip_rgb_connector_funcs = {
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.reset = drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int rockchip_rgb_connector_get_modes(struct drm_connector *connector)
+{
+	struct rockchip_rgb *rgb = connector_to_rgb(connector);
+	struct drm_panel *panel = rgb->panel;
+
+	return drm_panel_get_modes(panel);
+}
+
+static struct drm_encoder *
+rockchip_rgb_connector_best_encoder(struct drm_connector *connector)
+{
+	struct rockchip_rgb *rgb = connector_to_rgb(connector);
+
+	return &rgb->encoder;
+}
+
+static const
+struct drm_connector_helper_funcs rockchip_rgb_connector_helper_funcs = {
+	.get_modes = rockchip_rgb_connector_get_modes,
+	.best_encoder = rockchip_rgb_connector_best_encoder,
+};
+
+static void rockchip_rgb_encoder_enable(struct drm_encoder *encoder)
+{
+	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
+
+	drm_panel_prepare(rgb->panel);
+	drm_panel_enable(rgb->panel);
+}
+
+static void rockchip_rgb_encoder_disable(struct drm_encoder *encoder)
+{
+	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
+
+	drm_panel_disable(rgb->panel);
+	drm_panel_unprepare(rgb->panel);
+}
+
+static int
+rockchip_rgb_encoder_atomic_check(struct drm_encoder *encoder,
+				   struct drm_crtc_state *crtc_state,
+				   struct drm_connector_state *conn_state)
+{
+	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
+	struct drm_connector *connector = conn_state->connector;
+	struct drm_display_info *info = &connector->display_info;
+	u32 bus_format;
+
+	if (info->num_bus_formats)
+		bus_format = info->bus_formats[0];
+	else
+		bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+
+	switch (bus_format) {
+	case MEDIA_BUS_FMT_RGB666_1X18:
+		s->output_mode = ROCKCHIP_OUT_MODE_P666;
+		break;
+	case MEDIA_BUS_FMT_RGB565_1X16:
+		s->output_mode = ROCKCHIP_OUT_MODE_P565;
+		break;
+	case MEDIA_BUS_FMT_RGB888_1X24:
+	case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
+	default:
+		s->output_mode = ROCKCHIP_OUT_MODE_P888;
+		break;
+	}
+	s->output_type = DRM_MODE_CONNECTOR_LVDS;
+
+	return 0;
+}
+
+static const
+struct drm_encoder_helper_funcs rockchip_rgb_encoder_helper_funcs = {
+	.enable = rockchip_rgb_encoder_enable,
+	.disable = rockchip_rgb_encoder_disable,
+	.atomic_check = rockchip_rgb_encoder_atomic_check,
+};
+
+static const struct drm_encoder_funcs rockchip_rgb_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+static const struct of_device_id rockchip_rgb_dt_ids[] = {
+	{
+		.compatible = "rockchip,px30-rgb",
+	},
+	{
+		.compatible = "rockchip,rv1108-rgb",
+	},
+	{}
+};
+
+MODULE_DEVICE_TABLE(of, rockchip_rgb_dt_ids);
+
+static int rockchip_rgb_bind(struct device *dev, struct device *master,
+			     void *data)
+{
+	struct rockchip_rgb *rgb = dev_get_drvdata(dev);
+	struct drm_device *drm_dev = data;
+	struct drm_encoder *encoder;
+	struct drm_connector *connector;
+	struct device_node  *port, *endpoint;
+	u32 endpoint_id;
+	int ret = 0, child_count = 0;
+
+	rgb->drm_dev = drm_dev;
+	port = of_graph_get_port_by_id(dev->of_node, 1);
+	if (!port) {
+		DRM_DEV_ERROR(dev,
+			      "can't found port point, please init rgb panel port!\n");
+		return -EINVAL;
+	}
+	for_each_child_of_node(port, endpoint) {
+		child_count++;
+		if (of_property_read_u32(endpoint, "reg", &endpoint_id))
+			endpoint_id = 0;
+		ret = drm_of_find_panel_or_bridge(dev->of_node, 1, endpoint_id,
+						  &rgb->panel, &rgb->bridge);
+		if (!ret) {
+			of_node_put(endpoint);
+			break;
+		}
+	}
+	if (!child_count) {
+		DRM_DEV_ERROR(dev, "rgb port does not have any children\n");
+		ret = -EINVAL;
+		goto err_put_port;
+	} else if (ret) {
+		DRM_DEV_ERROR(dev, "failed to find panel and bridge node\n");
+		ret = -EPROBE_DEFER;
+		goto err_put_port;
+	}
+
+	encoder = &rgb->encoder;
+	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
+							     dev->of_node);
+
+	ret = drm_encoder_init(drm_dev, encoder, &rockchip_rgb_encoder_funcs,
+			       DRM_MODE_ENCODER_NONE, NULL);
+	if (ret < 0) {
+		DRM_DEV_ERROR(drm_dev->dev,
+			      "failed to initialize encoder: %d\n", ret);
+		goto err_put_port;
+	}
+
+	drm_encoder_helper_add(encoder, &rockchip_rgb_encoder_helper_funcs);
+
+	if (rgb->panel) {
+		connector = &rgb->connector;
+		connector->dpms = DRM_MODE_DPMS_OFF;
+		ret = drm_connector_init(drm_dev, connector,
+					 &rockchip_rgb_connector_funcs,
+					 DRM_MODE_CONNECTOR_Unknown);
+		if (ret < 0) {
+			DRM_DEV_ERROR(drm_dev->dev,
+				      "failed to initialize connector: %d\n",
+				      ret);
+			goto err_free_encoder;
+		}
+
+		drm_connector_helper_add(connector,
+					 &rockchip_rgb_connector_helper_funcs);
+
+		ret = drm_connector_attach_encoder(connector, encoder);
+		if (ret < 0) {
+			DRM_DEV_ERROR(drm_dev->dev,
+				      "failed to attach encoder: %d\n", ret);
+			goto err_free_connector;
+		}
+
+		ret = drm_panel_attach(rgb->panel, connector);
+		if (ret < 0) {
+			DRM_DEV_ERROR(drm_dev->dev,
+				      "failed to attach panel: %d\n", ret);
+			goto err_free_connector;
+		}
+	} else {
+		rgb->bridge->encoder = encoder;
+		ret = drm_bridge_attach(encoder, rgb->bridge, NULL);
+		if (ret) {
+			DRM_DEV_ERROR(drm_dev->dev,
+				      "failed to attach bridge: %d\n", ret);
+			goto err_free_encoder;
+		}
+		encoder->bridge = rgb->bridge;
+	}
+
+	of_node_put(port);
+
+	return 0;
+
+err_free_connector:
+	drm_connector_cleanup(connector);
+err_free_encoder:
+	drm_encoder_cleanup(encoder);
+err_put_port:
+	of_node_put(port);
+
+	return ret;
+}
+
+static void rockchip_rgb_unbind(struct device *dev, struct device *master,
+				void *data)
+{
+	struct rockchip_rgb *rgb = dev_get_drvdata(dev);
+
+	rockchip_rgb_encoder_disable(&rgb->encoder);
+	if (rgb->panel) {
+		drm_panel_detach(rgb->panel);
+		drm_connector_cleanup(&rgb->connector);
+	}
+	drm_encoder_cleanup(&rgb->encoder);
+}
+
+static const struct component_ops rockchip_rgb_component_ops = {
+	.bind = rockchip_rgb_bind,
+	.unbind = rockchip_rgb_unbind,
+};
+
+static int rockchip_rgb_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rockchip_rgb *rgb;
+	const struct of_device_id *match;
+	int ret;
+
+	if (!dev->of_node) {
+		DRM_DEV_ERROR(dev, "dev->of_node is null\n");
+		return -ENODEV;
+	}
+
+	rgb = devm_kzalloc(&pdev->dev, sizeof(*rgb), GFP_KERNEL);
+	if (!rgb)
+		return -ENOMEM;
+
+	rgb->dev = dev;
+	match = of_match_node(rockchip_rgb_dt_ids, dev->of_node);
+	if (!match) {
+		DRM_DEV_ERROR(dev, "match node failed\n");
+		return -ENODEV;
+	}
+
+	dev_set_drvdata(dev, rgb);
+	ret = component_add(&pdev->dev, &rockchip_rgb_component_ops);
+	if (ret < 0)
+		DRM_DEV_ERROR(dev, "failed to add component\n");
+
+	return ret;
+}
+
+static int rockchip_rgb_remove(struct platform_device *pdev)
+{
+	component_del(&pdev->dev, &rockchip_rgb_component_ops);
+
+	return 0;
+}
+
+struct platform_driver rockchip_rgb_driver = {
+	.probe = rockchip_rgb_probe,
+	.remove = rockchip_rgb_remove,
+	.driver = {
+		   .name = "rockchip-rgb",
+		   .of_match_table = of_match_ptr(rockchip_rgb_dt_ids),
+	},
+};
-- 
2.7.4

Re: [PATCH v5 1/2] devicetree/bindings: display: Add document for rockchip RGB output

[Michal Vokáč]

On 28.8.2018 10:12, Sandy Huang wrote:
> This path add support rv1108 and px30 rgb output interface driver.

Hi Sandy,
Seems like you forgot to address Rob's comment regarding the "driver" word.
Also fix the typo: s/path/patch
The subject could be improved as well: s/devicetree\/bindings/dt-bindings

Best regards, Michal
> 
> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/1509522765-118759-1-git-send-email-hjc@rock-chips.com
> ---
>   .../bindings/display/rockchip/rockchip-rgb.txt     | 70 ++++++++++++++++++++++
>   1 file changed, 70 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
> new file mode 100644
> index 0000000..1f4909c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-rgb.txt
> @@ -0,0 +1,70 @@
> +Rockchip RV1108 RGB interface
> +================================
> +
> +Required properties:
> +- compatible: matching the soc type:
> +	- "rockchip,px30-rgb";
> +	- "rockchip,rv1108-rgb";
> +
> +Optional properties:
> +- pinctrl-names: should be a "lcdc" entry or a "default" entry.
> +- pinctrl-0: pin control group to be used for this interface.
> +
> +The rgb has two video ports described by:
> +	Documentation/devicetree/bindings/media/video-interfaces.txt
> +Their connections are modeled using the OF graph bindings specified in
> +	Documentation/devicetree/bindings/graph.txt.
> +
> +- video port 0 for the VOP input, the remote endpoint maybe vopb/vopl/vop
> +- video port 1 for either a panel or subsequent encoder
> +
> +the panel described by:
> +	Documentation/devicetree/bindings/display/panel/simple-panel.txt
> +Panel other required properties:
> +- ports for remote rgb output.
> +
> +Example:
> +
> +panel: panel {
> +	compatible = "auo,b101ean01";
> +	enable-gpios = <&gpio7 21 GPIO_ACTIVE_HIGH>;
> +
> +	ports {
> +		panel_in_rgb: endpoint {
> +			remote-endpoint = <&rgb_out_panel>;
> +		};
> +	};
> +};
> +
> +For Rockchip RV1108:
> +
> +	rgb: rgb {
> +		compatible = "rockchip,rv1108-rgb";
> +		pinctrl-names = "lcdc";
> +		pinctrl-0 = <&lcdc_ctl>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			rgb_in: port@0 {
> +				reg = <0>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				rgb_in_vop: endpoint {
> +					remote-endpoint = <&vop_out_rgb>;
> +				};
> +			};
> +
> +			rgb_out: port@1 {
> +				reg = <1>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				rgb_out_panel: endpoint {
> +					remote-endpoint = <&panel_in_rgb>;
> +				};
> +			};
> +		};
> +	};
> 

Re: [PATCH v5 2/2] drm/rockchip: Add support for Rockchip Soc RGB output interface

[Heiko Stuebner]

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

Hi Sandy,

Am Dienstag, 28. August 2018, 10:12:34 CEST schrieb Sandy Huang:
> Some Rockchip CRTCs, like rv1108 and px30, can directly output parallel
> and serial RGB data to panel or conversion chip, so we add this driver
> to probe encoder and connector.
> 
> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
> Reviewed-by: Sean Paul <seanpaul@chromium.org>
> Reviewed-by: Mark Yao <mark.yao@rock-chips.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/1509522851-128181-1-git-send-email-hjc@rock-chips.com

This does not really address, Rob's concern about not being an actual
hardware block related to the devicetree node you are using.

I see your reply to my mail, but I guess you didn't see the alternative
approach I described a bit below in that mail?

Actual (unfinished) code included below. (2 patches)

Heiko



---- 8< -----
From: Heiko Stuebner <heiko@sntech.de>
Date: Sat, 25 Aug 2018 19:05:21 +0200
Subject: [PATCH 1/2] drm/tockchip: add function to check if endpoint is a
 subdriver

To be able to have both internal subdrivers and external bridge
drivers as output endpoints of vops, add a function to be able
to distinguish these.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 27 +++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |  1 +
 2 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 1d9c4a9201c8..d18f7f85aa23 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -24,6 +24,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/module.h>
 #include <linux/of_graph.h>
+#include <linux/of_platform.h>
 #include <linux/component.h>
 #include <linux/console.h>
 #include <linux/iommu.h>
@@ -267,6 +268,32 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
 static struct platform_driver *rockchip_sub_drivers[MAX_ROCKCHIP_SUB_DRIVERS];
 static int num_rockchip_sub_drivers;
 
+/*
+ * check if a vop output-endpoint is a subdriver or bridge.
+ */
+bool rockchip_drm_endpoint_is_subdriver(struct device_node *ep)
+{
+	struct device_node *node = of_graph_get_remote_port_parent(ep);
+	struct platform_device *pdev;
+	int i;
+
+	if (!node)
+		return false;
+
+	pdev = of_find_device_by_node(node);
+	if (!pdev)
+		return false;
+
+	for (i = 0; i < num_rockchip_sub_drivers; i++) {
+		struct device_driver *drv = pdev->dev.driver;
+
+		if (rockchip_sub_drivers[i] == to_platform_driver(drv))
+			return true;
+	}
+
+	return false;
+}
+
 static int compare_dev(struct device *dev, void *data)
 {
 	return dev == (struct device *)data;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index d67ad0a3cf36..305b4858c522 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -64,6 +64,7 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 				    struct device *dev);
 int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout);
 
+bool rockchip_drm_endpoint_is_subdriver(struct device_node *ep);
 extern struct platform_driver cdn_dp_driver;
 extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
 extern struct platform_driver dw_mipi_dsi_driver;
-- 
2.17.0
---- 8< -----
From: Sandy Huang <hjc@rock-chips.com>
Date: Tue, 26 Jun 2018 15:15:40 +0800
Subject: [PATCH 2/2] drm/rockchip: Add support for RGB output interface

Some Rockchip CRTCs, like rv1108 and px30, can directly output parallel
and serial RGB data to panel or conversion chip.

So add a feature-bit for vops to mark the ability for these direct outputs
and add an internal encoder in that case, that can attach to bridge chips
or panels.

Signed-off-by: Sandy Huang <hjc@rock-chips.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/gpu/drm/rockchip/Kconfig            |  11 +
 drivers/gpu/drm/rockchip/Makefile           |   1 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  16 ++
 drivers/gpu/drm/rockchip/rockchip_drm_vop.h |   1 +
 drivers/gpu/drm/rockchip/rockchip_rgb.c     | 212 ++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_rgb.h     |  19 ++
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c |   1 +
 7 files changed, 261 insertions(+)
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_rgb.c
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_rgb.h

diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index 0ccc76217ee4..e88eb715ae6b 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -8,6 +8,7 @@ config DRM_ROCKCHIP
 	select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
 	select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
 	select DRM_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
+	select DRM_RGB if ROCKCHIP_RGB
 	select SND_SOC_HDMI_CODEC if ROCKCHIP_CDN_DP && SND_SOC
 	help
 	  Choose this option if you have a Rockchip soc chipset.
@@ -66,4 +67,14 @@ config ROCKCHIP_LVDS
 	  Rockchip rk3288 SoC has LVDS TX Controller can be used, and it
 	  support LVDS, rgb, dual LVDS output mode. say Y to enable its
 	  driver.
+
+config ROCKCHIP_RGB
+	bool "Rockchip RGB support"
+	depends on DRM_ROCKCHIP
+	depends on PINCTRL
+	help
+	  Choose this option to enable support for Rockchip RGB output.
+	  Some Rockchip CRTCs, like rv1108, can directly output parallel
+	  and serial RGB format to panel or connect to a conversion chip.
+	  say Y to enable its driver.
 endif
diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
index a314e2109e76..868263ff0302 100644
--- a/drivers/gpu/drm/rockchip/Makefile
+++ b/drivers/gpu/drm/rockchip/Makefile
@@ -14,5 +14,6 @@ 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
 rockchipdrm-$(CONFIG_ROCKCHIP_LVDS) += rockchip_lvds.o
+rockchipdrm-$(CONFIG_ROCKCHIP_RGB) += rockchip_rgb.o
 
 obj-$(CONFIG_DRM_ROCKCHIP) += rockchipdrm.o
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 38f8cae7ef51..867b654fe2a3 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -41,6 +41,7 @@
 #include "rockchip_drm_fb.h"
 #include "rockchip_drm_psr.h"
 #include "rockchip_drm_vop.h"
+#include "rockchip_rgb.h"
 
 #define VOP_WIN_SET(x, win, name, v) \
 		vop_reg_set(vop, &win->phy->name, win->base, ~0, v, #name)
@@ -92,6 +93,7 @@ struct vop_win {
 	struct vop *vop;
 };
 
+struct rockchip_rgb;
 struct vop {
 	struct drm_crtc crtc;
 	struct device *dev;
@@ -135,6 +137,9 @@ struct vop {
 	/* vop dclk reset */
 	struct reset_control *dclk_rst;
 
+	/* optional internal rgb encoder */
+	struct rockchip_rgb *rgb;
+
 	struct vop_win win[];
 };
 
@@ -1638,6 +1643,14 @@ static int vop_bind(struct device *dev, struct device *master, void *data)
 	if (ret)
 		goto err_disable_pm_runtime;
 
+	if (vop->data->feature & VOP_FEATURE_INTERNAL_RGB) {
+		vop->rgb = rockchip_rgb_init(dev, &vop->crtc, vop->drm_dev);
+		if (IS_ERR(vop->rgb)) {
+			ret = PTR_ERR(vop->rgb);
+			goto err_disable_pm_runtime;
+		}
+	}
+
 	return 0;
 
 err_disable_pm_runtime:
@@ -1650,6 +1663,9 @@ static void vop_unbind(struct device *dev, struct device *master, void *data)
 {
 	struct vop *vop = dev_get_drvdata(dev);
 
+	if (vop->rgb)
+		rockchip_rgb_fini(vop->rgb);
+
 	pm_runtime_disable(dev);
 	vop_destroy_crtc(vop);
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
index fcb91041a666..fd5765dfd637 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h
@@ -162,6 +162,7 @@ struct vop_data {
 	unsigned int win_size;
 
 #define VOP_FEATURE_OUTPUT_RGB10	BIT(0)
+#define VOP_FEATURE_INTERNAL_RGB	BIT(1)
 	u64 feature;
 };
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c b/drivers/gpu/drm/rockchip/rockchip_rgb.c
new file mode 100644
index 000000000000..6180369e591f
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
@@ -0,0 +1,212 @@
+/*
+ * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
+ * Author:
+ *      Sandy Huang <hjc@rock-chips.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_dp_helper.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_of.h>
+
+#include <linux/component.h>
+#include <linux/of_graph.h>
+
+#include "rockchip_drm_drv.h"
+#include "rockchip_drm_vop.h"
+
+#define encoder_to_rgb(c) container_of(c, struct rockchip_rgb, encoder)
+
+struct rockchip_rgb {
+	struct device *dev;
+	struct drm_device *drm_dev;
+	struct drm_bridge *bridge;
+	struct drm_encoder encoder;
+//	struct dev_pin_info *pins;
+	int output_mode;
+};
+
+static inline int name_to_output_mode(const char *s)
+{
+	static const struct {
+		const char *name;
+		int format;
+	} formats[] = {
+		{ "p888", ROCKCHIP_OUT_MODE_P888 },
+		{ "p666", ROCKCHIP_OUT_MODE_P666 },
+		{ "p565", ROCKCHIP_OUT_MODE_P565 }
+	};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(formats); i++)
+		if (!strncmp(s, formats[i].name, strlen(formats[i].name)))
+			return formats[i].format;
+
+	return -EINVAL;
+}
+
+static void rockchip_rgb_encoder_enable(struct drm_encoder *encoder)
+{
+	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
+
+//	if (rgb->pins && !IS_ERR(rgb->pins->default_state))
+//		pinctrl_select_state(rgb->pins->p, rgb->pins->default_state);
+}
+
+static void rockchip_rgb_encoder_disable(struct drm_encoder *encoder)
+{
+	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
+
+//FIXME: pin-voodoo?
+}
+
+static int
+rockchip_rgb_encoder_atomic_check(struct drm_encoder *encoder,
+				   struct drm_crtc_state *crtc_state,
+				   struct drm_connector_state *conn_state)
+{
+	struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state);
+	struct rockchip_rgb *rgb = encoder_to_rgb(encoder);
+
+	s->output_mode = rgb->output_mode;
+	s->output_type = DRM_MODE_CONNECTOR_LVDS;
+
+	return 0;
+}
+
+static const
+struct drm_encoder_helper_funcs rockchip_rgb_encoder_helper_funcs = {
+	.enable = rockchip_rgb_encoder_enable,
+	.disable = rockchip_rgb_encoder_disable,
+	.atomic_check = rockchip_rgb_encoder_atomic_check,
+};
+
+static const struct drm_encoder_funcs rockchip_rgb_encoder_funcs = {
+	.destroy = drm_encoder_cleanup,
+};
+
+struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
+				       struct drm_crtc *crtc,
+				       struct drm_device *drm_dev)
+{
+	struct rockchip_rgb *rgb;
+	struct drm_encoder *encoder;
+	struct device_node *port, *endpoint;
+	u32 endpoint_id;
+	int ret = 0, child_count = 0;
+	struct drm_panel *panel;
+	struct drm_bridge *bridge;
+
+	rgb = devm_kzalloc(dev, sizeof(*rgb), GFP_KERNEL);
+	if (!rgb)
+		return ERR_PTR(-ENOMEM);
+
+	rgb->dev = dev;
+	rgb->drm_dev = drm_dev;
+
+/*	rgb->pins = devm_kzalloc(rgb->dev, sizeof(*rgb->pins), GFP_KERNEL);
+	if (!rgb->pins)
+		return -ENOMEM; */
+
+/*	rgb->pins->p = devm_pinctrl_get(rgb->dev);
+	if (IS_ERR(rgb->pins->p)) {
+		DRM_DEV_ERROR(dev, "no pinctrl handle\n");
+		devm_kfree(rgb->dev, rgb->pins);
+		rgb->pins = NULL;
+	} else {
+		rgb->pins->default_state =
+			pinctrl_lookup_state(rgb->pins->p, "lcdc");
+		if (IS_ERR(rgb->pins->default_state)) {
+			DRM_DEV_ERROR(dev, "no default pinctrl state\n");
+			devm_kfree(rgb->dev, rgb->pins);
+			rgb->pins = NULL;
+		}
+	} */
+
+	port = of_graph_get_port_by_id(dev->of_node, 0);
+	if (!port)
+		return ERR_PTR(-EINVAL);
+
+	for_each_child_of_node(port, endpoint) {
+		if (of_property_read_u32(endpoint, "reg", &endpoint_id))
+			endpoint_id = 0;
+
+		if (rockchip_drm_endpoint_is_subdriver(endpoint))
+			continue;
+
+		child_count++;
+		ret = drm_of_find_panel_or_bridge(dev->of_node, 0, endpoint_id,
+						  &panel, &bridge);
+		if (!ret)
+			break;
+	}
+
+	of_node_put(port);
+
+	/* if the rgb output is not connected to anything, just return */
+	if (!child_count)
+		return NULL;
+
+	if (ret < 0) {
+		if (ret != -EPROBE_DEFER)
+			DRM_DEV_ERROR(dev, "failed to find panel or bridge %d\n", ret);
+		return ERR_PTR(ret);
+	}
+
+	rgb->output_mode = ROCKCHIP_OUT_MODE_P888;
+
+	encoder = &rgb->encoder;
+	encoder->possible_crtcs = drm_crtc_mask(crtc);
+
+	ret = drm_encoder_init(drm_dev, encoder, &rockchip_rgb_encoder_funcs,
+			       DRM_MODE_ENCODER_NONE, NULL);
+	if (ret < 0) {
+		DRM_DEV_ERROR(drm_dev->dev,
+			      "failed to initialize encoder: %d\n", ret);
+		return ERR_PTR(ret);
+	}
+
+	drm_encoder_helper_add(encoder, &rockchip_rgb_encoder_helper_funcs);
+
+	if (panel) {
+		bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_LVDS);
+		if (IS_ERR(bridge))
+			return ERR_CAST(bridge);
+	}
+
+	rgb->bridge = bridge;
+
+	ret = drm_bridge_attach(encoder, rgb->bridge, NULL);
+	if (ret) {
+		DRM_DEV_ERROR(drm_dev->dev,
+			      "failed to attach bridge: %d\n", ret);
+		goto err_free_encoder;
+	}
+
+	return rgb;
+
+err_free_encoder:
+	drm_encoder_cleanup(encoder);
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(rockchip_rgb_init);
+
+void rockchip_rgb_fini(struct rockchip_rgb *rgb)
+{
+	rockchip_rgb_encoder_disable(&rgb->encoder);
+
+	drm_panel_bridge_remove(rgb->bridge);
+	drm_encoder_cleanup(&rgb->encoder);
+}
+EXPORT_SYMBOL_GPL(rockchip_rgb_fini);
diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.h b/drivers/gpu/drm/rockchip/rockchip_rgb.h
new file mode 100644
index 000000000000..16f05d50abc3
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
+ * Author:
+ *      Sandy Huang <hjc@rock-chips.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
+				       struct drm_crtc *crtc,
+				       struct drm_device *drm_dev);
+void rockchip_rgb_fini(struct rockchip_rgb *rgb);
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 0c5358350159..f4dd0b143467 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -487,6 +487,7 @@ static const struct vop_data rk3188_vop = {
 	.output = &rk3188_output,
 	.win = rk3188_vop_win_data,
 	.win_size = ARRAY_SIZE(rk3188_vop_win_data),
+	.feature = VOP_FEATURE_INTERNAL_RGB,
 };
 
 static const struct vop_scl_extension rk3288_win_full_scl_ext = {
-- 
2.17.0

[-- Attachment #2: 0001-drm-tockchip-add-function-to-check-if-endpoint-is-a-.patch --]
[-- Type: text/x-patch, Size: 2583 bytes --]

From c5a48c772f3564211dcd1ec71004de1d6dbb0307 Mon Sep 17 00:00:00 2001
From: Heiko Stuebner <heiko@sntech.de>
Date: Sat, 25 Aug 2018 19:05:21 +0200
Subject: [PATCH 1/2] drm/tockchip: add function to check if endpoint is a
 subdriver

To be able to have both internal subdrivers and external bridge
drivers as output endpoints of vops, add a function to be able
to distinguish these.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 27 +++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |  1 +
 2 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 1d9c4a9201c8..d18f7f85aa23 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -24,6 +24,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/module.h>
 #include <linux/of_graph.h>
+#include <linux/of_platform.h>
 #include <linux/component.h>
 #include <linux/console.h>
 #include <linux/iommu.h>
@@ -267,6 +268,32 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
 static struct platform_driver *rockchip_sub_drivers[MAX_ROCKCHIP_SUB_DRIVERS];
 static int num_rockchip_sub_drivers;
 
+/*
+ * check if a vop output-endpoint is a subdriver or bridge.
+ */
+bool rockchip_drm_endpoint_is_subdriver(struct device_node *ep)
+{
+	struct device_node *node = of_graph_get_remote_port_parent(ep);
+	struct platform_device *pdev;
+	int i;
+
+	if (!node)
+		return false;
+
+	pdev = of_find_device_by_node(node);
+	if (!pdev)
+		return false;
+
+	for (i = 0; i < num_rockchip_sub_drivers; i++) {
+		struct device_driver *drv = pdev->dev.driver;
+
+		if (rockchip_sub_drivers[i] == to_platform_driver(drv))
+			return true;
+	}
+
+	return false;
+}
+
 static int compare_dev(struct device *dev, void *data)
 {
 	return dev == (struct device *)data;
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
index d67ad0a3cf36..305b4858c522 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
@@ -64,6 +64,7 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
 				    struct device *dev);
 int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout);
 
+bool rockchip_drm_endpoint_is_subdriver(struct device_node *ep);
 extern struct platform_driver cdn_dp_driver;
 extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
 extern struct platform_driver dw_mipi_dsi_driver;
-- 
2.17.0

Re: [PATCH v5 2/2] drm/rockchip: Add support for Rockchip Soc RGB output interface

[Sandy Huang]

Hi hieko,

在 2018/8/28 16:31, Heiko Stuebner 写道:
> Hi Sandy,
> 
> Am Dienstag, 28. August 2018, 10:12:34 CEST schrieb Sandy Huang:
>> Some Rockchip CRTCs, like rv1108 and px30, can directly output parallel
>> and serial RGB data to panel or conversion chip, so we add this driver
>> to probe encoder and connector.
>>
>> Signed-off-by: Sandy Huang <hjc@rock-chips.com>
>> Reviewed-by: Sean Paul <seanpaul@chromium.org>
>> Reviewed-by: Mark Yao <mark.yao@rock-chips.com>
>> Link: https://patchwork.freedesktop.org/patch/msgid/1509522851-128181-1-git-send-email-hjc@rock-chips.com
> 
> This does not really address, Rob's concern about not being an actual
> hardware block related to the devicetree node you are using.
> 
> I see your reply to my mail, but I guess you didn't see the alternative
> approach I described a bit below in that mail?
> 
> Actual (unfinished) code included below. (2 patches)
> 
> Heiko
> 
> 
yes,i didn't see the behind described:(
As you said devicetree describes hardware blocks,
I think you implement driver for rgb output interface is more 
reasonable,  i will adjust this driver refer to your implementation, 
thanks:)


> 
> ---- 8< -----
> From: Heiko Stuebner <heiko@sntech.de>
> Date: Sat, 25 Aug 2018 19:05:21 +0200
> Subject: [PATCH 1/2] drm/tockchip: add function to check if endpoint is a
>   subdriver
> 
> To be able to have both internal subdrivers and external bridge
> drivers as output endpoints of vops, add a function to be able
> to distinguish these.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>   drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 27 +++++++++++++++++++++
>   drivers/gpu/drm/rockchip/rockchip_drm_drv.h |  1 +
>   2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 1d9c4a9201c8..d18f7f85aa23 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -24,6 +24,7 @@
>   #include <linux/pm_runtime.h>
>   #include <linux/module.h>
>   #include <linux/of_graph.h>
> +#include <linux/of_platform.h>
>   #include <linux/component.h>
>   #include <linux/console.h>
>   #include <linux/iommu.h>
> @@ -267,6 +268,32 @@ static const struct dev_pm_ops rockchip_drm_pm_ops = {
>   static struct platform_driver *rockchip_sub_drivers[MAX_ROCKCHIP_SUB_DRIVERS];
>   static int num_rockchip_sub_drivers;
>   
> +/*
> + * check if a vop output-endpoint is a subdriver or bridge.
> + */
> +bool rockchip_drm_endpoint_is_subdriver(struct device_node *ep)
> +{
> +	struct device_node *node = of_graph_get_remote_port_parent(ep);
> +	struct platform_device *pdev;
> +	int i;
> +
> +	if (!node)
> +		return false;
> +
> +	pdev = of_find_device_by_node(node);
> +	if (!pdev)
> +		return false;
> +
> +	for (i = 0; i < num_rockchip_sub_drivers; i++) {
> +		struct device_driver *drv = pdev->dev.driver;
> +
> +		if (rockchip_sub_drivers[i] == to_platform_driver(drv))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>   static int compare_dev(struct device *dev, void *data)
>   {
>   	return dev == (struct device *)data;
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> index d67ad0a3cf36..305b4858c522 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
> @@ -64,6 +64,7 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
>   				    struct device *dev);
>   int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout);
>   
> +bool rockchip_drm_endpoint_is_subdriver(struct device_node *ep);
>   extern struct platform_driver cdn_dp_driver;
>   extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
>   extern struct platform_driver dw_mipi_dsi_driver;
> 
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 

Re: [PATCH v5 1/2] devicetree/bindings: display: Add document for rockchip RGB output

[Heiko Stuebner]

Hi Michal,

Am Dienstag, 28. August 2018, 10:31:15 CEST schrieb Michal Vokáč:
> On 28.8.2018 10:12, Sandy Huang wrote:
> > This path add support rv1108 and px30 rgb output interface driver.
> 
> Hi Sandy,
> Seems like you forgot to address Rob's comment regarding the "driver" word.
> Also fix the typo: s/path/patch
> The subject could be improved as well: s/devicetree\/bindings/dt-bindings

We actually honored Rob's bigger concern of the rgb output not being
an actual device backed by programmable hardware and moved the rgb
output inside the driver, see v7 of the patchset [0].


Heiko

[0] https://patchwork.freedesktop.org/patch/246297/
https://patchwork.freedesktop.org/patch/246298/
https://patchwork.freedesktop.org/patch/246299/