diff mbox series

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

Message ID 20250114052548.763611-1-anarsoul@gmail.com
State New
Delegated to: Kever Yang
Headers show
Series [1/3] clk: rockchip: rk3328: use HDMIPHY PLL as a clock parent for VOP_DCLK | expand

Commit Message

Vasily Khoruzhick Jan. 14, 2025, 5:24 a.m. UTC
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>
---
 drivers/clk/rockchip/clk_rk3328.c | 76 +++++++------------------------
 1 file changed, 16 insertions(+), 60 deletions(-)

Comments

Matwey V. Kornilov Jan. 15, 2025, 1:58 p.m. UTC | #1
Thanks. Very interesting. I'll test it on my Rock64 at the weekend.

вт, 14 янв. 2025 г. в 08:26, Vasily Khoruzhick <anarsoul@gmail.com>:
>
> 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>
> ---
>  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.47.1
>
Matwey V. Kornilov Jan. 18, 2025, 12:52 p.m. UTC | #2
вт, 14 янв. 2025 г. в 08:26, Vasily Khoruzhick <anarsoul@gmail.com>:
>
> 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>

Both UART-console and HDMI-console works with this patcheset on my Rock64 board.

Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com>

> ---
>  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.47.1
>
Vasily Khoruzhick Jan. 19, 2025, 12:22 a.m. UTC | #3
On Sat, Jan 18, 2025 at 4:53 AM Matwey V. Kornilov
<matwey.kornilov@gmail.com> wrote:

> Both UART-console and HDMI-console works with this patcheset on my Rock64 board.
>
> Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com>

Thanks for testing!
diff mbox series

Patch

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;