[PATCH v8 0/3] Add support rockchip RGB output interface

[PATCH v8 0/3] Add support rockchip RGB output interface

[Heiko Stuebner]

This patches add support rockchip RGB output, Some Rockchip CRTCs, like
rv1108 and px30 can directly output parallel and serial RGB data to panel
or to 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.


Hi Sandy,

while reviewing my own code in patch1, I found that it was missing
correct put calls for node and pdev and also made it behave better
if no device is present.

While we don't need that right now, it makes it possible to
distinguish between subdriver, external bridge or no device later
on if needed.

I've also updated the documentation of the function a bit, so that
people looking at it can grasp it hopefully easier :-)

And finally I've dropped the Review tag from Mark. He isn't at
Rockchip for a while now and the driver did change over time a bit.

So if you have time, please take a final look at it.

Thanks
Heiko


Heiko Stuebner (1):
  drm/rockchip: add function to check if endpoint is a subdriver

Sandy Huang (2):
  drm/rockchip: Add support for Rockchip Soc RGB output interface
  drm/rockchip: Add directly output rgb feature for px30

 drivers/gpu/drm/rockchip/Kconfig            |  11 ++
 drivers/gpu/drm/rockchip/Makefile           |   1 +
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c |  48 ++++++
 drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   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     | 173 ++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_rgb.h     |  20 +++
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c |   2 +
 9 files changed, 273 insertions(+)
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_rgb.c
 create mode 100644 drivers/gpu/drm/rockchip/rockchip_rgb.h

-- 
2.17.0

[PATCH v8 1/3] drm/rockchip: add function to check if endpoint is a subdriver

[Heiko Stuebner]

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.

changes in v8:
- improved function documentation
- better error handling
- put calls for node and pdev references
changes in v6:
- added function to check subdriver vs. bridge

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

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 1d9c4a9201c8..5864cb452c5c 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,53 @@ 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 endpoint is leading to a rockchip subdriver or bridge.
+ * Should be called from the component bind stage of the drivers
+ * to ensure that all subdrivers are probed.
+ *
+ * @ep: endpoint of a rockchip vop
+ *
+ * returns true if subdriver, false if external bridge and -ENODEV
+ * if remote port does not contain a device.
+ */
+int rockchip_drm_endpoint_is_subdriver(struct device_node *ep)
+{
+	struct device_node *node = of_graph_get_remote_port_parent(ep);
+	struct platform_device *pdev;
+	struct device_driver *drv;
+	int i;
+
+	if (!node)
+		return -ENODEV;
+
+	/* status disabled will prevent creation of platform-devices */
+	pdev = of_find_device_by_node(node);
+	of_node_put(node);
+	if (!pdev)
+		return -ENODEV;
+
+	/*
+	 * All rockchip subdrivers have probed at this point, so
+	 * any device not having a driver now is an external bridge.
+	 */
+	drv = pdev->dev.driver;
+	if (!drv) {
+		platform_device_put(pdev);
+		return false;
+	}
+
+	for (i = 0; i < num_rockchip_sub_drivers; i++) {
+		if (rockchip_sub_drivers[i] == to_platform_driver(drv)) {
+			platform_device_put(pdev);
+			return true;
+		}
+	}
+
+	platform_device_put(pdev);
+	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..21a023a97bb8 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);
 
+int 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 v8 1/3] drm/rockchip: add function to check if endpoint is a subdriver

[Sandy Huang]

