| Message ID | 20180612200036.21483-1-jernej.skrabec@siol.net |
|---|---|
| Headers | show |
| Series | Add support for R40 HDMI pipeline | expand |
On Wed, Jun 13, 2018 at 4:00 AM, Jernej Skrabec <jernej.skrabec@siol.net> wrote: > According to documentation and experience with other similar SoCs, video > PLLs don't work stable if their output frequency is set below 192 MHz. > > Because of that, set minimal rate to both R40 video PLLs to 192 MHz. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> Reviewed-by: Chen-Yu Tsai <wens@csie.org> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 13, 2018 at 4:00 AM, Jernej Skrabec <jernej.skrabec@siol.net> wrote: > Display related peripherals need precise clocks to operate correctly. > > Allow DE2, TCONs and HDMI to set parent clock. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> Reviewed-by: Chen-Yu Tsai <wens@csie.org> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 12, 2018 at 10:00:33PM +0200, Jernej Skrabec wrote: > Function is useful when drm_of_find_possible_crtcs() can't be used and > custom parsing is needed. This can happen for example when there is a > node with multiple muxes between crtc and encoder. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > --- > drivers/gpu/drm/drm_of.c | 4 ++-- > include/drm/drm_of.h | 8 ++++++++ > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c > index 1fe122461298..2e9cea3287b2 100644 > --- a/drivers/gpu/drm/drm_of.c > +++ b/drivers/gpu/drm/drm_of.c > @@ -22,8 +22,8 @@ static void drm_release_of(struct device *dev, void *data) > * Given a port OF node, return the possible mask of the corresponding > * CRTC within a device's list of CRTCs. Returns zero if not found. > */ > -static uint32_t drm_crtc_port_mask(struct drm_device *dev, > - struct device_node *port) > +uint32_t drm_crtc_port_mask(struct drm_device *dev, > + struct device_node *port) It should probably be exported too? Maxime
On Tue, Jun 12, 2018 at 10:00:23PM +0200, Jernej Skrabec wrote: > TV TCONs are always connected to TV or HDMI encoder, so it doesn't make > sense to check if panel or bridge is connected to them. > > Check if TCON has channel 0 and only then check for connected panel or > bridges. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > --- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index b1205a7bc20f..c9ffa5381185 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -1189,13 +1189,19 @@ static const struct component_ops sun4i_tcon_ops = { > static int sun4i_tcon_probe(struct platform_device *pdev) > { > struct device_node *node = pdev->dev.of_node; > + const struct sun4i_tcon_quirks *quirks; > struct drm_bridge *bridge; > struct drm_panel *panel; > int ret; > > - ret = drm_of_find_panel_or_bridge(node, 1, 0, &panel, &bridge); > - if (ret == -EPROBE_DEFER) > - return ret; > + quirks = of_device_get_match_data(&pdev->dev); We should probably check ofr the pointer value before dereferencing it. Maxime
On Wed, Jun 13, 2018 at 3:46 PM, Maxime Ripard <maxime.ripard@bootlin.com> wrote: > On Tue, Jun 12, 2018 at 10:00:23PM +0200, Jernej Skrabec wrote: >> TV TCONs are always connected to TV or HDMI encoder, so it doesn't make >> sense to check if panel or bridge is connected to them. >> >> Check if TCON has channel 0 and only then check for connected panel or >> bridges. >> >> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> >> --- >> drivers/gpu/drm/sun4i/sun4i_tcon.c | 12 +++++++++--- >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> index b1205a7bc20f..c9ffa5381185 100644 >> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> @@ -1189,13 +1189,19 @@ static const struct component_ops sun4i_tcon_ops = { >> static int sun4i_tcon_probe(struct platform_device *pdev) >> { >> struct device_node *node = pdev->dev.of_node; >> + const struct sun4i_tcon_quirks *quirks; >> struct drm_bridge *bridge; >> struct drm_panel *panel; >> int ret; >> >> - ret = drm_of_find_panel_or_bridge(node, 1, 0, &panel, &bridge); >> - if (ret == -EPROBE_DEFER) >> - return ret; >> + quirks = of_device_get_match_data(&pdev->dev); > > We should probably check ofr the pointer value before dereferencing it. I think we've discussed this before. If the driver has data structures for all the supported compatible strings, and it is device tree only, then we should just let it blow up in the user's face, since they are obviously doing something they shouldn't be doing to get the driver to probe without a compatible string match. ChenYu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne sreda, 13. junij 2018 ob 09:36:05 CEST je Maxime Ripard napisal(a): > On Tue, Jun 12, 2018 at 10:00:33PM +0200, Jernej Skrabec wrote: > > Function is useful when drm_of_find_possible_crtcs() can't be used and > > custom parsing is needed. This can happen for example when there is a > > node with multiple muxes between crtc and encoder. > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > --- > > > > drivers/gpu/drm/drm_of.c | 4 ++-- > > include/drm/drm_of.h | 8 ++++++++ > > 2 files changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c > > index 1fe122461298..2e9cea3287b2 100644 > > --- a/drivers/gpu/drm/drm_of.c > > +++ b/drivers/gpu/drm/drm_of.c > > @@ -22,8 +22,8 @@ static void drm_release_of(struct device *dev, void > > *data)> > > * Given a port OF node, return the possible mask of the corresponding > > * CRTC within a device's list of CRTCs. Returns zero if not found. > > */ > > > > -static uint32_t drm_crtc_port_mask(struct drm_device *dev, > > - struct device_node *port) > > +uint32_t drm_crtc_port_mask(struct drm_device *dev, > > + struct device_node *port) > > It should probably be exported too? Yes, of course. It will be in next version. Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne sreda, 13. junij 2018 ob 10:04:20 CEST je Chen-Yu Tsai napisal(a): > On Wed, Jun 13, 2018 at 3:46 PM, Maxime Ripard > > <maxime.ripard@bootlin.com> wrote: > > On Tue, Jun 12, 2018 at 10:00:23PM +0200, Jernej Skrabec wrote: > >> TV TCONs are always connected to TV or HDMI encoder, so it doesn't make > >> sense to check if panel or bridge is connected to them. > >> > >> Check if TCON has channel 0 and only then check for connected panel or > >> bridges. > >> > >> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > >> --- > >> > >> drivers/gpu/drm/sun4i/sun4i_tcon.c | 12 +++++++++--- > >> 1 file changed, 9 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> b/drivers/gpu/drm/sun4i/sun4i_tcon.c index b1205a7bc20f..c9ffa5381185 > >> 100644 > >> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> @@ -1189,13 +1189,19 @@ static const struct component_ops sun4i_tcon_ops > >> = {>> > >> static int sun4i_tcon_probe(struct platform_device *pdev) > >> { > >> > >> struct device_node *node = pdev->dev.of_node; > >> > >> + const struct sun4i_tcon_quirks *quirks; > >> > >> struct drm_bridge *bridge; > >> struct drm_panel *panel; > >> int ret; > >> > >> - ret = drm_of_find_panel_or_bridge(node, 1, 0, &panel, &bridge); > >> - if (ret == -EPROBE_DEFER) > >> - return ret; > >> + quirks = of_device_get_match_data(&pdev->dev); > > > > We should probably check ofr the pointer value before dereferencing it. > > I think we've discussed this before. If the driver has data structures > for all the supported compatible strings, and it is device tree only, > then we should just let it blow up in the user's face, since they are > obviously doing something they shouldn't be doing to get the driver > to probe without a compatible string match. TCON can't work with no quirks specified, since that would mean that neither channels are present. Additionally, sun4i_tcon_bind() also doesn't check if quirks are NULL or not. So I concur with Chen-Yu here. Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 13, 2018 at 1:30 AM, Jernej Skrabec <jernej.skrabec@siol.net> wrote: > This series adds support for R40 HDMI pipeline. It is a bit special > than other already supported pipelines because it has additional unit > called TCON TOP responsible for relationship configuration between > mixers, TCONs and HDMI. Additionally, it has additional gates for DSI > and TV TCONs, TV encoder clock settings and pin muxing between LCD > and TV encoders. > > However, it seems that TCON TOP will become a norm, since newer > Allwinner SoCs like H6 also have this unit. > > I tested different possible configurations: > - mixer0 <> TCON-TV0 <> HDMI > - mixer0 <> TCON-TV1 <> HDMI > - mixer1 <> TCON-TV0 <> HDMI > - mixer1 <> TCON-TV1 <> HDMI > > Please review. > > Best regards, > Jernej > > Changes from v1: > - Split DT bindings patch and updated description > - Split HDMI PHY patch > - Move header file from TCON TOP patch to dt bindings patch > - Added Rob reviewed-by tag > - Used clk_hw_register_gate() instead of custom gate registration code > - Reworked TCON TOP to be part of of-graph. Because of that, a lot of > new patches were added. > - Droped mixer index quirk patch > - Reworked TCON support for TCON TOP > - Updated commit messages > > Jernej Skrabec (27): > clk: sunxi-ng: r40: Add minimal rate for video PLLs > clk: sunxi-ng: r40: Allow setting parent rate to display related > clocks > clk: sunxi-ng: r40: Export video PLLs > dt-bindings: display: sunxi-drm: Add TCON TOP description > drm/sun4i: Add TCON TOP driver > drm/sun4i: Fix releasing node when enumerating enpoints > drm/sun4i: Split out code for enumerating endpoints in output port > drm/sun4i: Add support for traversing graph with TCON TOP > drm/sun4i: Don't skip TCONs if they don't have channel 0 > dt-bindings: display: sun4i-drm: Add R40 TV TCON description > drm/sun4i: tcon: Add support for tcon-top gate > drm/sun4i: tcon: Generalize engine search algorithm > drm/sun4i: Don't check for LVDS and RGB when TCON has only ch1 > drm/sun4i: Don't check for panel or bridge on TV TCONs > drm/sun4i: Add support for R40 TV TCON > dt-bindings: display: sun4i-drm: Add R40 mixer compatibles > drm/sun4i: Add support for R40 mixers > dt-bindings: display: sun4i-drm: Add description of A64 HDMI PHY > drm/sun4i: Enable DW HDMI PHY clock > drm/sun4i: Don't change clock bits in DW HDMI PHY driver > drm/sun4i: DW HDMI PHY: Add support for second PLL > drm/sun4i: Add support for second clock parent to DW HDMI PHY clk > driver > drm/sun4i: Add support for A64 HDMI PHY > drm: of: Export drm_crtc_port_mask() > drm/sun4i: DW HDMI: Expand algorithm for possible crtcs > ARM: dts: sun8i: r40: Add HDMI pipeline > ARM: dts: sun8i: r40: Enable HDMI output on BananaPi M2 Ultra Tested whole series on top of linux-next. Tested-by: Jagan Teki <jagan@amarulasolutions.com> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne četrtek, 14. junij 2018 ob 09:12:41 CEST je Jagan Teki napisal(a): > On Wed, Jun 13, 2018 at 1:30 AM, Jernej Skrabec <jernej.skrabec@siol.net> wrote: > > This series adds support for R40 HDMI pipeline. It is a bit special > > than other already supported pipelines because it has additional unit > > called TCON TOP responsible for relationship configuration between > > mixers, TCONs and HDMI. Additionally, it has additional gates for DSI > > and TV TCONs, TV encoder clock settings and pin muxing between LCD > > and TV encoders. > > > > However, it seems that TCON TOP will become a norm, since newer > > Allwinner SoCs like H6 also have this unit. > > > > I tested different possible configurations: > > - mixer0 <> TCON-TV0 <> HDMI > > - mixer0 <> TCON-TV1 <> HDMI > > - mixer1 <> TCON-TV0 <> HDMI > > - mixer1 <> TCON-TV1 <> HDMI > > > > Please review. > > > > Best regards, > > Jernej > > > > Changes from v1: > > - Split DT bindings patch and updated description > > - Split HDMI PHY patch > > - Move header file from TCON TOP patch to dt bindings patch > > - Added Rob reviewed-by tag > > - Used clk_hw_register_gate() instead of custom gate registration code > > - Reworked TCON TOP to be part of of-graph. Because of that, a lot of > > > > new patches were added. > > > > - Droped mixer index quirk patch > > - Reworked TCON support for TCON TOP > > - Updated commit messages > > > > Jernej Skrabec (27): > > clk: sunxi-ng: r40: Add minimal rate for video PLLs > > clk: sunxi-ng: r40: Allow setting parent rate to display related > > > > clocks > > > > clk: sunxi-ng: r40: Export video PLLs > > dt-bindings: display: sunxi-drm: Add TCON TOP description > > drm/sun4i: Add TCON TOP driver > > drm/sun4i: Fix releasing node when enumerating enpoints > > drm/sun4i: Split out code for enumerating endpoints in output port > > drm/sun4i: Add support for traversing graph with TCON TOP > > drm/sun4i: Don't skip TCONs if they don't have channel 0 > > dt-bindings: display: sun4i-drm: Add R40 TV TCON description > > drm/sun4i: tcon: Add support for tcon-top gate > > drm/sun4i: tcon: Generalize engine search algorithm > > drm/sun4i: Don't check for LVDS and RGB when TCON has only ch1 > > drm/sun4i: Don't check for panel or bridge on TV TCONs > > drm/sun4i: Add support for R40 TV TCON > > dt-bindings: display: sun4i-drm: Add R40 mixer compatibles > > drm/sun4i: Add support for R40 mixers > > dt-bindings: display: sun4i-drm: Add description of A64 HDMI PHY > > drm/sun4i: Enable DW HDMI PHY clock > > drm/sun4i: Don't change clock bits in DW HDMI PHY driver > > drm/sun4i: DW HDMI PHY: Add support for second PLL > > drm/sun4i: Add support for second clock parent to DW HDMI PHY clk > > > > driver > > > > drm/sun4i: Add support for A64 HDMI PHY > > drm: of: Export drm_crtc_port_mask() > > drm/sun4i: DW HDMI: Expand algorithm for possible crtcs > > ARM: dts: sun8i: r40: Add HDMI pipeline > > ARM: dts: sun8i: r40: Enable HDMI output on BananaPi M2 Ultra > > Tested whole series on top of linux-next. > > Tested-by: Jagan Teki <jagan@amarulasolutions.com> Thanks! -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 14, 2018 at 8:04 PM, Jernej Škrabec <jernej.skrabec@siol.net> wrote: > Dne četrtek, 14. junij 2018 ob 09:12:41 CEST je Jagan Teki napisal(a): >> On Wed, Jun 13, 2018 at 1:30 AM, Jernej Skrabec <jernej.skrabec@siol.net> > wrote: >> > This series adds support for R40 HDMI pipeline. It is a bit special >> > than other already supported pipelines because it has additional unit >> > called TCON TOP responsible for relationship configuration between >> > mixers, TCONs and HDMI. Additionally, it has additional gates for DSI >> > and TV TCONs, TV encoder clock settings and pin muxing between LCD >> > and TV encoders. >> > >> > However, it seems that TCON TOP will become a norm, since newer >> > Allwinner SoCs like H6 also have this unit. >> > >> > I tested different possible configurations: >> > - mixer0 <> TCON-TV0 <> HDMI >> > - mixer0 <> TCON-TV1 <> HDMI >> > - mixer1 <> TCON-TV0 <> HDMI >> > - mixer1 <> TCON-TV1 <> HDMI >> > >> > Please review. >> > >> > Best regards, >> > Jernej >> > >> > Changes from v1: >> > - Split DT bindings patch and updated description >> > - Split HDMI PHY patch >> > - Move header file from TCON TOP patch to dt bindings patch >> > - Added Rob reviewed-by tag >> > - Used clk_hw_register_gate() instead of custom gate registration code >> > - Reworked TCON TOP to be part of of-graph. Because of that, a lot of >> > >> > new patches were added. >> > >> > - Droped mixer index quirk patch >> > - Reworked TCON support for TCON TOP >> > - Updated commit messages >> > >> > Jernej Skrabec (27): >> > clk: sunxi-ng: r40: Add minimal rate for video PLLs >> > clk: sunxi-ng: r40: Allow setting parent rate to display related >> > >> > clocks >> > >> > clk: sunxi-ng: r40: Export video PLLs >> > dt-bindings: display: sunxi-drm: Add TCON TOP description >> > drm/sun4i: Add TCON TOP driver >> > drm/sun4i: Fix releasing node when enumerating enpoints >> > drm/sun4i: Split out code for enumerating endpoints in output port >> > drm/sun4i: Add support for traversing graph with TCON TOP >> > drm/sun4i: Don't skip TCONs if they don't have channel 0 >> > dt-bindings: display: sun4i-drm: Add R40 TV TCON description >> > drm/sun4i: tcon: Add support for tcon-top gate >> > drm/sun4i: tcon: Generalize engine search algorithm >> > drm/sun4i: Don't check for LVDS and RGB when TCON has only ch1 >> > drm/sun4i: Don't check for panel or bridge on TV TCONs >> > drm/sun4i: Add support for R40 TV TCON >> > dt-bindings: display: sun4i-drm: Add R40 mixer compatibles >> > drm/sun4i: Add support for R40 mixers >> > dt-bindings: display: sun4i-drm: Add description of A64 HDMI PHY >> > drm/sun4i: Enable DW HDMI PHY clock >> > drm/sun4i: Don't change clock bits in DW HDMI PHY driver >> > drm/sun4i: DW HDMI PHY: Add support for second PLL >> > drm/sun4i: Add support for second clock parent to DW HDMI PHY clk >> > >> > driver >> > >> > drm/sun4i: Add support for A64 HDMI PHY >> > drm: of: Export drm_crtc_port_mask() >> > drm/sun4i: DW HDMI: Expand algorithm for possible crtcs >> > ARM: dts: sun8i: r40: Add HDMI pipeline >> > ARM: dts: sun8i: r40: Enable HDMI output on BananaPi M2 Ultra >> >> Tested whole series on top of linux-next. >> >> Tested-by: Jagan Teki <jagan@amarulasolutions.com> > > Thanks! I've V40 board, which is same as R40. I'm able to detect the HDMI but seems edid not detecting properly. [ 0.983007] sun4i-drm display-engine: bound 1100000.mixer (ops 0xc074a80c) [ 0.999043] sun4i-drm display-engine: bound 1200000.mixer (ops 0xc074a80c) [ 1.006229] sun4i-drm display-engine: bound 1c70000.tcon-top (ops 0xc074e2ac) [ 1.013609] sun4i-drm display-engine: bound 1c73000.lcd-controller (ops 0xc0747a28) [ 1.053988] sun8i-dw-hdmi 1ee0000.hdmi: Detected HDMI TX controller v1.32a with HDCP (sun8i_dw_hdmi_phy) [ 1.063913] sun8i-dw-hdmi 1ee0000.hdmi: registered DesignWare HDMI I2C bus driver [ 1.071683] sun4i-drm display-engine: bound 1ee0000.hdmi (ops 0xc074a298) [ 1.078484] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [ 1.085098] [drm] No driver support for vblank timestamp query. [ 1.091055] [drm] Cannot find any crtc or sizes [ 1.095995] [drm] Initialized sun4i-drm 1.0.0 20150629 for display-engine on minor 0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne četrtek, 14. junij 2018 ob 19:16:46 CEST je Jagan Teki napisal(a): > On Thu, Jun 14, 2018 at 8:04 PM, Jernej Škrabec <jernej.skrabec@siol.net> wrote: > > Dne četrtek, 14. junij 2018 ob 09:12:41 CEST je Jagan Teki napisal(a): > >> On Wed, Jun 13, 2018 at 1:30 AM, Jernej Skrabec <jernej.skrabec@siol.net> > > > > wrote: > >> > This series adds support for R40 HDMI pipeline. It is a bit special > >> > than other already supported pipelines because it has additional unit > >> > called TCON TOP responsible for relationship configuration between > >> > mixers, TCONs and HDMI. Additionally, it has additional gates for DSI > >> > and TV TCONs, TV encoder clock settings and pin muxing between LCD > >> > and TV encoders. > >> > > >> > However, it seems that TCON TOP will become a norm, since newer > >> > Allwinner SoCs like H6 also have this unit. > >> > > >> > I tested different possible configurations: > >> > - mixer0 <> TCON-TV0 <> HDMI > >> > - mixer0 <> TCON-TV1 <> HDMI > >> > - mixer1 <> TCON-TV0 <> HDMI > >> > - mixer1 <> TCON-TV1 <> HDMI > >> > > >> > Please review. > >> > > >> > Best regards, > >> > Jernej > >> > > >> > Changes from v1: > >> > - Split DT bindings patch and updated description > >> > - Split HDMI PHY patch > >> > - Move header file from TCON TOP patch to dt bindings patch > >> > - Added Rob reviewed-by tag > >> > - Used clk_hw_register_gate() instead of custom gate registration code > >> > - Reworked TCON TOP to be part of of-graph. Because of that, a lot of > >> > > >> > new patches were added. > >> > > >> > - Droped mixer index quirk patch > >> > - Reworked TCON support for TCON TOP > >> > - Updated commit messages > >> > > >> > Jernej Skrabec (27): > >> > clk: sunxi-ng: r40: Add minimal rate for video PLLs > >> > clk: sunxi-ng: r40: Allow setting parent rate to display related > >> > > >> > clocks > >> > > >> > clk: sunxi-ng: r40: Export video PLLs > >> > dt-bindings: display: sunxi-drm: Add TCON TOP description > >> > drm/sun4i: Add TCON TOP driver > >> > drm/sun4i: Fix releasing node when enumerating enpoints > >> > drm/sun4i: Split out code for enumerating endpoints in output port > >> > drm/sun4i: Add support for traversing graph with TCON TOP > >> > drm/sun4i: Don't skip TCONs if they don't have channel 0 > >> > dt-bindings: display: sun4i-drm: Add R40 TV TCON description > >> > drm/sun4i: tcon: Add support for tcon-top gate > >> > drm/sun4i: tcon: Generalize engine search algorithm > >> > drm/sun4i: Don't check for LVDS and RGB when TCON has only ch1 > >> > drm/sun4i: Don't check for panel or bridge on TV TCONs > >> > drm/sun4i: Add support for R40 TV TCON > >> > dt-bindings: display: sun4i-drm: Add R40 mixer compatibles > >> > drm/sun4i: Add support for R40 mixers > >> > dt-bindings: display: sun4i-drm: Add description of A64 HDMI PHY > >> > drm/sun4i: Enable DW HDMI PHY clock > >> > drm/sun4i: Don't change clock bits in DW HDMI PHY driver > >> > drm/sun4i: DW HDMI PHY: Add support for second PLL > >> > drm/sun4i: Add support for second clock parent to DW HDMI PHY clk > >> > > >> > driver > >> > > >> > drm/sun4i: Add support for A64 HDMI PHY > >> > drm: of: Export drm_crtc_port_mask() > >> > drm/sun4i: DW HDMI: Expand algorithm for possible crtcs > >> > ARM: dts: sun8i: r40: Add HDMI pipeline > >> > ARM: dts: sun8i: r40: Enable HDMI output on BananaPi M2 Ultra > >> > >> Tested whole series on top of linux-next. > >> > >> Tested-by: Jagan Teki <jagan@amarulasolutions.com> > > > > Thanks! > > I've V40 board, which is same as R40. I'm able to detect the HDMI but > seems edid not detecting properly. > > [ 0.983007] sun4i-drm display-engine: bound 1100000.mixer (ops > 0xc074a80c) [ 0.999043] sun4i-drm display-engine: bound 1200000.mixer > (ops 0xc074a80c) [ 1.006229] sun4i-drm display-engine: bound > 1c70000.tcon-top (ops 0xc074e2ac) [ 1.013609] sun4i-drm display-engine: > bound 1c73000.lcd-controller (ops 0xc0747a28) > [ 1.053988] sun8i-dw-hdmi 1ee0000.hdmi: Detected HDMI TX controller > v1.32a with HDCP (sun8i_dw_hdmi_phy) > [ 1.063913] sun8i-dw-hdmi 1ee0000.hdmi: registered DesignWare HDMI > I2C bus driver > [ 1.071683] sun4i-drm display-engine: bound 1ee0000.hdmi (ops 0xc074a298) > [ 1.078484] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). > [ 1.085098] [drm] No driver support for vblank timestamp query. [ > 1.091055] [drm] Cannot find any crtc or sizes > [ 1.095995] [drm] Initialized sun4i-drm 1.0.0 20150629 for > display-engine on minor 0 This seems like DT issue. Can you post somewhere your V40 DTSI (if it is different to R40) and board DTS? Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec wrote: > TV TCONs connected to TCON TOP have to enable additional gate in order > to work. > > Add support for such TCONs. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > --- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++ > drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++++ > 2 files changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index 08747fc3ee71..0afb5a94a414 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device *dev, > dev_err(dev, "Couldn't get the TCON bus clock\n"); > return PTR_ERR(tcon->clk); > } > + > + if (tcon->quirks->has_tcon_top_gate) { > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); > + if (IS_ERR(tcon->top_clk)) { > + dev_err(dev, "Couldn't get the TCON TOP bus clock\n"); > + return PTR_ERR(tcon->top_clk); > + } > + clk_prepare_enable(tcon->top_clk); > + } > + Is it required for the TCON itself to operate, or does the TCON requires the TCON TOP, which in turn requires that clock to be functional? I find it quite odd to have a clock that isn't meant for a particular device to actually be wired to another device. I'm not saying this isn't the case, but it would be a first. Maxime
Hi, Dne petek, 15. junij 2018 ob 10:31:10 CEST je Maxime Ripard napisal(a): > Hi, > > On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec wrote: > > TV TCONs connected to TCON TOP have to enable additional gate in order > > to work. > > > > Add support for such TCONs. > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > --- > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++ > > drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++++ > > 2 files changed, 15 insertions(+) > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..0afb5a94a414 > > 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device *dev, > > > > dev_err(dev, "Couldn't get the TCON bus clock\n"); > > return PTR_ERR(tcon->clk); > > > > } > > > > + > > + if (tcon->quirks->has_tcon_top_gate) { > > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); > > + if (IS_ERR(tcon->top_clk)) { > > + dev_err(dev, "Couldn't get the TCON TOP bus clock\n"); > > + return PTR_ERR(tcon->top_clk); > > + } > > + clk_prepare_enable(tcon->top_clk); > > + } > > + > > Is it required for the TCON itself to operate, or does the TCON > requires the TCON TOP, which in turn requires that clock to be > functional? > > I find it quite odd to have a clock that isn't meant for a particular > device to actually be wired to another device. I'm not saying this > isn't the case, but it would be a first. Documentation doesn't say much about that gate. I did few tests and TCON registers can be read and written even if TCON TOP TV TCON gate is disabled. However, there is no image, as expected. More interestingly, I enabled test pattern directly in TCON to eliminate influence of the mixer. As soon as I disabled that gate, test pattern on HDMI screen was gone, which suggest that this gate influences something inside TCON. Another test I did was that I moved enable/disable gate code to sun4i_tcon_channel_set_status() and it worked just as well. I'll ask AW engineer what that gate actually does, but from what I saw, I would say that most appropriate location to enable/disable TCON TOP TV TCON gate is TCON driver. Alternatively, TCON TOP driver could check if any TV TCON is in use and enable appropriate gate. However, that doesn't sound right to me for some reason. Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne torek, 12. junij 2018 ob 22:00:29 CEST je Jernej Skrabec napisal(a): > DW HDMI PHY driver and PHY clock driver share same registers. Make sure > that DW HDMI PHY setup code doesn't change any clock related bits and > set them to 0 during initialization. > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > --- > drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 2 +- > drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 12 +++++++++++- > 2 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h index 79154f0f674a..3ba71aff92fc > 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > +++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h > @@ -98,7 +98,7 @@ > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO2_EN BIT(29) > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO1_EN BIT(28) > #define SUN8I_HDMI_PHY_PLL_CFG1_HV_IS_33 BIT(27) > -#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL BIT(26) > +#define SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK BIT(26) > #define SUN8I_HDMI_PHY_PLL_CFG1_PLLEN BIT(25) > #define SUN8I_HDMI_PHY_PLL_CFG1_LDO_VSET(x) ((x) << 22) > #define SUN8I_HDMI_PHY_PLL_CFG1_UNKNOWN(x) ((x) << 20) > diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c index 966688f04741..cd07ceb71601 > 100644 > --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c > @@ -183,7 +183,13 @@ static int sun8i_hdmi_phy_config_h3(struct dw_hdmi > *hdmi, regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_ANA_CFG1_REG, > SUN8I_HDMI_PHY_ANA_CFG1_TXEN_MASK, 0); > > - regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, pll_cfg1_init); > + /* > + * NOTE: We have to be careful not to overwrite PHY parent > + * clock selection bit and clock divider. > + */ > + regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, > + (u32)~SUN8I_HDMI_PHY_PLL_CFG1_CKIN_SEL_MSK, > + pll_cfg1_init); > regmap_update_bits(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG, > (u32)~SUN8I_HDMI_PHY_PLL_CFG2_PREDIV_MSK, > pll_cfg2_init); > @@ -352,6 +358,10 @@ static void sun8i_hdmi_phy_init_h3(struct > sun8i_hdmi_phy *phy) SUN8I_HDMI_PHY_ANA_CFG3_SCLEN | > SUN8I_HDMI_PHY_ANA_CFG3_SDAEN); > > + /* reset PLL clock configuration */ > + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG1_REG, 0); > + regmap_write(phy->regs, SUN8I_HDMI_PHY_PLL_CFG2_REG, 0); > + For some reason, this change breaks HDMI on H3. Clearing only PLL parent selection bit works ok, though. I'll fix it in next revision. Best regards, Jernej > /* set HW control of CEC pins */ > regmap_write(phy->regs, SUN8I_HDMI_PHY_CEC_REG, 0); -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec <jernej.skrabec@siol.net> wrote: > Hi, > > Dne petek, 15. junij 2018 ob 10:31:10 CEST je Maxime Ripard napisal(a): >> Hi, >> >> On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec wrote: >> > TV TCONs connected to TCON TOP have to enable additional gate in order >> > to work. >> > >> > Add support for such TCONs. >> > >> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> >> > --- >> > >> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++ >> > drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++++ >> > 2 files changed, 15 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..0afb5a94a414 >> > 100644 >> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device *dev, >> > >> > dev_err(dev, "Couldn't get the TCON bus clock\n"); >> > return PTR_ERR(tcon->clk); >> > >> > } >> > >> > + >> > + if (tcon->quirks->has_tcon_top_gate) { >> > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); >> > + if (IS_ERR(tcon->top_clk)) { >> > + dev_err(dev, "Couldn't get the TCON TOP bus clock\n"); >> > + return PTR_ERR(tcon->top_clk); >> > + } >> > + clk_prepare_enable(tcon->top_clk); >> > + } >> > + >> >> Is it required for the TCON itself to operate, or does the TCON >> requires the TCON TOP, which in turn requires that clock to be >> functional? >> >> I find it quite odd to have a clock that isn't meant for a particular >> device to actually be wired to another device. I'm not saying this >> isn't the case, but it would be a first. > > Documentation doesn't say much about that gate. I did few tests and TCON > registers can be read and written even if TCON TOP TV TCON gate is disabled. > However, there is no image, as expected. The R40 manual does include it in the diagram, on page 504. There's also a mux to select whether the clock comes directly from the CCU or the TV encoder (a feedback mode?). I assume this is the gate you are referring to here, in which case it is not a bus clock, but rather the TCON module or channel clock, strangely routed. > More interestingly, I enabled test pattern directly in TCON to eliminate > influence of the mixer. As soon as I disabled that gate, test pattern on HDMI > screen was gone, which suggest that this gate influences something inside > TCON. > > Another test I did was that I moved enable/disable gate code to > sun4i_tcon_channel_set_status() and it worked just as well. > > I'll ask AW engineer what that gate actually does, but from what I saw, I > would say that most appropriate location to enable/disable TCON TOP TV TCON > gate is TCON driver. Alternatively, TCON TOP driver could check if any TV TCON > is in use and enable appropriate gate. However, that doesn't sound right to me > for some reason. If what I said above it true, then yes, the appropriate location to enable it is the TCON driver, but moreover, the representation of the clock tree should be fixed such that the TCON takes the clock from the TCON TOP as its channel/ module clock instead. That way you don't need this patch, but you'd add another for all the clock routing. ChenYu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai napisal(a): > On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec > > <jernej.skrabec@siol.net> wrote: > > Hi, > > > > Dne petek, 15. junij 2018 ob 10:31:10 CEST je Maxime Ripard napisal(a): > >> Hi, > >> > >> On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec wrote: > >> > TV TCONs connected to TCON TOP have to enable additional gate in order > >> > to work. > >> > > >> > Add support for such TCONs. > >> > > >> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > >> > --- > >> > > >> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++ > >> > drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++++ > >> > 2 files changed, 15 insertions(+) > >> > > >> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..0afb5a94a414 > >> > 100644 > >> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device > >> > *dev, > >> > > >> > dev_err(dev, "Couldn't get the TCON bus clock\n"); > >> > return PTR_ERR(tcon->clk); > >> > > >> > } > >> > > >> > + > >> > + if (tcon->quirks->has_tcon_top_gate) { > >> > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); > >> > + if (IS_ERR(tcon->top_clk)) { > >> > + dev_err(dev, "Couldn't get the TCON TOP bus > >> > clock\n"); > >> > + return PTR_ERR(tcon->top_clk); > >> > + } > >> > + clk_prepare_enable(tcon->top_clk); > >> > + } > >> > + > >> > >> Is it required for the TCON itself to operate, or does the TCON > >> requires the TCON TOP, which in turn requires that clock to be > >> functional? > >> > >> I find it quite odd to have a clock that isn't meant for a particular > >> device to actually be wired to another device. I'm not saying this > >> isn't the case, but it would be a first. > > > > Documentation doesn't say much about that gate. I did few tests and TCON > > registers can be read and written even if TCON TOP TV TCON gate is > > disabled. However, there is no image, as expected. > > The R40 manual does include it in the diagram, on page 504. There's also a > mux to select whether the clock comes directly from the CCU or the TV > encoder (a feedback mode?). I assume this is the gate you are referring to > here, in which case it is not a bus clock, but rather the TCON module or > channel clock, strangely routed. > > > More interestingly, I enabled test pattern directly in TCON to eliminate > > influence of the mixer. As soon as I disabled that gate, test pattern on > > HDMI screen was gone, which suggest that this gate influences something > > inside TCON. > > > > Another test I did was that I moved enable/disable gate code to > > sun4i_tcon_channel_set_status() and it worked just as well. > > > > I'll ask AW engineer what that gate actually does, but from what I saw, I > > would say that most appropriate location to enable/disable TCON TOP TV > > TCON > > gate is TCON driver. Alternatively, TCON TOP driver could check if any TV > > TCON is in use and enable appropriate gate. However, that doesn't sound > > right to me for some reason. > > If what I said above it true, then yes, the appropriate location to enable > it is the TCON driver, but moreover, the representation of the clock tree > should be fixed such that the TCON takes the clock from the TCON TOP as its > channel/ module clock instead. That way you don't need this patch, but > you'd add another for all the clock routing. Can you be more specific? I not sure what you mean here. Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec <jernej.skrabec@siol.net> wrote: > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai napisal(a): >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec >> >> <jernej.skrabec@siol.net> wrote: >> > Hi, >> > >> > Dne petek, 15. junij 2018 ob 10:31:10 CEST je Maxime Ripard napisal(a): >> >> Hi, >> >> >> >> On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec wrote: >> >> > TV TCONs connected to TCON TOP have to enable additional gate in order >> >> > to work. >> >> > >> >> > Add support for such TCONs. >> >> > >> >> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> >> >> > --- >> >> > >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++ >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++++ >> >> > 2 files changed, 15 insertions(+) >> >> > >> >> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> >> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 08747fc3ee71..0afb5a94a414 >> >> > 100644 >> >> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> >> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> >> > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device >> >> > *dev, >> >> > >> >> > dev_err(dev, "Couldn't get the TCON bus clock\n"); >> >> > return PTR_ERR(tcon->clk); >> >> > >> >> > } >> >> > >> >> > + >> >> > + if (tcon->quirks->has_tcon_top_gate) { >> >> > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); >> >> > + if (IS_ERR(tcon->top_clk)) { >> >> > + dev_err(dev, "Couldn't get the TCON TOP bus >> >> > clock\n"); >> >> > + return PTR_ERR(tcon->top_clk); >> >> > + } >> >> > + clk_prepare_enable(tcon->top_clk); >> >> > + } >> >> > + >> >> >> >> Is it required for the TCON itself to operate, or does the TCON >> >> requires the TCON TOP, which in turn requires that clock to be >> >> functional? >> >> >> >> I find it quite odd to have a clock that isn't meant for a particular >> >> device to actually be wired to another device. I'm not saying this >> >> isn't the case, but it would be a first. >> > >> > Documentation doesn't say much about that gate. I did few tests and TCON >> > registers can be read and written even if TCON TOP TV TCON gate is >> > disabled. However, there is no image, as expected. >> >> The R40 manual does include it in the diagram, on page 504. There's also a >> mux to select whether the clock comes directly from the CCU or the TV >> encoder (a feedback mode?). I assume this is the gate you are referring to >> here, in which case it is not a bus clock, but rather the TCON module or >> channel clock, strangely routed. >> >> > More interestingly, I enabled test pattern directly in TCON to eliminate >> > influence of the mixer. As soon as I disabled that gate, test pattern on >> > HDMI screen was gone, which suggest that this gate influences something >> > inside TCON. >> > >> > Another test I did was that I moved enable/disable gate code to >> > sun4i_tcon_channel_set_status() and it worked just as well. >> > >> > I'll ask AW engineer what that gate actually does, but from what I saw, I >> > would say that most appropriate location to enable/disable TCON TOP TV >> > TCON >> > gate is TCON driver. Alternatively, TCON TOP driver could check if any TV >> > TCON is in use and enable appropriate gate. However, that doesn't sound >> > right to me for some reason. >> >> If what I said above it true, then yes, the appropriate location to enable >> it is the TCON driver, but moreover, the representation of the clock tree >> should be fixed such that the TCON takes the clock from the TCON TOP as its >> channel/ module clock instead. That way you don't need this patch, but >> you'd add another for all the clock routing. > > Can you be more specific? I not sure what you mean here. For clock related properties in the device tree: &tcon_top { clocks = <&ccu CLK_BUS_TCON_TOP>, <&ccu CLK_TCON_TV0>, <&tve0>, <&ccu CLK_TCON_TV1>, <&tve1>; clock-names = "bus", "tcon-tv0", "tve0", "tcon-tv1", "tve1"; clock-output-names = "tcon-top-tv0", "tcon-top-tv1"; }; &tcon_tv0 { clocks = <&ccu CLK_BUS_TCON_TV0>, <&tcon_top 0>' clock-names = "ahb", "tcon-ch1"; }; A diagram would look like: | This part is TCON TOP | v v CCU CLK_TCON_TV0 --|----\ | | mux ---- gate ----|-- TCON_TV0 TVE0 --------------|----/ | And the same goes for TCON_TV1 and TVE1. The user manual is a bit lacking on how TVE outputs a clock though. ChenYu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 14, 2018 at 10:59 PM, Jernej Škrabec <jernej.skrabec@siol.net> wrote: > Dne četrtek, 14. junij 2018 ob 19:16:46 CEST je Jagan Teki napisal(a): >> On Thu, Jun 14, 2018 at 8:04 PM, Jernej Škrabec <jernej.skrabec@siol.net> > wrote: >> > Dne četrtek, 14. junij 2018 ob 09:12:41 CEST je Jagan Teki napisal(a): >> >> On Wed, Jun 13, 2018 at 1:30 AM, Jernej Skrabec <jernej.skrabec@siol.net> >> > >> > wrote: >> >> > This series adds support for R40 HDMI pipeline. It is a bit special >> >> > than other already supported pipelines because it has additional unit >> >> > called TCON TOP responsible for relationship configuration between >> >> > mixers, TCONs and HDMI. Additionally, it has additional gates for DSI >> >> > and TV TCONs, TV encoder clock settings and pin muxing between LCD >> >> > and TV encoders. >> >> > >> >> > However, it seems that TCON TOP will become a norm, since newer >> >> > Allwinner SoCs like H6 also have this unit. >> >> > >> >> > I tested different possible configurations: >> >> > - mixer0 <> TCON-TV0 <> HDMI >> >> > - mixer0 <> TCON-TV1 <> HDMI >> >> > - mixer1 <> TCON-TV0 <> HDMI >> >> > - mixer1 <> TCON-TV1 <> HDMI >> >> > >> >> > Please review. >> >> > >> >> > Best regards, >> >> > Jernej >> >> > >> >> > Changes from v1: >> >> > - Split DT bindings patch and updated description >> >> > - Split HDMI PHY patch >> >> > - Move header file from TCON TOP patch to dt bindings patch >> >> > - Added Rob reviewed-by tag >> >> > - Used clk_hw_register_gate() instead of custom gate registration code >> >> > - Reworked TCON TOP to be part of of-graph. Because of that, a lot of >> >> > >> >> > new patches were added. >> >> > >> >> > - Droped mixer index quirk patch >> >> > - Reworked TCON support for TCON TOP >> >> > - Updated commit messages >> >> > >> >> > Jernej Skrabec (27): >> >> > clk: sunxi-ng: r40: Add minimal rate for video PLLs >> >> > clk: sunxi-ng: r40: Allow setting parent rate to display related >> >> > >> >> > clocks >> >> > >> >> > clk: sunxi-ng: r40: Export video PLLs >> >> > dt-bindings: display: sunxi-drm: Add TCON TOP description >> >> > drm/sun4i: Add TCON TOP driver >> >> > drm/sun4i: Fix releasing node when enumerating enpoints >> >> > drm/sun4i: Split out code for enumerating endpoints in output port >> >> > drm/sun4i: Add support for traversing graph with TCON TOP >> >> > drm/sun4i: Don't skip TCONs if they don't have channel 0 >> >> > dt-bindings: display: sun4i-drm: Add R40 TV TCON description >> >> > drm/sun4i: tcon: Add support for tcon-top gate >> >> > drm/sun4i: tcon: Generalize engine search algorithm >> >> > drm/sun4i: Don't check for LVDS and RGB when TCON has only ch1 >> >> > drm/sun4i: Don't check for panel or bridge on TV TCONs >> >> > drm/sun4i: Add support for R40 TV TCON >> >> > dt-bindings: display: sun4i-drm: Add R40 mixer compatibles >> >> > drm/sun4i: Add support for R40 mixers >> >> > dt-bindings: display: sun4i-drm: Add description of A64 HDMI PHY >> >> > drm/sun4i: Enable DW HDMI PHY clock >> >> > drm/sun4i: Don't change clock bits in DW HDMI PHY driver >> >> > drm/sun4i: DW HDMI PHY: Add support for second PLL >> >> > drm/sun4i: Add support for second clock parent to DW HDMI PHY clk >> >> > >> >> > driver >> >> > >> >> > drm/sun4i: Add support for A64 HDMI PHY >> >> > drm: of: Export drm_crtc_port_mask() >> >> > drm/sun4i: DW HDMI: Expand algorithm for possible crtcs >> >> > ARM: dts: sun8i: r40: Add HDMI pipeline >> >> > ARM: dts: sun8i: r40: Enable HDMI output on BananaPi M2 Ultra >> >> >> >> Tested whole series on top of linux-next. >> >> >> >> Tested-by: Jagan Teki <jagan@amarulasolutions.com> >> > >> > Thanks! >> >> I've V40 board, which is same as R40. I'm able to detect the HDMI but >> seems edid not detecting properly. >> >> [ 0.983007] sun4i-drm display-engine: bound 1100000.mixer (ops >> 0xc074a80c) [ 0.999043] sun4i-drm display-engine: bound 1200000.mixer >> (ops 0xc074a80c) [ 1.006229] sun4i-drm display-engine: bound >> 1c70000.tcon-top (ops 0xc074e2ac) [ 1.013609] sun4i-drm display-engine: >> bound 1c73000.lcd-controller (ops 0xc0747a28) >> [ 1.053988] sun8i-dw-hdmi 1ee0000.hdmi: Detected HDMI TX controller >> v1.32a with HDCP (sun8i_dw_hdmi_phy) >> [ 1.063913] sun8i-dw-hdmi 1ee0000.hdmi: registered DesignWare HDMI >> I2C bus driver >> [ 1.071683] sun4i-drm display-engine: bound 1ee0000.hdmi (ops 0xc074a298) >> [ 1.078484] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). >> [ 1.085098] [drm] No driver support for vblank timestamp query. [ >> 1.091055] [drm] Cannot find any crtc or sizes >> [ 1.095995] [drm] Initialized sun4i-drm 1.0.0 20150629 for >> display-engine on minor 0 > > This seems like DT issue. Can you post somewhere your V40 DTSI (if it is > different to R40) and board DTS? same dtsi shared between r40 and v40, here is board dts support for HDMI[1] [1] https://paste.ubuntu.com/p/wqVz38BHrM/ -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne ponedeljek, 18. junij 2018 ob 14:58:02 CEST je Jagan Teki napisal(a): > On Thu, Jun 14, 2018 at 10:59 PM, Jernej Škrabec > > <jernej.skrabec@siol.net> wrote: > > Dne četrtek, 14. junij 2018 ob 19:16:46 CEST je Jagan Teki napisal(a): > >> On Thu, Jun 14, 2018 at 8:04 PM, Jernej Škrabec <jernej.skrabec@siol.net> > > > > wrote: > >> > Dne četrtek, 14. junij 2018 ob 09:12:41 CEST je Jagan Teki napisal(a): > >> >> On Wed, Jun 13, 2018 at 1:30 AM, Jernej Skrabec > >> >> <jernej.skrabec@siol.net> > >> > > >> > wrote: > >> >> > This series adds support for R40 HDMI pipeline. It is a bit special > >> >> > than other already supported pipelines because it has additional > >> >> > unit > >> >> > called TCON TOP responsible for relationship configuration between > >> >> > mixers, TCONs and HDMI. Additionally, it has additional gates for > >> >> > DSI > >> >> > and TV TCONs, TV encoder clock settings and pin muxing between LCD > >> >> > and TV encoders. > >> >> > > >> >> > However, it seems that TCON TOP will become a norm, since newer > >> >> > Allwinner SoCs like H6 also have this unit. > >> >> > > >> >> > I tested different possible configurations: > >> >> > - mixer0 <> TCON-TV0 <> HDMI > >> >> > - mixer0 <> TCON-TV1 <> HDMI > >> >> > - mixer1 <> TCON-TV0 <> HDMI > >> >> > - mixer1 <> TCON-TV1 <> HDMI > >> >> > > >> >> > Please review. > >> >> > > >> >> > Best regards, > >> >> > Jernej > >> >> > > >> >> > Changes from v1: > >> >> > - Split DT bindings patch and updated description > >> >> > - Split HDMI PHY patch > >> >> > - Move header file from TCON TOP patch to dt bindings patch > >> >> > - Added Rob reviewed-by tag > >> >> > - Used clk_hw_register_gate() instead of custom gate registration > >> >> > code > >> >> > - Reworked TCON TOP to be part of of-graph. Because of that, a lot > >> >> > of > >> >> > > >> >> > new patches were added. > >> >> > > >> >> > - Droped mixer index quirk patch > >> >> > - Reworked TCON support for TCON TOP > >> >> > - Updated commit messages > >> >> > > >> >> > Jernej Skrabec (27): > >> >> > clk: sunxi-ng: r40: Add minimal rate for video PLLs > >> >> > clk: sunxi-ng: r40: Allow setting parent rate to display related > >> >> > > >> >> > clocks > >> >> > > >> >> > clk: sunxi-ng: r40: Export video PLLs > >> >> > dt-bindings: display: sunxi-drm: Add TCON TOP description > >> >> > drm/sun4i: Add TCON TOP driver > >> >> > drm/sun4i: Fix releasing node when enumerating enpoints > >> >> > drm/sun4i: Split out code for enumerating endpoints in output port > >> >> > drm/sun4i: Add support for traversing graph with TCON TOP > >> >> > drm/sun4i: Don't skip TCONs if they don't have channel 0 > >> >> > dt-bindings: display: sun4i-drm: Add R40 TV TCON description > >> >> > drm/sun4i: tcon: Add support for tcon-top gate > >> >> > drm/sun4i: tcon: Generalize engine search algorithm > >> >> > drm/sun4i: Don't check for LVDS and RGB when TCON has only ch1 > >> >> > drm/sun4i: Don't check for panel or bridge on TV TCONs > >> >> > drm/sun4i: Add support for R40 TV TCON > >> >> > dt-bindings: display: sun4i-drm: Add R40 mixer compatibles > >> >> > drm/sun4i: Add support for R40 mixers > >> >> > dt-bindings: display: sun4i-drm: Add description of A64 HDMI PHY > >> >> > drm/sun4i: Enable DW HDMI PHY clock > >> >> > drm/sun4i: Don't change clock bits in DW HDMI PHY driver > >> >> > drm/sun4i: DW HDMI PHY: Add support for second PLL > >> >> > drm/sun4i: Add support for second clock parent to DW HDMI PHY clk > >> >> > > >> >> > driver > >> >> > > >> >> > drm/sun4i: Add support for A64 HDMI PHY > >> >> > drm: of: Export drm_crtc_port_mask() > >> >> > drm/sun4i: DW HDMI: Expand algorithm for possible crtcs > >> >> > ARM: dts: sun8i: r40: Add HDMI pipeline > >> >> > ARM: dts: sun8i: r40: Enable HDMI output on BananaPi M2 Ultra > >> >> > >> >> Tested whole series on top of linux-next. > >> >> > >> >> Tested-by: Jagan Teki <jagan@amarulasolutions.com> > >> > > >> > Thanks! > >> > >> I've V40 board, which is same as R40. I'm able to detect the HDMI but > >> seems edid not detecting properly. > >> > >> [ 0.983007] sun4i-drm display-engine: bound 1100000.mixer (ops > >> 0xc074a80c) [ 0.999043] sun4i-drm display-engine: bound 1200000.mixer > >> (ops 0xc074a80c) [ 1.006229] sun4i-drm display-engine: bound > >> 1c70000.tcon-top (ops 0xc074e2ac) [ 1.013609] sun4i-drm > >> display-engine: > >> bound 1c73000.lcd-controller (ops 0xc0747a28) > >> [ 1.053988] sun8i-dw-hdmi 1ee0000.hdmi: Detected HDMI TX controller > >> v1.32a with HDCP (sun8i_dw_hdmi_phy) > >> [ 1.063913] sun8i-dw-hdmi 1ee0000.hdmi: registered DesignWare HDMI > >> I2C bus driver > >> [ 1.071683] sun4i-drm display-engine: bound 1ee0000.hdmi (ops > >> 0xc074a298) [ 1.078484] [drm] Supports vblank timestamp caching Rev 2 > >> (21.10.2013). [ 1.085098] [drm] No driver support for vblank > >> timestamp query. [ 1.091055] [drm] Cannot find any crtc or sizes > >> [ 1.095995] [drm] Initialized sun4i-drm 1.0.0 20150629 for > >> display-engine on minor 0 > > > > This seems like DT issue. Can you post somewhere your V40 DTSI (if it is > > different to R40) and board DTS? > > same dtsi shared between r40 and v40, here is board dts support for HDMI[1] > > [1] https://paste.ubuntu.com/p/wqVz38BHrM/ This patch looks like exactly the same as mine for BananaPi M2U, so there should be no issues. What about VCC-HDMI? Is powered? Can you measure it to check? Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
在 2018-06-18一的 16:43 +0200，Jernej Škrabec写道： > Dne ponedeljek, 18. junij 2018 ob 14:58:02 CEST je Jagan Teki > napisal(a): > > On Thu, Jun 14, 2018 at 10:59 PM, Jernej Škrabec > > > > <jernej.skrabec@siol.net> wrote: > > > Dne četrtek, 14. junij 2018 ob 19:16:46 CEST je Jagan Teki > > > napisal(a): > > > > On Thu, Jun 14, 2018 at 8:04 PM, Jernej Škrabec <jernej.skrabec > > > > @siol.net> > > > > > > wrote: > > > > > Dne četrtek, 14. junij 2018 ob 09:12:41 CEST je Jagan Teki > > > > > napisal(a): > > > > > > On Wed, Jun 13, 2018 at 1:30 AM, Jernej Skrabec > > > > > > <jernej.skrabec@siol.net> > > > > > > > > > > wrote: > > > > > > > This series adds support for R40 HDMI pipeline. It is a > > > > > > > bit special > > > > > > > than other already supported pipelines because it has > > > > > > > additional > > > > > > > unit > > > > > > > called TCON TOP responsible for relationship > > > > > > > configuration between > > > > > > > mixers, TCONs and HDMI. Additionally, it has additional > > > > > > > gates for > > > > > > > DSI > > > > > > > and TV TCONs, TV encoder clock settings and pin muxing > > > > > > > between LCD > > > > > > > and TV encoders. > > > > > > > > > > > > > > However, it seems that TCON TOP will become a norm, since > > > > > > > newer > > > > > > > Allwinner SoCs like H6 also have this unit. > > > > > > > > > > > > > > I tested different possible configurations: > > > > > > > - mixer0 <> TCON-TV0 <> HDMI > > > > > > > - mixer0 <> TCON-TV1 <> HDMI > > > > > > > - mixer1 <> TCON-TV0 <> HDMI > > > > > > > - mixer1 <> TCON-TV1 <> HDMI > > > > > > > > > > > > > > Please review. > > > > > > > > > > > > > > Best regards, > > > > > > > Jernej > > > > > > > > > > > > > > Changes from v1: > > > > > > > - Split DT bindings patch and updated description > > > > > > > - Split HDMI PHY patch > > > > > > > - Move header file from TCON TOP patch to dt bindings > > > > > > > patch > > > > > > > - Added Rob reviewed-by tag > > > > > > > - Used clk_hw_register_gate() instead of custom gate > > > > > > > registration > > > > > > > code > > > > > > > - Reworked TCON TOP to be part of of-graph. Because of > > > > > > > that, a lot > > > > > > > of > > > > > > > > > > > > > > new patches were added. > > > > > > > > > > > > > > - Droped mixer index quirk patch > > > > > > > - Reworked TCON support for TCON TOP > > > > > > > - Updated commit messages > > > > > > > > > > > > > > Jernej Skrabec (27): > > > > > > > clk: sunxi-ng: r40: Add minimal rate for video PLLs > > > > > > > clk: sunxi-ng: r40: Allow setting parent rate to > > > > > > > display related > > > > > > > > > > > > > > clocks > > > > > > > > > > > > > > clk: sunxi-ng: r40: Export video PLLs > > > > > > > dt-bindings: display: sunxi-drm: Add TCON TOP > > > > > > > description > > > > > > > drm/sun4i: Add TCON TOP driver > > > > > > > drm/sun4i: Fix releasing node when enumerating enpoints > > > > > > > drm/sun4i: Split out code for enumerating endpoints in > > > > > > > output port > > > > > > > drm/sun4i: Add support for traversing graph with TCON > > > > > > > TOP > > > > > > > drm/sun4i: Don't skip TCONs if they don't have channel > > > > > > > 0 > > > > > > > dt-bindings: display: sun4i-drm: Add R40 TV TCON > > > > > > > description > > > > > > > drm/sun4i: tcon: Add support for tcon-top gate > > > > > > > drm/sun4i: tcon: Generalize engine search algorithm > > > > > > > drm/sun4i: Don't check for LVDS and RGB when TCON has > > > > > > > only ch1 > > > > > > > drm/sun4i: Don't check for panel or bridge on TV TCONs > > > > > > > drm/sun4i: Add support for R40 TV TCON > > > > > > > dt-bindings: display: sun4i-drm: Add R40 mixer > > > > > > > compatibles > > > > > > > drm/sun4i: Add support for R40 mixers > > > > > > > dt-bindings: display: sun4i-drm: Add description of A64 > > > > > > > HDMI PHY > > > > > > > drm/sun4i: Enable DW HDMI PHY clock > > > > > > > drm/sun4i: Don't change clock bits in DW HDMI PHY > > > > > > > driver > > > > > > > drm/sun4i: DW HDMI PHY: Add support for second PLL > > > > > > > drm/sun4i: Add support for second clock parent to DW > > > > > > > HDMI PHY clk > > > > > > > > > > > > > > driver > > > > > > > > > > > > > > drm/sun4i: Add support for A64 HDMI PHY > > > > > > > drm: of: Export drm_crtc_port_mask() > > > > > > > drm/sun4i: DW HDMI: Expand algorithm for possible crtcs > > > > > > > ARM: dts: sun8i: r40: Add HDMI pipeline > > > > > > > ARM: dts: sun8i: r40: Enable HDMI output on BananaPi M2 > > > > > > > Ultra > > > > > > > > > > > > Tested whole series on top of linux-next. > > > > > > > > > > > > Tested-by: Jagan Teki <jagan@amarulasolutions.com> > > > > > > > > > > Thanks! > > > > > > > > I've V40 board, which is same as R40. I'm able to detect the > > > > HDMI but > > > > seems edid not detecting properly. > > > > > > > > [ 0.983007] sun4i-drm display-engine: bound 1100000.mixer > > > > (ops > > > > 0xc074a80c) [ 0.999043] sun4i-drm display-engine: bound > > > > 1200000.mixer > > > > (ops 0xc074a80c) [ 1.006229] sun4i-drm display-engine: bound > > > > 1c70000.tcon-top (ops 0xc074e2ac) [ 1.013609] sun4i-drm > > > > display-engine: > > > > bound 1c73000.lcd-controller (ops 0xc0747a28) > > > > [ 1.053988] sun8i-dw-hdmi 1ee0000.hdmi: Detected HDMI TX > > > > controller > > > > v1.32a with HDCP (sun8i_dw_hdmi_phy) > > > > [ 1.063913] sun8i-dw-hdmi 1ee0000.hdmi: registered > > > > DesignWare HDMI > > > > I2C bus driver > > > > [ 1.071683] sun4i-drm display-engine: bound 1ee0000.hdmi > > > > (ops > > > > 0xc074a298) [ 1.078484] [drm] Supports vblank timestamp > > > > caching Rev 2 > > > > (21.10.2013). [ 1.085098] [drm] No driver support for vblank > > > > timestamp query. [ 1.091055] [drm] Cannot find any crtc or > > > > sizes > > > > [ 1.095995] [drm] Initialized sun4i-drm 1.0.0 20150629 for > > > > display-engine on minor 0 > > > > > > This seems like DT issue. Can you post somewhere your V40 DTSI > > > (if it is > > > different to R40) and board DTS? > > > > same dtsi shared between r40 and v40, here is board dts support for > > HDMI[1] > > > > [1] https://paste.ubuntu.com/p/wqVz38BHrM/ > > This patch looks like exactly the same as mine for BananaPi M2U, so > there > should be no issues. As I know, M2B is designed to be compatible with M2U, so most things should be the same. The stock firmware even use the same images for both M2U and M2B. > > What about VCC-HDMI? Is powered? Can you measure it to check? > > Best regards, > Jernej > > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai napisal(a): > On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec <jernej.skrabec@siol.net> wrote: > > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai napisal(a): > >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec > >> > >> <jernej.skrabec@siol.net> wrote: > >> > Hi, > >> > > >> > Dne petek, 15. junij 2018 ob 10:31:10 CEST je Maxime Ripard napisal(a): > >> >> Hi, > >> >> > >> >> On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec wrote: > >> >> > TV TCONs connected to TCON TOP have to enable additional gate in > >> >> > order > >> >> > to work. > >> >> > > >> >> > Add support for such TCONs. > >> >> > > >> >> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > >> >> > --- > >> >> > > >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++ > >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++++ > >> >> > 2 files changed, 15 insertions(+) > >> >> > > >> >> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> >> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index > >> >> > 08747fc3ee71..0afb5a94a414 > >> >> > 100644 > >> >> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> >> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> >> > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device > >> >> > *dev, > >> >> > > >> >> > dev_err(dev, "Couldn't get the TCON bus clock\n"); > >> >> > return PTR_ERR(tcon->clk); > >> >> > > >> >> > } > >> >> > > >> >> > + > >> >> > + if (tcon->quirks->has_tcon_top_gate) { > >> >> > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); > >> >> > + if (IS_ERR(tcon->top_clk)) { > >> >> > + dev_err(dev, "Couldn't get the TCON TOP bus > >> >> > clock\n"); > >> >> > + return PTR_ERR(tcon->top_clk); > >> >> > + } > >> >> > + clk_prepare_enable(tcon->top_clk); > >> >> > + } > >> >> > + > >> >> > >> >> Is it required for the TCON itself to operate, or does the TCON > >> >> requires the TCON TOP, which in turn requires that clock to be > >> >> functional? > >> >> > >> >> I find it quite odd to have a clock that isn't meant for a particular > >> >> device to actually be wired to another device. I'm not saying this > >> >> isn't the case, but it would be a first. > >> > > >> > Documentation doesn't say much about that gate. I did few tests and > >> > TCON > >> > registers can be read and written even if TCON TOP TV TCON gate is > >> > disabled. However, there is no image, as expected. > >> > >> The R40 manual does include it in the diagram, on page 504. There's also > >> a > >> mux to select whether the clock comes directly from the CCU or the TV > >> encoder (a feedback mode?). I assume this is the gate you are referring > >> to > >> here, in which case it is not a bus clock, but rather the TCON module or > >> channel clock, strangely routed. > >> > >> > More interestingly, I enabled test pattern directly in TCON to > >> > eliminate > >> > influence of the mixer. As soon as I disabled that gate, test pattern > >> > on > >> > HDMI screen was gone, which suggest that this gate influences something > >> > inside TCON. > >> > > >> > Another test I did was that I moved enable/disable gate code to > >> > sun4i_tcon_channel_set_status() and it worked just as well. > >> > > >> > I'll ask AW engineer what that gate actually does, but from what I saw, > >> > I > >> > would say that most appropriate location to enable/disable TCON TOP TV > >> > TCON > >> > gate is TCON driver. Alternatively, TCON TOP driver could check if any > >> > TV > >> > TCON is in use and enable appropriate gate. However, that doesn't sound > >> > right to me for some reason. > >> > >> If what I said above it true, then yes, the appropriate location to > >> enable > >> it is the TCON driver, but moreover, the representation of the clock tree > >> should be fixed such that the TCON takes the clock from the TCON TOP as > >> its > >> channel/ module clock instead. That way you don't need this patch, but > >> you'd add another for all the clock routing. > > > > Can you be more specific? I not sure what you mean here. > > For clock related properties in the device tree: > > &tcon_top { > clocks = <&ccu CLK_BUS_TCON_TOP>, > <&ccu CLK_TCON_TV0>, > <&tve0>, > <&ccu CLK_TCON_TV1>, > <&tve1>; > clock-names = "bus", "tcon-tv0", "tve0", "tcon-tv1", "tve1"; > clock-output-names = "tcon-top-tv0", "tcon-top-tv1"; > }; > > &tcon_tv0 { > clocks = <&ccu CLK_BUS_TCON_TV0>, <&tcon_top 0>' > clock-names = "ahb", "tcon-ch1"; > }; > > A diagram would look like: > | This part is TCON TOP | > > v v > CCU CLK_TCON_TV0 --|----\ | > > | mux ---- gate ----|-- TCON_TV0 > > TVE0 --------------|----/ | > > And the same goes for TCON_TV1 and TVE1. > > The user manual is a bit lacking on how TVE outputs a clock though. I didn't yet received any response on HW details from AW till now, but I would like to post new version of patches soon. While chaining like you described could be implemented easily, I don't think it really represents HW as it is. Tests showed that these two clocks are independent, otherwise register writes/reads wouldn't be possible with tcon- top gate disabled. I chose tcon-top bus clock as a parent becase if it is not enabled, it simply won't work. However, if everyone feels chaining is the best way to implement it, I'll do it. Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec <jernej.skrabec@siol.net> wrote: > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai napisal(a): >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec <jernej.skrabec@siol.net> > wrote: >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai napisal(a): >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec >> >> >> >> <jernej.skrabec@siol.net> wrote: >> >> > Hi, >> >> > >> >> > Dne petek, 15. junij 2018 ob 10:31:10 CEST je Maxime Ripard napisal(a): >> >> >> Hi, >> >> >> >> >> >> On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec wrote: >> >> >> > TV TCONs connected to TCON TOP have to enable additional gate in >> >> >> > order >> >> >> > to work. >> >> >> > >> >> >> > Add support for such TCONs. >> >> >> > >> >> >> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> >> >> >> > --- >> >> >> > >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++ >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++++ >> >> >> > 2 files changed, 15 insertions(+) >> >> >> > >> >> >> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> >> >> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index >> >> >> > 08747fc3ee71..0afb5a94a414 >> >> >> > 100644 >> >> >> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> >> >> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> >> >> > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct device >> >> >> > *dev, >> >> >> > >> >> >> > dev_err(dev, "Couldn't get the TCON bus clock\n"); >> >> >> > return PTR_ERR(tcon->clk); >> >> >> > >> >> >> > } >> >> >> > >> >> >> > + >> >> >> > + if (tcon->quirks->has_tcon_top_gate) { >> >> >> > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); >> >> >> > + if (IS_ERR(tcon->top_clk)) { >> >> >> > + dev_err(dev, "Couldn't get the TCON TOP bus >> >> >> > clock\n"); >> >> >> > + return PTR_ERR(tcon->top_clk); >> >> >> > + } >> >> >> > + clk_prepare_enable(tcon->top_clk); >> >> >> > + } >> >> >> > + >> >> >> >> >> >> Is it required for the TCON itself to operate, or does the TCON >> >> >> requires the TCON TOP, which in turn requires that clock to be >> >> >> functional? >> >> >> >> >> >> I find it quite odd to have a clock that isn't meant for a particular >> >> >> device to actually be wired to another device. I'm not saying this >> >> >> isn't the case, but it would be a first. >> >> > >> >> > Documentation doesn't say much about that gate. I did few tests and >> >> > TCON >> >> > registers can be read and written even if TCON TOP TV TCON gate is >> >> > disabled. However, there is no image, as expected. >> >> >> >> The R40 manual does include it in the diagram, on page 504. There's also >> >> a >> >> mux to select whether the clock comes directly from the CCU or the TV >> >> encoder (a feedback mode?). I assume this is the gate you are referring >> >> to >> >> here, in which case it is not a bus clock, but rather the TCON module or >> >> channel clock, strangely routed. >> >> >> >> > More interestingly, I enabled test pattern directly in TCON to >> >> > eliminate >> >> > influence of the mixer. As soon as I disabled that gate, test pattern >> >> > on >> >> > HDMI screen was gone, which suggest that this gate influences something >> >> > inside TCON. >> >> > >> >> > Another test I did was that I moved enable/disable gate code to >> >> > sun4i_tcon_channel_set_status() and it worked just as well. >> >> > >> >> > I'll ask AW engineer what that gate actually does, but from what I saw, >> >> > I >> >> > would say that most appropriate location to enable/disable TCON TOP TV >> >> > TCON >> >> > gate is TCON driver. Alternatively, TCON TOP driver could check if any >> >> > TV >> >> > TCON is in use and enable appropriate gate. However, that doesn't sound >> >> > right to me for some reason. >> >> >> >> If what I said above it true, then yes, the appropriate location to >> >> enable >> >> it is the TCON driver, but moreover, the representation of the clock tree >> >> should be fixed such that the TCON takes the clock from the TCON TOP as >> >> its >> >> channel/ module clock instead. That way you don't need this patch, but >> >> you'd add another for all the clock routing. >> > >> > Can you be more specific? I not sure what you mean here. >> >> For clock related properties in the device tree: >> >> &tcon_top { >> clocks = <&ccu CLK_BUS_TCON_TOP>, >> <&ccu CLK_TCON_TV0>, >> <&tve0>, >> <&ccu CLK_TCON_TV1>, >> <&tve1>; >> clock-names = "bus", "tcon-tv0", "tve0", "tcon-tv1", "tve1"; >> clock-output-names = "tcon-top-tv0", "tcon-top-tv1"; >> }; >> >> &tcon_tv0 { >> clocks = <&ccu CLK_BUS_TCON_TV0>, <&tcon_top 0>' >> clock-names = "ahb", "tcon-ch1"; >> }; >> >> A diagram would look like: >> | This part is TCON TOP | >> >> v v >> CCU CLK_TCON_TV0 --|----\ | >> >> | mux ---- gate ----|-- TCON_TV0 >> >> TVE0 --------------|----/ | >> >> And the same goes for TCON_TV1 and TVE1. >> >> The user manual is a bit lacking on how TVE outputs a clock though. > > I didn't yet received any response on HW details from AW till now, but I would > like to post new version of patches soon. > > While chaining like you described could be implemented easily, I don't think > it really represents HW as it is. Tests showed that these two clocks are > independent, otherwise register writes/reads wouldn't be possible with tcon- > top gate disabled. I chose tcon-top bus clock as a parent becase if it is not > enabled, it simply won't work. AFAIK with the TCONs, even when the TCON channel clock (not the bus clock) is disabled, register accesses still work. I'm saying that the TCON TOP gate is downstream from the TCON channel clock in the CCU. These are not related to the TCON bus clock in the CCU, which affects register access. Did Allwinner provide any information regarding the hierarchy of the clocks? > However, if everyone feels chaining is the best way to implement it, I'll do > it. I would like to get it right and match actual hardware. My proposal is based on my understanding from the diagrams in the user manual. Regards ChenYu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne četrtek, 21. junij 2018 ob 03:23:27 CEST je Chen-Yu Tsai napisal(a): > On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec <jernej.skrabec@siol.net> wrote: > > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai napisal(a): > >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec <jernej.skrabec@siol.net> > > > > wrote: > >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai napisal(a): > >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec > >> >> > >> >> <jernej.skrabec@siol.net> wrote: > >> >> > Hi, > >> >> > > >> >> > Dne petek, 15. junij 2018 ob 10:31:10 CEST je Maxime Ripard napisal(a): > >> >> >> Hi, > >> >> >> > >> >> >> On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec wrote: > >> >> >> > TV TCONs connected to TCON TOP have to enable additional gate in > >> >> >> > order > >> >> >> > to work. > >> >> >> > > >> >> >> > Add support for such TCONs. > >> >> >> > > >> >> >> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > >> >> >> > --- > >> >> >> > > >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++ > >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++++ > >> >> >> > 2 files changed, 15 insertions(+) > >> >> >> > > >> >> >> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> >> >> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index > >> >> >> > 08747fc3ee71..0afb5a94a414 > >> >> >> > 100644 > >> >> >> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> >> >> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> >> >> > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct > >> >> >> > device > >> >> >> > *dev, > >> >> >> > > >> >> >> > dev_err(dev, "Couldn't get the TCON bus clock\n"); > >> >> >> > return PTR_ERR(tcon->clk); > >> >> >> > > >> >> >> > } > >> >> >> > > >> >> >> > + > >> >> >> > + if (tcon->quirks->has_tcon_top_gate) { > >> >> >> > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); > >> >> >> > + if (IS_ERR(tcon->top_clk)) { > >> >> >> > + dev_err(dev, "Couldn't get the TCON TOP bus > >> >> >> > clock\n"); > >> >> >> > + return PTR_ERR(tcon->top_clk); > >> >> >> > + } > >> >> >> > + clk_prepare_enable(tcon->top_clk); > >> >> >> > + } > >> >> >> > + > >> >> >> > >> >> >> Is it required for the TCON itself to operate, or does the TCON > >> >> >> requires the TCON TOP, which in turn requires that clock to be > >> >> >> functional? > >> >> >> > >> >> >> I find it quite odd to have a clock that isn't meant for a > >> >> >> particular > >> >> >> device to actually be wired to another device. I'm not saying this > >> >> >> isn't the case, but it would be a first. > >> >> > > >> >> > Documentation doesn't say much about that gate. I did few tests and > >> >> > TCON > >> >> > registers can be read and written even if TCON TOP TV TCON gate is > >> >> > disabled. However, there is no image, as expected. > >> >> > >> >> The R40 manual does include it in the diagram, on page 504. There's > >> >> also > >> >> a > >> >> mux to select whether the clock comes directly from the CCU or the TV > >> >> encoder (a feedback mode?). I assume this is the gate you are > >> >> referring > >> >> to > >> >> here, in which case it is not a bus clock, but rather the TCON module > >> >> or > >> >> channel clock, strangely routed. > >> >> > >> >> > More interestingly, I enabled test pattern directly in TCON to > >> >> > eliminate > >> >> > influence of the mixer. As soon as I disabled that gate, test > >> >> > pattern > >> >> > on > >> >> > HDMI screen was gone, which suggest that this gate influences > >> >> > something > >> >> > inside TCON. > >> >> > > >> >> > Another test I did was that I moved enable/disable gate code to > >> >> > sun4i_tcon_channel_set_status() and it worked just as well. > >> >> > > >> >> > I'll ask AW engineer what that gate actually does, but from what I > >> >> > saw, > >> >> > I > >> >> > would say that most appropriate location to enable/disable TCON TOP > >> >> > TV > >> >> > TCON > >> >> > gate is TCON driver. Alternatively, TCON TOP driver could check if > >> >> > any > >> >> > TV > >> >> > TCON is in use and enable appropriate gate. However, that doesn't > >> >> > sound > >> >> > right to me for some reason. > >> >> > >> >> If what I said above it true, then yes, the appropriate location to > >> >> enable > >> >> it is the TCON driver, but moreover, the representation of the clock > >> >> tree > >> >> should be fixed such that the TCON takes the clock from the TCON TOP > >> >> as > >> >> its > >> >> channel/ module clock instead. That way you don't need this patch, but > >> >> you'd add another for all the clock routing. > >> > > >> > Can you be more specific? I not sure what you mean here. > >> > >> For clock related properties in the device tree: > >> > >> &tcon_top { > >> > >> clocks = <&ccu CLK_BUS_TCON_TOP>, > >> > >> <&ccu CLK_TCON_TV0>, > >> <&tve0>, > >> <&ccu CLK_TCON_TV1>, > >> <&tve1>; > >> > >> clock-names = "bus", "tcon-tv0", "tve0", "tcon-tv1", "tve1"; > >> clock-output-names = "tcon-top-tv0", "tcon-top-tv1"; > >> > >> }; > >> > >> &tcon_tv0 { > >> > >> clocks = <&ccu CLK_BUS_TCON_TV0>, <&tcon_top 0>' > >> clock-names = "ahb", "tcon-ch1"; > >> > >> }; > >> > >> A diagram would look like: > >> | This part is TCON TOP | > >> > >> v v > >> > >> CCU CLK_TCON_TV0 --|----\ | > >> > >> | mux ---- gate ----|-- TCON_TV0 > >> > >> TVE0 --------------|----/ | > >> > >> And the same goes for TCON_TV1 and TVE1. > >> > >> The user manual is a bit lacking on how TVE outputs a clock though. > > > > I didn't yet received any response on HW details from AW till now, but I > > would like to post new version of patches soon. > > > > While chaining like you described could be implemented easily, I don't > > think it really represents HW as it is. Tests showed that these two > > clocks are independent, otherwise register writes/reads wouldn't be > > possible with tcon- top gate disabled. I chose tcon-top bus clock as a > > parent becase if it is not enabled, it simply won't work. > > AFAIK with the TCONs, even when the TCON channel clock (not the bus clock) > is disabled, register accesses still work. You're right, I just tested that. > I'm saying that the TCON TOP > gate is downstream from the TCON channel clock in the CCU. These are not > related to the TCON bus clock in the CCU, which affects register access. > > Did Allwinner provide any information regarding the hierarchy of the clocks? No reponse for now. > > However, if everyone feels chaining is the best way to implement it, I'll > > do it. > > I would like to get it right and match actual hardware. My proposal is > based on my understanding from the diagrams in the user manual. So for now, your explanation is the most reasonable. Should we go ahead and implement your idea? Please note that H6 has TCON-TOP too, but it has only one LCD TCON and one TV TCON instead of two of each kind. That means we will have hole in indices (tcon_lcd0 is 1, tcon_tv0 is 3, which is aligned with R40) and different TCON- TOP binding (no tcon_tv1 channel clock), but setup is exactly the same. Best regard, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne četrtek, 21. junij 2018 ob 17:35:45 CEST je Jernej Škrabec napisal(a): > Dne četrtek, 21. junij 2018 ob 03:23:27 CEST je Chen-Yu Tsai napisal(a): > > On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec <jernej.skrabec@siol.net> > > wrote: > > > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai napisal(a): > > >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec > > >> <jernej.skrabec@siol.net> > > > > > > wrote: > > >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai napisal(a): > > >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec > > >> >> > > >> >> <jernej.skrabec@siol.net> wrote: > > >> >> > Hi, > > >> >> > > > >> >> > Dne petek, 15. junij 2018 ob 10:31:10 CEST je Maxime Ripard > > napisal(a): > > >> >> >> Hi, > > >> >> >> > > >> >> >> On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec wrote: > > >> >> >> > TV TCONs connected to TCON TOP have to enable additional gate > > >> >> >> > in > > >> >> >> > order > > >> >> >> > to work. > > >> >> >> > > > >> >> >> > Add support for such TCONs. > > >> >> >> > > > >> >> >> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > > >> >> >> > --- > > >> >> >> > > > >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++ > > >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++++ > > >> >> >> > 2 files changed, 15 insertions(+) > > >> >> >> > > > >> >> >> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > >> >> >> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index > > >> >> >> > 08747fc3ee71..0afb5a94a414 > > >> >> >> > 100644 > > >> >> >> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > >> >> >> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > >> >> >> > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct > > >> >> >> > device > > >> >> >> > *dev, > > >> >> >> > > > >> >> >> > dev_err(dev, "Couldn't get the TCON bus clock\n"); > > >> >> >> > return PTR_ERR(tcon->clk); > > >> >> >> > > > >> >> >> > } > > >> >> >> > > > >> >> >> > + > > >> >> >> > + if (tcon->quirks->has_tcon_top_gate) { > > >> >> >> > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); > > >> >> >> > + if (IS_ERR(tcon->top_clk)) { > > >> >> >> > + dev_err(dev, "Couldn't get the TCON TOP bus > > >> >> >> > clock\n"); > > >> >> >> > + return PTR_ERR(tcon->top_clk); > > >> >> >> > + } > > >> >> >> > + clk_prepare_enable(tcon->top_clk); > > >> >> >> > + } > > >> >> >> > + > > >> >> >> > > >> >> >> Is it required for the TCON itself to operate, or does the TCON > > >> >> >> requires the TCON TOP, which in turn requires that clock to be > > >> >> >> functional? > > >> >> >> > > >> >> >> I find it quite odd to have a clock that isn't meant for a > > >> >> >> particular > > >> >> >> device to actually be wired to another device. I'm not saying > > >> >> >> this > > >> >> >> isn't the case, but it would be a first. > > >> >> > > > >> >> > Documentation doesn't say much about that gate. I did few tests > > >> >> > and > > >> >> > TCON > > >> >> > registers can be read and written even if TCON TOP TV TCON gate is > > >> >> > disabled. However, there is no image, as expected. > > >> >> > > >> >> The R40 manual does include it in the diagram, on page 504. There's > > >> >> also > > >> >> a > > >> >> mux to select whether the clock comes directly from the CCU or the > > >> >> TV > > >> >> encoder (a feedback mode?). I assume this is the gate you are > > >> >> referring > > >> >> to > > >> >> here, in which case it is not a bus clock, but rather the TCON > > >> >> module > > >> >> or > > >> >> channel clock, strangely routed. > > >> >> > > >> >> > More interestingly, I enabled test pattern directly in TCON to > > >> >> > eliminate > > >> >> > influence of the mixer. As soon as I disabled that gate, test > > >> >> > pattern > > >> >> > on > > >> >> > HDMI screen was gone, which suggest that this gate influences > > >> >> > something > > >> >> > inside TCON. > > >> >> > > > >> >> > Another test I did was that I moved enable/disable gate code to > > >> >> > sun4i_tcon_channel_set_status() and it worked just as well. > > >> >> > > > >> >> > I'll ask AW engineer what that gate actually does, but from what I > > >> >> > saw, > > >> >> > I > > >> >> > would say that most appropriate location to enable/disable TCON > > >> >> > TOP > > >> >> > TV > > >> >> > TCON > > >> >> > gate is TCON driver. Alternatively, TCON TOP driver could check if > > >> >> > any > > >> >> > TV > > >> >> > TCON is in use and enable appropriate gate. However, that doesn't > > >> >> > sound > > >> >> > right to me for some reason. > > >> >> > > >> >> If what I said above it true, then yes, the appropriate location to > > >> >> enable > > >> >> it is the TCON driver, but moreover, the representation of the clock > > >> >> tree > > >> >> should be fixed such that the TCON takes the clock from the TCON TOP > > >> >> as > > >> >> its > > >> >> channel/ module clock instead. That way you don't need this patch, > > >> >> but > > >> >> you'd add another for all the clock routing. > > >> > > > >> > Can you be more specific? I not sure what you mean here. > > >> > > >> For clock related properties in the device tree: > > >> > > >> &tcon_top { > > >> > > >> clocks = <&ccu CLK_BUS_TCON_TOP>, > > >> > > >> <&ccu CLK_TCON_TV0>, > > >> <&tve0>, > > >> <&ccu CLK_TCON_TV1>, > > >> <&tve1>; > > >> > > >> clock-names = "bus", "tcon-tv0", "tve0", "tcon-tv1", "tve1"; > > >> clock-output-names = "tcon-top-tv0", "tcon-top-tv1"; > > >> > > >> }; > > >> > > >> &tcon_tv0 { > > >> > > >> clocks = <&ccu CLK_BUS_TCON_TV0>, <&tcon_top 0>' > > >> clock-names = "ahb", "tcon-ch1"; > > >> > > >> }; > > >> > > >> A diagram would look like: > > >> | This part is TCON TOP | > > >> > > >> v v > > >> > > >> CCU CLK_TCON_TV0 --|----\ | > > >> > > >> | mux ---- gate ----|-- TCON_TV0 > > >> > > >> TVE0 --------------|----/ | > > >> > > >> And the same goes for TCON_TV1 and TVE1. > > >> > > >> The user manual is a bit lacking on how TVE outputs a clock though. > > > > > > I didn't yet received any response on HW details from AW till now, but I > > > would like to post new version of patches soon. > > > > > > While chaining like you described could be implemented easily, I don't > > > think it really represents HW as it is. Tests showed that these two > > > clocks are independent, otherwise register writes/reads wouldn't be > > > possible with tcon- top gate disabled. I chose tcon-top bus clock as a > > > parent becase if it is not enabled, it simply won't work. > > > > AFAIK with the TCONs, even when the TCON channel clock (not the bus clock) > > is disabled, register accesses still work. > > You're right, I just tested that. > > > I'm saying that the TCON TOP > > gate is downstream from the TCON channel clock in the CCU. These are not > > related to the TCON bus clock in the CCU, which affects register access. > > > > Did Allwinner provide any information regarding the hierarchy of the > > clocks? > No reponse for now. > > > > However, if everyone feels chaining is the best way to implement it, > > > I'll > > > do it. > > > > I would like to get it right and match actual hardware. My proposal is > > based on my understanding from the diagrams in the user manual. > > So for now, your explanation is the most reasonable. Should we go ahead and > implement your idea? > > Please note that H6 has TCON-TOP too, but it has only one LCD TCON and one > TV TCON instead of two of each kind. That means we will have hole in > indices (tcon_lcd0 is 1, tcon_tv0 is 3, which is aligned with R40) and > different TCON- TOP binding (no tcon_tv1 channel clock), but setup is > exactly the same. I just noticed issue with this proposal. If we have following clock chain for HDMI, everythings is ok: TCON-TV0 -> TCON-TOP-TV0 TCON TV sets TCON-TOP-TV0 clock rate, which in turn sets TCON-TV0 clock and everything works. However, when TVE will be configured, it would look like this: TVE0 -> TCON-TOP-TV0 TVE driver will set TVE0 clock to 216 MHz and TCON TV would set TCON-TOP-TV0 rate which in turn sets TVE0 clock to something like 13.5 MHz (or whatever is the right clock rate for PAL and NTSC). As you can see, same clock is set to two different rates by two different drivers. It *might* still work, since encoders set clock rate after TCON (at least that is my experience for HDMI pipeline), but that is still wrong. To overcome above issue, I would stick to original proposal with additional clock specified in TCON TV DT node. That way TCON driver would always set clock rate to TCON-TV0 clock. If TVE0 is enabled, TCON wouldn't interfere with setting clock rate, because TCON-TV0 clock would be decoupled in TCON-TOP mux. What do you think? Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 25, 2018 at 3:52 AM, Jernej Škrabec <jernej.skrabec@gmail.com> wrote: > Dne četrtek, 21. junij 2018 ob 17:35:45 CEST je Jernej Škrabec napisal(a): >> Dne četrtek, 21. junij 2018 ob 03:23:27 CEST je Chen-Yu Tsai napisal(a): >> > On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec <jernej.skrabec@siol.net> >> >> wrote: >> > > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai napisal(a): >> > >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec >> > >> <jernej.skrabec@siol.net> >> > > >> > > wrote: >> > >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai > napisal(a): >> > >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec >> > >> >> >> > >> >> <jernej.skrabec@siol.net> wrote: >> > >> >> > Hi, >> > >> >> > >> > >> >> > Dne petek, 15. junij 2018 ob 10:31:10 CEST je Maxime Ripard >> >> napisal(a): >> > >> >> >> Hi, >> > >> >> >> >> > >> >> >> On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec wrote: >> > >> >> >> > TV TCONs connected to TCON TOP have to enable additional gate >> > >> >> >> > in >> > >> >> >> > order >> > >> >> >> > to work. >> > >> >> >> > >> > >> >> >> > Add support for such TCONs. >> > >> >> >> > >> > >> >> >> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> >> > >> >> >> > --- >> > >> >> >> > >> > >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++ >> > >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++++ >> > >> >> >> > 2 files changed, 15 insertions(+) >> > >> >> >> > >> > >> >> >> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> > >> >> >> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index >> > >> >> >> > 08747fc3ee71..0afb5a94a414 >> > >> >> >> > 100644 >> > >> >> >> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> > >> >> >> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> > >> >> >> > @@ -688,6 +688,16 @@ static int sun4i_tcon_init_clocks(struct >> > >> >> >> > device >> > >> >> >> > *dev, >> > >> >> >> > >> > >> >> >> > dev_err(dev, "Couldn't get the TCON bus clock\n"); >> > >> >> >> > return PTR_ERR(tcon->clk); >> > >> >> >> > >> > >> >> >> > } >> > >> >> >> > >> > >> >> >> > + >> > >> >> >> > + if (tcon->quirks->has_tcon_top_gate) { >> > >> >> >> > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); >> > >> >> >> > + if (IS_ERR(tcon->top_clk)) { >> > >> >> >> > + dev_err(dev, "Couldn't get the TCON TOP bus >> > >> >> >> > clock\n"); >> > >> >> >> > + return PTR_ERR(tcon->top_clk); >> > >> >> >> > + } >> > >> >> >> > + clk_prepare_enable(tcon->top_clk); >> > >> >> >> > + } >> > >> >> >> > + >> > >> >> >> >> > >> >> >> Is it required for the TCON itself to operate, or does the TCON >> > >> >> >> requires the TCON TOP, which in turn requires that clock to be >> > >> >> >> functional? >> > >> >> >> >> > >> >> >> I find it quite odd to have a clock that isn't meant for a >> > >> >> >> particular >> > >> >> >> device to actually be wired to another device. I'm not saying >> > >> >> >> this >> > >> >> >> isn't the case, but it would be a first. >> > >> >> > >> > >> >> > Documentation doesn't say much about that gate. I did few tests >> > >> >> > and >> > >> >> > TCON >> > >> >> > registers can be read and written even if TCON TOP TV TCON gate is >> > >> >> > disabled. However, there is no image, as expected. >> > >> >> >> > >> >> The R40 manual does include it in the diagram, on page 504. There's >> > >> >> also >> > >> >> a >> > >> >> mux to select whether the clock comes directly from the CCU or the >> > >> >> TV >> > >> >> encoder (a feedback mode?). I assume this is the gate you are >> > >> >> referring >> > >> >> to >> > >> >> here, in which case it is not a bus clock, but rather the TCON >> > >> >> module >> > >> >> or >> > >> >> channel clock, strangely routed. >> > >> >> >> > >> >> > More interestingly, I enabled test pattern directly in TCON to >> > >> >> > eliminate >> > >> >> > influence of the mixer. As soon as I disabled that gate, test >> > >> >> > pattern >> > >> >> > on >> > >> >> > HDMI screen was gone, which suggest that this gate influences >> > >> >> > something >> > >> >> > inside TCON. >> > >> >> > >> > >> >> > Another test I did was that I moved enable/disable gate code to >> > >> >> > sun4i_tcon_channel_set_status() and it worked just as well. >> > >> >> > >> > >> >> > I'll ask AW engineer what that gate actually does, but from what I >> > >> >> > saw, >> > >> >> > I >> > >> >> > would say that most appropriate location to enable/disable TCON >> > >> >> > TOP >> > >> >> > TV >> > >> >> > TCON >> > >> >> > gate is TCON driver. Alternatively, TCON TOP driver could check if >> > >> >> > any >> > >> >> > TV >> > >> >> > TCON is in use and enable appropriate gate. However, that doesn't >> > >> >> > sound >> > >> >> > right to me for some reason. >> > >> >> >> > >> >> If what I said above it true, then yes, the appropriate location to >> > >> >> enable >> > >> >> it is the TCON driver, but moreover, the representation of the clock >> > >> >> tree >> > >> >> should be fixed such that the TCON takes the clock from the TCON TOP >> > >> >> as >> > >> >> its >> > >> >> channel/ module clock instead. That way you don't need this patch, >> > >> >> but >> > >> >> you'd add another for all the clock routing. >> > >> > >> > >> > Can you be more specific? I not sure what you mean here. >> > >> >> > >> For clock related properties in the device tree: >> > >> >> > >> &tcon_top { >> > >> >> > >> clocks = <&ccu CLK_BUS_TCON_TOP>, >> > >> >> > >> <&ccu CLK_TCON_TV0>, >> > >> <&tve0>, >> > >> <&ccu CLK_TCON_TV1>, >> > >> <&tve1>; >> > >> >> > >> clock-names = "bus", "tcon-tv0", "tve0", "tcon-tv1", "tve1"; >> > >> clock-output-names = "tcon-top-tv0", "tcon-top-tv1"; >> > >> >> > >> }; >> > >> >> > >> &tcon_tv0 { >> > >> >> > >> clocks = <&ccu CLK_BUS_TCON_TV0>, <&tcon_top 0>' >> > >> clock-names = "ahb", "tcon-ch1"; >> > >> >> > >> }; >> > >> >> > >> A diagram would look like: >> > >> | This part is TCON TOP | >> > >> >> > >> v v >> > >> >> > >> CCU CLK_TCON_TV0 --|----\ | >> > >> >> > >> | mux ---- gate ----|-- TCON_TV0 >> > >> >> > >> TVE0 --------------|----/ | >> > >> >> > >> And the same goes for TCON_TV1 and TVE1. >> > >> >> > >> The user manual is a bit lacking on how TVE outputs a clock though. >> > > >> > > I didn't yet received any response on HW details from AW till now, but I >> > > would like to post new version of patches soon. >> > > >> > > While chaining like you described could be implemented easily, I don't >> > > think it really represents HW as it is. Tests showed that these two >> > > clocks are independent, otherwise register writes/reads wouldn't be >> > > possible with tcon- top gate disabled. I chose tcon-top bus clock as a >> > > parent becase if it is not enabled, it simply won't work. >> > >> > AFAIK with the TCONs, even when the TCON channel clock (not the bus clock) >> > is disabled, register accesses still work. >> >> You're right, I just tested that. >> >> > I'm saying that the TCON TOP >> > gate is downstream from the TCON channel clock in the CCU. These are not >> > related to the TCON bus clock in the CCU, which affects register access. >> > >> > Did Allwinner provide any information regarding the hierarchy of the >> > clocks? >> No reponse for now. >> >> > > However, if everyone feels chaining is the best way to implement it, >> > > I'll >> > > do it. >> > >> > I would like to get it right and match actual hardware. My proposal is >> > based on my understanding from the diagrams in the user manual. >> >> So for now, your explanation is the most reasonable. Should we go ahead and >> implement your idea? >> >> Please note that H6 has TCON-TOP too, but it has only one LCD TCON and one >> TV TCON instead of two of each kind. That means we will have hole in >> indices (tcon_lcd0 is 1, tcon_tv0 is 3, which is aligned with R40) and >> different TCON- TOP binding (no tcon_tv1 channel clock), but setup is >> exactly the same. > > I just noticed issue with this proposal. If we have following clock chain for > HDMI, everythings is ok: > > TCON-TV0 -> TCON-TOP-TV0 > > TCON TV sets TCON-TOP-TV0 clock rate, which in turn sets TCON-TV0 clock and > everything works. > > However, when TVE will be configured, it would look like this: > > TVE0 -> TCON-TOP-TV0 > > TVE driver will set TVE0 clock to 216 MHz and TCON TV would set TCON-TOP-TV0 > rate which in turn sets TVE0 clock to something like 13.5 MHz (or whatever is > the right clock rate for PAL and NTSC). As you can see, same clock is set to > two different rates by two different drivers. > > It *might* still work, since encoders set clock rate after TCON (at least that > is my experience for HDMI pipeline), but that is still wrong. > > To overcome above issue, I would stick to original proposal with additional > clock specified in TCON TV DT node. That way TCON driver would always set > clock rate to TCON-TV0 clock. If TVE0 is enabled, TCON wouldn't interfere with > setting clock rate, because TCON-TV0 clock would be decoupled in TCON-TOP mux. > > What do you think? I think this is the wrong representation, and worse, you are trying to work around software issues with it. So to confirm some details, the TVE expects 216 MHz clock, and it expects the TCON to run and output data at 216 MHz as well. Is that correct? Would any settings for the TCON differ between when HDMI or TVE is used? Does TVE and TCON run at 216 MHz regardless of resolution? I kind of doubt it. It might be expecting 297 MHz for PC resolutions. I think these kinds of quirks should be handled in the software, instead of being papered over. ChenYu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne ponedeljek, 25. junij 2018 ob 05:51:41 CEST je Chen-Yu Tsai napisal(a): > On Mon, Jun 25, 2018 at 3:52 AM, Jernej Škrabec > > <jernej.skrabec@gmail.com> wrote: > > Dne četrtek, 21. junij 2018 ob 17:35:45 CEST je Jernej Škrabec napisal(a): > >> Dne četrtek, 21. junij 2018 ob 03:23:27 CEST je Chen-Yu Tsai napisal(a): > >> > On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec > >> > <jernej.skrabec@siol.net> > >> > >> wrote: > >> > > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai napisal(a): > >> > >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec > >> > >> <jernej.skrabec@siol.net> > >> > > > >> > > wrote: > >> > >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai > > > > napisal(a): > >> > >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec > >> > >> >> > >> > >> >> <jernej.skrabec@siol.net> wrote: > >> > >> >> > Hi, > >> > >> >> > > >> > >> >> > Dne petek, 15. junij 2018 ob 10:31:10 CEST je Maxime Ripard > >> > >> napisal(a): > >> > >> >> >> Hi, > >> > >> >> >> > >> > >> >> >> On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec wrote: > >> > >> >> >> > TV TCONs connected to TCON TOP have to enable additional > >> > >> >> >> > gate > >> > >> >> >> > in > >> > >> >> >> > order > >> > >> >> >> > to work. > >> > >> >> >> > > >> > >> >> >> > Add support for such TCONs. > >> > >> >> >> > > >> > >> >> >> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > >> > >> >> >> > --- > >> > >> >> >> > > >> > >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++ > >> > >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++++ > >> > >> >> >> > 2 files changed, 15 insertions(+) > >> > >> >> >> > > >> > >> >> >> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> > >> >> >> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index > >> > >> >> >> > 08747fc3ee71..0afb5a94a414 > >> > >> >> >> > 100644 > >> > >> >> >> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> > >> >> >> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> > >> >> >> > @@ -688,6 +688,16 @@ static int > >> > >> >> >> > sun4i_tcon_init_clocks(struct > >> > >> >> >> > device > >> > >> >> >> > *dev, > >> > >> >> >> > > >> > >> >> >> > dev_err(dev, "Couldn't get the TCON bus > >> > >> >> >> > clock\n"); > >> > >> >> >> > return PTR_ERR(tcon->clk); > >> > >> >> >> > > >> > >> >> >> > } > >> > >> >> >> > > >> > >> >> >> > + > >> > >> >> >> > + if (tcon->quirks->has_tcon_top_gate) { > >> > >> >> >> > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); > >> > >> >> >> > + if (IS_ERR(tcon->top_clk)) { > >> > >> >> >> > + dev_err(dev, "Couldn't get the TCON TOP > >> > >> >> >> > bus > >> > >> >> >> > clock\n"); > >> > >> >> >> > + return PTR_ERR(tcon->top_clk); > >> > >> >> >> > + } > >> > >> >> >> > + clk_prepare_enable(tcon->top_clk); > >> > >> >> >> > + } > >> > >> >> >> > + > >> > >> >> >> > >> > >> >> >> Is it required for the TCON itself to operate, or does the > >> > >> >> >> TCON > >> > >> >> >> requires the TCON TOP, which in turn requires that clock to be > >> > >> >> >> functional? > >> > >> >> >> > >> > >> >> >> I find it quite odd to have a clock that isn't meant for a > >> > >> >> >> particular > >> > >> >> >> device to actually be wired to another device. I'm not saying > >> > >> >> >> this > >> > >> >> >> isn't the case, but it would be a first. > >> > >> >> > > >> > >> >> > Documentation doesn't say much about that gate. I did few tests > >> > >> >> > and > >> > >> >> > TCON > >> > >> >> > registers can be read and written even if TCON TOP TV TCON gate > >> > >> >> > is > >> > >> >> > disabled. However, there is no image, as expected. > >> > >> >> > >> > >> >> The R40 manual does include it in the diagram, on page 504. > >> > >> >> There's > >> > >> >> also > >> > >> >> a > >> > >> >> mux to select whether the clock comes directly from the CCU or > >> > >> >> the > >> > >> >> TV > >> > >> >> encoder (a feedback mode?). I assume this is the gate you are > >> > >> >> referring > >> > >> >> to > >> > >> >> here, in which case it is not a bus clock, but rather the TCON > >> > >> >> module > >> > >> >> or > >> > >> >> channel clock, strangely routed. > >> > >> >> > >> > >> >> > More interestingly, I enabled test pattern directly in TCON to > >> > >> >> > eliminate > >> > >> >> > influence of the mixer. As soon as I disabled that gate, test > >> > >> >> > pattern > >> > >> >> > on > >> > >> >> > HDMI screen was gone, which suggest that this gate influences > >> > >> >> > something > >> > >> >> > inside TCON. > >> > >> >> > > >> > >> >> > Another test I did was that I moved enable/disable gate code to > >> > >> >> > sun4i_tcon_channel_set_status() and it worked just as well. > >> > >> >> > > >> > >> >> > I'll ask AW engineer what that gate actually does, but from > >> > >> >> > what I > >> > >> >> > saw, > >> > >> >> > I > >> > >> >> > would say that most appropriate location to enable/disable TCON > >> > >> >> > TOP > >> > >> >> > TV > >> > >> >> > TCON > >> > >> >> > gate is TCON driver. Alternatively, TCON TOP driver could check > >> > >> >> > if > >> > >> >> > any > >> > >> >> > TV > >> > >> >> > TCON is in use and enable appropriate gate. However, that > >> > >> >> > doesn't > >> > >> >> > sound > >> > >> >> > right to me for some reason. > >> > >> >> > >> > >> >> If what I said above it true, then yes, the appropriate location > >> > >> >> to > >> > >> >> enable > >> > >> >> it is the TCON driver, but moreover, the representation of the > >> > >> >> clock > >> > >> >> tree > >> > >> >> should be fixed such that the TCON takes the clock from the TCON > >> > >> >> TOP > >> > >> >> as > >> > >> >> its > >> > >> >> channel/ module clock instead. That way you don't need this > >> > >> >> patch, > >> > >> >> but > >> > >> >> you'd add another for all the clock routing. > >> > >> > > >> > >> > Can you be more specific? I not sure what you mean here. > >> > >> > >> > >> For clock related properties in the device tree: > >> > >> > >> > >> &tcon_top { > >> > >> > >> > >> clocks = <&ccu CLK_BUS_TCON_TOP>, > >> > >> > >> > >> <&ccu CLK_TCON_TV0>, > >> > >> <&tve0>, > >> > >> <&ccu CLK_TCON_TV1>, > >> > >> <&tve1>; > >> > >> > >> > >> clock-names = "bus", "tcon-tv0", "tve0", "tcon-tv1", "tve1"; > >> > >> clock-output-names = "tcon-top-tv0", "tcon-top-tv1"; > >> > >> > >> > >> }; > >> > >> > >> > >> &tcon_tv0 { > >> > >> > >> > >> clocks = <&ccu CLK_BUS_TCON_TV0>, <&tcon_top 0>' > >> > >> clock-names = "ahb", "tcon-ch1"; > >> > >> > >> > >> }; > >> > >> > >> > >> A diagram would look like: > >> > >> | This part is TCON TOP | > >> > >> > >> > >> v v > >> > >> > >> > >> CCU CLK_TCON_TV0 --|----\ | > >> > >> > >> > >> | mux ---- gate ----|-- TCON_TV0 > >> > >> > >> > >> TVE0 --------------|----/ | > >> > >> > >> > >> And the same goes for TCON_TV1 and TVE1. > >> > >> > >> > >> The user manual is a bit lacking on how TVE outputs a clock though. > >> > > > >> > > I didn't yet received any response on HW details from AW till now, > >> > > but I > >> > > would like to post new version of patches soon. > >> > > > >> > > While chaining like you described could be implemented easily, I > >> > > don't > >> > > think it really represents HW as it is. Tests showed that these two > >> > > clocks are independent, otherwise register writes/reads wouldn't be > >> > > possible with tcon- top gate disabled. I chose tcon-top bus clock as > >> > > a > >> > > parent becase if it is not enabled, it simply won't work. > >> > > >> > AFAIK with the TCONs, even when the TCON channel clock (not the bus > >> > clock) > >> > is disabled, register accesses still work. > >> > >> You're right, I just tested that. > >> > >> > I'm saying that the TCON TOP > >> > gate is downstream from the TCON channel clock in the CCU. These are > >> > not > >> > related to the TCON bus clock in the CCU, which affects register > >> > access. > >> > > >> > Did Allwinner provide any information regarding the hierarchy of the > >> > clocks? > >> > >> No reponse for now. > >> > >> > > However, if everyone feels chaining is the best way to implement it, > >> > > I'll > >> > > do it. > >> > > >> > I would like to get it right and match actual hardware. My proposal is > >> > based on my understanding from the diagrams in the user manual. > >> > >> So for now, your explanation is the most reasonable. Should we go ahead > >> and > >> implement your idea? > >> > >> Please note that H6 has TCON-TOP too, but it has only one LCD TCON and > >> one > >> TV TCON instead of two of each kind. That means we will have hole in > >> indices (tcon_lcd0 is 1, tcon_tv0 is 3, which is aligned with R40) and > >> different TCON- TOP binding (no tcon_tv1 channel clock), but setup is > >> exactly the same. > > > > I just noticed issue with this proposal. If we have following clock chain > > for HDMI, everythings is ok: > > > > TCON-TV0 -> TCON-TOP-TV0 > > > > TCON TV sets TCON-TOP-TV0 clock rate, which in turn sets TCON-TV0 clock > > and > > everything works. > > > > However, when TVE will be configured, it would look like this: > > > > TVE0 -> TCON-TOP-TV0 > > > > TVE driver will set TVE0 clock to 216 MHz and TCON TV would set > > TCON-TOP-TV0 rate which in turn sets TVE0 clock to something like 13.5 > > MHz (or whatever is the right clock rate for PAL and NTSC). As you can > > see, same clock is set to two different rates by two different drivers. > > > > It *might* still work, since encoders set clock rate after TCON (at least > > that is my experience for HDMI pipeline), but that is still wrong. > > > > To overcome above issue, I would stick to original proposal with > > additional > > clock specified in TCON TV DT node. That way TCON driver would always set > > clock rate to TCON-TV0 clock. If TVE0 is enabled, TCON wouldn't interfere > > with setting clock rate, because TCON-TV0 clock would be decoupled in > > TCON-TOP mux. > > > > What do you think? > > I think this is the wrong representation, and worse, you are trying to work > around software issues with it. > > So to confirm some details, the TVE expects 216 MHz clock, and it expects > the TCON to run and output data at 216 MHz as well. Is that correct? Yes, from my understanding. 216 MHz is correct at least for PAL and NTSC, e.g. TV mode. TVE on R40 is also capable of RGB mode (VGA connector). > > Would any settings for the TCON differ between when HDMI or TVE is used? > Apart of clock, no, other settings would be the same. > Does TVE and TCON run at 216 MHz regardless of resolution? I kind of doubt > it. It might be expecting 297 MHz for PC resolutions. Please check this table in BSP: https://github.com/BPI-SINOVOIP/BPI-M2U-bsp/blob/master/linux-sunxi/drivers/ video/sunxi/disp2/tv/drv_tv.c#L24 216 MHz is applicable for low resolution, interlaced modes. Modes like 1080P, 1080I have expected standard timing. > > I think these kinds of quirks should be handled in the software, instead of > being papered over. Ok, that works for me too. I would just like to have such design that would later allow implementing TVE driver without much issues. BTW, H3 TV TCON which is connected to TVE doesn't have TCON-TV channel clock at all, since it is controlled with TVE clock (same case as it would be here, if TCON TOP mux is switched to TVE clock source). Maybe quirk can be added that it doesn't set clock rate at all if it is connected to TVE? Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 25, 2018 at 3:58 PM, Jernej Škrabec <jernej.skrabec@gmail.com> wrote: > Dne ponedeljek, 25. junij 2018 ob 05:51:41 CEST je Chen-Yu Tsai napisal(a): >> On Mon, Jun 25, 2018 at 3:52 AM, Jernej Škrabec >> >> <jernej.skrabec@gmail.com> wrote: >> > Dne četrtek, 21. junij 2018 ob 17:35:45 CEST je Jernej Škrabec napisal(a): >> >> Dne četrtek, 21. junij 2018 ob 03:23:27 CEST je Chen-Yu Tsai napisal(a): >> >> > On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec >> >> > <jernej.skrabec@siol.net> >> >> >> >> wrote: >> >> > > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai > napisal(a): >> >> > >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec >> >> > >> <jernej.skrabec@siol.net> >> >> > > >> >> > > wrote: >> >> > >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai >> > >> > napisal(a): >> >> > >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec >> >> > >> >> >> >> > >> >> <jernej.skrabec@siol.net> wrote: >> >> > >> >> > Hi, >> >> > >> >> > >> >> > >> >> > Dne petek, 15. junij 2018 ob 10:31:10 CEST je Maxime Ripard >> >> >> >> napisal(a): >> >> > >> >> >> Hi, >> >> > >> >> >> >> >> > >> >> >> On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec > wrote: >> >> > >> >> >> > TV TCONs connected to TCON TOP have to enable additional >> >> > >> >> >> > gate >> >> > >> >> >> > in >> >> > >> >> >> > order >> >> > >> >> >> > to work. >> >> > >> >> >> > >> >> > >> >> >> > Add support for such TCONs. >> >> > >> >> >> > >> >> > >> >> >> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> >> >> > >> >> >> > --- >> >> > >> >> >> > >> >> > >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++ >> >> > >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++++ >> >> > >> >> >> > 2 files changed, 15 insertions(+) >> >> > >> >> >> > >> >> > >> >> >> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> >> > >> >> >> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index >> >> > >> >> >> > 08747fc3ee71..0afb5a94a414 >> >> > >> >> >> > 100644 >> >> > >> >> >> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >> >> > >> >> >> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >> >> > >> >> >> > @@ -688,6 +688,16 @@ static int >> >> > >> >> >> > sun4i_tcon_init_clocks(struct >> >> > >> >> >> > device >> >> > >> >> >> > *dev, >> >> > >> >> >> > >> >> > >> >> >> > dev_err(dev, "Couldn't get the TCON bus >> >> > >> >> >> > clock\n"); >> >> > >> >> >> > return PTR_ERR(tcon->clk); >> >> > >> >> >> > >> >> > >> >> >> > } >> >> > >> >> >> > >> >> > >> >> >> > + >> >> > >> >> >> > + if (tcon->quirks->has_tcon_top_gate) { >> >> > >> >> >> > + tcon->top_clk = devm_clk_get(dev, "tcon-top"); >> >> > >> >> >> > + if (IS_ERR(tcon->top_clk)) { >> >> > >> >> >> > + dev_err(dev, "Couldn't get the TCON TOP >> >> > >> >> >> > bus >> >> > >> >> >> > clock\n"); >> >> > >> >> >> > + return PTR_ERR(tcon->top_clk); >> >> > >> >> >> > + } >> >> > >> >> >> > + clk_prepare_enable(tcon->top_clk); >> >> > >> >> >> > + } >> >> > >> >> >> > + >> >> > >> >> >> >> >> > >> >> >> Is it required for the TCON itself to operate, or does the >> >> > >> >> >> TCON >> >> > >> >> >> requires the TCON TOP, which in turn requires that clock to be >> >> > >> >> >> functional? >> >> > >> >> >> >> >> > >> >> >> I find it quite odd to have a clock that isn't meant for a >> >> > >> >> >> particular >> >> > >> >> >> device to actually be wired to another device. I'm not saying >> >> > >> >> >> this >> >> > >> >> >> isn't the case, but it would be a first. >> >> > >> >> > >> >> > >> >> > Documentation doesn't say much about that gate. I did few tests >> >> > >> >> > and >> >> > >> >> > TCON >> >> > >> >> > registers can be read and written even if TCON TOP TV TCON gate >> >> > >> >> > is >> >> > >> >> > disabled. However, there is no image, as expected. >> >> > >> >> >> >> > >> >> The R40 manual does include it in the diagram, on page 504. >> >> > >> >> There's >> >> > >> >> also >> >> > >> >> a >> >> > >> >> mux to select whether the clock comes directly from the CCU or >> >> > >> >> the >> >> > >> >> TV >> >> > >> >> encoder (a feedback mode?). I assume this is the gate you are >> >> > >> >> referring >> >> > >> >> to >> >> > >> >> here, in which case it is not a bus clock, but rather the TCON >> >> > >> >> module >> >> > >> >> or >> >> > >> >> channel clock, strangely routed. >> >> > >> >> >> >> > >> >> > More interestingly, I enabled test pattern directly in TCON to >> >> > >> >> > eliminate >> >> > >> >> > influence of the mixer. As soon as I disabled that gate, test >> >> > >> >> > pattern >> >> > >> >> > on >> >> > >> >> > HDMI screen was gone, which suggest that this gate influences >> >> > >> >> > something >> >> > >> >> > inside TCON. >> >> > >> >> > >> >> > >> >> > Another test I did was that I moved enable/disable gate code to >> >> > >> >> > sun4i_tcon_channel_set_status() and it worked just as well. >> >> > >> >> > >> >> > >> >> > I'll ask AW engineer what that gate actually does, but from >> >> > >> >> > what I >> >> > >> >> > saw, >> >> > >> >> > I >> >> > >> >> > would say that most appropriate location to enable/disable TCON >> >> > >> >> > TOP >> >> > >> >> > TV >> >> > >> >> > TCON >> >> > >> >> > gate is TCON driver. Alternatively, TCON TOP driver could check >> >> > >> >> > if >> >> > >> >> > any >> >> > >> >> > TV >> >> > >> >> > TCON is in use and enable appropriate gate. However, that >> >> > >> >> > doesn't >> >> > >> >> > sound >> >> > >> >> > right to me for some reason. >> >> > >> >> >> >> > >> >> If what I said above it true, then yes, the appropriate location >> >> > >> >> to >> >> > >> >> enable >> >> > >> >> it is the TCON driver, but moreover, the representation of the >> >> > >> >> clock >> >> > >> >> tree >> >> > >> >> should be fixed such that the TCON takes the clock from the TCON >> >> > >> >> TOP >> >> > >> >> as >> >> > >> >> its >> >> > >> >> channel/ module clock instead. That way you don't need this >> >> > >> >> patch, >> >> > >> >> but >> >> > >> >> you'd add another for all the clock routing. >> >> > >> > >> >> > >> > Can you be more specific? I not sure what you mean here. >> >> > >> >> >> > >> For clock related properties in the device tree: >> >> > >> >> >> > >> &tcon_top { >> >> > >> >> >> > >> clocks = <&ccu CLK_BUS_TCON_TOP>, >> >> > >> >> >> > >> <&ccu CLK_TCON_TV0>, >> >> > >> <&tve0>, >> >> > >> <&ccu CLK_TCON_TV1>, >> >> > >> <&tve1>; >> >> > >> >> >> > >> clock-names = "bus", "tcon-tv0", "tve0", "tcon-tv1", "tve1"; >> >> > >> clock-output-names = "tcon-top-tv0", "tcon-top-tv1"; >> >> > >> >> >> > >> }; >> >> > >> >> >> > >> &tcon_tv0 { >> >> > >> >> >> > >> clocks = <&ccu CLK_BUS_TCON_TV0>, <&tcon_top 0>' >> >> > >> clock-names = "ahb", "tcon-ch1"; >> >> > >> >> >> > >> }; >> >> > >> >> >> > >> A diagram would look like: >> >> > >> | This part is TCON TOP | >> >> > >> >> >> > >> v v >> >> > >> >> >> > >> CCU CLK_TCON_TV0 --|----\ | >> >> > >> >> >> > >> | mux ---- gate ----|-- TCON_TV0 >> >> > >> >> >> > >> TVE0 --------------|----/ | >> >> > >> >> >> > >> And the same goes for TCON_TV1 and TVE1. >> >> > >> >> >> > >> The user manual is a bit lacking on how TVE outputs a clock though. >> >> > > >> >> > > I didn't yet received any response on HW details from AW till now, >> >> > > but I >> >> > > would like to post new version of patches soon. >> >> > > >> >> > > While chaining like you described could be implemented easily, I >> >> > > don't >> >> > > think it really represents HW as it is. Tests showed that these two >> >> > > clocks are independent, otherwise register writes/reads wouldn't be >> >> > > possible with tcon- top gate disabled. I chose tcon-top bus clock as >> >> > > a >> >> > > parent becase if it is not enabled, it simply won't work. >> >> > >> >> > AFAIK with the TCONs, even when the TCON channel clock (not the bus >> >> > clock) >> >> > is disabled, register accesses still work. >> >> >> >> You're right, I just tested that. >> >> >> >> > I'm saying that the TCON TOP >> >> > gate is downstream from the TCON channel clock in the CCU. These are >> >> > not >> >> > related to the TCON bus clock in the CCU, which affects register >> >> > access. >> >> > >> >> > Did Allwinner provide any information regarding the hierarchy of the >> >> > clocks? >> >> >> >> No reponse for now. >> >> >> >> > > However, if everyone feels chaining is the best way to implement it, >> >> > > I'll >> >> > > do it. >> >> > >> >> > I would like to get it right and match actual hardware. My proposal is >> >> > based on my understanding from the diagrams in the user manual. >> >> >> >> So for now, your explanation is the most reasonable. Should we go ahead >> >> and >> >> implement your idea? >> >> >> >> Please note that H6 has TCON-TOP too, but it has only one LCD TCON and >> >> one >> >> TV TCON instead of two of each kind. That means we will have hole in >> >> indices (tcon_lcd0 is 1, tcon_tv0 is 3, which is aligned with R40) and >> >> different TCON- TOP binding (no tcon_tv1 channel clock), but setup is >> >> exactly the same. >> > >> > I just noticed issue with this proposal. If we have following clock chain >> > for HDMI, everythings is ok: >> > >> > TCON-TV0 -> TCON-TOP-TV0 >> > >> > TCON TV sets TCON-TOP-TV0 clock rate, which in turn sets TCON-TV0 clock >> > and >> > everything works. >> > >> > However, when TVE will be configured, it would look like this: >> > >> > TVE0 -> TCON-TOP-TV0 >> > >> > TVE driver will set TVE0 clock to 216 MHz and TCON TV would set >> > TCON-TOP-TV0 rate which in turn sets TVE0 clock to something like 13.5 >> > MHz (or whatever is the right clock rate for PAL and NTSC). As you can >> > see, same clock is set to two different rates by two different drivers. >> > >> > It *might* still work, since encoders set clock rate after TCON (at least >> > that is my experience for HDMI pipeline), but that is still wrong. >> > >> > To overcome above issue, I would stick to original proposal with >> > additional >> > clock specified in TCON TV DT node. That way TCON driver would always set >> > clock rate to TCON-TV0 clock. If TVE0 is enabled, TCON wouldn't interfere >> > with setting clock rate, because TCON-TV0 clock would be decoupled in >> > TCON-TOP mux. >> > >> > What do you think? >> >> I think this is the wrong representation, and worse, you are trying to work >> around software issues with it. >> >> So to confirm some details, the TVE expects 216 MHz clock, and it expects >> the TCON to run and output data at 216 MHz as well. Is that correct? > > Yes, from my understanding. 216 MHz is correct at least for PAL and NTSC, e.g. > TV mode. TVE on R40 is also capable of RGB mode (VGA connector). > >> >> Would any settings for the TCON differ between when HDMI or TVE is used? >> > > Apart of clock, no, other settings would be the same. > >> Does TVE and TCON run at 216 MHz regardless of resolution? I kind of doubt >> it. It might be expecting 297 MHz for PC resolutions. > > Please check this table in BSP: > https://github.com/BPI-SINOVOIP/BPI-M2U-bsp/blob/master/linux-sunxi/drivers/ > video/sunxi/disp2/tv/drv_tv.c#L24 > > 216 MHz is applicable for low resolution, interlaced modes. Modes like 1080P, > 1080I have expected standard timing. That's weird. So it only applies to SDTV video resolutions. I remember seeing an "up sampling" setting for composite in the TVE, which goes all the way up to 216 MHz. Maybe that's the reason? I wonder how the TCON manages this though. I mean with the dot clock this high, doesn't that mean the frame rate is much higher? >> >> I think these kinds of quirks should be handled in the software, instead of >> being papered over. > > Ok, that works for me too. I would just like to have such design that would > later allow implementing TVE driver without much issues. > > BTW, H3 TV TCON which is connected to TVE doesn't have TCON-TV channel clock > at all, since it is controlled with TVE clock (same case as it would be here, > if TCON TOP mux is switched to TVE clock source). Does it require 216 MHz as well? > Maybe quirk can be added that it doesn't set clock rate at all if it is > connected to TVE? A quirk yes. But the dot clock would be 216 MHz instead of not setting it, and only for certain display modes. To be honest I think we can get by with just a TODO note for now. ChenYu -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dne ponedeljek, 25. junij 2018 ob 10:14:52 CEST je Chen-Yu Tsai napisal(a): > On Mon, Jun 25, 2018 at 3:58 PM, Jernej Škrabec > > <jernej.skrabec@gmail.com> wrote: > > Dne ponedeljek, 25. junij 2018 ob 05:51:41 CEST je Chen-Yu Tsai napisal(a): > >> On Mon, Jun 25, 2018 at 3:52 AM, Jernej Škrabec > >> > >> <jernej.skrabec@gmail.com> wrote: > >> > Dne četrtek, 21. junij 2018 ob 17:35:45 CEST je Jernej Škrabec napisal(a): > >> >> Dne četrtek, 21. junij 2018 ob 03:23:27 CEST je Chen-Yu Tsai napisal(a): > >> >> > On Thu, Jun 21, 2018 at 3:37 AM, Jernej Škrabec > >> >> > <jernej.skrabec@siol.net> > >> >> > >> >> wrote: > >> >> > > Dne sobota, 16. junij 2018 ob 07:48:38 CEST je Chen-Yu Tsai > > > > napisal(a): > >> >> > >> On Sat, Jun 16, 2018 at 1:33 AM, Jernej Škrabec > >> >> > >> <jernej.skrabec@siol.net> > >> >> > > > >> >> > > wrote: > >> >> > >> > Dne petek, 15. junij 2018 ob 19:13:17 CEST je Chen-Yu Tsai > >> > > >> > napisal(a): > >> >> > >> >> On Sat, Jun 16, 2018 at 12:41 AM, Jernej Škrabec > >> >> > >> >> > >> >> > >> >> <jernej.skrabec@siol.net> wrote: > >> >> > >> >> > Hi, > >> >> > >> >> > > >> >> > >> >> > Dne petek, 15. junij 2018 ob 10:31:10 CEST je Maxime Ripard > >> >> > >> >> napisal(a): > >> >> > >> >> >> Hi, > >> >> > >> >> >> > >> >> > >> >> >> On Tue, Jun 12, 2018 at 10:00:20PM +0200, Jernej Skrabec > > > > wrote: > >> >> > >> >> >> > TV TCONs connected to TCON TOP have to enable additional > >> >> > >> >> >> > gate > >> >> > >> >> >> > in > >> >> > >> >> >> > order > >> >> > >> >> >> > to work. > >> >> > >> >> >> > > >> >> > >> >> >> > Add support for such TCONs. > >> >> > >> >> >> > > >> >> > >> >> >> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net> > >> >> > >> >> >> > --- > >> >> > >> >> >> > > >> >> > >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.c | 11 +++++++++++ > >> >> > >> >> >> > drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 ++++ > >> >> > >> >> >> > 2 files changed, 15 insertions(+) > >> >> > >> >> >> > > >> >> > >> >> >> > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> >> > >> >> >> > b/drivers/gpu/drm/sun4i/sun4i_tcon.c index > >> >> > >> >> >> > 08747fc3ee71..0afb5a94a414 > >> >> > >> >> >> > 100644 > >> >> > >> >> >> > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> >> > >> >> >> > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > >> >> > >> >> >> > @@ -688,6 +688,16 @@ static int > >> >> > >> >> >> > sun4i_tcon_init_clocks(struct > >> >> > >> >> >> > device > >> >> > >> >> >> > *dev, > >> >> > >> >> >> > > >> >> > >> >> >> > dev_err(dev, "Couldn't get the TCON bus > >> >> > >> >> >> > clock\n"); > >> >> > >> >> >> > return PTR_ERR(tcon->clk); > >> >> > >> >> >> > > >> >> > >> >> >> > } > >> >> > >> >> >> > > >> >> > >> >> >> > + > >> >> > >> >> >> > + if (tcon->quirks->has_tcon_top_gate) { > >> >> > >> >> >> > + tcon->top_clk = devm_clk_get(dev, > >> >> > >> >> >> > "tcon-top"); > >> >> > >> >> >> > + if (IS_ERR(tcon->top_clk)) { > >> >> > >> >> >> > + dev_err(dev, "Couldn't get the TCON > >> >> > >> >> >> > TOP > >> >> > >> >> >> > bus > >> >> > >> >> >> > clock\n"); > >> >> > >> >> >> > + return PTR_ERR(tcon->top_clk); > >> >> > >> >> >> > + } > >> >> > >> >> >> > + clk_prepare_enable(tcon->top_clk); > >> >> > >> >> >> > + } > >> >> > >> >> >> > + > >> >> > >> >> >> > >> >> > >> >> >> Is it required for the TCON itself to operate, or does the > >> >> > >> >> >> TCON > >> >> > >> >> >> requires the TCON TOP, which in turn requires that clock to > >> >> > >> >> >> be > >> >> > >> >> >> functional? > >> >> > >> >> >> > >> >> > >> >> >> I find it quite odd to have a clock that isn't meant for a > >> >> > >> >> >> particular > >> >> > >> >> >> device to actually be wired to another device. I'm not > >> >> > >> >> >> saying > >> >> > >> >> >> this > >> >> > >> >> >> isn't the case, but it would be a first. > >> >> > >> >> > > >> >> > >> >> > Documentation doesn't say much about that gate. I did few > >> >> > >> >> > tests > >> >> > >> >> > and > >> >> > >> >> > TCON > >> >> > >> >> > registers can be read and written even if TCON TOP TV TCON > >> >> > >> >> > gate > >> >> > >> >> > is > >> >> > >> >> > disabled. However, there is no image, as expected. > >> >> > >> >> > >> >> > >> >> The R40 manual does include it in the diagram, on page 504. > >> >> > >> >> There's > >> >> > >> >> also > >> >> > >> >> a > >> >> > >> >> mux to select whether the clock comes directly from the CCU or > >> >> > >> >> the > >> >> > >> >> TV > >> >> > >> >> encoder (a feedback mode?). I assume this is the gate you are > >> >> > >> >> referring > >> >> > >> >> to > >> >> > >> >> here, in which case it is not a bus clock, but rather the TCON > >> >> > >> >> module > >> >> > >> >> or > >> >> > >> >> channel clock, strangely routed. > >> >> > >> >> > >> >> > >> >> > More interestingly, I enabled test pattern directly in TCON > >> >> > >> >> > to > >> >> > >> >> > eliminate > >> >> > >> >> > influence of the mixer. As soon as I disabled that gate, > >> >> > >> >> > test > >> >> > >> >> > pattern > >> >> > >> >> > on > >> >> > >> >> > HDMI screen was gone, which suggest that this gate > >> >> > >> >> > influences > >> >> > >> >> > something > >> >> > >> >> > inside TCON. > >> >> > >> >> > > >> >> > >> >> > Another test I did was that I moved enable/disable gate code > >> >> > >> >> > to > >> >> > >> >> > sun4i_tcon_channel_set_status() and it worked just as well. > >> >> > >> >> > > >> >> > >> >> > I'll ask AW engineer what that gate actually does, but from > >> >> > >> >> > what I > >> >> > >> >> > saw, > >> >> > >> >> > I > >> >> > >> >> > would say that most appropriate location to enable/disable > >> >> > >> >> > TCON > >> >> > >> >> > TOP > >> >> > >> >> > TV > >> >> > >> >> > TCON > >> >> > >> >> > gate is TCON driver. Alternatively, TCON TOP driver could > >> >> > >> >> > check > >> >> > >> >> > if > >> >> > >> >> > any > >> >> > >> >> > TV > >> >> > >> >> > TCON is in use and enable appropriate gate. However, that > >> >> > >> >> > doesn't > >> >> > >> >> > sound > >> >> > >> >> > right to me for some reason. > >> >> > >> >> > >> >> > >> >> If what I said above it true, then yes, the appropriate > >> >> > >> >> location > >> >> > >> >> to > >> >> > >> >> enable > >> >> > >> >> it is the TCON driver, but moreover, the representation of the > >> >> > >> >> clock > >> >> > >> >> tree > >> >> > >> >> should be fixed such that the TCON takes the clock from the > >> >> > >> >> TCON > >> >> > >> >> TOP > >> >> > >> >> as > >> >> > >> >> its > >> >> > >> >> channel/ module clock instead. That way you don't need this > >> >> > >> >> patch, > >> >> > >> >> but > >> >> > >> >> you'd add another for all the clock routing. > >> >> > >> > > >> >> > >> > Can you be more specific? I not sure what you mean here. > >> >> > >> > >> >> > >> For clock related properties in the device tree: > >> >> > >> > >> >> > >> &tcon_top { > >> >> > >> > >> >> > >> clocks = <&ccu CLK_BUS_TCON_TOP>, > >> >> > >> > >> >> > >> <&ccu CLK_TCON_TV0>, > >> >> > >> <&tve0>, > >> >> > >> <&ccu CLK_TCON_TV1>, > >> >> > >> <&tve1>; > >> >> > >> > >> >> > >> clock-names = "bus", "tcon-tv0", "tve0", "tcon-tv1", "tve1"; > >> >> > >> clock-output-names = "tcon-top-tv0", "tcon-top-tv1"; > >> >> > >> > >> >> > >> }; > >> >> > >> > >> >> > >> &tcon_tv0 { > >> >> > >> > >> >> > >> clocks = <&ccu CLK_BUS_TCON_TV0>, <&tcon_top 0>' > >> >> > >> clock-names = "ahb", "tcon-ch1"; > >> >> > >> > >> >> > >> }; > >> >> > >> > >> >> > >> A diagram would look like: > >> >> > >> | This part is TCON TOP | > >> >> > >> > >> >> > >> v v > >> >> > >> > >> >> > >> CCU CLK_TCON_TV0 --|----\ | > >> >> > >> > >> >> > >> | mux ---- gate ----|-- TCON_TV0 > >> >> > >> > >> >> > >> TVE0 --------------|----/ | > >> >> > >> > >> >> > >> And the same goes for TCON_TV1 and TVE1. > >> >> > >> > >> >> > >> The user manual is a bit lacking on how TVE outputs a clock > >> >> > >> though. > >> >> > > > >> >> > > I didn't yet received any response on HW details from AW till now, > >> >> > > but I > >> >> > > would like to post new version of patches soon. > >> >> > > > >> >> > > While chaining like you described could be implemented easily, I > >> >> > > don't > >> >> > > think it really represents HW as it is. Tests showed that these > >> >> > > two > >> >> > > clocks are independent, otherwise register writes/reads wouldn't > >> >> > > be > >> >> > > possible with tcon- top gate disabled. I chose tcon-top bus clock > >> >> > > as > >> >> > > a > >> >> > > parent becase if it is not enabled, it simply won't work. > >> >> > > >> >> > AFAIK with the TCONs, even when the TCON channel clock (not the bus > >> >> > clock) > >> >> > is disabled, register accesses still work. > >> >> > >> >> You're right, I just tested that. > >> >> > >> >> > I'm saying that the TCON TOP > >> >> > gate is downstream from the TCON channel clock in the CCU. These are > >> >> > not > >> >> > related to the TCON bus clock in the CCU, which affects register > >> >> > access. > >> >> > > >> >> > Did Allwinner provide any information regarding the hierarchy of the > >> >> > clocks? > >> >> > >> >> No reponse for now. > >> >> > >> >> > > However, if everyone feels chaining is the best way to implement > >> >> > > it, > >> >> > > I'll > >> >> > > do it. > >> >> > > >> >> > I would like to get it right and match actual hardware. My proposal > >> >> > is > >> >> > based on my understanding from the diagrams in the user manual. > >> >> > >> >> So for now, your explanation is the most reasonable. Should we go > >> >> ahead > >> >> and > >> >> implement your idea? > >> >> > >> >> Please note that H6 has TCON-TOP too, but it has only one LCD TCON and > >> >> one > >> >> TV TCON instead of two of each kind. That means we will have hole in > >> >> indices (tcon_lcd0 is 1, tcon_tv0 is 3, which is aligned with R40) and > >> >> different TCON- TOP binding (no tcon_tv1 channel clock), but setup is > >> >> exactly the same. > >> > > >> > I just noticed issue with this proposal. If we have following clock > >> > chain > >> > for HDMI, everythings is ok: > >> > > >> > TCON-TV0 -> TCON-TOP-TV0 > >> > > >> > TCON TV sets TCON-TOP-TV0 clock rate, which in turn sets TCON-TV0 clock > >> > and > >> > everything works. > >> > > >> > However, when TVE will be configured, it would look like this: > >> > > >> > TVE0 -> TCON-TOP-TV0 > >> > > >> > TVE driver will set TVE0 clock to 216 MHz and TCON TV would set > >> > TCON-TOP-TV0 rate which in turn sets TVE0 clock to something like 13.5 > >> > MHz (or whatever is the right clock rate for PAL and NTSC). As you can > >> > see, same clock is set to two different rates by two different drivers. > >> > > >> > It *might* still work, since encoders set clock rate after TCON (at > >> > least > >> > that is my experience for HDMI pipeline), but that is still wrong. > >> > > >> > To overcome above issue, I would stick to original proposal with > >> > additional > >> > clock specified in TCON TV DT node. That way TCON driver would always > >> > set > >> > clock rate to TCON-TV0 clock. If TVE0 is enabled, TCON wouldn't > >> > interfere > >> > with setting clock rate, because TCON-TV0 clock would be decoupled in > >> > TCON-TOP mux. > >> > > >> > What do you think? > >> > >> I think this is the wrong representation, and worse, you are trying to > >> work > >> around software issues with it. > >> > >> So to confirm some details, the TVE expects 216 MHz clock, and it expects > >> the TCON to run and output data at 216 MHz as well. Is that correct? > > > > Yes, from my understanding. 216 MHz is correct at least for PAL and NTSC, > > e.g. TV mode. TVE on R40 is also capable of RGB mode (VGA connector). > > > >> Would any settings for the TCON differ between when HDMI or TVE is used? > > > > Apart of clock, no, other settings would be the same. > > > >> Does TVE and TCON run at 216 MHz regardless of resolution? I kind of > >> doubt > >> it. It might be expecting 297 MHz for PC resolutions. > > > > Please check this table in BSP: > > https://github.com/BPI-SINOVOIP/BPI-M2U-bsp/blob/master/linux-sunxi/driver > > s/ video/sunxi/disp2/tv/drv_tv.c#L24 > > > > 216 MHz is applicable for low resolution, interlaced modes. Modes like > > 1080P, 1080I have expected standard timing. > > That's weird. So it only applies to SDTV video resolutions. I remember > seeing an "up sampling" setting for composite in the TVE, which goes all > the way up to 216 MHz. Maybe that's the reason? > Probably. If upsampling is set to 0, it still needs 27 MHz, which is 2x more than standard PAL/NTSC clock. After studying AC200 manual (which is similar TV encoder) and its driver, it seems the reason for that is 8 bit parallel interface between TCON and TVE and 16 bit data (CCIR656). However, actual tests would be needed to confirm all that. > I wonder how the TCON manages this though. I mean with the dot clock this > high, doesn't that mean the frame rate is much higher? > Not sure, but IMO it is downscaled somehow in TVE HW to get proper rates at the end. > >> I think these kinds of quirks should be handled in the software, instead > >> of > >> being papered over. > > > > Ok, that works for me too. I would just like to have such design that > > would > > later allow implementing TVE driver without much issues. > > > > BTW, H3 TV TCON which is connected to TVE doesn't have TCON-TV channel > > clock at all, since it is controlled with TVE clock (same case as it > > would be here, if TCON TOP mux is switched to TVE clock source). > > Does it require 216 MHz as well? Yes. It supports only PAL and NTSC (only one DAC), so BSP driver sets TVE clock to 216 MHz. > > > Maybe quirk can be added that it doesn't set clock rate at all if it is > > connected to TVE? > > A quirk yes. But the dot clock would be 216 MHz instead of not setting it, > and only for certain display modes. To be honest I think we can get by with > just a TODO note for now. Why not leave control of channel rate to TVE, since it knows if oversampling is enabled or not? But that's debate for another time. I will send new R40 HDMI patches according your original proposal. Best regards, Jernej -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
This series adds support for R40 HDMI pipeline. It is a bit special than other already supported pipelines because it has additional unit called TCON TOP responsible for relationship configuration between mixers, TCONs and HDMI. Additionally, it has additional gates for DSI and TV TCONs, TV encoder clock settings and pin muxing between LCD and TV encoders. However, it seems that TCON TOP will become a norm, since newer Allwinner SoCs like H6 also have this unit. I tested different possible configurations: - mixer0 <> TCON-TV0 <> HDMI - mixer0 <> TCON-TV1 <> HDMI - mixer1 <> TCON-TV0 <> HDMI - mixer1 <> TCON-TV1 <> HDMI Please review. Best regards, Jernej Changes from v1: - Split DT bindings patch and updated description - Split HDMI PHY patch - Move header file from TCON TOP patch to dt bindings patch - Added Rob reviewed-by tag - Used clk_hw_register_gate() instead of custom gate registration code - Reworked TCON TOP to be part of of-graph. Because of that, a lot of new patches were added. - Droped mixer index quirk patch - Reworked TCON support for TCON TOP - Updated commit messages Jernej Skrabec (27): clk: sunxi-ng: r40: Add minimal rate for video PLLs clk: sunxi-ng: r40: Allow setting parent rate to display related clocks clk: sunxi-ng: r40: Export video PLLs dt-bindings: display: sunxi-drm: Add TCON TOP description drm/sun4i: Add TCON TOP driver drm/sun4i: Fix releasing node when enumerating enpoints drm/sun4i: Split out code for enumerating endpoints in output port drm/sun4i: Add support for traversing graph with TCON TOP drm/sun4i: Don't skip TCONs if they don't have channel 0 dt-bindings: display: sun4i-drm: Add R40 TV TCON description drm/sun4i: tcon: Add support for tcon-top gate drm/sun4i: tcon: Generalize engine search algorithm drm/sun4i: Don't check for LVDS and RGB when TCON has only ch1 drm/sun4i: Don't check for panel or bridge on TV TCONs drm/sun4i: Add support for R40 TV TCON dt-bindings: display: sun4i-drm: Add R40 mixer compatibles drm/sun4i: Add support for R40 mixers dt-bindings: display: sun4i-drm: Add description of A64 HDMI PHY drm/sun4i: Enable DW HDMI PHY clock drm/sun4i: Don't change clock bits in DW HDMI PHY driver drm/sun4i: DW HDMI PHY: Add support for second PLL drm/sun4i: Add support for second clock parent to DW HDMI PHY clk driver drm/sun4i: Add support for A64 HDMI PHY drm: of: Export drm_crtc_port_mask() drm/sun4i: DW HDMI: Expand algorithm for possible crtcs ARM: dts: sun8i: r40: Add HDMI pipeline ARM: dts: sun8i: r40: Enable HDMI output on BananaPi M2 Ultra .../bindings/display/sunxi/sun4i-drm.txt | 56 +++- .../boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 45 +++ arch/arm/boot/dts/sun8i-r40.dtsi | 257 ++++++++++++++++++ drivers/clk/sunxi-ng/ccu-sun8i-r40.c | 58 ++-- drivers/clk/sunxi-ng/ccu-sun8i-r40.h | 8 +- drivers/gpu/drm/drm_of.c | 4 +- drivers/gpu/drm/sun4i/Makefile | 3 +- drivers/gpu/drm/sun4i/sun4i_drv.c | 121 ++++++--- drivers/gpu/drm/sun4i/sun4i_tcon.c | 83 ++++-- drivers/gpu/drm/sun4i/sun4i_tcon.h | 4 + drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 46 +++- drivers/gpu/drm/sun4i/sun8i_dw_hdmi.h | 8 +- drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 54 +++- drivers/gpu/drm/sun4i/sun8i_hdmi_phy_clk.c | 90 ++++-- drivers/gpu/drm/sun4i/sun8i_mixer.c | 24 ++ drivers/gpu/drm/sun4i/sun8i_tcon_top.c | 248 +++++++++++++++++ drivers/gpu/drm/sun4i/sun8i_tcon_top.h | 38 +++ include/drm/drm_of.h | 8 + include/dt-bindings/clock/sun8i-r40-ccu.h | 4 + include/dt-bindings/clock/sun8i-tcon-top.h | 11 + 20 files changed, 1047 insertions(+), 123 deletions(-) create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.c create mode 100644 drivers/gpu/drm/sun4i/sun8i_tcon_top.h create mode 100644 include/dt-bindings/clock/sun8i-tcon-top.h