[PATCH v3 10/12] video: rockchip: Add HDMI support for RK3568

[Jonas Karlman]

Hi,

On 2025-04-12 16:27, Dang Huynh wrote:
> HDMI on RK3568 is mostly simplified, all this does is enabling
> DDC for display timings and HPD.
> 
> Signed-off-by: Dang Huynh <danct12 at riseup.net>
> ---
>  drivers/video/rockchip/Makefile      |  1 +
>  drivers/video/rockchip/rk3568_hdmi.c | 71 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+)
> 
> diff --git a/drivers/video/rockchip/Makefile b/drivers/video/rockchip/Makefile
> index 2f89a979a2848733be5a6d05817ad76ce3ad3a34..b751c969b46f1933e91a6c0434f31227a709d8e5 100644
> --- a/drivers/video/rockchip/Makefile
> +++ b/drivers/video/rockchip/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_DISPLAY_ROCKCHIP_LVDS) += rk_lvds.o
>  obj-hdmi-$(CONFIG_ROCKCHIP_RK3288) += rk3288_hdmi.o
>  obj-hdmi-$(CONFIG_ROCKCHIP_RK3328) += rk3328_hdmi.o
>  obj-hdmi-$(CONFIG_ROCKCHIP_RK3399) += rk3399_hdmi.o
> +obj-hdmi-$(CONFIG_ROCKCHIP_RK3568) += rk3568_hdmi.o
>  obj-$(CONFIG_DISPLAY_ROCKCHIP_HDMI) += rk_hdmi.o $(obj-hdmi-y)
>  obj-mipi-$(CONFIG_ROCKCHIP_RK3288) += rk3288_mipi.o
>  obj-mipi-$(CONFIG_ROCKCHIP_RK3399) += rk3399_mipi.o
> diff --git a/drivers/video/rockchip/rk3568_hdmi.c b/drivers/video/rockchip/rk3568_hdmi.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..5578ea37243a5735713d92572639cdc8ba9865cf
> --- /dev/null
> +++ b/drivers/video/rockchip/rk3568_hdmi.c
> @@ -0,0 +1,71 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2024 Dang Huynh <danct12 at riseup.net>
> + */
> +
> +#include <display.h>
> +#include <dm.h>
> +#include <dw_hdmi.h>
> +#include <edid.h>
> +#include <asm/arch-rockchip/hardware.h>
> +#include <asm/arch-rockchip/grf_rk3568.h>
> +#include "rk_hdmi.h"
> +
> +#define RK3568_IO_DDC_IN_MSK             ((3 << 14) | (3 << (14 + 16)))
> +
> +static int rk3568_hdmi_enable(struct udevice *dev, int panel_bpp,
> +			      const struct display_timing *edid)
> +{
> +	struct rk_hdmi_priv *priv = dev_get_priv(dev);
> +
> +	return dw_hdmi_enable(&priv->hdmi, edid);
> +}
> +
> +static int rk3568_hdmi_of_to_plat(struct udevice *dev)
> +{
> +	struct rk_hdmi_priv *priv = dev_get_priv(dev);
> +	struct dw_hdmi *hdmi = &priv->hdmi;
> +
> +	hdmi->i2c_clk_high = 0x71;
> +	hdmi->i2c_clk_low = 0x76;

It is interesting that U-Boot will do I2C clock configuration, something
that Linux will not do.

Where did you get these values? According to the dw-hdmi databook these
should be configured similar to following:

 Minimum High and Minimum Low @100 kbps:
 MIN_SCL_*time Variable		Specification Value	Compliance Value
 MIN_SCL_HIGHtime		4000 ns			4500 ns
 MIN_SCL_LOWtime		4500 ns			5200 ns

 SFR Clock Frequency (Hz) = SFRFREQ
 SFRFREQ = 27 MHz
 I2Cmode = standard, 100 kbit/s
 MIN_SCL_HIGHtime = 4000 ns.
 MIN_SCL_LOWtime = 4700 ns.

 IC_xCNT = (ROUNDUP(MIN_SCL_HIGH_LOWtime*SFRFREQ,0))
 IC_HCNT = (ROUNDUP(4000 ns * 27 MHz,0))
 IC_HCNTSCL PERIOD = 108 (0x6c)

 IC_LCNT = (ROUNDUP(4700 ns * 27 MHz,0))
 IC_LCNTSCL PERIOD = 127 (0x7f)

Above are the defaults for a 27 MHz source clock, however Rockchip use a
24 MHz source clock.

With current values the difference between high and low seem rather low:

 i2c_clk_high = 0x71 (4708 ns)
 i2c_clk_low = 0x76 (4916 ns)

I would expect these to possible be better options:

 i2c_clk_high = 0x6c (4500 ns)
 i2c_clk_low = 0x7d (5208 ns)

As that more closer matches the recommended values for compliance test.

> +
> +	return rk_hdmi_of_to_plat(dev);
> +}
> +
> +static const char * const rk3568_hdmi_reg_names[] = {
> +	"avdd-0v9",
> +	"avdd-1v8"
> +};
> +
> +static int rk3568_hdmi_probe(struct udevice *dev)
> +{
> +	struct rk_hdmi_priv *priv = dev_get_priv(dev);
> +	struct rk3568_grf *grf = priv->grf;
> +
> +	rk_hdmi_probe_regulators(dev, rk3568_hdmi_reg_names,
> +				 ARRAY_SIZE(rk3568_hdmi_reg_names));
> +
> +	writel(RK3568_IO_DDC_IN_MSK, &grf->vo_con1);
> +
> +	return rk_hdmi_probe(dev);
> +}
> +
> +static const struct dm_display_ops rk3568_hdmi_ops = {
> +	.read_edid = rk_hdmi_read_edid,
> +	.enable = rk3568_hdmi_enable,
> +};
> +
> +static const struct udevice_id rk3568_hdmi_ids[] = {
> +	{ .compatible = "rockchip,rk3568-dw-hdmi" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(rk3568_hdmi_rockchip) = {
> +	.name = "rk3568_hdmi_rockchip",
> +	.id = UCLASS_DISPLAY,
> +	.of_match = rk3568_hdmi_ids,
> +	.ops = &rk3568_hdmi_ops,
> +	.of_to_plat = rk3568_hdmi_of_to_plat,
> +	.probe = rk3568_hdmi_probe,
> +	.priv_auto	= sizeof(struct rk_hdmi_priv),

No need to add a tab here since none of the values are aligned, please
replace the tab with a space.

Regards,
Jonas

> +};
>