[PATCH 1/2] clk: sunxi-ng: v3s: Fix de clock definition

[PATCH 1/2] clk: sunxi-ng: v3s: Fix de clock definition

[Paul Kocialkowski]

The de clock is marked with CLK_SET_RATE_PARENT, which is really not
necessary (as confirmed from experimentation) and significantly
restricts flexibility for other clocks using the same parent.

In addition the source selection (parent) field is marked as using
2 bits, when it the documentation reports that it uses 3.

Fix both issues in the de clock definition.

Fixes: d0f11d14b0bc ("clk: sunxi-ng: add support for V3s CCU")
Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
---
 drivers/clk/sunxi-ng/ccu-sun8i-v3s.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
index 52e4369664c5..df345a620d8d 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
@@ -347,8 +347,7 @@ static SUNXI_CCU_GATE(dram_ohci_clk,	"dram-ohci",	"dram",
 
 static const char * const de_parents[] = { "pll-video", "pll-periph0" };
 static SUNXI_CCU_M_WITH_MUX_GATE(de_clk, "de", de_parents,
-				 0x104, 0, 4, 24, 2, BIT(31),
-				 CLK_SET_RATE_PARENT);
+				 0x104, 0, 4, 24, 3, BIT(31), 0);
 
 static const char * const tcon_parents[] = { "pll-video", "pll-periph0" };
 static SUNXI_CCU_M_WITH_MUX_GATE(tcon_clk, "tcon", tcon_parents,
-- 
2.49.0

[PATCH 2/2] clk: sunxi-ng: v3s: Assign the de and tcon clocks to the video pll

[Paul Kocialkowski]

It appears (based on experimentation) that both the de and tcon clocks
need to have the same parent for the two units to work together.

Assign them both to the video pll by manually clearing the parent
selection bits (effectively setting index 0) and marking the clocks
with the CLK_SET_RATE_NO_REPARENT flag, which ensures that they will
never use a different parent.

The video pll is also a possible parent for the camera subsystem,
but it can use the dedicated isp pll if needed so there should be
no negative side-effect due to this change.

Note that ccu_mux_helper_set_parent cannot be used at this stage as
it requires the clock driver to be initialized and this configuration
is best done before the clock driver is available to consumers.

Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
---
 drivers/clk/sunxi-ng/ccu-sun8i-v3s.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
index df345a620d8d..05595ac51b76 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
@@ -347,11 +347,13 @@ static SUNXI_CCU_GATE(dram_ohci_clk,	"dram-ohci",	"dram",
 
 static const char * const de_parents[] = { "pll-video", "pll-periph0" };
 static SUNXI_CCU_M_WITH_MUX_GATE(de_clk, "de", de_parents,
-				 0x104, 0, 4, 24, 3, BIT(31), 0);
+				 0x104, 0, 4, 24, 3, BIT(31),
+				 CLK_SET_RATE_NO_REPARENT);
 
 static const char * const tcon_parents[] = { "pll-video", "pll-periph0" };
 static SUNXI_CCU_M_WITH_MUX_GATE(tcon_clk, "tcon", tcon_parents,
-				 0x118, 0, 4, 24, 3, BIT(31), 0);
+				 0x118, 0, 4, 24, 3, BIT(31),
+				 CLK_SET_RATE_NO_REPARENT);
 
 static SUNXI_CCU_GATE(csi_misc_clk,	"csi-misc",	"osc24M",
 		      0x130, BIT(31), 0);
@@ -753,6 +755,21 @@ static int sun8i_v3s_ccu_probe(struct platform_device *pdev)
 	val &= ~GENMASK(19, 16);
 	writel(val, reg + SUN8I_V3S_PLL_AUDIO_REG);
 