在 2018/8/31 5:12, Heiko Stuebner 写道:
> 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.
> 
> changes in v8:
> - improved function documentation
> - better error handling
> - put calls for node and pdev references
> changes in v6:
> - added function to check subdriver vs. bridge
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>   drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 48 +++++++++++++++++++++
>   drivers/gpu/drm/rockchip/rockchip_drm_drv.h |  1 +
>   2 files changed, 49 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 1d9c4a9201c8..5864cb452c5c 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,53 @@ 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 endpoint is leading to a rockchip subdriver or bridge.
> + * Should be called from the component bind stage of the drivers
> + * to ensure that all subdrivers are probed.
> + *
> + * @ep: endpoint of a rockchip vop
> + *
> + * returns true if subdriver, false if external bridge and -ENODEV
> + * if remote port does not contain a device.
> + */
> +int rockchip_drm_endpoint_is_subdriver(struct device_node *ep)
> +{
> +	struct device_node *node = of_graph_get_remote_port_parent(ep);
> +	struct platform_device *pdev;
> +	struct device_driver *drv;
> +	int i;
> +
> +	if (!node)
> +		return -ENODEV;
> +
> +	/* status disabled will prevent creation of platform-devices */
> +	pdev = of_find_device_by_node(node);
> +	of_node_put(node);
> +	if (!pdev)
> +		return -ENODEV;
> +
> +	/*
> +	 * All rockchip subdrivers have probed at this point, so
> +	 * any device not having a driver now is an external bridge.
> +	 */
> +	drv = pdev->dev.driver;
> +	if (!drv) {
> +		platform_device_put(pdev);
> +		return false;
> +	}
> +
> +	for (i = 0; i < num_rockchip_sub_drivers; i++) {
> +		if (rockchip_sub_drivers[i] == to_platform_driver(drv)) {
> +			platform_device_put(pdev);
> +			return true;
> +		}
> +	}
> +
> +	platform_device_put(pdev);
> +	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..21a023a97bb8 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);
>   
> +int 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;
> 

Reviewed-by Sandy Huang <hjc@rock-chips.com>

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

[Heiko Stuebner]

From: Sandy Huang <hjc@rock-chips.com>

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 chipsor panels.

Changes in v7:
    1. forget to delete rockchip_rgb_driver and delete it.
Changes in v6:
    1. Update according to Heiko Stuebner' implemention, rgb output is
       part of vop's feature, should not register as a independent
       driver.
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.

Signed-off-by: Sandy Huang <hjc@rock-chips.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
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     | 173 ++++++++++++++++++++
 drivers/gpu/drm/rockchip/rockchip_rgb.h     |  20 +++
 6 files changed, 222 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 0534dc1ca32a..26438d45732b 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 17f9ed688841..0c35a88e33dd 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -42,6 +42,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)
@@ -93,6 +94,7 @@ struct vop_win {
 	struct vop *vop;
 };
 
+struct rockchip_rgb;
 struct vop {
 	struct drm_crtc crtc;
 	struct device *dev;
@@ -136,6 +138,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..96ac1458a59c
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
@@ -0,0 +1,173 @@
+//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 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;
+	int output_mode;
+};
+
+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 = {
+	.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;
+
+	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) > 0)
+			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);
+	}
+
+	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)
+{
+	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..c712a673d1e3
--- /dev/null
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.h
@@ -0,0 +1,20 @@
+//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.
+ */
+
+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);
-- 
2.17.0

Re: [v8, 2/3] drm/rockchip: Add support for Rockchip Soc RGB output interface

[Thomas Gleixner]

Sandy, Heiko,

On Thu, 30 Aug 2018, Heiko Stuebner wrote:

> +++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
> @@ -0,0 +1,173 @@
> +//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.

The licensing in this file and in the corresponding header is broken.

> --- /dev/null
> +++ b/drivers/gpu/drm/rockchip/rockchip_rgb.h
> @@ -0,0 +1,20 @@
> +//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.
> + */

Both files use the SPDX license identifier for GPL v2 or later, but the
license notice is clearly v2 only.

So the copyright holder needs to decide which one. And please remove the
boiler plate text. It's redundant.

See also Documentation/process/license-rules.rst

Please fix ASAP, add a Fixes tag and cc stable so the inconsistency is
fixed in 4.20 as well.

Thanks,

	tglx

Re: [v8, 2/3] drm/rockchip: Add support for Rockchip Soc RGB output interface

[Heiko Stuebner]

Hi Thomas,

