Hi Maxime, On 9/24/24 5:02 PM, Maxime Ripard wrote: > Hi, > > On Sat, Sep 14, 2024 at 09:56:53PM GMT, Cristian Ciocaltea wrote: >> The RK3588 SoC family integrates the newer Synopsys DesignWare HDMI 2.1 >> Quad-Pixel (QP) TX controller IP and a HDMI/eDP TX Combo PHY based on a >> Samsung IP block. >> >> Add just the basic support for now, i.e. RGB output up to 4K@60Hz, >> without audio, CEC or any of the HDMI 2.1 specific features. >> >> Co-developed-by: Algea Cao <algea.cao@xxxxxxxxxxxxxx> >> Signed-off-by: Algea Cao <algea.cao@xxxxxxxxxxxxxx> >> Tested-by: Heiko Stuebner <heiko@xxxxxxxxx> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@xxxxxxxxxxxxx> >> --- >> drivers/gpu/drm/rockchip/Kconfig | 9 + >> drivers/gpu/drm/rockchip/Makefile | 1 + >> drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c | 436 +++++++++++++++++++++++++ >> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 + >> drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 + >> 5 files changed, 449 insertions(+) >> >> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig >> index 23c49e91f1cc..448fadd4ba15 100644 >> --- a/drivers/gpu/drm/rockchip/Kconfig >> +++ b/drivers/gpu/drm/rockchip/Kconfig >> @@ -8,6 +8,7 @@ config DRM_ROCKCHIP >> select VIDEOMODE_HELPERS >> select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_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 >> select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI >> select GENERIC_PHY_MIPI_DPHY if ROCKCHIP_DW_MIPI_DSI >> @@ -63,6 +64,14 @@ config ROCKCHIP_DW_HDMI >> enable HDMI on RK3288 or RK3399 based SoC, you should select >> this option. >> >> +config ROCKCHIP_DW_HDMI_QP >> + bool "Rockchip specific extensions for Synopsys DW HDMI QP" >> + select DRM_BRIDGE_CONNECTOR >> + help >> + This selects support for Rockchip SoC specific extensions >> + for the Synopsys DesignWare HDMI QP driver. If you want to >> + enable HDMI on RK3588 based SoC, you should select this option. >> + >> config ROCKCHIP_DW_MIPI_DSI >> bool "Rockchip specific extensions for Synopsys DW MIPI DSI" >> select GENERIC_PHY_MIPI_DPHY >> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile >> index 3ff7b21c0414..3eab662a5a1d 100644 >> --- a/drivers/gpu/drm/rockchip/Makefile >> +++ b/drivers/gpu/drm/rockchip/Makefile >> @@ -11,6 +11,7 @@ rockchipdrm-$(CONFIG_ROCKCHIP_VOP) += rockchip_drm_vop.o rockchip_vop_reg.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_HDMI_QP) += dw_hdmi_qp-rockchip.o >> rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi-rockchip.o >> rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o >> rockchipdrm-$(CONFIG_ROCKCHIP_LVDS) += rockchip_lvds.o >> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c >> new file mode 100644 >> index 000000000000..19d407c926bd >> --- /dev/null >> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c >> @@ -0,0 +1,436 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * Copyright (c) 2021-2022 Rockchip Electronics Co., Ltd. >> + * Copyright (c) 2024 Collabora Ltd. >> + * >> + * Author: Algea Cao <algea.cao@xxxxxxxxxxxxxx> >> + * Author: Cristian Ciocaltea <cristian.ciocaltea@xxxxxxxxxxxxx> >> + */ >> + >> +#include <linux/clk.h> >> +#include <linux/gpio/consumer.h> >> +#include <linux/mfd/syscon.h> >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> +#include <linux/phy/phy.h> >> +#include <linux/regmap.h> >> +#include <linux/workqueue.h> >> + >> +#include <drm/bridge/dw_hdmi_qp.h> >> +#include <drm/drm_bridge_connector.h> >> +#include <drm/drm_of.h> >> +#include <drm/drm_probe_helper.h> >> +#include <drm/drm_simple_kms_helper.h> >> + >> +#include "rockchip_drm_drv.h" >> + >> +#define RK3588_GRF_SOC_CON2 0x0308 >> +#define RK3588_HDMI0_HPD_INT_MSK BIT(13) >> +#define RK3588_HDMI0_HPD_INT_CLR BIT(12) >> +#define RK3588_GRF_SOC_CON7 0x031c >> +#define RK3588_SET_HPD_PATH_MASK GENMASK(13, 12) >> +#define RK3588_GRF_SOC_STATUS1 0x0384 >> +#define RK3588_HDMI0_LEVEL_INT BIT(16) >> +#define RK3588_GRF_VO1_CON3 0x000c >> +#define RK3588_SCLIN_MASK BIT(9) >> +#define RK3588_SDAIN_MASK BIT(10) >> +#define RK3588_MODE_MASK BIT(11) >> +#define RK3588_I2S_SEL_MASK BIT(13) >> +#define RK3588_GRF_VO1_CON9 0x0024 >> +#define RK3588_HDMI0_GRANT_SEL BIT(10) >> + >> +#define HIWORD_UPDATE(val, mask) ((val) | (mask) << 16) >> + >> +struct rockchip_hdmi_qp { >> + struct device *dev; >> + struct regmap *regmap; >> + struct regmap *vo_regmap; >> + struct rockchip_encoder encoder; >> + struct clk *ref_clk; >> + struct dw_hdmi_qp *hdmi; >> + struct phy *phy; >> + struct gpio_desc *enable_gpio; >> + struct delayed_work hpd_work; >> +}; >> + >> +static struct rockchip_hdmi_qp *to_rockchip_hdmi_qp(struct drm_encoder *encoder) >> +{ >> + struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder); >> + >> + return container_of(rkencoder, struct rockchip_hdmi_qp, encoder); >> +} >> + >> +static void >> +dw_hdmi_qp_rockchip_encoder_mode_set(struct drm_encoder *encoder, >> + struct drm_display_mode *mode, >> + struct drm_display_mode *adj_mode) >> +{ >> + struct rockchip_hdmi_qp *hdmi = to_rockchip_hdmi_qp(encoder); >> + >> + clk_set_rate(hdmi->ref_clk, adj_mode->clock * 1000); >> +} > > I'm not sure you can do that. mode_set can be called multiple times > while the connector is enabled. It would be better to drop the mode_set > implementation, and just put it in the encoder enable. I dropped this in v8 [1], as the ref_clk rate adjustment has been already handled via the encoder enable. Thanks for the review, Cristian [1] https://lore.kernel.org/all/20240929-b4-rk3588-bridge-upstream-v8-0-83538c2cc325@xxxxxxxxxxxxx/