Re:Re: [PATCH 3/6] drm/rockchip: Add RK3588 DPTX output support

["Andy Yan" <andyshrk@163.com>]

Hi Dmitry,

At 2025-03-02 02:19:24, "Dmitry Baryshkov" <dmitry.baryshkov@linaro.org> wrote:
>On Sun, Feb 23, 2025 at 07:30:26PM +0800, Andy Yan wrote:
>> From: Andy Yan <andy.yan@rock-chips.com>
>> 
>> Add driver extension for Synopsys DesignWare DPTX IP used
>> on Rockchip RK3588 SoC.
>> 
>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com>
>> ---
>> 
>>  drivers/gpu/drm/rockchip/Kconfig            |   7 +
>>  drivers/gpu/drm/rockchip/Makefile           |   1 +
>>  drivers/gpu/drm/rockchip/dw_dp-rockchip.c   | 162 ++++++++++++++++++++
>>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c |   1 +
>>  drivers/gpu/drm/rockchip/rockchip_drm_drv.h |   1 +
>>  5 files changed, 172 insertions(+)
>>  create mode 100644 drivers/gpu/drm/rockchip/dw_dp-rockchip.c
>> 
>> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
>> index 26c4410b2407..c8638baf9641 100644
>> --- a/drivers/gpu/drm/rockchip/Kconfig
>> +++ b/drivers/gpu/drm/rockchip/Kconfig
>> @@ -8,6 +8,7 @@ config DRM_ROCKCHIP
>>  	select DRM_PANEL
>>  	select VIDEOMODE_HELPERS
>>  	select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
>> +	select DRM_DW_DP if ROCKCHIP_DW_DP
>>  	select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
>>  	select DRM_DW_HDMI_QP if ROCKCHIP_DW_HDMI_QP
>>  	select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
>> @@ -58,6 +59,12 @@ config ROCKCHIP_CDN_DP
>>  	  RK3399 based SoC, you should select this
>>  	  option.
>>  
>> +config ROCKCHIP_DW_DP
>> +	bool "Rockchip specific extensions for Synopsys DW DP"
>> +	help
>> +	  Choose this option for Synopsys DesignWare Cores DisplayPort
>> +	  transmit controller support on Rockchip SoC.
>> +
>>  config ROCKCHIP_DW_HDMI
>>  	bool "Rockchip specific extensions for Synopsys DW HDMI"
>>  	help
>> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
>> index 2b867cebbc12..097f062399c7 100644
>> --- a/drivers/gpu/drm/rockchip/Makefile
>> +++ b/drivers/gpu/drm/rockchip/Makefile
>> @@ -14,6 +14,7 @@ rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
>>  rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI_QP) += dw_hdmi_qp-rockchip.o
>>  rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi-rockchip.o
>>  rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI2) += dw-mipi-dsi2-rockchip.o
>> +rockchipdrm-$(CONFIG_ROCKCHIP_DW_DP) += dw_dp-rockchip.o
>>  rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
>>  rockchipdrm-$(CONFIG_ROCKCHIP_LVDS) += rockchip_lvds.o
>>  rockchipdrm-$(CONFIG_ROCKCHIP_RGB) += rockchip_rgb.o
>> diff --git a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
>> new file mode 100644
>> index 000000000000..b41a41eb74d7
>> --- /dev/null
>> +++ b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
>> @@ -0,0 +1,162 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2020 Rockchip Electronics Co., Ltd.
>> + *
>> + * Author: Zhang Yubing <yubing.zhang@rock-chips.com>
>> + * Author: Andy Yan <andy.yan@rock-chips.com>
>> + */
>> +
>> +#include <linux/component.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <drm/bridge/dw_dp.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_bridge.h>
>> +#include <drm/drm_bridge_connector.h>
>> +#include <drm/drm_of.h>
>> +#include <drm/drm_print.h>
>> +#include <drm/drm_probe_helper.h>
>> +#include <drm/drm_simple_kms_helper.h>
>> +
>> +#include <uapi/linux/media-bus-format.h>
>> +#include <uapi/linux/videodev2.h>
>
>I'd say, include those two headers directly. Rockchip / Synopsys are the
>only drivers including these uapi headers directly.

Okay, I will include them directly in V2 like this:

+#include <linux/media-bus-format.h>
+#include <linux/videodev2.h>

