diff mbox series

[5/9] ARM: dts: sun8i: r40: Add TCON TOP LCD clocking

Message ID 20190613185241.22800-6-jagan@amarulasolutions.com
State New
Headers
Series drm/sun4i: Allwinner R40 MIPI-DSI support |

Commit Message

Jagan Teki June 13, 2019, 6:52 p.m. UTC 
According to Fig 7-2. TCON Top Block Diagram in User manual.

TCON TOP can have an hierarchy for TCON_LCD0, LCD1 like
TCON_TV0, TV1 so, the tcon top would handle the clocks of
TCON_LCD0, LCD1 similar like TV0, TV1.

But, the current tcon_top node is using dsi clock name with
CLK_DSI_DPHY which is ideally handle via dphy which indeed
a separate interface block.

So, use tcon-lcd0 instead of dsi which would refer the
CLK_TCON_LCD0 similar like CLK_TCON_TV0 with tcon-tv0.

This way we can refer CLK_TCON_LCD0 from tcon_top clock in
tcon_lcd0 node and the actual DSI_DPHY clock node would
refer in dphy node.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm/boot/dts/sun8i-r40.dtsi           | 6 +++---
 drivers/gpu/drm/sun4i/sun8i_tcon_top.c     | 6 +++---
 include/dt-bindings/clock/sun8i-tcon-top.h | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

Comments

Chen-Yu Tsai June 14, 2019, 3:46 a.m. UTC | #1 
On Fri, Jun 14, 2019 at 2:54 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> According to Fig 7-2. TCON Top Block Diagram in User manual.
>
> TCON TOP can have an hierarchy for TCON_LCD0, LCD1 like
> TCON_TV0, TV1 so, the tcon top would handle the clocks of
> TCON_LCD0, LCD1 similar like TV0, TV1.

That is not guaranteed. The diagram shows the pixel data path,
not necessarily the clocks. In addition, the LCD TCONs have an
internal clock gate for the dot-clock output, which the TV variants
do not. That might explain the need for the gates in TCON TOP.

> But, the current tcon_top node is using dsi clock name with
> CLK_DSI_DPHY which is ideally handle via dphy which indeed
> a separate interface block.
>
> So, use tcon-lcd0 instead of dsi which would refer the
> CLK_TCON_LCD0 similar like CLK_TCON_TV0 with tcon-tv0.
>
> This way we can refer CLK_TCON_LCD0 from tcon_top clock in
> tcon_lcd0 node and the actual DSI_DPHY clock node would
> refer in dphy node.

That doesn't make sense. What about TCON_LCD1?

The CCU already has CLK_TCON_LCD0 and CLK_TCON_LCD1. What makes
you think that the TCONs don't use them directly?

Or maybe they do go through TCON_TOP, but there's no gate,
so we don't know about it.

You need to rethink this. What are you trying to deal with?

> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  arch/arm/boot/dts/sun8i-r40.dtsi           | 6 +++---
>  drivers/gpu/drm/sun4i/sun8i_tcon_top.c     | 6 +++---
>  include/dt-bindings/clock/sun8i-tcon-top.h | 2 +-

This is going to be a pain to merge.

First, you need to split the driver parts from the DT parts.

Second, you might need to revert CLK_DSI_DPHY back to a raw
number for now, so that when the patches get merged through
different trees, nothing breaks.

Third, you'll come back after everything is merged, and change
the raw number to the new macro.

ChenYu
Jagan Teki June 14, 2019, 9:48 a.m. UTC | #2 
On Fri, Jun 14, 2019 at 9:16 AM Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Fri, Jun 14, 2019 at 2:54 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > According to Fig 7-2. TCON Top Block Diagram in User manual.
> >
> > TCON TOP can have an hierarchy for TCON_LCD0, LCD1 like
> > TCON_TV0, TV1 so, the tcon top would handle the clocks of
> > TCON_LCD0, LCD1 similar like TV0, TV1.
>
> That is not guaranteed. The diagram shows the pixel data path,
> not necessarily the clocks. In addition, the LCD TCONs have an
> internal clock gate for the dot-clock output, which the TV variants
> do not. That might explain the need for the gates in TCON TOP.

