[PATCH v2 1/3] clk: rockchip: rk3328: use HDMIPHY PLL as a clock parent for VOP_DCLK

[PATCH v2 1/3] clk: rockchip: rk3328: use HDMIPHY PLL as a clock parent for VOP_DCLK

[Vasily Khoruzhick]

The only video mode that currently works on rk3328 in u-boot is 1080p,
because it uses GPLL for VOP_DCLK clock parent.

Linux driver uses HDMIPHY PLL as a clock parent for VOP_DCLK, since using
GPLL or CPLL is not feasible due these PLL being used as a clock parent
for other devices. It would be cumbersome to recalculate dividers
for the rest of the devices, and u-boot doesn't do it anyway. As a
result, u-boot is not able to set desired dot clock for most
resolutions.

Switch to using HDMIPHY as a clock parent for VOP_DCLK

Tested with 768p, 1080p, 1440p monitors.

Fixes: 92edae779f8c ("clk: rockchip: rk3328: Add VOP clk support")
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
v2: no change

 drivers/clk/rockchip/clk_rk3328.c | 76 +++++++------------------------
 1 file changed, 16 insertions(+), 60 deletions(-)

diff --git a/drivers/clk/rockchip/clk_rk3328.c b/drivers/clk/rockchip/clk_rk3328.c
index 7701a9734ee..537af6b3290 100644
--- a/drivers/clk/rockchip/clk_rk3328.c
+++ b/drivers/clk/rockchip/clk_rk3328.c
@@ -79,6 +79,12 @@ enum {
 	PLL_MODE_SLOW			= 0,
 	PLL_MODE_NORM,
 
+	/* MISC_CON */
+	HDMIPHY_24M_SEL_SHIFT		= 13,
+	HDMIPHY_24M_SEL_MASK		= 1 << HDMIPHY_24M_SEL_SHIFT,
+	HDMIPHY_24M_SEL_PCLK		= 0,
+	HDMIPHY_24M_SEL_24MHZ		= 1,
+
 	/* CLKSEL_CON0 */
 	CLK_CORE_PLL_SEL_APLL		= 0,
 	CLK_CORE_PLL_SEL_GPLL,
@@ -583,41 +589,11 @@ static ulong rk3328_spi_set_clk(struct rk3328_cru *cru, uint hz)
 }
 
 #ifndef CONFIG_XPL_BUILD
-static ulong rk3328_vop_get_clk(struct rk3328_clk_priv *priv, ulong clk_id)
-{
-	struct rk3328_cru *cru = priv->cru;
-	u32 div, con, parent;
-
-	switch (clk_id) {
-	case ACLK_VOP_PRE:
-		con = readl(&cru->clksel_con[39]);
-		div = (con & ACLK_VOP_DIV_CON_MASK) >> ACLK_VOP_DIV_CON_SHIFT;
-		parent = GPLL_HZ;
-		break;
-	case ACLK_VIO_PRE:
-		con = readl(&cru->clksel_con[37]);
-		div = (con & ACLK_VIO_DIV_CON_MASK) >> ACLK_VIO_DIV_CON_SHIFT;
-		parent = GPLL_HZ;
-		break;
-	case DCLK_LCDC:
-		con = readl(&cru->clksel_con[40]);
-		div = (con & DCLK_LCDC_DIV_CON_MASK) >> DCLK_LCDC_DIV_CON_SHIFT;
-		parent = GPLL_HZ;
-		break;
-	default:
-		printf("%s: Unsupported vop get clk#%ld\n", __func__, clk_id);
-		return -ENOENT;
-	}
-
-	return DIV_TO_RATE(parent, div);
-}
-
 static ulong rk3328_vop_set_clk(struct rk3328_clk_priv *priv,
 				ulong clk_id, uint hz)
 {
 	struct rk3328_cru *cru = priv->cru;
 	int src_clk_div;
-	u32 con, parent;
 
 	src_clk_div = DIV_ROUND_UP(GPLL_HZ, hz);
 	assert(src_clk_div - 1 < 31);
@@ -636,42 +612,25 @@ static ulong rk3328_vop_set_clk(struct rk3328_clk_priv *priv,
 			     (src_clk_div - 1) << ACLK_VIO_DIV_CON_SHIFT);
 		break;
 	case DCLK_LCDC:
-		con = readl(&cru->clksel_con[40]);
-		con = (con & DCLK_LCDC_SEL_MASK) >> DCLK_LCDC_SEL_SHIFT;
-		if (con) {
-			parent = readl(&cru->clksel_con[40]);
-			parent = (parent & DCLK_LCDC_PLL_SEL_MASK) >>
-				 DCLK_LCDC_PLL_SEL_SHIFT;
-			if (parent)
-				src_clk_div = DIV_ROUND_UP(GPLL_HZ, hz);
-			else
-				src_clk_div = DIV_ROUND_UP(GPLL_HZ, hz);
-
-			rk_clrsetreg(&cru->clksel_con[40],
-				     DCLK_LCDC_DIV_CON_MASK,
-				     (src_clk_div - 1) <<
-				     DCLK_LCDC_DIV_CON_SHIFT);
-		}
+		/* Set HDMIPHY clock output to pixel clock */
+		rk_clrsetreg(&cru->misc, HDMIPHY_24M_SEL_MASK,
+			     HDMIPHY_24M_SEL_PCLK << HDMIPHY_24M_SEL_SHIFT);
+		/* Set HDMIPHY as a parent of DCLK_LCDC, and set all divisors to 1 */
+		rk_clrsetreg(&cru->clksel_con[40],
+			     DCLK_LCDC_DIV_CON_MASK | DCLK_LCDC_SEL_MASK | CLK_HDMIPHY_DIV_CON_MASK,
+			     (1 - 1) << CLK_HDMIPHY_DIV_CON_SHIFT |
+			     DCLK_LCDC_SEL_HDMIPHY << DCLK_LCDC_SEL_SHIFT |
+			     (1 - 1) << DCLK_LCDC_DIV_CON_SHIFT);
 		break;
 	default:
 		printf("%s: Unable to set vop clk#%ld\n", __func__, clk_id);
 		return -EINVAL;
 	}
 