+	/*
+	 * Assign the DE and TCON clock to the video PLL. Both clocks need to
+	 * have the same parent for the units to work together.
+	 */
+
+	val = readl(reg + de_clk.common.reg);
+	val &= ~GENMASK(de_clk.mux.shift + de_clk.mux.width - 1,
+			de_clk.mux.shift);
+	writel(val, reg + de_clk.common.reg);
+
+	val = readl(reg + tcon_clk.common.reg);
+	val &= ~GENMASK(tcon_clk.mux.shift + tcon_clk.mux.width - 1,
+			tcon_clk.mux.shift);
+	writel(val, reg + tcon_clk.common.reg);
+
 	return devm_sunxi_ccu_probe(&pdev->dev, reg, desc);
 }
 
-- 
2.49.0

Re: [PATCH 1/2] clk: sunxi-ng: v3s: Fix de clock definition

[Icenowy Zheng]

在 2025-07-04星期五的 17:40 +0200，Paul Kocialkowski写道：
> The de clock is marked with CLK_SET_RATE_PARENT, which is really not
> necessary (as confirmed from experimentation) and significantly
> restricts flexibility for other clocks using the same parent.

With it not setting parent, is arbitary pixel clocks still possible?

> 
> In addition the source selection (parent) field is marked as using
> 2 bits, when it the documentation reports that it uses 3.
> 
> Fix both issues in the de clock definition.
> 
> Fixes: d0f11d14b0bc ("clk: sunxi-ng: add support for V3s CCU")
> Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
> ---
>  drivers/clk/sunxi-ng/ccu-sun8i-v3s.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> index 52e4369664c5..df345a620d8d 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> @@ -347,8 +347,7 @@ static
> SUNXI_CCU_GATE(dram_ohci_clk,        "dram-ohci",    "dram",
>  
>  static const char * const de_parents[] = { "pll-video", "pll-
> periph0" };
>  static SUNXI_CCU_M_WITH_MUX_GATE(de_clk, "de", de_parents,
> -                                0x104, 0, 4, 24, 2, BIT(31),
> -                                CLK_SET_RATE_PARENT);
> +                                0x104, 0, 4, 24, 3, BIT(31), 0);
>  
>  static const char * const tcon_parents[] = { "pll-video", "pll-
> periph0" };
>  static SUNXI_CCU_M_WITH_MUX_GATE(tcon_clk, "tcon", tcon_parents,

Re: [PATCH 1/2] clk: sunxi-ng: v3s: Fix de clock definition

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 4610 bytes --]

Hi,

Le Fri 04 Jul 25, 23:54, Icenowy Zheng a écrit :
> 在 2025-07-04星期五的 17:40 +0200，Paul Kocialkowski写道：
> > The de clock is marked with CLK_SET_RATE_PARENT, which is really not
> > necessary (as confirmed from experimentation) and significantly
> > restricts flexibility for other clocks using the same parent.
> 
> With it not setting parent, is arbitary pixel clocks still possible?

Absolutely and the clock tree is very much improved, I think the flag was the
reason that was preventing it from naturally keeping the tcon and de clocks
under the video pll in my case.

Now it can provide both the 33 Mhz for the pixel clock and runs the mixer at
nearly 150 MHz. The video pll now runs at 297 MHz which is a perfect fit for
csi-sclk camera main clock, so the algorithm is doing its job at its best!

So this means that I no longer have to change the mixer clock to 297 MHz to
keep it under the video pll. It pretty much solves all my problems at once.

Here is the relevant clk_summary extract:
    pll-video                        2       2        2        297000000   50000      0     50000      Y      deviceless                      no_connection_id         
       csi-sclk                      0       0        3        297000000   50000      0     50000      N         1cb8000.isp                     mod                      
                                                                                                                 1cb0000.camera                  mod                      
                                                                                                                 1cb1000.csi                     mod                      
       tcon                          2       2        1        33000000    50000      0     50000      Y         1c0c000.lcd-controller          tcon-ch0                 
          tcon-data-clock            1       1        1        33000000    50000      0     50000      Y            deviceless                      no_connection_id         
       de                            2       2        0        297000000   50000      0     50000      Y         1000000.clock                   mod                      
          wb-div                     0       0        0        297000000   50000      0     50000      Y            deviceless                      no_connection_id         
             wb                      0       0        0        297000000   50000      0     50000      N               deviceless                      no_connection_id         
          mixer0-div                 1       1        0        148500000   50000      0     50000      Y            deviceless                      no_connection_id         
             mixer0                  1       1        0        148500000   50000      0     50000      Y               1100000.mixer                   mod                      

Cheers,

Paul

> > 
> > In addition the source selection (parent) field is marked as using
> > 2 bits, when it the documentation reports that it uses 3.
> > 
> > Fix both issues in the de clock definition.
> > 
> > Fixes: d0f11d14b0bc ("clk: sunxi-ng: add support for V3s CCU")
> > Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
> > ---
> >  drivers/clk/sunxi-ng/ccu-sun8i-v3s.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> > b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> > index 52e4369664c5..df345a620d8d 100644
> > --- a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> > +++ b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> > @@ -347,8 +347,7 @@ static
> > SUNXI_CCU_GATE(dram_ohci_clk,        "dram-ohci",    "dram",
> >  
> >  static const char * const de_parents[] = { "pll-video", "pll-
> > periph0" };
> >  static SUNXI_CCU_M_WITH_MUX_GATE(de_clk, "de", de_parents,
> > -                                0x104, 0, 4, 24, 2, BIT(31),
> > -                                CLK_SET_RATE_PARENT);
> > +                                0x104, 0, 4, 24, 3, BIT(31), 0);
> >  
> >  static const char * const tcon_parents[] = { "pll-video", "pll-
> > periph0" };
> >  static SUNXI_CCU_M_WITH_MUX_GATE(tcon_clk, "tcon", tcon_parents,
> 

-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Expert in multimedia, graphics and embedded hardware support with Linux.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] clk: sunxi-ng: v3s: Fix de clock definition

[Icenowy Zheng]

在 2025-07-04星期五的 23:52 +0200，Paul Kocialkowski写道：
> Hi,
> 
> Le Fri 04 Jul 25, 23:54, Icenowy Zheng a écrit :
> > 在 2025-07-04星期五的 17:40 +0200，Paul Kocialkowski写道：
> > > The de clock is marked with CLK_SET_RATE_PARENT, which is really
> > > not
> > > necessary (as confirmed from experimentation) and significantly
> > > restricts flexibility for other clocks using the same parent.
> > 
> > With it not setting parent, is arbitary pixel clocks still
> > possible?
> 
> Absolutely and the clock tree is very much improved, I think the flag
> was the
> reason that was preventing it from naturally keeping the tcon and de
> clocks
> under the video pll in my case.
> 
> Now it can provide both the 33 Mhz for the pixel clock and runs the
> mixer at
> nearly 150 MHz. The video pll now runs at 297 MHz which is a perfect

Did you test other pixel clocks?

I suggest you to try a RGB-to-VGA bridge and test possible pixel clocks
on the VGA port.

> fit for
> csi-sclk camera main clock, so the algorithm is doing its job at its
> best!
> 
> So this means that I no longer have to change the mixer clock to 297
> MHz to
> keep it under the video pll. It pretty much solves all my problems at
> once.
> 
> Here is the relevant clk_summary extract:
>     pll-video                        2       2        2       
> 297000000   50000      0     50000      Y     
> deviceless                      no_connection_id         
>        csi-sclk                      0       0        3       
> 297000000   50000      0     50000      N        
> 1cb8000.isp                     mod                      
>                                                                      
>                                            
> 1cb0000.camera                  mod                      
>                                                                      
>                                            
> 1cb1000.csi                     mod                      
>        tcon                          2       2        1       
> 33000000    50000      0     50000      Y         1c0c000.lcd-
> controller          tcon-ch0                 
>           tcon-data-clock            1       1        1       
> 33000000    50000      0     50000      Y           
> deviceless                      no_connection_id         
>        de                            2       2        0       
> 297000000   50000      0     50000      Y        
> 1000000.clock                   mod                      
>           wb-div                     0       0        0       
> 297000000   50000      0     50000      Y           
> deviceless                      no_connection_id         
>              wb                      0       0        0       
> 297000000   50000      0     50000      N              
> deviceless                      no_connection_id         
>           mixer0-div                 1       1        0       
> 148500000   50000      0     50000      Y           
> deviceless                      no_connection_id         
>              mixer0                  1       1        0       
> 148500000   50000      0     50000      Y              
> 1100000.mixer                   mod                      
> 
> Cheers,
> 
> Paul
> 
> > > 
> > > In addition the source selection (parent) field is marked as
> > > using
> > > 2 bits, when it the documentation reports that it uses 3.
> > > 
> > > Fix both issues in the de clock definition.
> > > 
> > > Fixes: d0f11d14b0bc ("clk: sunxi-ng: add support for V3s CCU")
> > > Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
> > > ---
> > >  drivers/clk/sunxi-ng/ccu-sun8i-v3s.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> > > b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> > > index 52e4369664c5..df345a620d8d 100644
> > > --- a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> > > +++ b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> > > @@ -347,8 +347,7 @@ static
> > > SUNXI_CCU_GATE(dram_ohci_clk,        "dram-ohci",    "dram",
> > >  
> > >  static const char * const de_parents[] = { "pll-video", "pll-
> > > periph0" };
> > >  static SUNXI_CCU_M_WITH_MUX_GATE(de_clk, "de", de_parents,
> > > -                                0x104, 0, 4, 24, 2, BIT(31),
> > > -                                CLK_SET_RATE_PARENT);
> > > +                                0x104, 0, 4, 24, 3, BIT(31), 0);
> > >  
> > >  static const char * const tcon_parents[] = { "pll-video", "pll-
> > > periph0" };
> > >  static SUNXI_CCU_M_WITH_MUX_GATE(tcon_clk, "tcon", tcon_parents,
> > 
> 

Re: [PATCH 1/2] clk: sunxi-ng: v3s: Fix de clock definition

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 6808 bytes --]

Hi,

Le Sat 05 Jul 25, 14:38, Icenowy Zheng a écrit :
> 在 2025-07-04星期五的 23:52 +0200，Paul Kocialkowski写道：
> > Hi,
> > 
> > Le Fri 04 Jul 25, 23:54, Icenowy Zheng a écrit :
> > > 在 2025-07-04星期五的 17:40 +0200，Paul Kocialkowski写道：
> > > > The de clock is marked with CLK_SET_RATE_PARENT, which is really
> > > > not
> > > > necessary (as confirmed from experimentation) and significantly
> > > > restricts flexibility for other clocks using the same parent.
> > > 
> > > With it not setting parent, is arbitary pixel clocks still
> > > possible?
> > 
> > Absolutely and the clock tree is very much improved, I think the flag
> > was the
> > reason that was preventing it from naturally keeping the tcon and de
> > clocks
> > under the video pll in my case.
> > 
> > Now it can provide both the 33 Mhz for the pixel clock and runs the
> > mixer at
> > nearly 150 MHz. The video pll now runs at 297 MHz which is a perfect
> 
> Did you test other pixel clocks?
> 
> I suggest you to try a RGB-to-VGA bridge and test possible pixel clocks
> on the VGA port.

I don't have any such bridge and the V3/V3s boards I have would not easily
allow connecting such a display. But I would be glad if someone wants to test
the patch with such a setup.

Cheers,

Paul


> > fit for
> > csi-sclk camera main clock, so the algorithm is doing its job at its
> > best!
> > 
> > So this means that I no longer have to change the mixer clock to 297
> > MHz to
> > keep it under the video pll. It pretty much solves all my problems at
> > once.
> > 
> > Here is the relevant clk_summary extract:
> >     pll-video                        2       2        2       
> > 297000000   50000      0     50000      Y     
> > deviceless                      no_connection_id         
> >        csi-sclk                      0       0        3       
> > 297000000   50000      0     50000      N        
> > 1cb8000.isp                     mod                      
> >                                                                      
> >                                            
> > 1cb0000.camera                  mod                      
> >                                                                      
> >                                            
> > 1cb1000.csi                     mod                      
> >        tcon                          2       2        1       
> > 33000000    50000      0     50000      Y         1c0c000.lcd-
> > controller          tcon-ch0                 
> >           tcon-data-clock            1       1        1       
> > 33000000    50000      0     50000      Y           
> > deviceless                      no_connection_id         
> >        de                            2       2        0       
> > 297000000   50000      0     50000      Y        
> > 1000000.clock                   mod                      
> >           wb-div                     0       0        0       
> > 297000000   50000      0     50000      Y           
> > deviceless                      no_connection_id         
> >              wb                      0       0        0       
> > 297000000   50000      0     50000      N              
> > deviceless                      no_connection_id         
> >           mixer0-div                 1       1        0       
> > 148500000   50000      0     50000      Y           
> > deviceless                      no_connection_id         
> >              mixer0                  1       1        0       
> > 148500000   50000      0     50000      Y              
> > 1100000.mixer                   mod                      
> > 
> > Cheers,
> > 
> > Paul
> > 
> > > > 
> > > > In addition the source selection (parent) field is marked as
> > > > using
> > > > 2 bits, when it the documentation reports that it uses 3.
> > > > 
> > > > Fix both issues in the de clock definition.
> > > > 
> > > > Fixes: d0f11d14b0bc ("clk: sunxi-ng: add support for V3s CCU")
> > > > Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
> > > > ---
> > > >  drivers/clk/sunxi-ng/ccu-sun8i-v3s.c | 3 +--
> > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> > > > b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> > > > index 52e4369664c5..df345a620d8d 100644
> > > > --- a/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> > > > +++ b/drivers/clk/sunxi-ng/ccu-sun8i-v3s.c
> > > > @@ -347,8 +347,7 @@ static
> > > > SUNXI_CCU_GATE(dram_ohci_clk,        "dram-ohci",    "dram",
> > > >  
> > > >  static const char * const de_parents[] = { "pll-video", "pll-
> > > > periph0" };
> > > >  static SUNXI_CCU_M_WITH_MUX_GATE(de_clk, "de", de_parents,
> > > > -                                0x104, 0, 4, 24, 2, BIT(31),
> > > > -                                CLK_SET_RATE_PARENT);
> > > > +                                0x104, 0, 4, 24, 3, BIT(31), 0);
> > > >  
> > > >  static const char * const tcon_parents[] = { "pll-video", "pll-
> > > > periph0" };
> > > >  static SUNXI_CCU_M_WITH_MUX_GATE(tcon_clk, "tcon", tcon_parents,
> > > 
> > 
> 

-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Expert in multimedia, graphics and embedded hardware support with Linux.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/2] clk: sunxi-ng: v3s: Fix de clock definition

[Chen-Yu Tsai]

On Fri, 04 Jul 2025 17:40:07 +0200, Paul Kocialkowski wrote:
> The de clock is marked with CLK_SET_RATE_PARENT, which is really not
> necessary (as confirmed from experimentation) and significantly
> restricts flexibility for other clocks using the same parent.
> 
> In addition the source selection (parent) field is marked as using
> 2 bits, when it the documentation reports that it uses 3.
> 
> [...]

Applied to sunxi/clk-for-6.17 in local tree, thanks!

[1/2] clk: sunxi-ng: v3s: Fix de clock definition
      commit: e8ab346f9907a1a3aa2f0e5decf849925c06ae2e
[2/2] clk: sunxi-ng: v3s: Assign the de and tcon clocks to the video pll
      commit: ea879ce83d360aa13acd54cf6af913885b69ed44

Best regards,
-- 
Chen-Yu Tsai <wens@csie.org>