Correct, the actual idea about explanation here is to mention the
clocks definition in tcon top level and internal tv and lcd can handle
as you explained.

>
> > But, the current tcon_top node is using dsi clock name with
> > CLK_DSI_DPHY which is ideally handle via dphy which indeed
> > a separate interface block.
> >
> > So, use tcon-lcd0 instead of dsi which would refer the
> > CLK_TCON_LCD0 similar like CLK_TCON_TV0 with tcon-tv0.
> >
> > This way we can refer CLK_TCON_LCD0 from tcon_top clock in
> > tcon_lcd0 node and the actual DSI_DPHY clock node would
> > refer in dphy node.
>
> That doesn't make sense. What about TCON_LCD1?
>
> The CCU already has CLK_TCON_LCD0 and CLK_TCON_LCD1. What makes
> you think that the TCONs don't use them directly?
>
> Or maybe they do go through TCON_TOP, but there's no gate,
> so we don't know about it.
>
> You need to rethink this. What are you trying to deal with?

Yes, I understand what your asking for and indeed this is where I get
confused and tried this way initially and attach the dsi reference in
dphy something like

tcon_lcd0 {
                clocks = <&ccu CLK_BUS_TCON_LCD0>, <&ccu CLK_TCON_LCD0>;
                clock-names = "ahb", "tcon-ch0";
};

dphy {
               clocks = <&ccu CLK_BUS_MIPI_DSI>,
                              <&tcon_top CLK_TCON_TOP_DSI>;
               clock-names = "bus", "mod";
};

This would ended-up, phy wont getting the mod clock keep probing for
-EPROBE-DEFER since tcon top driver might not be loaded at the time
mipi driver. This way we have tv0, tv1 and dsi gates supported as
existed. Does it make sense?
Chen-Yu Tsai June 14, 2019, 9:53 a.m. UTC | #3 
On Fri, Jun 14, 2019 at 5:48 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Fri, Jun 14, 2019 at 9:16 AM Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > On Fri, Jun 14, 2019 at 2:54 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > According to Fig 7-2. TCON Top Block Diagram in User manual.
> > >
> > > TCON TOP can have an hierarchy for TCON_LCD0, LCD1 like
> > > TCON_TV0, TV1 so, the tcon top would handle the clocks of
> > > TCON_LCD0, LCD1 similar like TV0, TV1.
> >
> > That is not guaranteed. The diagram shows the pixel data path,
> > not necessarily the clocks. In addition, the LCD TCONs have an
> > internal clock gate for the dot-clock output, which the TV variants
> > do not. That might explain the need for the gates in TCON TOP.
>
> Correct, the actual idea about explanation here is to mention the
> clocks definition in tcon top level and internal tv and lcd can handle
> as you explained.
>
> >
> > > But, the current tcon_top node is using dsi clock name with
> > > CLK_DSI_DPHY which is ideally handle via dphy which indeed
> > > a separate interface block.
> > >
> > > So, use tcon-lcd0 instead of dsi which would refer the
> > > CLK_TCON_LCD0 similar like CLK_TCON_TV0 with tcon-tv0.
> > >
> > > This way we can refer CLK_TCON_LCD0 from tcon_top clock in
> > > tcon_lcd0 node and the actual DSI_DPHY clock node would
> > > refer in dphy node.
> >
> > That doesn't make sense. What about TCON_LCD1?
> >
> > The CCU already has CLK_TCON_LCD0 and CLK_TCON_LCD1. What makes
> > you think that the TCONs don't use them directly?
> >
> > Or maybe they do go through TCON_TOP, but there's no gate,
> > so we don't know about it.
> >
> > You need to rethink this. What are you trying to deal with?
>
> Yes, I understand what your asking for and indeed this is where I get
> confused and tried this way initially and attach the dsi reference in
> dphy something like
>
> tcon_lcd0 {
>                 clocks = <&ccu CLK_BUS_TCON_LCD0>, <&ccu CLK_TCON_LCD0>;
>                 clock-names = "ahb", "tcon-ch0";
> };
>
> dphy {
>                clocks = <&ccu CLK_BUS_MIPI_DSI>,
>                               <&tcon_top CLK_TCON_TOP_DSI>;
>                clock-names = "bus", "mod";
> };
>
> This would ended-up, phy wont getting the mod clock keep probing for
> -EPROBE-DEFER since tcon top driver might not be loaded at the time
> mipi driver. This way we have tv0, tv1 and dsi gates supported as
> existed. Does it make sense?