-	return rk3328_vop_get_clk(priv, clk_id);
+	return hz;
 }
 #endif
 
-static ulong rk3328_hdmiphy_get_clk(struct rk3328_cru *cru)
-{
-	u32 div, con;
-
-	con = readl(&cru->clksel_con[40]);
-	div = (con & CLK_HDMIPHY_DIV_CON_MASK) >> CLK_HDMIPHY_DIV_CON_SHIFT;
-
-	return DIV_TO_RATE(GPLL_HZ, div);
-}
-
 static ulong rk3328_clk_get_rate(struct clk *clk)
 {
 	struct rk3328_clk_priv *priv = dev_get_priv(clk->dev);
@@ -701,9 +660,6 @@ static ulong rk3328_clk_get_rate(struct clk *clk)
 	case SCLK_SPI:
 		rate = rk3328_spi_get_clk(priv->cru);
 		break;
-	case PCLK_HDMIPHY:
-		rate = rk3328_hdmiphy_get_clk(priv->cru);
-		break;
 	case SCLK_USB3OTG_REF:
 		rate = OSC_HZ;
 		break;
-- 
2.49.0

[PATCH v2 2/3] video: rockchip: rk3328: filter unsupported modes

[Vasily Khoruzhick]

While RK3328 is capable of 4K resolutions, dw-hdmi driver in u-boot
seems to have some issue with 4K. 1440p or lower works fine.

Limit max resolutions to 2560x1440x60Hz, by filtering the modes with
pixel clock > 297MHz

Fixes: f3ea872970d603 ("video: rockchip: Add rk3328 hdmi support")
Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
v2: add a comment that max pixel clock limit is a workaround

drivers/video/rockchip/rk3328_hdmi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/video/rockchip/rk3328_hdmi.c b/drivers/video/rockchip/rk3328_hdmi.c
index 763669c09be..ff351aaac7c 100644
--- a/drivers/video/rockchip/rk3328_hdmi.c
+++ b/drivers/video/rockchip/rk3328_hdmi.c
@@ -105,9 +105,20 @@ static int rk3328_hdmi_probe(struct udevice *dev)
 	return 0;
 }
 