Am Freitag, 18. Januar 2019, 01:40:03 CET schrieb Thomas Gleixner:
> On Thu, 30 Aug 2018, Heiko Stuebner wrote:
> > +++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
> > @@ -0,0 +1,173 @@
> > +//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.
> 
> The licensing in this file and in the corresponding header is broken.
> 
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rockchip/rockchip_rgb.h
> > @@ -0,0 +1,20 @@
> > +//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.
> > + */
> 
> Both files use the SPDX license identifier for GPL v2 or later, but the
> license notice is clearly v2 only.
> 
> So the copyright holder needs to decide which one. And please remove the
> boiler plate text. It's redundant.

Not sure how to proceed as I haven't seen Sandy around for quite a bit,
but the first versions didn't have a SPDX header and were GPL2-only (v1-v4)
and the SPDX header was added in v5.

So as a stopgap measure, I'd guess we could assume that the copyright
intended GPL2-only?


> Please fix ASAP, add a Fixes tag and cc stable so the inconsistency is
> fixed in 4.20 as well.

ok, once we have the direction of change set.


Heiko

Re: [v8, 2/3] drm/rockchip: Add support for Rockchip Soc RGB output interface

[sandy.huang]

Hi heiko and thomas,

the SPDX license identifier is updated by the following patch, please 
help to review, thanks.

https://patchwork.kernel.org/patch/10776935/

在 2019/1/18 下午8:34, Heiko Stuebner 写道:
> Hi Thomas,
>
> Am Freitag, 18. Januar 2019, 01:40:03 CET schrieb Thomas Gleixner:
>> On Thu, 30 Aug 2018, Heiko Stuebner wrote:
>>> +++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
>>> @@ -0,0 +1,173 @@
>>> +//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.
>> The licensing in this file and in the corresponding header is broken.
>>
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/rockchip/rockchip_rgb.h
>>> @@ -0,0 +1,20 @@
>>> +//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.
>>> + */
>> Both files use the SPDX license identifier for GPL v2 or later, but the
>> license notice is clearly v2 only.
>>
>> So the copyright holder needs to decide which one. And please remove the
>> boiler plate text. It's redundant.
> Not sure how to proceed as I haven't seen Sandy around for quite a bit,
> but the first versions didn't have a SPDX header and were GPL2-only (v1-v4)
> and the SPDX header was added in v5.
>
> So as a stopgap measure, I'd guess we could assume that the copyright
> intended GPL2-only?
>
>
>> Please fix ASAP, add a Fixes tag and cc stable so the inconsistency is
>> fixed in 4.20 as well.
> ok, once we have the direction of change set.
>
>
> Heiko
>
>
>
>
>

[PATCH v8 3/3] drm/rockchip: Add directly output rgb feature for px30

[Heiko Stuebner]

From: Sandy Huang <hjc@rock-chips.com>

Add this feature bit indicate px30 vop can directly output
parallel or serial rgb data.

Signed-off-by: Sandy Huang <hjc@rock-chips.com>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
index 7f29913824ce..09910d3b01ce 100644
--- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
+++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
@@ -276,6 +276,7 @@ static const struct vop_win_data px30_vop_big_win_data[] = {
 
 static const struct vop_data px30_vop_big = {
 	.intr = &px30_intr,
+	.feature = VOP_FEATURE_INTERNAL_RGB,
 	.common = &px30_common,
 	.modeset = &px30_modeset,
 	.output = &px30_output,
@@ -290,6 +291,7 @@ static const struct vop_win_data px30_vop_lit_win_data[] = {
 
 static const struct vop_data px30_vop_lit = {
 	.intr = &px30_intr,
+	.feature = VOP_FEATURE_INTERNAL_RGB,
 	.common = &px30_common,
 	.modeset = &px30_modeset,
 	.output = &px30_output,
-- 
2.17.0

Re: [PATCH v8 0/3] Add support rockchip RGB output interface

[Heiko Stuebner]

Am Donnerstag, 30. August 2018, 23:12:04 CEST schrieb Heiko Stuebner:
> This patches add support rockchip RGB output, Some Rockchip CRTCs, like
> rv1108 and px30 can directly output parallel and serial RGB data to panel
> or to 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.

applied to drm-misc-next