Looks like that happens because the clocks are only registered at
the component bind phase, rather than the probe phase. And to bind
all the components, the DSI controller wants the DPHY available,
which isn't because it's still waiting for the clock.

So you could try moving the bits that register the clocks in the
TCON TOP driver to the probe function, and see if that solves
the circular dependency issue.

ChenYu
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
index 219d2dca16b3..12576536df4a 100644
--- a/arch/arm/boot/dts/sun8i-r40.dtsi
+++ b/arch/arm/boot/dts/sun8i-r40.dtsi
@@ -591,16 +591,16 @@ 
 				 <&ccu CLK_TVE0>,
 				 <&ccu CLK_TCON_TV1>,
 				 <&ccu CLK_TVE1>,
-				 <&ccu CLK_DSI_DPHY>;
+				 <&ccu CLK_TCON_LCD0>;
 			clock-names = "bus",
 				      "tcon-tv0",
 				      "tve0",
 				      "tcon-tv1",
 				      "tve1",
-				      "dsi";
+				      "tcon-lcd0";
 			clock-output-names = "tcon-top-tv0",
 					     "tcon-top-tv1",
-					     "tcon-top-dsi";
+					     "tcon-top-lcd0";
 			resets = <&ccu RST_BUS_TCON_TOP>;
 			#clock-cells = <1>;
 
diff --git a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
index 465e9b0cdfee..e23c19f18986 100644
--- a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
+++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
@@ -205,11 +205,11 @@  static int sun8i_tcon_top_bind(struct device *dev, struct device *master,
 						     CLK_TCON_TOP_TV1);
 
 	if (quirks->has_dsi)
-		clk_data->hws[CLK_TCON_TOP_DSI] =
-			sun8i_tcon_top_register_gate(dev, "dsi", regs,
+		clk_data->hws[CLK_TCON_TOP_LCD0] =
+			sun8i_tcon_top_register_gate(dev, "tcon-lcd0", regs,
 						     &tcon_top->reg_lock,
 						     TCON_TOP_TCON_DSI_GATE,
-						     CLK_TCON_TOP_DSI);
+						     CLK_TCON_TOP_LCD0);
 
 	for (i = 0; i < CLK_NUM; i++)
 		if (IS_ERR(clk_data->hws[i])) {
diff --git a/include/dt-bindings/clock/sun8i-tcon-top.h b/include/dt-bindings/clock/sun8i-tcon-top.h
index 25164d767835..88de3f2ba335 100644
--- a/include/dt-bindings/clock/sun8i-tcon-top.h
+++ b/include/dt-bindings/clock/sun8i-tcon-top.h
@@ -6,6 +6,6 @@ 
 
 #define CLK_TCON_TOP_TV0	0
 #define CLK_TCON_TOP_TV1	1
-#define CLK_TCON_TOP_DSI	2
+#define CLK_TCON_TOP_LCD0	2
 
 #endif /* _DT_BINDINGS_CLOCK_SUN8I_TCON_TOP_H_ */