+static bool rk3328_hdmi_mode_valid(struct udevice *dev,
+				   const struct display_timing *timing)
+{
+	/* Limit pixel clock to 297MHz. While RK3328 support higher rates and it
+	 * works in Linux, it doesn't seem to work in u-boot. That limits max
+	 * resolution to 1440p
+	 */
+	return timing->pixelclock.typ <= 297000000;
+}
+
 static const struct dm_display_ops rk3328_hdmi_ops = {
 	.read_edid = rk_hdmi_read_edid,
 	.enable = rk3328_hdmi_enable,
+	.mode_valid = rk3328_hdmi_mode_valid,
 };
 
 static const struct udevice_id rk3328_hdmi_ids[] = {
-- 
2.49.0

[PATCH v2 3/3] rockchip: rock64-rk3328_defconfig: enable HDMI output and USB keyboard

[Vasily Khoruzhick]

Enable HDMI output and USB keyboard on Rock64 board

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
---
v2: no change

 configs/rock64-rk3328_defconfig | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/configs/rock64-rk3328_defconfig b/configs/rock64-rk3328_defconfig
index 6d00b52e62f..ae3512625cf 100644
--- a/configs/rock64-rk3328_defconfig
+++ b/configs/rock64-rk3328_defconfig
@@ -21,7 +21,9 @@ CONFIG_FIT_VERBOSE=y
 CONFIG_SPL_FIT_SIGNATURE=y
 CONFIG_SPL_LOAD_FIT=y
 CONFIG_LEGACY_IMAGE_FORMAT=y
+CONFIG_USE_PREBOOT=y
 CONFIG_DEFAULT_FDT_FILE="rockchip/rk3328-rock64.dtb"
+# CONFIG_SYS_DEVICE_NULLDEV is not set
 # CONFIG_DISPLAY_CPUINFO is not set
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_SPL_MAX_SIZE=0x40000
@@ -72,6 +74,7 @@ CONFIG_DM_ETH_PHY=y
 CONFIG_PHY_GIGE=y
 CONFIG_ETH_DESIGNWARE=y
 CONFIG_GMAC_ROCKCHIP=y
+CONFIG_PHY_ROCKCHIP_INNO_HDMI=y
 CONFIG_PHY_ROCKCHIP_INNO_USB2=y
 CONFIG_PINCTRL=y
 CONFIG_SPL_PINCTRL=y
@@ -80,6 +83,8 @@ CONFIG_PMIC_RK8XX=y
 CONFIG_SPL_DM_REGULATOR=y
 CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_SPL_DM_REGULATOR_FIXED=y
+CONFIG_DM_REGULATOR_GPIO=y
+CONFIG_SPL_DM_REGULATOR_GPIO=y
 CONFIG_REGULATOR_RK8XX=y
 CONFIG_PWM_ROCKCHIP=y
 CONFIG_RAM=y
@@ -104,6 +109,11 @@ CONFIG_USB_OHCI_GENERIC=y
 CONFIG_USB_DWC2=y
 CONFIG_USB_DWC3=y
 CONFIG_USB_DWC3_GENERIC=y
+CONFIG_USB_KEYBOARD=y
+CONFIG_VIDEO=y
+CONFIG_DISPLAY=y
+CONFIG_VIDEO_ROCKCHIP=y
+CONFIG_DISPLAY_ROCKCHIP_HDMI=y
 CONFIG_SPL_TINY_MEMSET=y
 CONFIG_TPL_TINY_MEMSET=y
 CONFIG_ERRNO_STR=y
-- 
2.49.0

Re: [PATCH v2 2/3] video: rockchip: rk3328: filter unsupported modes

[Matwey V. Kornilov]

You say that the 4K works in Linux. Wouldn't it be better to fix the 4K
support in u-boot?

пн, 24 мар. 2025 г., 01:51 Vasily Khoruzhick <anarsoul@gmail.com>:

> While RK3328 is capable of 4K resolutions, dw-hdmi driver in u-boot
> seems to have some issue with 4K. 1440p or lower works fine.
>
> Limit max resolutions to 2560x1440x60Hz, by filtering the modes with
> pixel clock > 297MHz
>
> Fixes: f3ea872970d603 ("video: rockchip: Add rk3328 hdmi support")
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
> v2: add a comment that max pixel clock limit is a workaround
>
> drivers/video/rockchip/rk3328_hdmi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/video/rockchip/rk3328_hdmi.c
> b/drivers/video/rockchip/rk3328_hdmi.c
> index 763669c09be..ff351aaac7c 100644
> --- a/drivers/video/rockchip/rk3328_hdmi.c
> +++ b/drivers/video/rockchip/rk3328_hdmi.c
> @@ -105,9 +105,20 @@ static int rk3328_hdmi_probe(struct udevice *dev)
>         return 0;
>  }
>
> +static bool rk3328_hdmi_mode_valid(struct udevice *dev,
> +                                  const struct display_timing *timing)
> +{
> +       /* Limit pixel clock to 297MHz. While RK3328 support higher rates
> and it
> +        * works in Linux, it doesn't seem to work in u-boot. That limits
> max
> +        * resolution to 1440p
> +        */
> +       return timing->pixelclock.typ <= 297000000;
> +}
> +
>  static const struct dm_display_ops rk3328_hdmi_ops = {
>         .read_edid = rk_hdmi_read_edid,
>         .enable = rk3328_hdmi_enable,
> +       .mode_valid = rk3328_hdmi_mode_valid,
>  };
>
>  static const struct udevice_id rk3328_hdmi_ids[] = {
> --
> 2.49.0
>
>

Re: [PATCH v2 2/3] video: rockchip: rk3328: filter unsupported modes

[Vasily Khoruzhick]

On Sun, Mar 23, 2025, 22:36 Matwey V. Kornilov <matwey.kornilov@gmail.com>
wrote:

> You say that the 4K works in Linux. Wouldn't it be better to fix the 4K
> support in u-boot?
>

Hey Matwey,

A proper fix would certainly be better, however I don't really have neither
time nor motivation to debug it further.

We can either leave it broken as it is now and wait for someone else to fix
it or plug it with a workaround for a time being.

I'd be happy to test the fix if you volunteer to debug it.

Regards,
Vasily

пн, 24 мар. 2025 г., 01:51 Vasily Khoruzhick <anarsoul@gmail.com>:
>
>> While RK3328 is capable of 4K resolutions, dw-hdmi driver in u-boot
>> seems to have some issue with 4K. 1440p or lower works fine.
>>
>> Limit max resolutions to 2560x1440x60Hz, by filtering the modes with
>> pixel clock > 297MHz
>>
>> Fixes: f3ea872970d603 ("video: rockchip: Add rk3328 hdmi support")
>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>> ---
>> v2: add a comment that max pixel clock limit is a workaround
>>
>> drivers/video/rockchip/rk3328_hdmi.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/video/rockchip/rk3328_hdmi.c
>> b/drivers/video/rockchip/rk3328_hdmi.c
>> index 763669c09be..ff351aaac7c 100644
>> --- a/drivers/video/rockchip/rk3328_hdmi.c
>> +++ b/drivers/video/rockchip/rk3328_hdmi.c
>> @@ -105,9 +105,20 @@ static int rk3328_hdmi_probe(struct udevice *dev)
>>         return 0;
>>  }
>>
>> +static bool rk3328_hdmi_mode_valid(struct udevice *dev,
>> +                                  const struct display_timing *timing)
>> +{
>> +       /* Limit pixel clock to 297MHz. While RK3328 support higher rates
>> and it
>> +        * works in Linux, it doesn't seem to work in u-boot. That limits
>> max
>> +        * resolution to 1440p
>> +        */
>> +       return timing->pixelclock.typ <= 297000000;
>> +}
>> +
>>  static const struct dm_display_ops rk3328_hdmi_ops = {
>>         .read_edid = rk_hdmi_read_edid,
>>         .enable = rk3328_hdmi_enable,
>> +       .mode_valid = rk3328_hdmi_mode_valid,
>>  };
>>
>>  static const struct udevice_id rk3328_hdmi_ids[] = {
>> --
>> 2.49.0
>>
>>

Re: [PATCH v2 3/3] rockchip: rock64-rk3328_defconfig: enable HDMI output and USB keyboard

[Jonas Karlman]

Hi Vasily,

On 2025-03-23 23:50, Vasily Khoruzhick wrote:
> Enable HDMI output and USB keyboard on Rock64 board
> 
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
> v2: no change
> 
>  configs/rock64-rk3328_defconfig | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/configs/rock64-rk3328_defconfig b/configs/rock64-rk3328_defconfig
> index 6d00b52e62f..ae3512625cf 100644
> --- a/configs/rock64-rk3328_defconfig
> +++ b/configs/rock64-rk3328_defconfig
> @@ -21,7 +21,9 @@ CONFIG_FIT_VERBOSE=y
>  CONFIG_SPL_FIT_SIGNATURE=y
>  CONFIG_SPL_LOAD_FIT=y
>  CONFIG_LEGACY_IMAGE_FORMAT=y
> +CONFIG_USE_PREBOOT=y

This will add a large delay during boot as it forces a run of the slow
"usb start" and is something we want to avoid for normal use, please
drop this.

>  CONFIG_DEFAULT_FDT_FILE="rockchip/rk3328-rock64.dtb"
> +# CONFIG_SYS_DEVICE_NULLDEV is not set
>  # CONFIG_DISPLAY_CPUINFO is not set
>  CONFIG_DISPLAY_BOARDINFO_LATE=y
>  CONFIG_SPL_MAX_SIZE=0x40000
> @@ -72,6 +74,7 @@ CONFIG_DM_ETH_PHY=y
>  CONFIG_PHY_GIGE=y
>  CONFIG_ETH_DESIGNWARE=y
>  CONFIG_GMAC_ROCKCHIP=y
> +CONFIG_PHY_ROCKCHIP_INNO_HDMI=y
>  CONFIG_PHY_ROCKCHIP_INNO_USB2=y
>  CONFIG_PINCTRL=y
>  CONFIG_SPL_PINCTRL=y
> @@ -80,6 +83,8 @@ CONFIG_PMIC_RK8XX=y
>  CONFIG_SPL_DM_REGULATOR=y
>  CONFIG_DM_REGULATOR_FIXED=y
>  CONFIG_SPL_DM_REGULATOR_FIXED=y
> +CONFIG_DM_REGULATOR_GPIO=y
> +CONFIG_SPL_DM_REGULATOR_GPIO=y

The ROCK64 does not use any regulator-gpio, this look unrelated and can
be dropped?

>  CONFIG_REGULATOR_RK8XX=y
>  CONFIG_PWM_ROCKCHIP=y
>  CONFIG_RAM=y
> @@ -104,6 +109,11 @@ CONFIG_USB_OHCI_GENERIC=y
>  CONFIG_USB_DWC2=y
>  CONFIG_USB_DWC3=y
>  CONFIG_USB_DWC3_GENERIC=y
> +CONFIG_USB_KEYBOARD=y
> +CONFIG_VIDEO=y
> +CONFIG_DISPLAY=y
> +CONFIG_VIDEO_ROCKCHIP=y
> +CONFIG_DISPLAY_ROCKCHIP_HDMI=y

This will force a small delay of at least a 300ms timeout for all users
that does not have HDMI connected during boot.

Maybe this could be extracted to a rockchip-hdmi.config-file to make it
more optional?

E.g. use "make rock64-rk3328_defconfig rockchip-hdmi.config" to build
with something like:

  > cat rockchip-hdmi.config
  CONFIG_VIDEO=y
  CONFIG_DISPLAY=y
  CONFIG_VIDEO_ROCKCHIP=y
  CONFIG_DISPLAY_ROCKCHIP_HDMI=y

Regards,
Jonas

>  CONFIG_SPL_TINY_MEMSET=y
>  CONFIG_TPL_TINY_MEMSET=y
>  CONFIG_ERRNO_STR=y

Re: [PATCH v2 3/3] rockchip: rock64-rk3328_defconfig: enable HDMI output and USB keyboard

[Matwey V. Kornilov]

вс, 30 мар. 2025 г. в 14:45, Jonas Karlman <jonas@kwiboo.se>:
>
> Hi Vasily,
>
> On 2025-03-23 23:50, Vasily Khoruzhick wrote:
> > Enable HDMI output and USB keyboard on Rock64 board
> >
> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > ---
> > v2: no change
> >
> >  configs/rock64-rk3328_defconfig | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/configs/rock64-rk3328_defconfig b/configs/rock64-rk3328_defconfig
> > index 6d00b52e62f..ae3512625cf 100644
> > --- a/configs/rock64-rk3328_defconfig
> > +++ b/configs/rock64-rk3328_defconfig
> > @@ -21,7 +21,9 @@ CONFIG_FIT_VERBOSE=y
> >  CONFIG_SPL_FIT_SIGNATURE=y
> >  CONFIG_SPL_LOAD_FIT=y
> >  CONFIG_LEGACY_IMAGE_FORMAT=y
> > +CONFIG_USE_PREBOOT=y
>
> This will add a large delay during boot as it forces a run of the slow
> "usb start" and is something we want to avoid for normal use, please
> drop this.
>
> >  CONFIG_DEFAULT_FDT_FILE="rockchip/rk3328-rock64.dtb"
> > +# CONFIG_SYS_DEVICE_NULLDEV is not set
> >  # CONFIG_DISPLAY_CPUINFO is not set
> >  CONFIG_DISPLAY_BOARDINFO_LATE=y
> >  CONFIG_SPL_MAX_SIZE=0x40000
> > @@ -72,6 +74,7 @@ CONFIG_DM_ETH_PHY=y
> >  CONFIG_PHY_GIGE=y
> >  CONFIG_ETH_DESIGNWARE=y
> >  CONFIG_GMAC_ROCKCHIP=y
> > +CONFIG_PHY_ROCKCHIP_INNO_HDMI=y
> >  CONFIG_PHY_ROCKCHIP_INNO_USB2=y
> >  CONFIG_PINCTRL=y
> >  CONFIG_SPL_PINCTRL=y
> > @@ -80,6 +83,8 @@ CONFIG_PMIC_RK8XX=y
> >  CONFIG_SPL_DM_REGULATOR=y
> >  CONFIG_DM_REGULATOR_FIXED=y
> >  CONFIG_SPL_DM_REGULATOR_FIXED=y
> > +CONFIG_DM_REGULATOR_GPIO=y
> > +CONFIG_SPL_DM_REGULATOR_GPIO=y
>
> The ROCK64 does not use any regulator-gpio, this look unrelated and can
> be dropped?
>
> >  CONFIG_REGULATOR_RK8XX=y
> >  CONFIG_PWM_ROCKCHIP=y
> >  CONFIG_RAM=y
> > @@ -104,6 +109,11 @@ CONFIG_USB_OHCI_GENERIC=y
> >  CONFIG_USB_DWC2=y
> >  CONFIG_USB_DWC3=y
> >  CONFIG_USB_DWC3_GENERIC=y
> > +CONFIG_USB_KEYBOARD=y
> > +CONFIG_VIDEO=y
> > +CONFIG_DISPLAY=y
> > +CONFIG_VIDEO_ROCKCHIP=y
> > +CONFIG_DISPLAY_ROCKCHIP_HDMI=y
>
> This will force a small delay of at least a 300ms timeout for all users
> that does not have HDMI connected during boot.

It already takes about 2 sec to initialize Ethernet. I would say that
nobody notices another 300ms.

>
> Maybe this could be extracted to a rockchip-hdmi.config-file to make it
> more optional?
>
> E.g. use "make rock64-rk3328_defconfig rockchip-hdmi.config" to build
> with something like:
>
>   > cat rockchip-hdmi.config
>   CONFIG_VIDEO=y
>   CONFIG_DISPLAY=y
>   CONFIG_VIDEO_ROCKCHIP=y
>   CONFIG_DISPLAY_ROCKCHIP_HDMI=y
>
> Regards,
> Jonas
>
> >  CONFIG_SPL_TINY_MEMSET=y
> >  CONFIG_TPL_TINY_MEMSET=y
> >  CONFIG_ERRNO_STR=y
>


-- 
With best regards,
Matwey V. Kornilov

Re: [PATCH v2 2/3] video: rockchip: rk3328: filter unsupported modes

[Jonas Karlman]

Hi Vasily,

On 2025-03-23 23:50, Vasily Khoruzhick wrote:
> While RK3328 is capable of 4K resolutions, dw-hdmi driver in u-boot
> seems to have some issue with 4K. 1440p or lower works fine.
> 
> Limit max resolutions to 2560x1440x60Hz, by filtering the modes with
> pixel clock > 297MHz
> 
> Fixes: f3ea872970d603 ("video: rockchip: Add rk3328 hdmi support")
> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> ---
> v2: add a comment that max pixel clock limit is a workaround
> 
> drivers/video/rockchip/rk3328_hdmi.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/video/rockchip/rk3328_hdmi.c b/drivers/video/rockchip/rk3328_hdmi.c
> index 763669c09be..ff351aaac7c 100644
> --- a/drivers/video/rockchip/rk3328_hdmi.c
> +++ b/drivers/video/rockchip/rk3328_hdmi.c
> @@ -105,9 +105,20 @@ static int rk3328_hdmi_probe(struct udevice *dev)
>  	return 0;
>  }
>  
> +static bool rk3328_hdmi_mode_valid(struct udevice *dev,
> +				   const struct display_timing *timing)
> +{
> +	/* Limit pixel clock to 297MHz. While RK3328 support higher rates and it
> +	 * works in Linux, it doesn't seem to work in u-boot. That limits max
> +	 * resolution to 1440p
> +	 */
> +	return timing->pixelclock.typ <= 297000000;

I think it make more sense to limit to the HDMI 1.4 max 340 MHz instead
of arbitrary 297 MHz. Anything above 340 MHz will require scrambling etc
and will not work in U-Boot unless at least dw-hdmi driver is updated.

Regards,
Jonas

> +}
> +
>  static const struct dm_display_ops rk3328_hdmi_ops = {
>  	.read_edid = rk_hdmi_read_edid,
>  	.enable = rk3328_hdmi_enable,
> +	.mode_valid = rk3328_hdmi_mode_valid,
>  };
>  
>  static const struct udevice_id rk3328_hdmi_ids[] = {

Re: [PATCH v2 3/3] rockchip: rock64-rk3328_defconfig: enable HDMI output and USB keyboard

[Jonas Karlman]

Hi Matwey,

On 2025-03-30 13:52, Matwey V. Kornilov wrote:
> вс, 30 мар. 2025 г. в 14:45, Jonas Karlman <jonas@kwiboo.se>:
>>
>> Hi Vasily,
>>
>> On 2025-03-23 23:50, Vasily Khoruzhick wrote:
>>> Enable HDMI output and USB keyboard on Rock64 board
>>>
>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>>> ---
>>> v2: no change
>>>
>>>  configs/rock64-rk3328_defconfig | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/configs/rock64-rk3328_defconfig b/configs/rock64-rk3328_defconfig
>>> index 6d00b52e62f..ae3512625cf 100644
>>> --- a/configs/rock64-rk3328_defconfig
>>> +++ b/configs/rock64-rk3328_defconfig
>>> @@ -21,7 +21,9 @@ CONFIG_FIT_VERBOSE=y
>>>  CONFIG_SPL_FIT_SIGNATURE=y
>>>  CONFIG_SPL_LOAD_FIT=y
>>>  CONFIG_LEGACY_IMAGE_FORMAT=y
>>> +CONFIG_USE_PREBOOT=y
>>
>> This will add a large delay during boot as it forces a run of the slow
>> "usb start" and is something we want to avoid for normal use, please
>> drop this.
>>
>>>  CONFIG_DEFAULT_FDT_FILE="rockchip/rk3328-rock64.dtb"
>>> +# CONFIG_SYS_DEVICE_NULLDEV is not set
>>>  # CONFIG_DISPLAY_CPUINFO is not set
>>>  CONFIG_DISPLAY_BOARDINFO_LATE=y
>>>  CONFIG_SPL_MAX_SIZE=0x40000
>>> @@ -72,6 +74,7 @@ CONFIG_DM_ETH_PHY=y
>>>  CONFIG_PHY_GIGE=y
>>>  CONFIG_ETH_DESIGNWARE=y
>>>  CONFIG_GMAC_ROCKCHIP=y
>>> +CONFIG_PHY_ROCKCHIP_INNO_HDMI=y
>>>  CONFIG_PHY_ROCKCHIP_INNO_USB2=y
>>>  CONFIG_PINCTRL=y
>>>  CONFIG_SPL_PINCTRL=y
>>> @@ -80,6 +83,8 @@ CONFIG_PMIC_RK8XX=y
>>>  CONFIG_SPL_DM_REGULATOR=y
>>>  CONFIG_DM_REGULATOR_FIXED=y
>>>  CONFIG_SPL_DM_REGULATOR_FIXED=y
>>> +CONFIG_DM_REGULATOR_GPIO=y
>>> +CONFIG_SPL_DM_REGULATOR_GPIO=y
>>
>> The ROCK64 does not use any regulator-gpio, this look unrelated and can
>> be dropped?
>>
>>>  CONFIG_REGULATOR_RK8XX=y
>>>  CONFIG_PWM_ROCKCHIP=y
>>>  CONFIG_RAM=y
>>> @@ -104,6 +109,11 @@ CONFIG_USB_OHCI_GENERIC=y
>>>  CONFIG_USB_DWC2=y
>>>  CONFIG_USB_DWC3=y
>>>  CONFIG_USB_DWC3_GENERIC=y
>>> +CONFIG_USB_KEYBOARD=y
>>> +CONFIG_VIDEO=y
>>> +CONFIG_DISPLAY=y
>>> +CONFIG_VIDEO_ROCKCHIP=y
>>> +CONFIG_DISPLAY_ROCKCHIP_HDMI=y
>>
>> This will force a small delay of at least a 300ms timeout for all users
>> that does not have HDMI connected during boot.
> 
> It already takes about 2 sec to initialize Ethernet. I would say that
> nobody notices another 300ms.

If it takes 2 seconds then I think you may have something wrong in your
configuration? Please share a "bootstage report".

The Ethernet initialization should be fast, trying to use the Ethernet
will however be slow. The Ethernet PHY reset should be around 10+50ms.

Reaching eth_common_init may take some time, but eth_initialize should
be relative fast.

Regards,
Jonas

> 
>>
>> Maybe this could be extracted to a rockchip-hdmi.config-file to make it
>> more optional?
>>
>> E.g. use "make rock64-rk3328_defconfig rockchip-hdmi.config" to build
>> with something like:
>>
>>   > cat rockchip-hdmi.config
>>   CONFIG_VIDEO=y
>>   CONFIG_DISPLAY=y
>>   CONFIG_VIDEO_ROCKCHIP=y
>>   CONFIG_DISPLAY_ROCKCHIP_HDMI=y
>>
>> Regards,
>> Jonas
>>
>>>  CONFIG_SPL_TINY_MEMSET=y
>>>  CONFIG_TPL_TINY_MEMSET=y
>>>  CONFIG_ERRNO_STR=y
>>
> 
> 

Re: [PATCH v2 3/3] rockchip: rock64-rk3328_defconfig: enable HDMI output and USB keyboard

[Vasily Khoruzhick]

On Sun, Mar 30, 2025 at 4:45 AM Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Hi Vasily,
>
> On 2025-03-23 23:50, Vasily Khoruzhick wrote:
> > Enable HDMI output and USB keyboard on Rock64 board
> >
> > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
> > ---
> > v2: no change
> >
> >  configs/rock64-rk3328_defconfig | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/configs/rock64-rk3328_defconfig b/configs/rock64-rk3328_defconfig
> > index 6d00b52e62f..ae3512625cf 100644
> > --- a/configs/rock64-rk3328_defconfig
> > +++ b/configs/rock64-rk3328_defconfig
> > @@ -21,7 +21,9 @@ CONFIG_FIT_VERBOSE=y
> >  CONFIG_SPL_FIT_SIGNATURE=y
> >  CONFIG_SPL_LOAD_FIT=y
> >  CONFIG_LEGACY_IMAGE_FORMAT=y
> > +CONFIG_USE_PREBOOT=y
>
> This will add a large delay during boot as it forces a run of the slow
> "usb start" and is something we want to avoid for normal use, please
> drop this.

Well, I can just drop the patch altogether.

Personally, I do need USB and HDMI support, since it allows me to boot
from a secondary storage in case the OS on main storage doesn't boot.
Not having USB support defeats the purpose of this patch.

> >  CONFIG_DEFAULT_FDT_FILE="rockchip/rk3328-rock64.dtb"
> > +# CONFIG_SYS_DEVICE_NULLDEV is not set
> >  # CONFIG_DISPLAY_CPUINFO is not set
> >  CONFIG_DISPLAY_BOARDINFO_LATE=y
> >  CONFIG_SPL_MAX_SIZE=0x40000
> > @@ -72,6 +74,7 @@ CONFIG_DM_ETH_PHY=y
> >  CONFIG_PHY_GIGE=y
> >  CONFIG_ETH_DESIGNWARE=y
> >  CONFIG_GMAC_ROCKCHIP=y
> > +CONFIG_PHY_ROCKCHIP_INNO_HDMI=y
> >  CONFIG_PHY_ROCKCHIP_INNO_USB2=y
> >  CONFIG_PINCTRL=y
> >  CONFIG_SPL_PINCTRL=y
> > @@ -80,6 +83,8 @@ CONFIG_PMIC_RK8XX=y
> >  CONFIG_SPL_DM_REGULATOR=y
> >  CONFIG_DM_REGULATOR_FIXED=y
> >  CONFIG_SPL_DM_REGULATOR_FIXED=y
> > +CONFIG_DM_REGULATOR_GPIO=y
> > +CONFIG_SPL_DM_REGULATOR_GPIO=y
>
> The ROCK64 does not use any regulator-gpio, this look unrelated and can
> be dropped?
>
> >  CONFIG_REGULATOR_RK8XX=y
> >  CONFIG_PWM_ROCKCHIP=y
> >  CONFIG_RAM=y
> > @@ -104,6 +109,11 @@ CONFIG_USB_OHCI_GENERIC=y
> >  CONFIG_USB_DWC2=y
> >  CONFIG_USB_DWC3=y
> >  CONFIG_USB_DWC3_GENERIC=y
> > +CONFIG_USB_KEYBOARD=y
> > +CONFIG_VIDEO=y
> > +CONFIG_DISPLAY=y
> > +CONFIG_VIDEO_ROCKCHIP=y
> > +CONFIG_DISPLAY_ROCKCHIP_HDMI=y
>
> This will force a small delay of at least a 300ms timeout for all users
> that does not have HDMI connected during boot.
>
> Maybe this could be extracted to a rockchip-hdmi.config-file to make it
> more optional?
>
> E.g. use "make rock64-rk3328_defconfig rockchip-hdmi.config" to build
> with something like:
>
>   > cat rockchip-hdmi.config
>   CONFIG_VIDEO=y
>   CONFIG_DISPLAY=y
>   CONFIG_VIDEO_ROCKCHIP=y
>   CONFIG_DISPLAY_ROCKCHIP_HDMI=y
>
> Regards,
> Jonas
>
> >  CONFIG_SPL_TINY_MEMSET=y
> >  CONFIG_TPL_TINY_MEMSET=y
> >  CONFIG_ERRNO_STR=y
>

Re: [PATCH v2 2/3] video: rockchip: rk3328: filter unsupported modes

[Vasily Khoruzhick]

On Sun, Mar 30, 2025 at 4:57 AM Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Hi Vasily,

> > +     /* Limit pixel clock to 297MHz. While RK3328 support higher rates and it
> > +      * works in Linux, it doesn't seem to work in u-boot. That limits max
> > +      * resolution to 1440p
> > +      */
> > +     return timing->pixelclock.typ <= 297000000;
>
> I think it make more sense to limit to the HDMI 1.4 max 340 MHz instead
> of arbitrary 297 MHz. Anything above 340 MHz will require scrambling etc
> and will not work in U-Boot unless at least dw-hdmi driver is updated.

Sure, sounds reasonable. Will do in v3.

> Regards,
> Jonas
>
> > +}
> > +
> >  static const struct dm_display_ops rk3328_hdmi_ops = {
> >       .read_edid = rk_hdmi_read_edid,
> >       .enable = rk3328_hdmi_enable,
> > +     .mode_valid = rk3328_hdmi_mode_valid,
> >  };
> >
> >  static const struct udevice_id rk3328_hdmi_ids[] = {
>

Re: [PATCH v2 3/3] rockchip: rock64-rk3328_defconfig: enable HDMI output and USB keyboard

[Jonas Karlman]

Hi Vasily,

On 2025-04-01 00:16, Vasily Khoruzhick wrote:
> On Sun, Mar 30, 2025 at 4:45 AM Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> Hi Vasily,
>>
>> On 2025-03-23 23:50, Vasily Khoruzhick wrote:
>>> Enable HDMI output and USB keyboard on Rock64 board
>>>
>>> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
>>> ---
>>> v2: no change
>>>
>>>  configs/rock64-rk3328_defconfig | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/configs/rock64-rk3328_defconfig b/configs/rock64-rk3328_defconfig
>>> index 6d00b52e62f..ae3512625cf 100644
>>> --- a/configs/rock64-rk3328_defconfig
>>> +++ b/configs/rock64-rk3328_defconfig
>>> @@ -21,7 +21,9 @@ CONFIG_FIT_VERBOSE=y
>>>  CONFIG_SPL_FIT_SIGNATURE=y
>>>  CONFIG_SPL_LOAD_FIT=y
>>>  CONFIG_LEGACY_IMAGE_FORMAT=y
>>> +CONFIG_USE_PREBOOT=y
>>
>> This will add a large delay during boot as it forces a run of the slow
>> "usb start" and is something we want to avoid for normal use, please
>> drop this.
> 
> Well, I can just drop the patch altogether.
> 
> Personally, I do need USB and HDMI support, since it allows me to boot
> from a secondary storage in case the OS on main storage doesn't boot.
> Not having USB support defeats the purpose of this patch.

With bootstd the storage media is initialized in fastest to slowest
(initialization time) order. So USB will be initialized if nothing it
found on mmc nvme etc, see boot_targets env var.

However, if you want usb in efi bootmgr you must initialize usb (and any
other storage media) before starting bootstd loop. To my knowledge
someone should be working on a change to the boot order to load efi
bootmgr just before removable media and have it initialize usb etc.

Since ROCK64 has env by default you can easily change the boot command
to run "usb start" before booting on your device, until the reordered
efi bootmgr work lands.

Slowing down boot (USE_PREBOOT=y) for everyone by several seconds to
force initialize USB when it is not really needed for i.e. a quick
extlinux/scripts boot from MMC is not ideal.

We could look into options using e.g. recovery button, env var or
similar to use an alternative boot flow.

Regards,
Jonas

> 
>>>  CONFIG_DEFAULT_FDT_FILE="rockchip/rk3328-rock64.dtb"
>>> +# CONFIG_SYS_DEVICE_NULLDEV is not set
>>>  # CONFIG_DISPLAY_CPUINFO is not set
>>>  CONFIG_DISPLAY_BOARDINFO_LATE=y
>>>  CONFIG_SPL_MAX_SIZE=0x40000
>>> @@ -72,6 +74,7 @@ CONFIG_DM_ETH_PHY=y
>>>  CONFIG_PHY_GIGE=y
>>>  CONFIG_ETH_DESIGNWARE=y
>>>  CONFIG_GMAC_ROCKCHIP=y
>>> +CONFIG_PHY_ROCKCHIP_INNO_HDMI=y
>>>  CONFIG_PHY_ROCKCHIP_INNO_USB2=y
>>>  CONFIG_PINCTRL=y
>>>  CONFIG_SPL_PINCTRL=y
>>> @@ -80,6 +83,8 @@ CONFIG_PMIC_RK8XX=y
>>>  CONFIG_SPL_DM_REGULATOR=y
>>>  CONFIG_DM_REGULATOR_FIXED=y
>>>  CONFIG_SPL_DM_REGULATOR_FIXED=y
>>> +CONFIG_DM_REGULATOR_GPIO=y
>>> +CONFIG_SPL_DM_REGULATOR_GPIO=y
>>
>> The ROCK64 does not use any regulator-gpio, this look unrelated and can
>> be dropped?
>>
>>>  CONFIG_REGULATOR_RK8XX=y
>>>  CONFIG_PWM_ROCKCHIP=y
>>>  CONFIG_RAM=y
>>> @@ -104,6 +109,11 @@ CONFIG_USB_OHCI_GENERIC=y
>>>  CONFIG_USB_DWC2=y
>>>  CONFIG_USB_DWC3=y
>>>  CONFIG_USB_DWC3_GENERIC=y
>>> +CONFIG_USB_KEYBOARD=y
>>> +CONFIG_VIDEO=y
>>> +CONFIG_DISPLAY=y
>>> +CONFIG_VIDEO_ROCKCHIP=y
>>> +CONFIG_DISPLAY_ROCKCHIP_HDMI=y
>>
>> This will force a small delay of at least a 300ms timeout for all users
>> that does not have HDMI connected during boot.
>>
>> Maybe this could be extracted to a rockchip-hdmi.config-file to make it
>> more optional?
>>
>> E.g. use "make rock64-rk3328_defconfig rockchip-hdmi.config" to build
>> with something like:
>>
>>   > cat rockchip-hdmi.config
>>   CONFIG_VIDEO=y
>>   CONFIG_DISPLAY=y
>>   CONFIG_VIDEO_ROCKCHIP=y
>>   CONFIG_DISPLAY_ROCKCHIP_HDMI=y
>>
>> Regards,
>> Jonas
>>
>>>  CONFIG_SPL_TINY_MEMSET=y
>>>  CONFIG_TPL_TINY_MEMSET=y
>>>  CONFIG_ERRNO_STR=y
>>