>
>> +
>> +#include "rockchip_drm_drv.h"
>> +#include "rockchip_drm_vop.h"
>> +
>> +struct rockchip_dw_dp {
>> +	struct dw_dp *base;
>> +	struct device *dev;
>> +	struct rockchip_encoder encoder;
>> +};
>> +
>> +static inline struct rockchip_dw_dp *encoder_to_dp(struct drm_encoder *encoder)
>> +{
>> +	struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder);
>> +
>> +	return container_of(rkencoder, struct rockchip_dw_dp, encoder);
>> +}
>> +
>> +static int dw_dp_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_atomic_state *state = conn_state->state;
>> +	struct drm_display_info *di = &conn_state->connector->display_info;
>> +	struct drm_bridge *bridge  = drm_bridge_chain_get_first_bridge(encoder);
>> +	struct drm_bridge_state *bridge_state = drm_atomic_get_new_bridge_state(state, bridge);
>> +	u32 bus_format = bridge_state->input_bus_cfg.format;
>> +
>> +	switch (bus_format) {
>> +	case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
>> +	case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
>> +		s->output_mode = ROCKCHIP_OUT_MODE_YUV420;
>> +		break;
>> +	case MEDIA_BUS_FMT_YUYV10_1X20:
>> +	case MEDIA_BUS_FMT_YUYV8_1X16:
>> +		s->output_mode = ROCKCHIP_OUT_MODE_S888_DUMMY;
>> +		break;
>> +	case MEDIA_BUS_FMT_RGB101010_1X30:
>> +	case MEDIA_BUS_FMT_RGB888_1X24:
>> +	case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
>> +	case MEDIA_BUS_FMT_YUV10_1X30:
>> +	case MEDIA_BUS_FMT_YUV8_1X24:
>> +	default:
>> +		s->output_mode = ROCKCHIP_OUT_MODE_AAAA;
>> +		break;
>> +	}
>> +
>> +	s->output_type = DRM_MODE_CONNECTOR_DisplayPort;
>> +	s->bus_format = bus_format;
>> +	s->bus_flags = di->bus_flags;
>> +	s->color_space = V4L2_COLORSPACE_DEFAULT;
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct drm_encoder_helper_funcs dw_dp_encoder_helper_funcs = {
>> +	.atomic_check		= dw_dp_encoder_atomic_check,
>> +};
>> +
>> +static int dw_dp_rockchip_bind(struct device *dev, struct device *master, void *data)
>> +{
>> +	struct dw_dp_plat_data plat_data;
>> +	struct drm_device *drm_dev = data;
>> +	struct rockchip_dw_dp *dp;
>> +	struct drm_encoder *encoder;
>> +	struct drm_connector *connector;
>> +	int ret;
>> +
>> +	dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
>> +	if (!dp)
>> +		return -ENOMEM;
>> +
>> +	dp->dev = dev;
>> +	plat_data.max_link_rate = 810000;
>> +	encoder = &dp->encoder.encoder;
>> +	encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev, dev->of_node);
>> +	rockchip_drm_encoder_set_crtc_endpoint_id(&dp->encoder, dev->of_node, 0, 0);
>> +
>> +	drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_TMDS);
>
>drmm_encoder_init() ? This will allow you to get rid of
>drm_encoder_cleanup() calls.

Okay, I will try in V2.


>
>> +	drm_encoder_helper_add(encoder, &dw_dp_encoder_helper_funcs);
>> +
>> +	dp->base = dw_dp_bind(dev, encoder, &plat_data);
>> +	if (IS_ERR(dp->base)) {
>> +		ret = PTR_ERR(dp->base);
>> +		drm_encoder_cleanup(encoder);
>> +		return ret;
>> +	}
>> +
>> +	connector = drm_bridge_connector_init(drm_dev, encoder);
>> +	if (IS_ERR(connector)) {
>> +		ret = PTR_ERR(connector);
>> +		dev_err(dev, "Failed to init bridge connector: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	drm_connector_attach_encoder(connector, encoder);
>> +
>> +	return 0;
>> +}
>> +
>> +static void dw_dp_rockchip_unbind(struct device *dev, struct device *master, void *data)
>> +{
>> +	struct rockchip_dw_dp *dp = dev_get_drvdata(dev);
>> +
>> +	drm_encoder_cleanup(&dp->encoder.encoder);
>> +}
>> +
>> +static const struct component_ops dw_dp_rockchip_component_ops = {
>> +	.bind = dw_dp_rockchip_bind,
>> +	.unbind = dw_dp_rockchip_unbind,
>> +};
>> +
>> +static int dw_dp_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +
>> +	return component_add(dev, &dw_dp_rockchip_component_ops);
>> +}
>> +
>> +static void dw_dp_remove(struct platform_device *pdev)
>> +{
>> +	struct rockchip_dw_dp *dp = platform_get_drvdata(pdev);
>> +
>> +	component_del(dp->dev, &dw_dp_rockchip_component_ops);
>> +}
>> +
>> +static const struct of_device_id dw_dp_of_match[] = {
>> +	{ .compatible = "rockchip,rk3588-dp", },
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, dw_dp_of_match);
>> +
>> +struct platform_driver dw_dp_driver = {
>> +	.probe	= dw_dp_probe,
>> +	.remove = dw_dp_remove,
>> +	.driver = {
>> +		.name = "dw-dp",
>> +		.of_match_table = dw_dp_of_match,
>> +	},
>> +};
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> index 9cf311b5dec1..2b245491c71d 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> @@ -529,6 +529,7 @@ static int __init rockchip_drm_init(void)
>>  	ADD_ROCKCHIP_SUB_DRIVER(rockchip_dp_driver,
>>  				CONFIG_ROCKCHIP_ANALOGIX_DP);
>>  	ADD_ROCKCHIP_SUB_DRIVER(cdn_dp_driver, CONFIG_ROCKCHIP_CDN_DP);
>> +	ADD_ROCKCHIP_SUB_DRIVER(dw_dp_driver, CONFIG_ROCKCHIP_DW_DP);
>>  	ADD_ROCKCHIP_SUB_DRIVER(dw_hdmi_rockchip_pltfm_driver,
>>  				CONFIG_ROCKCHIP_DW_HDMI);
>>  	ADD_ROCKCHIP_SUB_DRIVER(dw_hdmi_qp_rockchip_pltfm_driver,
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> index c183e82a42a5..2e86ad00979c 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>> @@ -87,6 +87,7 @@ int rockchip_drm_encoder_set_crtc_endpoint_id(struct rockchip_encoder *rencoder,
>>  					      struct device_node *np, int port, int reg);
>>  int rockchip_drm_endpoint_is_subdriver(struct device_node *ep);
>>  extern struct platform_driver cdn_dp_driver;
>> +extern struct platform_driver dw_dp_driver;
>>  extern struct platform_driver dw_hdmi_rockchip_pltfm_driver;
>>  extern struct platform_driver dw_hdmi_qp_rockchip_pltfm_driver;
>>  extern struct platform_driver dw_mipi_dsi_rockchip_driver;
>> -- 
>> 2.34.1
>> 
>
>-- 
>With best wishes
>Dmitry