2012-11-09 13:20:09

by Thierry Reding

[permalink] [raw]
Subject: [PATCH 0/2] Device tree updates for host1x support

This set of patches are in preparation for the Tegra DRM driver. They
add the necessary nodes to the DTSI files and setup the clocks necessary
for host1x and the display controllers to work properly. The AUXDATA
table is updated with the entries for the newly added nodes and the PLL
frequency tables are extended with some frequencies required to make a
few of the more common LVDS panel and HDMI display modes work.

By default, all output resources (RGB, HDMI, DSI and TVO) are disabled,
and individual boards need to enable those that they require.

Note that there has been talk to replace the frequency tables by an
algorithmic computation of the dividers in order to avoid endless
additions to these tables. No code has surfaced yet, but eventually this
should replace the static tables.

Thierry

Thierry Reding (2):
ARM: tegra: Add Tegra20 host1x support
ARM: tegra: Add Tegra30 host1x support

arch/arm/boot/dts/tegra20.dtsi | 87 +++++++++++++++++++++++++++++++
arch/arm/boot/dts/tegra30.dtsi | 87 +++++++++++++++++++++++++++++++
arch/arm/mach-tegra/board-dt-tegra20.c | 9 ++++
arch/arm/mach-tegra/board-dt-tegra30.c | 10 ++++
arch/arm/mach-tegra/tegra20_clocks_data.c | 23 ++++----
arch/arm/mach-tegra/tegra30_clocks_data.c | 11 ++--
6 files changed, 214 insertions(+), 13 deletions(-)

--
1.8.0


2012-11-09 13:20:11

by Thierry Reding

[permalink] [raw]
Subject: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

This commit adds the host1x node along with its children to the Tegra20
DTSI. Furthermore the OF auxiliary data table is updated to have proper
names assigned to the platform devices instantiated from the device
tree. Moreover, the clocks required by host1x and the two display
controllers are initialized and the pll_d frequency table is completed
with a few entries to support common HDMI and LVDS display modes.

Signed-off-by: Thierry Reding <[email protected]>
---
arch/arm/boot/dts/tegra20.dtsi | 87 +++++++++++++++++++++++++++++++
arch/arm/mach-tegra/board-dt-tegra20.c | 9 ++++
arch/arm/mach-tegra/tegra20_clocks_data.c | 23 ++++----
3 files changed, 110 insertions(+), 9 deletions(-)

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 71a650d..2b4dec9 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -4,6 +4,93 @@
compatible = "nvidia,tegra20";
interrupt-parent = <&intc>;

+ host1x {
+ compatible = "nvidia,tegra20-host1x", "simple-bus";
+ reg = <0x50000000 0x00024000>;
+ interrupts = <0 65 0x04 /* mpcore syncpt */
+ 0 67 0x04>; /* mpcore general */
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ ranges = <0x54000000 0x54000000 0x04000000>;
+
+ mpe {
+ compatible = "nvidia,tegra20-mpe";
+ reg = <0x54040000 0x00040000>;
+ interrupts = <0 68 0x04>;
+ };
+
+ vi {
+ compatible = "nvidia,tegra20-vi";
+ reg = <0x54080000 0x00040000>;
+ interrupts = <0 69 0x04>;
+ };
+
+ epp {
+ compatible = "nvidia,tegra20-epp";
+ reg = <0x540c0000 0x00040000>;
+ interrupts = <0 70 0x04>;
+ };
+
+ isp {
+ compatible = "nvidia,tegra20-isp";
+ reg = <0x54100000 0x00040000>;
+ interrupts = <0 71 0x04>;
+ };
+
+ gr2d {
+ compatible = "nvidia,tegra20-gr2d";
+ reg = <0x54140000 0x00040000>;
+ interrupts = <0 72 0x04>;
+ };
+
+ gr3d {
+ compatible = "nvidia,tegra20-gr3d";
+ reg = <0x54180000 0x00040000>;
+ };
+
+ dc@54200000 {
+ compatible = "nvidia,tegra20-dc";
+ reg = <0x54200000 0x00040000>;
+ interrupts = <0 73 0x04>;
+
+ rgb {
+ status = "disabled";
+ };
+ };
+
+ dc@54240000 {
+ compatible = "nvidia,tegra20-dc";
+ reg = <0x54240000 0x00040000>;
+ interrupts = <0 74 0x04>;
+
+ rgb {
+ status = "disabled";
+ };
+ };
+
+ hdmi {
+ compatible = "nvidia,tegra20-hdmi";
+ reg = <0x54280000 0x00040000>;
+ interrupts = <0 75 0x04>;
+ status = "disabled";
+ };
+
+ tvo {
+ compatible = "nvidia,tegra20-tvo";
+ reg = <0x542c0000 0x00040000>;
+ interrupts = <0 76 0x04>;
+ status = "disabled";
+ };
+
+ dsi {
+ compatible = "nvidia,tegra20-dsi";
+ reg = <0x54300000 0x00040000>;
+ status = "disabled";
+ };
+ };
+
timer@50004600 {
compatible = "arm,cortex-a9-twd-timer";
reg = <0x50040600 0x20>;
diff --git a/arch/arm/mach-tegra/board-dt-tegra20.c b/arch/arm/mach-tegra/board-dt-tegra20.c
index 22f5a9b..1afbdd1 100644
--- a/arch/arm/mach-tegra/board-dt-tegra20.c
+++ b/arch/arm/mach-tegra/board-dt-tegra20.c
@@ -93,6 +93,12 @@ struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = {
OF_DEV_AUXDATA("nvidia,tegra20-slink", 0x7000D600, "spi_tegra.1", NULL),
OF_DEV_AUXDATA("nvidia,tegra20-slink", 0x7000D800, "spi_tegra.2", NULL),
OF_DEV_AUXDATA("nvidia,tegra20-slink", 0x7000DA00, "spi_tegra.3", NULL),
+ OF_DEV_AUXDATA("nvidia,tegra20-host1x", 0x50000000, "tegra-host1x", NULL),
+ OF_DEV_AUXDATA("nvidia,tegra20-dc", 0x54200000, "tegra-dc.0", NULL),
+ OF_DEV_AUXDATA("nvidia,tegra20-dc", 0x54240000, "tegra-dc.1", NULL),
+ OF_DEV_AUXDATA("nvidia,tegra20-hdmi", 0x54280000, "tegra-hdmi", NULL),
+ OF_DEV_AUXDATA("nvidia,tegra20-dsi", 0x54300000, "tegra-dsi", NULL),
+ OF_DEV_AUXDATA("nvidia,tegra20-tvo", 0x542c0000, "tegra-tvo", NULL),
{}
};

@@ -116,6 +122,9 @@ static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
{ "sbc2", "pll_p", 100000000, false },
{ "sbc3", "pll_p", 100000000, false },
{ "sbc4", "pll_p", 100000000, false },
+ { "host1x", "pll_c", 144000000, false },
+ { "disp1", "pll_p", 600000000, false },
+ { "disp2", "pll_p", 600000000, false },
{ NULL, NULL, 0, 0},
};

diff --git a/arch/arm/mach-tegra/tegra20_clocks_data.c b/arch/arm/mach-tegra/tegra20_clocks_data.c
index 9615ee3..2e247f2 100644
--- a/arch/arm/mach-tegra/tegra20_clocks_data.c
+++ b/arch/arm/mach-tegra/tegra20_clocks_data.c
@@ -246,11 +246,16 @@ static struct clk_pll_freq_table tegra_pll_d_freq_table[] = {
{ 19200000, 216000000, 135, 12, 1, 3},
{ 26000000, 216000000, 216, 26, 1, 4},

+ { 12000000, 297000000, 99, 4, 1, 4 },
+ { 12000000, 339000000, 113, 4, 1, 4 },
+
{ 12000000, 594000000, 594, 12, 1, 8},
{ 13000000, 594000000, 594, 13, 1, 8},
{ 19200000, 594000000, 495, 16, 1, 8},
{ 26000000, 594000000, 594, 26, 1, 8},

+ { 12000000, 616000000, 616, 12, 1, 8},
+
{ 12000000, 1000000000, 1000, 12, 1, 12},
{ 13000000, 1000000000, 1000, 13, 1, 12},
{ 19200000, 1000000000, 625, 12, 1, 8},
@@ -930,17 +935,17 @@ PERIPH_CLK(vi, "tegra_camera", "vi", 20, 0x148, 150000000, mux_pllm_pllc_pllp_
PERIPH_CLK(vi_sensor, "tegra_camera", "vi_sensor", 20, 0x1a8, 150000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | PERIPH_NO_RESET); /* scales with voltage and process_id */
PERIPH_CLK(epp, "epp", NULL, 19, 0x16c, 300000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
PERIPH_CLK(mpe, "mpe", NULL, 60, 0x170, 250000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
-PERIPH_CLK(host1x, "host1x", NULL, 28, 0x180, 166000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
+PERIPH_CLK(host1x, "tegra-host1x", NULL, 28, 0x180, 166000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
PERIPH_CLK(cve, "cve", NULL, 49, 0x140, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
-PERIPH_CLK(tvo, "tvo", NULL, 49, 0x188, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
-PERIPH_CLK(hdmi, "hdmi", NULL, 51, 0x18c, 600000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
+PERIPH_CLK(tvo, "tegra-tvo", NULL, 49, 0x188, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
+PERIPH_CLK(hdmi, "tegra-hdmi", NULL, 51, 0x18c, 600000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
PERIPH_CLK(tvdac, "tvdac", NULL, 53, 0x194, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
-PERIPH_CLK(disp1, "tegradc.0", NULL, 27, 0x138, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
-PERIPH_CLK(disp2, "tegradc.1", NULL, 26, 0x13c, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
+PERIPH_CLK(disp1, "tegra-dc.0", NULL, 27, 0x138, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
+PERIPH_CLK(disp2, "tegra-dc.1", NULL, 26, 0x13c, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
PERIPH_CLK(usbd, "fsl-tegra-udc", NULL, 22, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
PERIPH_CLK(usb2, "tegra-ehci.1", NULL, 58, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
PERIPH_CLK(usb3, "tegra-ehci.2", NULL, 59, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
-PERIPH_CLK(dsi, "dsi", NULL, 48, 0, 500000000, mux_plld, 0); /* scales with voltage */
+PERIPH_CLK(dsi, "tegra-dsi", NULL, 48, 0, 500000000, mux_plld, 0); /* scales with voltage */
PERIPH_CLK(csi, "tegra_camera", "csi", 52, 0, 72000000, mux_pllp_out3, 0);
PERIPH_CLK(isp, "tegra_camera", "isp", 23, 0, 150000000, mux_clk_m, 0); /* same frequency as VI */
PERIPH_CLK(csus, "tegra_camera", "csus", 92, 0, 150000000, mux_clk_m, PERIPH_NO_RESET);
@@ -1036,9 +1041,6 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
CLK_DUPLICATE("usbd", "utmip-pad", NULL),
CLK_DUPLICATE("usbd", "tegra-ehci.0", NULL),
CLK_DUPLICATE("usbd", "tegra-otg", NULL),
- CLK_DUPLICATE("hdmi", "tegradc.0", "hdmi"),
- CLK_DUPLICATE("hdmi", "tegradc.1", "hdmi"),
- CLK_DUPLICATE("host1x", "tegra_grhost", "host1x"),
CLK_DUPLICATE("2d", "tegra_grhost", "gr2d"),
CLK_DUPLICATE("3d", "tegra_grhost", "gr3d"),
CLK_DUPLICATE("epp", "tegra_grhost", "epp"),
@@ -1051,6 +1053,9 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
CLK_DUPLICATE("pll_p_out3", "tegra-i2c.1", "fast-clk"),
CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
+ CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
+ CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
+ CLK_DUPLICATE("pll_d_out0", "tegra-hdmi", "parent"),
};

#define CLK(dev, con, ck) \
--
1.8.0

2012-11-09 13:20:22

by Thierry Reding

[permalink] [raw]
Subject: [PATCH 2/2] ARM: tegra: Add Tegra30 host1x support

This commit adds the host1x node along with its children to the Tegra30
DTSI. Furthermore the OF auxiliary data table is updated to have proper
names assigned to the platform devices instantiated from the device
tree. Moreover, the clocks required by host1x and the two display
controllers are setup and initialized.

Signed-off-by: Thierry Reding <[email protected]>
---
arch/arm/boot/dts/tegra30.dtsi | 87 +++++++++++++++++++++++++++++++
arch/arm/mach-tegra/board-dt-tegra30.c | 10 ++++
arch/arm/mach-tegra/tegra30_clocks_data.c | 11 ++--
3 files changed, 104 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index b8e33c3..529fdb8 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -4,6 +4,93 @@
compatible = "nvidia,tegra30";
interrupt-parent = <&intc>;

+ host1x {
+ compatible = "nvidia,tegra30-host1x", "simple-bus";
+ reg = <0x50000000 0x00024000>;
+ interrupts = <0 65 0x04 /* mpcore syncpt */
+ 0 67 0x04>; /* mpcore general */
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ ranges = <0x54000000 0x54000000 0x04000000>;
+
+ mpe {
+ compatible = "nvidia,tegra30-mpe";
+ reg = <0x54040000 0x00040000>;
+ interrupts = <0 68 0x04>;
+ };
+
+ vi {
+ compatible = "nvidia,tegra30-vi";
+ reg = <0x54080000 0x00040000>;
+ interrupts = <0 69 0x04>;
+ };
+
+ epp {
+ compatible = "nvidia,tegra30-epp";
+ reg = <0x540c0000 0x00040000>;
+ interrupts = <0 70 0x04>;
+ };
+
+ isp {
+ compatible = "nvidia,tegra30-isp";
+ reg = <0x54100000 0x00040000>;
+ interrupts = <0 71 0x04>;
+ };
+
+ gr2d {
+ compatible = "nvidia,tegra30-gr2d";
+ reg = <0x54140000 0x00040000>;
+ interrupts = <0 72 0x04>;
+ };
+
+ gr3d {
+ compatible = "nvidia,tegra30-gr3d";
+ reg = <0x54180000 0x00040000>;
+ };
+
+ dc@54200000 {
+ compatible = "nvidia,tegra30-dc";
+ reg = <0x54200000 0x00040000>;
+ interrupts = <0 73 0x04>;
+
+ rgb {
+ status = "disabled";
+ };
+ };
+
+ dc@54240000 {
+ compatible = "nvidia,tegra30-dc";
+ reg = <0x54240000 0x00040000>;
+ interrupts = <0 74 0x04>;
+
+ rgb {
+ status = "disabled";
+ };
+ };
+
+ hdmi {
+ compatible = "nvidia,tegra30-hdmi";
+ reg = <0x54280000 0x00040000>;
+ interrupts = <0 75 0x04>;
+ status = "disabled";
+ };
+
+ tvo {
+ compatible = "nvidia,tegra30-tvo";
+ reg = <0x542c0000 0x00040000>;
+ interrupts = <0 76 0x04>;
+ status = "disabled";
+ };
+
+ dsi {
+ compatible = "nvidia,tegra30-dsi";
+ reg = <0x54300000 0x00040000>;
+ status = "disabled";
+ };
+ };
+
timer@50004600 {
compatible = "arm,cortex-a9-twd-timer";
reg = <0x50040600 0x20>;
diff --git a/arch/arm/mach-tegra/board-dt-tegra30.c b/arch/arm/mach-tegra/board-dt-tegra30.c
index cd30338..ff21107 100644
--- a/arch/arm/mach-tegra/board-dt-tegra30.c
+++ b/arch/arm/mach-tegra/board-dt-tegra30.c
@@ -57,6 +57,12 @@ struct of_dev_auxdata tegra30_auxdata_lookup[] __initdata = {
OF_DEV_AUXDATA("nvidia,tegra30-slink", 0x7000DA00, "spi_tegra.3", NULL),
OF_DEV_AUXDATA("nvidia,tegra30-slink", 0x7000DC00, "spi_tegra.4", NULL),
OF_DEV_AUXDATA("nvidia,tegra30-slink", 0x7000DE00, "spi_tegra.5", NULL),
+ OF_DEV_AUXDATA("nvidia,tegra30-host1x", 0x50000000, "tegra-host1x", NULL),
+ OF_DEV_AUXDATA("nvidia,tegra30-dc", 0x54200000, "tegra-dc.0", NULL),
+ OF_DEV_AUXDATA("nvidia,tegra30-dc", 0x54240000, "tegra-dc.1", NULL),
+ OF_DEV_AUXDATA("nvidia,tegra30-hdmi", 0x54280000, "tegra-hdmi", NULL),
+ OF_DEV_AUXDATA("nvidia,tegra30-dsi", 0x54300000, "tegra-dsi", NULL),
+ OF_DEV_AUXDATA("nvidia,tegra30-tvo", 0x542c0000, "tegra-tvo", NULL),
{}
};

@@ -82,6 +88,10 @@ static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
{ "sbc4", "pll_p", 100000000, false},
{ "sbc5", "pll_p", 100000000, false},
{ "sbc6", "pll_p", 100000000, false},
+ { "host1x", "pll_c", 144000000, false},
+ { "disp1", "pll_p", 74000000, false},
+ { "disp2", "pll_d2_out0", 148500000, false},
+ { "hdmi", "pll_d2_out0", 148500000, false},
{ NULL, NULL, 0, 0},
};

diff --git a/arch/arm/mach-tegra/tegra30_clocks_data.c b/arch/arm/mach-tegra/tegra30_clocks_data.c
index 7bc8b1d..210b8a4 100644
--- a/arch/arm/mach-tegra/tegra30_clocks_data.c
+++ b/arch/arm/mach-tegra/tegra30_clocks_data.c
@@ -1132,14 +1132,14 @@ PERIPH_CLK(2d, "2d", NULL, 21, 0x15c, 520000000, mux_pllm_pllc_pllp_plla, MUX
PERIPH_CLK(vi_sensor, "tegra_camera", "vi_sensor", 20, 0x1a8, 150000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | PERIPH_NO_RESET);
PERIPH_CLK(epp, "epp", NULL, 19, 0x16c, 520000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | DIV_U71_INT);
PERIPH_CLK(mpe, "mpe", NULL, 60, 0x170, 520000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | DIV_U71_INT);
-PERIPH_CLK(host1x, "host1x", NULL, 28, 0x180, 260000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | DIV_U71_INT);
+PERIPH_CLK(host1x, "tegra-host1x", NULL, 28, 0x180, 260000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | DIV_U71_INT);
PERIPH_CLK(cve, "cve", NULL, 49, 0x140, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
PERIPH_CLK(tvo, "tvo", NULL, 49, 0x188, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
PERIPH_CLK(dtv, "dtv", NULL, 79, 0x1dc, 250000000, mux_clk_m, 0);
-PERIPH_CLK(hdmi, "hdmi", NULL, 51, 0x18c, 148500000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8 | DIV_U71);
+PERIPH_CLK(hdmi, "tegra-hdmi", NULL, 51, 0x18c, 148500000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8 | DIV_U71);
PERIPH_CLK(tvdac, "tvdac", NULL, 53, 0x194, 220000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
-PERIPH_CLK(disp1, "tegradc.0", NULL, 27, 0x138, 600000000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8);
-PERIPH_CLK(disp2, "tegradc.1", NULL, 26, 0x13c, 600000000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8);
+PERIPH_CLK(disp1, "tegra-dc.0", NULL, 27, 0x138, 600000000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8);
+PERIPH_CLK(disp2, "tegra-dc.1", NULL, 26, 0x13c, 600000000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8);
PERIPH_CLK(usbd, "fsl-tegra-udc", NULL, 22, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
PERIPH_CLK(usb2, "tegra-ehci.1", NULL, 58, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
PERIPH_CLK(usb3, "tegra-ehci.2", NULL, 59, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
@@ -1337,6 +1337,9 @@ struct clk_duplicate tegra_clk_duplicates[] = {
CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
CLK_DUPLICATE("pll_p_out3", "tegra-i2c.4", "fast-clk"),
+ CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
+ CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
+ CLK_DUPLICATE("pll_d2_out0", "tegra-hdmi", "parent"),
};

struct clk *tegra_ptr_clks[] = {
--
1.8.0

2012-11-09 17:34:45

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 0/2] Device tree updates for host1x support

On 11/09/2012 06:20 AM, Thierry Reding wrote:
> This set of patches are in preparation for the Tegra DRM driver. They
> add the necessary nodes to the DTSI files and setup the clocks necessary
> for host1x and the display controllers to work properly. The AUXDATA
> table is updated with the entries for the newly added nodes and the PLL
> frequency tables are extended with some frequencies required to make a
> few of the more common LVDS panel and HDMI display modes work.
>
> By default, all output resources (RGB, HDMI, DSI and TVO) are disabled,
> and individual boards need to enable those that they require.
>
> Note that there has been talk to replace the frequency tables by an
> algorithmic computation of the dividers in order to avoid endless
> additions to these tables. No code has surfaced yet, but eventually this
> should replace the static tables.

This basically looks reasonable.

I assume the DT bindings are documented in the tegradrm driver series? I
haven't looked at that yet.

I'd like to see explicit acks for the DT changes from e.g. Terje and
Mark Zhang, and anyone else involved in previous discussions about them.

Could you please split up the patches a little differently? I'd like to see:

1) Changes to the clock driver
2) Changes to add AUXDATA
3) Changes to .dts files

The reason being that these are logically separate changes. (1) and (2)
would be applied to the "soc" branch, and (3) to the "dt" branch.

Finally, I'd prefer not to rename the match parameters in the clock
drivers any more; just set up the AUXDATA to match whatever driver names
the clock driver is expecting. That will reduce churn to the clock
driver. I really hope soon that Tegra will support DT bindings for
clocks so we can get rid of the AUXDATA, but unfortunately I haven't
seen any movement towards this:-(

Thanks.

2012-11-09 18:44:20

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 0/2] Device tree updates for host1x support

On Fri, Nov 09, 2012 at 10:34:41AM -0700, Stephen Warren wrote:
> On 11/09/2012 06:20 AM, Thierry Reding wrote:
> > This set of patches are in preparation for the Tegra DRM driver. They
> > add the necessary nodes to the DTSI files and setup the clocks necessary
> > for host1x and the display controllers to work properly. The AUXDATA
> > table is updated with the entries for the newly added nodes and the PLL
> > frequency tables are extended with some frequencies required to make a
> > few of the more common LVDS panel and HDMI display modes work.
> >
> > By default, all output resources (RGB, HDMI, DSI and TVO) are disabled,
> > and individual boards need to enable those that they require.
> >
> > Note that there has been talk to replace the frequency tables by an
> > algorithmic computation of the dividers in order to avoid endless
> > additions to these tables. No code has surfaced yet, but eventually this
> > should replace the static tables.
>
> This basically looks reasonable.
>
> I assume the DT bindings are documented in the tegradrm driver series? I
> haven't looked at that yet.
>
> I'd like to see explicit acks for the DT changes from e.g. Terje and
> Mark Zhang, and anyone else involved in previous discussions about them.
>
> Could you please split up the patches a little differently? I'd like to see:
>
> 1) Changes to the clock driver
> 2) Changes to add AUXDATA
> 3) Changes to .dts files
>
> The reason being that these are logically separate changes. (1) and (2)
> would be applied to the "soc" branch, and (3) to the "dt" branch.

I'll split the patches up some more then.

> Finally, I'd prefer not to rename the match parameters in the clock
> drivers any more; just set up the AUXDATA to match whatever driver names
> the clock driver is expecting. That will reduce churn to the clock
> driver.

I updated the table mainly because the devices were named very
inconsistently. For instance, the output resources and host1x didn't
have the tegra- prefix, but the display controllers did. I still think
there's enough value in having a consistent set of device names, but
this is your tree, so I'll drop those changes and update the AUXDATA
table if that's what you want.

> I really hope soon that Tegra will support DT bindings for
> clocks so we can get rid of the AUXDATA, but unfortunately I haven't
> seen any movement towards this:-(

Oh yes, please. We wouldn't have any of the above problem if we had
proper DT bindings for this. Who needs to be prodded to speed this up?

Thierry


Attachments:
(No filename) (2.48 kB)
(No filename) (836.00 B)
Download all attachments

2012-11-09 21:10:35

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 0/2] Device tree updates for host1x support

On 11/09/2012 11:44 AM, Thierry Reding wrote:
> On Fri, Nov 09, 2012 at 10:34:41AM -0700, Stephen Warren wrote:
...
>> I really hope soon that Tegra will support DT bindings for clocks
>> so we can get rid of the AUXDATA, but unfortunately I haven't
>> seen any movement towards this:-(
>
> Oh yes, please. We wouldn't have any of the above problem if we
> had proper DT bindings for this. Who needs to be prodded to speed
> this up?

I believe one of Peter De Schrijver or Prashant Gaikwad are supposed
to be working on this.

2012-11-12 09:39:24

by Mark Zhang

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 11/09/2012 09:20 PM, Thierry Reding wrote:
> This commit adds the host1x node along with its children to the Tegra20
> DTSI. Furthermore the OF auxiliary data table is updated to have proper
> names assigned to the platform devices instantiated from the device
> tree. Moreover, the clocks required by host1x and the two display
> controllers are initialized and the pll_d frequency table is completed
> with a few entries to support common HDMI and LVDS display modes.
>
> Signed-off-by: Thierry Reding <[email protected]>
> ---
> arch/arm/boot/dts/tegra20.dtsi | 87 +++++++++++++++++++++++++++++++
> arch/arm/mach-tegra/board-dt-tegra20.c | 9 ++++
> arch/arm/mach-tegra/tegra20_clocks_data.c | 23 ++++----
> 3 files changed, 110 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 71a650d..2b4dec9 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -4,6 +4,93 @@
> compatible = "nvidia,tegra20";
> interrupt-parent = <&intc>;
>
> + host1x {
> + compatible = "nvidia,tegra20-host1x", "simple-bus";
> + reg = <0x50000000 0x00024000>;
> + interrupts = <0 65 0x04 /* mpcore syncpt */
> + 0 67 0x04>; /* mpcore general */
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + ranges = <0x54000000 0x54000000 0x04000000>;
> +
> + mpe {
> + compatible = "nvidia,tegra20-mpe";
> + reg = <0x54040000 0x00040000>;
> + interrupts = <0 68 0x04>;
> + };
> +
> + vi {
> + compatible = "nvidia,tegra20-vi";
> + reg = <0x54080000 0x00040000>;
> + interrupts = <0 69 0x04>;
> + };
> +
> + epp {
> + compatible = "nvidia,tegra20-epp";
> + reg = <0x540c0000 0x00040000>;
> + interrupts = <0 70 0x04>;
> + };
> +
> + isp {
> + compatible = "nvidia,tegra20-isp";
> + reg = <0x54100000 0x00040000>;
> + interrupts = <0 71 0x04>;
> + };
> +
> + gr2d {
> + compatible = "nvidia,tegra20-gr2d";
> + reg = <0x54140000 0x00040000>;
> + interrupts = <0 72 0x04>;
> + };
> +
> + gr3d {
> + compatible = "nvidia,tegra20-gr3d";
> + reg = <0x54180000 0x00040000>;
> + };
> +
> + dc@54200000 {
> + compatible = "nvidia,tegra20-dc";
> + reg = <0x54200000 0x00040000>;
> + interrupts = <0 73 0x04>;
> +
> + rgb {
> + status = "disabled";
> + };
> + };
> +
> + dc@54240000 {
> + compatible = "nvidia,tegra20-dc";
> + reg = <0x54240000 0x00040000>;
> + interrupts = <0 74 0x04>;
> +
> + rgb {
> + status = "disabled";
> + };
> + };
> +
> + hdmi {
> + compatible = "nvidia,tegra20-hdmi";
> + reg = <0x54280000 0x00040000>;
> + interrupts = <0 75 0x04>;
> + status = "disabled";
> + };
> +
> + tvo {
> + compatible = "nvidia,tegra20-tvo";
> + reg = <0x542c0000 0x00040000>;
> + interrupts = <0 76 0x04>;
> + status = "disabled";
> + };
> +
> + dsi {
> + compatible = "nvidia,tegra20-dsi";
> + reg = <0x54300000 0x00040000>;
> + status = "disabled";
> + };
> + };
> +
> timer@50004600 {
> compatible = "arm,cortex-a9-twd-timer";
> reg = <0x50040600 0x20>;
> diff --git a/arch/arm/mach-tegra/board-dt-tegra20.c b/arch/arm/mach-tegra/board-dt-tegra20.c
> index 22f5a9b..1afbdd1 100644
> --- a/arch/arm/mach-tegra/board-dt-tegra20.c
> +++ b/arch/arm/mach-tegra/board-dt-tegra20.c
> @@ -93,6 +93,12 @@ struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = {
> OF_DEV_AUXDATA("nvidia,tegra20-slink", 0x7000D600, "spi_tegra.1", NULL),
> OF_DEV_AUXDATA("nvidia,tegra20-slink", 0x7000D800, "spi_tegra.2", NULL),
> OF_DEV_AUXDATA("nvidia,tegra20-slink", 0x7000DA00, "spi_tegra.3", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra20-host1x", 0x50000000, "tegra-host1x", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra20-dc", 0x54200000, "tegra-dc.0", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra20-dc", 0x54240000, "tegra-dc.1", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra20-hdmi", 0x54280000, "tegra-hdmi", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra20-dsi", 0x54300000, "tegra-dsi", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra20-tvo", 0x542c0000, "tegra-tvo", NULL),
> {}
> };
>
> @@ -116,6 +122,9 @@ static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
> { "sbc2", "pll_p", 100000000, false },
> { "sbc3", "pll_p", 100000000, false },
> { "sbc4", "pll_p", 100000000, false },
> + { "host1x", "pll_c", 144000000, false },
> + { "disp1", "pll_p", 600000000, false },
> + { "disp2", "pll_p", 600000000, false },
> { NULL, NULL, 0, 0},
> };
>
> diff --git a/arch/arm/mach-tegra/tegra20_clocks_data.c b/arch/arm/mach-tegra/tegra20_clocks_data.c
> index 9615ee3..2e247f2 100644
> --- a/arch/arm/mach-tegra/tegra20_clocks_data.c
> +++ b/arch/arm/mach-tegra/tegra20_clocks_data.c
> @@ -246,11 +246,16 @@ static struct clk_pll_freq_table tegra_pll_d_freq_table[] = {
> { 19200000, 216000000, 135, 12, 1, 3},
> { 26000000, 216000000, 216, 26, 1, 4},
>
> + { 12000000, 297000000, 99, 4, 1, 4 },
> + { 12000000, 339000000, 113, 4, 1, 4 },
> +

The patch is OK. I'm just curious about how you get the cpcon value
here. According to Tegra 2's TRM, it's not very clear about how to
calculate the cpcon value for pll_d(pll_d is type of CLKPLL1G_MIPI).

I've skimmed the l4t codes as well, the cpcon is hardcode to 8 in the
dynamic m/n/p calculating code blocks.

So, Peter or Prashant, could you help to take a look at this?

> { 12000000, 594000000, 594, 12, 1, 8},
> { 13000000, 594000000, 594, 13, 1, 8},
> { 19200000, 594000000, 495, 16, 1, 8},
> { 26000000, 594000000, 594, 26, 1, 8},
>
> + { 12000000, 616000000, 616, 12, 1, 8},
> +
> { 12000000, 1000000000, 1000, 12, 1, 12},
> { 13000000, 1000000000, 1000, 13, 1, 12},
> { 19200000, 1000000000, 625, 12, 1, 8},
> @@ -930,17 +935,17 @@ PERIPH_CLK(vi, "tegra_camera", "vi", 20, 0x148, 150000000, mux_pllm_pllc_pllp_
> PERIPH_CLK(vi_sensor, "tegra_camera", "vi_sensor", 20, 0x1a8, 150000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | PERIPH_NO_RESET); /* scales with voltage and process_id */
> PERIPH_CLK(epp, "epp", NULL, 19, 0x16c, 300000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
> PERIPH_CLK(mpe, "mpe", NULL, 60, 0x170, 250000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
> -PERIPH_CLK(host1x, "host1x", NULL, 28, 0x180, 166000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
> +PERIPH_CLK(host1x, "tegra-host1x", NULL, 28, 0x180, 166000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
> PERIPH_CLK(cve, "cve", NULL, 49, 0x140, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> -PERIPH_CLK(tvo, "tvo", NULL, 49, 0x188, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> -PERIPH_CLK(hdmi, "hdmi", NULL, 51, 0x18c, 600000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> +PERIPH_CLK(tvo, "tegra-tvo", NULL, 49, 0x188, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> +PERIPH_CLK(hdmi, "tegra-hdmi", NULL, 51, 0x18c, 600000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> PERIPH_CLK(tvdac, "tvdac", NULL, 53, 0x194, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> -PERIPH_CLK(disp1, "tegradc.0", NULL, 27, 0x138, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
> -PERIPH_CLK(disp2, "tegradc.1", NULL, 26, 0x13c, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
> +PERIPH_CLK(disp1, "tegra-dc.0", NULL, 27, 0x138, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
> +PERIPH_CLK(disp2, "tegra-dc.1", NULL, 26, 0x13c, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
> PERIPH_CLK(usbd, "fsl-tegra-udc", NULL, 22, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
> PERIPH_CLK(usb2, "tegra-ehci.1", NULL, 58, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
> PERIPH_CLK(usb3, "tegra-ehci.2", NULL, 59, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
> -PERIPH_CLK(dsi, "dsi", NULL, 48, 0, 500000000, mux_plld, 0); /* scales with voltage */
> +PERIPH_CLK(dsi, "tegra-dsi", NULL, 48, 0, 500000000, mux_plld, 0); /* scales with voltage */
> PERIPH_CLK(csi, "tegra_camera", "csi", 52, 0, 72000000, mux_pllp_out3, 0);
> PERIPH_CLK(isp, "tegra_camera", "isp", 23, 0, 150000000, mux_clk_m, 0); /* same frequency as VI */
> PERIPH_CLK(csus, "tegra_camera", "csus", 92, 0, 150000000, mux_clk_m, PERIPH_NO_RESET);
> @@ -1036,9 +1041,6 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
> CLK_DUPLICATE("usbd", "utmip-pad", NULL),
> CLK_DUPLICATE("usbd", "tegra-ehci.0", NULL),
> CLK_DUPLICATE("usbd", "tegra-otg", NULL),
> - CLK_DUPLICATE("hdmi", "tegradc.0", "hdmi"),
> - CLK_DUPLICATE("hdmi", "tegradc.1", "hdmi"),
> - CLK_DUPLICATE("host1x", "tegra_grhost", "host1x"),
> CLK_DUPLICATE("2d", "tegra_grhost", "gr2d"),
> CLK_DUPLICATE("3d", "tegra_grhost", "gr3d"),
> CLK_DUPLICATE("epp", "tegra_grhost", "epp"),
> @@ -1051,6 +1053,9 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.1", "fast-clk"),
> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
> + CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
> + CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
> + CLK_DUPLICATE("pll_d_out0", "tegra-hdmi", "parent"),
> };
>
> #define CLK(dev, con, ck) \
> --
> 1.8.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2012-11-13 04:37:31

by Mark Zhang

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: tegra: Add Tegra30 host1x support

On 11/09/2012 09:20 PM, Thierry Reding wrote:
> This commit adds the host1x node along with its children to the Tegra30
> DTSI. Furthermore the OF auxiliary data table is updated to have proper
> names assigned to the platform devices instantiated from the device
> tree. Moreover, the clocks required by host1x and the two display
> controllers are setup and initialized.
>
> Signed-off-by: Thierry Reding <[email protected]>
> ---
> arch/arm/boot/dts/tegra30.dtsi | 87 +++++++++++++++++++++++++++++++
> arch/arm/mach-tegra/board-dt-tegra30.c | 10 ++++
> arch/arm/mach-tegra/tegra30_clocks_data.c | 11 ++--
> 3 files changed, 104 insertions(+), 4 deletions(-)
>
...
>
> diff --git a/arch/arm/mach-tegra/tegra30_clocks_data.c b/arch/arm/mach-tegra/tegra30_clocks_data.c
> index 7bc8b1d..210b8a4 100644
> --- a/arch/arm/mach-tegra/tegra30_clocks_data.c
> +++ b/arch/arm/mach-tegra/tegra30_clocks_data.c
> @@ -1132,14 +1132,14 @@ PERIPH_CLK(2d, "2d", NULL, 21, 0x15c, 520000000, mux_pllm_pllc_pllp_plla, MUX
> PERIPH_CLK(vi_sensor, "tegra_camera", "vi_sensor", 20, 0x1a8, 150000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | PERIPH_NO_RESET);
> PERIPH_CLK(epp, "epp", NULL, 19, 0x16c, 520000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | DIV_U71_INT);
> PERIPH_CLK(mpe, "mpe", NULL, 60, 0x170, 520000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | DIV_U71_INT);
> -PERIPH_CLK(host1x, "host1x", NULL, 28, 0x180, 260000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | DIV_U71_INT);
> +PERIPH_CLK(host1x, "tegra-host1x", NULL, 28, 0x180, 260000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | DIV_U71_INT);
> PERIPH_CLK(cve, "cve", NULL, 49, 0x140, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> PERIPH_CLK(tvo, "tvo", NULL, 49, 0x188, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> PERIPH_CLK(dtv, "dtv", NULL, 79, 0x1dc, 250000000, mux_clk_m, 0);
> -PERIPH_CLK(hdmi, "hdmi", NULL, 51, 0x18c, 148500000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8 | DIV_U71);
> +PERIPH_CLK(hdmi, "tegra-hdmi", NULL, 51, 0x18c, 148500000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8 | DIV_U71);
> PERIPH_CLK(tvdac, "tvdac", NULL, 53, 0x194, 220000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> -PERIPH_CLK(disp1, "tegradc.0", NULL, 27, 0x138, 600000000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8);
> -PERIPH_CLK(disp2, "tegradc.1", NULL, 26, 0x13c, 600000000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8);
> +PERIPH_CLK(disp1, "tegra-dc.0", NULL, 27, 0x138, 600000000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8);
> +PERIPH_CLK(disp2, "tegra-dc.1", NULL, 26, 0x13c, 600000000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8);
> PERIPH_CLK(usbd, "fsl-tegra-udc", NULL, 22, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
> PERIPH_CLK(usb2, "tegra-ehci.1", NULL, 58, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
> PERIPH_CLK(usb3, "tegra-ehci.2", NULL, 59, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
> @@ -1337,6 +1337,9 @@ struct clk_duplicate tegra_clk_duplicates[] = {
> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.4", "fast-clk"),
> + CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
> + CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
> + CLK_DUPLICATE("pll_d2_out0", "tegra-hdmi", "parent"),
> };

Why we need this? Set the parent of "tegra-dc.0" & "tegra-dc.1" to "pll_p"?

Mark
>
> struct clk *tegra_ptr_clks[] = {
> --
> 1.8.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2012-11-13 04:38:46

by Mark Zhang

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 11/12/2012 05:39 PM, Mark Zhang wrote:
> On 11/09/2012 09:20 PM, Thierry Reding wrote:
>> This commit adds the host1x node along with its children to the Tegra20
>> DTSI. Furthermore the OF auxiliary data table is updated to have proper
>> names assigned to the platform devices instantiated from the device
>> tree. Moreover, the clocks required by host1x and the two display
>> controllers are initialized and the pll_d frequency table is completed
>> with a few entries to support common HDMI and LVDS display modes.
>>
>> Signed-off-by: Thierry Reding <[email protected]>
>> ---
>> arch/arm/boot/dts/tegra20.dtsi | 87 +++++++++++++++++++++++++++++++
>> arch/arm/mach-tegra/board-dt-tegra20.c | 9 ++++
>> arch/arm/mach-tegra/tegra20_clocks_data.c | 23 ++++----
>> 3 files changed, 110 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
>> index 71a650d..2b4dec9 100644
>> --- a/arch/arm/boot/dts/tegra20.dtsi
>> +++ b/arch/arm/boot/dts/tegra20.dtsi
>> @@ -4,6 +4,93 @@
>> compatible = "nvidia,tegra20";
>> interrupt-parent = <&intc>;
>>
>> + host1x {
>> + compatible = "nvidia,tegra20-host1x", "simple-bus";
>> + reg = <0x50000000 0x00024000>;
>> + interrupts = <0 65 0x04 /* mpcore syncpt */
>> + 0 67 0x04>; /* mpcore general */
>> +
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + ranges = <0x54000000 0x54000000 0x04000000>;
>> +
>> + mpe {
>> + compatible = "nvidia,tegra20-mpe";
>> + reg = <0x54040000 0x00040000>;
>> + interrupts = <0 68 0x04>;
>> + };
>> +
>> + vi {
>> + compatible = "nvidia,tegra20-vi";
>> + reg = <0x54080000 0x00040000>;
>> + interrupts = <0 69 0x04>;
>> + };
>> +
>> + epp {
>> + compatible = "nvidia,tegra20-epp";
>> + reg = <0x540c0000 0x00040000>;
>> + interrupts = <0 70 0x04>;
>> + };
>> +
>> + isp {
>> + compatible = "nvidia,tegra20-isp";
>> + reg = <0x54100000 0x00040000>;
>> + interrupts = <0 71 0x04>;
>> + };
>> +
>> + gr2d {
>> + compatible = "nvidia,tegra20-gr2d";
>> + reg = <0x54140000 0x00040000>;
>> + interrupts = <0 72 0x04>;
>> + };
>> +
>> + gr3d {
>> + compatible = "nvidia,tegra20-gr3d";
>> + reg = <0x54180000 0x00040000>;
>> + };
>> +
>> + dc@54200000 {
>> + compatible = "nvidia,tegra20-dc";
>> + reg = <0x54200000 0x00040000>;
>> + interrupts = <0 73 0x04>;
>> +
>> + rgb {
>> + status = "disabled";
>> + };
>> + };
>> +
>> + dc@54240000 {
>> + compatible = "nvidia,tegra20-dc";
>> + reg = <0x54240000 0x00040000>;
>> + interrupts = <0 74 0x04>;
>> +
>> + rgb {
>> + status = "disabled";
>> + };
>> + };
>> +
>> + hdmi {
>> + compatible = "nvidia,tegra20-hdmi";
>> + reg = <0x54280000 0x00040000>;
>> + interrupts = <0 75 0x04>;
>> + status = "disabled";
>> + };
>> +
>> + tvo {
>> + compatible = "nvidia,tegra20-tvo";
>> + reg = <0x542c0000 0x00040000>;
>> + interrupts = <0 76 0x04>;
>> + status = "disabled";
>> + };
>> +
>> + dsi {
>> + compatible = "nvidia,tegra20-dsi";
>> + reg = <0x54300000 0x00040000>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> timer@50004600 {
>> compatible = "arm,cortex-a9-twd-timer";
>> reg = <0x50040600 0x20>;
>> diff --git a/arch/arm/mach-tegra/board-dt-tegra20.c b/arch/arm/mach-tegra/board-dt-tegra20.c
>> index 22f5a9b..1afbdd1 100644
>> --- a/arch/arm/mach-tegra/board-dt-tegra20.c
>> +++ b/arch/arm/mach-tegra/board-dt-tegra20.c
>> @@ -93,6 +93,12 @@ struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = {
>> OF_DEV_AUXDATA("nvidia,tegra20-slink", 0x7000D600, "spi_tegra.1", NULL),
>> OF_DEV_AUXDATA("nvidia,tegra20-slink", 0x7000D800, "spi_tegra.2", NULL),
>> OF_DEV_AUXDATA("nvidia,tegra20-slink", 0x7000DA00, "spi_tegra.3", NULL),
>> + OF_DEV_AUXDATA("nvidia,tegra20-host1x", 0x50000000, "tegra-host1x", NULL),
>> + OF_DEV_AUXDATA("nvidia,tegra20-dc", 0x54200000, "tegra-dc.0", NULL),
>> + OF_DEV_AUXDATA("nvidia,tegra20-dc", 0x54240000, "tegra-dc.1", NULL),
>> + OF_DEV_AUXDATA("nvidia,tegra20-hdmi", 0x54280000, "tegra-hdmi", NULL),
>> + OF_DEV_AUXDATA("nvidia,tegra20-dsi", 0x54300000, "tegra-dsi", NULL),
>> + OF_DEV_AUXDATA("nvidia,tegra20-tvo", 0x542c0000, "tegra-tvo", NULL),
>> {}
>> };
>>
>> @@ -116,6 +122,9 @@ static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
>> { "sbc2", "pll_p", 100000000, false },
>> { "sbc3", "pll_p", 100000000, false },
>> { "sbc4", "pll_p", 100000000, false },
>> + { "host1x", "pll_c", 144000000, false },
>> + { "disp1", "pll_p", 600000000, false },
>> + { "disp2", "pll_p", 600000000, false },
>> { NULL, NULL, 0, 0},
>> };
>>
>> diff --git a/arch/arm/mach-tegra/tegra20_clocks_data.c b/arch/arm/mach-tegra/tegra20_clocks_data.c
>> index 9615ee3..2e247f2 100644
>> --- a/arch/arm/mach-tegra/tegra20_clocks_data.c
>> +++ b/arch/arm/mach-tegra/tegra20_clocks_data.c
>> @@ -246,11 +246,16 @@ static struct clk_pll_freq_table tegra_pll_d_freq_table[] = {
>> { 19200000, 216000000, 135, 12, 1, 3},
>> { 26000000, 216000000, 216, 26, 1, 4},
>>
>> + { 12000000, 297000000, 99, 4, 1, 4 },
>> + { 12000000, 339000000, 113, 4, 1, 4 },
>> +
>
> The patch is OK. I'm just curious about how you get the cpcon value
> here. According to Tegra 2's TRM, it's not very clear about how to
> calculate the cpcon value for pll_d(pll_d is type of CLKPLL1G_MIPI).
>
> I've skimmed the l4t codes as well, the cpcon is hardcode to 8 in the
> dynamic m/n/p calculating code blocks.
>
> So, Peter or Prashant, could you help to take a look at this?
>
>> { 12000000, 594000000, 594, 12, 1, 8},
>> { 13000000, 594000000, 594, 13, 1, 8},
>> { 19200000, 594000000, 495, 16, 1, 8},
>> { 26000000, 594000000, 594, 26, 1, 8},
>>
>> + { 12000000, 616000000, 616, 12, 1, 8},
>> +
>> { 12000000, 1000000000, 1000, 12, 1, 12},
>> { 13000000, 1000000000, 1000, 13, 1, 12},
>> { 19200000, 1000000000, 625, 12, 1, 8},
>> @@ -930,17 +935,17 @@ PERIPH_CLK(vi, "tegra_camera", "vi", 20, 0x148, 150000000, mux_pllm_pllc_pllp_
>> PERIPH_CLK(vi_sensor, "tegra_camera", "vi_sensor", 20, 0x1a8, 150000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | PERIPH_NO_RESET); /* scales with voltage and process_id */
>> PERIPH_CLK(epp, "epp", NULL, 19, 0x16c, 300000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
>> PERIPH_CLK(mpe, "mpe", NULL, 60, 0x170, 250000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
>> -PERIPH_CLK(host1x, "host1x", NULL, 28, 0x180, 166000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
>> +PERIPH_CLK(host1x, "tegra-host1x", NULL, 28, 0x180, 166000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
>> PERIPH_CLK(cve, "cve", NULL, 49, 0x140, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
>> -PERIPH_CLK(tvo, "tvo", NULL, 49, 0x188, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
>> -PERIPH_CLK(hdmi, "hdmi", NULL, 51, 0x18c, 600000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
>> +PERIPH_CLK(tvo, "tegra-tvo", NULL, 49, 0x188, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
>> +PERIPH_CLK(hdmi, "tegra-hdmi", NULL, 51, 0x18c, 600000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
>> PERIPH_CLK(tvdac, "tvdac", NULL, 53, 0x194, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
>> -PERIPH_CLK(disp1, "tegradc.0", NULL, 27, 0x138, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
>> -PERIPH_CLK(disp2, "tegradc.1", NULL, 26, 0x13c, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
>> +PERIPH_CLK(disp1, "tegra-dc.0", NULL, 27, 0x138, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
>> +PERIPH_CLK(disp2, "tegra-dc.1", NULL, 26, 0x13c, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
>> PERIPH_CLK(usbd, "fsl-tegra-udc", NULL, 22, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
>> PERIPH_CLK(usb2, "tegra-ehci.1", NULL, 58, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
>> PERIPH_CLK(usb3, "tegra-ehci.2", NULL, 59, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
>> -PERIPH_CLK(dsi, "dsi", NULL, 48, 0, 500000000, mux_plld, 0); /* scales with voltage */
>> +PERIPH_CLK(dsi, "tegra-dsi", NULL, 48, 0, 500000000, mux_plld, 0); /* scales with voltage */
>> PERIPH_CLK(csi, "tegra_camera", "csi", 52, 0, 72000000, mux_pllp_out3, 0);
>> PERIPH_CLK(isp, "tegra_camera", "isp", 23, 0, 150000000, mux_clk_m, 0); /* same frequency as VI */
>> PERIPH_CLK(csus, "tegra_camera", "csus", 92, 0, 150000000, mux_clk_m, PERIPH_NO_RESET);
>> @@ -1036,9 +1041,6 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
>> CLK_DUPLICATE("usbd", "utmip-pad", NULL),
>> CLK_DUPLICATE("usbd", "tegra-ehci.0", NULL),
>> CLK_DUPLICATE("usbd", "tegra-otg", NULL),
>> - CLK_DUPLICATE("hdmi", "tegradc.0", "hdmi"),
>> - CLK_DUPLICATE("hdmi", "tegradc.1", "hdmi"),
>> - CLK_DUPLICATE("host1x", "tegra_grhost", "host1x"),
>> CLK_DUPLICATE("2d", "tegra_grhost", "gr2d"),
>> CLK_DUPLICATE("3d", "tegra_grhost", "gr3d"),
>> CLK_DUPLICATE("epp", "tegra_grhost", "epp"),
>> @@ -1051,6 +1053,9 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.1", "fast-clk"),
>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
>> + CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
>> + CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
>> + CLK_DUPLICATE("pll_d_out0", "tegra-hdmi", "parent"),
>> };

Why we need this "CLK_DUPLICATE"? Set the clock parent of the dc
controllers to pll_p?

>>
>> #define CLK(dev, con, ck) \
>> --
>> 1.8.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2012-11-13 06:41:45

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On Tue, Nov 13, 2012 at 12:38:34PM +0800, Mark Zhang wrote:
> On 11/12/2012 05:39 PM, Mark Zhang wrote:
> > On 11/09/2012 09:20 PM, Thierry Reding wrote:
[...]
> >> @@ -1036,9 +1041,6 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
> >> CLK_DUPLICATE("usbd", "utmip-pad", NULL),
> >> CLK_DUPLICATE("usbd", "tegra-ehci.0", NULL),
> >> CLK_DUPLICATE("usbd", "tegra-otg", NULL),
> >> - CLK_DUPLICATE("hdmi", "tegradc.0", "hdmi"),
> >> - CLK_DUPLICATE("hdmi", "tegradc.1", "hdmi"),
> >> - CLK_DUPLICATE("host1x", "tegra_grhost", "host1x"),
> >> CLK_DUPLICATE("2d", "tegra_grhost", "gr2d"),
> >> CLK_DUPLICATE("3d", "tegra_grhost", "gr3d"),
> >> CLK_DUPLICATE("epp", "tegra_grhost", "epp"),
> >> @@ -1051,6 +1053,9 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
> >> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.1", "fast-clk"),
> >> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
> >> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
> >> + CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
> >> + CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
> >> + CLK_DUPLICATE("pll_d_out0", "tegra-hdmi", "parent"),
> >> };
>
> Why we need this "CLK_DUPLICATE"? Set the clock parent of the dc
> controllers to pll_p?

This was the method proposed by Stephen to abstract away the clock tree
differences between Tegra20 and Tegra30. The way this works is that we
can {devm_,}clk_get(&pdev->dev, "parent") in the display and HDMI
controllers' .probe() and it'll obtain the correct parent clock
independent of which version of Tegra is used.

Thierry


Attachments:
(No filename) (1.67 kB)
(No filename) (836.00 B)
Download all attachments

2012-11-13 07:37:29

by Mark Zhang

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 11/13/2012 02:41 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Tue, Nov 13, 2012 at 12:38:34PM +0800, Mark Zhang wrote:
>> On 11/12/2012 05:39 PM, Mark Zhang wrote:
>>> On 11/09/2012 09:20 PM, Thierry Reding wrote:
> [...]
>>>> @@ -1036,9 +1041,6 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
>>>> CLK_DUPLICATE("usbd", "utmip-pad", NULL),
>>>> CLK_DUPLICATE("usbd", "tegra-ehci.0", NULL),
>>>> CLK_DUPLICATE("usbd", "tegra-otg", NULL),
>>>> - CLK_DUPLICATE("hdmi", "tegradc.0", "hdmi"),
>>>> - CLK_DUPLICATE("hdmi", "tegradc.1", "hdmi"),
>>>> - CLK_DUPLICATE("host1x", "tegra_grhost", "host1x"),
>>>> CLK_DUPLICATE("2d", "tegra_grhost", "gr2d"),
>>>> CLK_DUPLICATE("3d", "tegra_grhost", "gr3d"),
>>>> CLK_DUPLICATE("epp", "tegra_grhost", "epp"),
>>>> @@ -1051,6 +1053,9 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
>>>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.1", "fast-clk"),
>>>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
>>>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
>>>> + CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
>>>> + CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
>>>> + CLK_DUPLICATE("pll_d_out0", "tegra-hdmi", "parent"),
>>>> };
>>
>> Why we need this "CLK_DUPLICATE"? Set the clock parent of the dc
>> controllers to pll_p?
>
> This was the method proposed by Stephen to abstract away the clock tree
> differences between Tegra20 and Tegra30. The way this works is that we
> can {devm_,}clk_get(&pdev->dev, "parent") in the display and HDMI
> controllers' .probe() and it'll obtain the correct parent clock
> independent of which version of Tegra is used.
>

Yes, after reading the corresponding codes in rgb.c & hdmi.c, I got how
this works. It's better than our previous version, I mean, the version
which we hard-code the clock parent setting logics in clock.c. But I
wanna say, it looks weird if you don't read the code of drm driver.

> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>

2012-11-13 07:45:19

by Mark Zhang

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 11/09/2012 09:20 PM, Thierry Reding wrote:
> This commit adds the host1x node along with its children to the Tegra20
> DTSI. Furthermore the OF auxiliary data table is updated to have proper
> names assigned to the platform devices instantiated from the device
> tree. Moreover, the clocks required by host1x and the two display
> controllers are initialized and the pll_d frequency table is completed
> with a few entries to support common HDMI and LVDS display modes.
>
> Signed-off-by: Thierry Reding <[email protected]>
> ---
> arch/arm/boot/dts/tegra20.dtsi | 87 +++++++++++++++++++++++++++++++
> arch/arm/mach-tegra/board-dt-tegra20.c | 9 ++++
> arch/arm/mach-tegra/tegra20_clocks_data.c | 23 ++++----
> 3 files changed, 110 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 71a650d..2b4dec9 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -4,6 +4,93 @@
> compatible = "nvidia,tegra20";
> interrupt-parent = <&intc>;
>
> + host1x {
> + compatible = "nvidia,tegra20-host1x", "simple-bus";
> + reg = <0x50000000 0x00024000>;
> + interrupts = <0 65 0x04 /* mpcore syncpt */
> + 0 67 0x04>; /* mpcore general */
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + ranges = <0x54000000 0x54000000 0x04000000>;
> +
> + mpe {
> + compatible = "nvidia,tegra20-mpe";
> + reg = <0x54040000 0x00040000>;
> + interrupts = <0 68 0x04>;
> + };
> +
> + vi {
> + compatible = "nvidia,tegra20-vi";
> + reg = <0x54080000 0x00040000>;
> + interrupts = <0 69 0x04>;
> + };
> +
> + epp {
> + compatible = "nvidia,tegra20-epp";
> + reg = <0x540c0000 0x00040000>;
> + interrupts = <0 70 0x04>;
> + };
> +
> + isp {
> + compatible = "nvidia,tegra20-isp";
> + reg = <0x54100000 0x00040000>;
> + interrupts = <0 71 0x04>;
> + };
> +
> + gr2d {
> + compatible = "nvidia,tegra20-gr2d";
> + reg = <0x54140000 0x00040000>;
> + interrupts = <0 72 0x04>;
> + };
> +
> + gr3d {
> + compatible = "nvidia,tegra20-gr3d";
> + reg = <0x54180000 0x00040000>;
> + };
> +
> + dc@54200000 {
> + compatible = "nvidia,tegra20-dc";
> + reg = <0x54200000 0x00040000>;
> + interrupts = <0 73 0x04>;
> +
> + rgb {
> + status = "disabled";
> + };
> + };
> +
> + dc@54240000 {
> + compatible = "nvidia,tegra20-dc";
> + reg = <0x54240000 0x00040000>;
> + interrupts = <0 74 0x04>;
> +
> + rgb {
> + status = "disabled";
> + };
> + };
> +
> + hdmi {
> + compatible = "nvidia,tegra20-hdmi";
> + reg = <0x54280000 0x00040000>;
> + interrupts = <0 75 0x04>;
> + status = "disabled";
> + };
> +
> + tvo {
> + compatible = "nvidia,tegra20-tvo";
> + reg = <0x542c0000 0x00040000>;
> + interrupts = <0 76 0x04>;
> + status = "disabled";
> + };
> +
> + dsi {
> + compatible = "nvidia,tegra20-dsi";
> + reg = <0x54300000 0x00040000>;
> + status = "disabled";
> + };
> + };
> +
> timer@50004600 {
> compatible = "arm,cortex-a9-twd-timer";
> reg = <0x50040600 0x20>;
> diff --git a/arch/arm/mach-tegra/board-dt-tegra20.c b/arch/arm/mach-tegra/board-dt-tegra20.c
> index 22f5a9b..1afbdd1 100644
> --- a/arch/arm/mach-tegra/board-dt-tegra20.c
> +++ b/arch/arm/mach-tegra/board-dt-tegra20.c
> @@ -93,6 +93,12 @@ struct of_dev_auxdata tegra20_auxdata_lookup[] __initdata = {
> OF_DEV_AUXDATA("nvidia,tegra20-slink", 0x7000D600, "spi_tegra.1", NULL),
> OF_DEV_AUXDATA("nvidia,tegra20-slink", 0x7000D800, "spi_tegra.2", NULL),
> OF_DEV_AUXDATA("nvidia,tegra20-slink", 0x7000DA00, "spi_tegra.3", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra20-host1x", 0x50000000, "tegra-host1x", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra20-dc", 0x54200000, "tegra-dc.0", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra20-dc", 0x54240000, "tegra-dc.1", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra20-hdmi", 0x54280000, "tegra-hdmi", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra20-dsi", 0x54300000, "tegra-dsi", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra20-tvo", 0x542c0000, "tegra-tvo", NULL),
> {}
> };
>
> @@ -116,6 +122,9 @@ static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
> { "sbc2", "pll_p", 100000000, false },
> { "sbc3", "pll_p", 100000000, false },
> { "sbc4", "pll_p", 100000000, false },
> + { "host1x", "pll_c", 144000000, false },
> + { "disp1", "pll_p", 600000000, false },
> + { "disp2", "pll_p", 600000000, false },

I think here the parent of disp2 should be "pll_d_out0", not "pll_p".
Right now pll_p has not a proper clock setting to make 148.5MHz 1080p
HDMI happy. In addition, you add the 297MHz in pll_d frequency table
next and I think this is for disp2 has a proper clock rate to support HDMI.

> { NULL, NULL, 0, 0},
> };
>
> diff --git a/arch/arm/mach-tegra/tegra20_clocks_data.c b/arch/arm/mach-tegra/tegra20_clocks_data.c
> index 9615ee3..2e247f2 100644
> --- a/arch/arm/mach-tegra/tegra20_clocks_data.c
> +++ b/arch/arm/mach-tegra/tegra20_clocks_data.c
> @@ -246,11 +246,16 @@ static struct clk_pll_freq_table tegra_pll_d_freq_table[] = {
> { 19200000, 216000000, 135, 12, 1, 3},
> { 26000000, 216000000, 216, 26, 1, 4},
>
> + { 12000000, 297000000, 99, 4, 1, 4 },
> + { 12000000, 339000000, 113, 4, 1, 4 },
> +
> { 12000000, 594000000, 594, 12, 1, 8},
> { 13000000, 594000000, 594, 13, 1, 8},
> { 19200000, 594000000, 495, 16, 1, 8},
> { 26000000, 594000000, 594, 26, 1, 8},
>
> + { 12000000, 616000000, 616, 12, 1, 8},
> +
> { 12000000, 1000000000, 1000, 12, 1, 12},
> { 13000000, 1000000000, 1000, 13, 1, 12},
> { 19200000, 1000000000, 625, 12, 1, 8},
> @@ -930,17 +935,17 @@ PERIPH_CLK(vi, "tegra_camera", "vi", 20, 0x148, 150000000, mux_pllm_pllc_pllp_
> PERIPH_CLK(vi_sensor, "tegra_camera", "vi_sensor", 20, 0x1a8, 150000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | PERIPH_NO_RESET); /* scales with voltage and process_id */
> PERIPH_CLK(epp, "epp", NULL, 19, 0x16c, 300000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
> PERIPH_CLK(mpe, "mpe", NULL, 60, 0x170, 250000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
> -PERIPH_CLK(host1x, "host1x", NULL, 28, 0x180, 166000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
> +PERIPH_CLK(host1x, "tegra-host1x", NULL, 28, 0x180, 166000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71); /* scales with voltage and process_id */
> PERIPH_CLK(cve, "cve", NULL, 49, 0x140, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> -PERIPH_CLK(tvo, "tvo", NULL, 49, 0x188, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> -PERIPH_CLK(hdmi, "hdmi", NULL, 51, 0x18c, 600000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> +PERIPH_CLK(tvo, "tegra-tvo", NULL, 49, 0x188, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> +PERIPH_CLK(hdmi, "tegra-hdmi", NULL, 51, 0x18c, 600000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> PERIPH_CLK(tvdac, "tvdac", NULL, 53, 0x194, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> -PERIPH_CLK(disp1, "tegradc.0", NULL, 27, 0x138, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
> -PERIPH_CLK(disp2, "tegradc.1", NULL, 26, 0x13c, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
> +PERIPH_CLK(disp1, "tegra-dc.0", NULL, 27, 0x138, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
> +PERIPH_CLK(disp2, "tegra-dc.1", NULL, 26, 0x13c, 600000000, mux_pllp_plld_pllc_clkm, MUX); /* scales with voltage and process_id */
> PERIPH_CLK(usbd, "fsl-tegra-udc", NULL, 22, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
> PERIPH_CLK(usb2, "tegra-ehci.1", NULL, 58, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
> PERIPH_CLK(usb3, "tegra-ehci.2", NULL, 59, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
> -PERIPH_CLK(dsi, "dsi", NULL, 48, 0, 500000000, mux_plld, 0); /* scales with voltage */
> +PERIPH_CLK(dsi, "tegra-dsi", NULL, 48, 0, 500000000, mux_plld, 0); /* scales with voltage */
> PERIPH_CLK(csi, "tegra_camera", "csi", 52, 0, 72000000, mux_pllp_out3, 0);
> PERIPH_CLK(isp, "tegra_camera", "isp", 23, 0, 150000000, mux_clk_m, 0); /* same frequency as VI */
> PERIPH_CLK(csus, "tegra_camera", "csus", 92, 0, 150000000, mux_clk_m, PERIPH_NO_RESET);
> @@ -1036,9 +1041,6 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
> CLK_DUPLICATE("usbd", "utmip-pad", NULL),
> CLK_DUPLICATE("usbd", "tegra-ehci.0", NULL),
> CLK_DUPLICATE("usbd", "tegra-otg", NULL),
> - CLK_DUPLICATE("hdmi", "tegradc.0", "hdmi"),
> - CLK_DUPLICATE("hdmi", "tegradc.1", "hdmi"),
> - CLK_DUPLICATE("host1x", "tegra_grhost", "host1x"),
> CLK_DUPLICATE("2d", "tegra_grhost", "gr2d"),
> CLK_DUPLICATE("3d", "tegra_grhost", "gr3d"),
> CLK_DUPLICATE("epp", "tegra_grhost", "epp"),
> @@ -1051,6 +1053,9 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.1", "fast-clk"),
> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
> + CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
> + CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
> + CLK_DUPLICATE("pll_d_out0", "tegra-hdmi", "parent"),
> };

The same with my above comments, the tegra-dc.1's parent should be
pll_d_out0.

>
> #define CLK(dev, con, ck) \
> --
> 1.8.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2012-11-13 07:46:08

by Mark Zhang

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: tegra: Add Tegra30 host1x support

On 11/09/2012 09:20 PM, Thierry Reding wrote:
> This commit adds the host1x node along with its children to the Tegra30
> DTSI. Furthermore the OF auxiliary data table is updated to have proper
> names assigned to the platform devices instantiated from the device
> tree. Moreover, the clocks required by host1x and the two display
> controllers are setup and initialized.
>
> Signed-off-by: Thierry Reding <[email protected]>
> ---
> arch/arm/boot/dts/tegra30.dtsi | 87 +++++++++++++++++++++++++++++++
> arch/arm/mach-tegra/board-dt-tegra30.c | 10 ++++
> arch/arm/mach-tegra/tegra30_clocks_data.c | 11 ++--
> 3 files changed, 104 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
> index b8e33c3..529fdb8 100644
> --- a/arch/arm/boot/dts/tegra30.dtsi
> +++ b/arch/arm/boot/dts/tegra30.dtsi
> @@ -4,6 +4,93 @@
> compatible = "nvidia,tegra30";
> interrupt-parent = <&intc>;
>
> + host1x {
> + compatible = "nvidia,tegra30-host1x", "simple-bus";
> + reg = <0x50000000 0x00024000>;
> + interrupts = <0 65 0x04 /* mpcore syncpt */
> + 0 67 0x04>; /* mpcore general */
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + ranges = <0x54000000 0x54000000 0x04000000>;
> +
> + mpe {
> + compatible = "nvidia,tegra30-mpe";
> + reg = <0x54040000 0x00040000>;
> + interrupts = <0 68 0x04>;
> + };
> +
> + vi {
> + compatible = "nvidia,tegra30-vi";
> + reg = <0x54080000 0x00040000>;
> + interrupts = <0 69 0x04>;
> + };
> +
> + epp {
> + compatible = "nvidia,tegra30-epp";
> + reg = <0x540c0000 0x00040000>;
> + interrupts = <0 70 0x04>;
> + };
> +
> + isp {
> + compatible = "nvidia,tegra30-isp";
> + reg = <0x54100000 0x00040000>;
> + interrupts = <0 71 0x04>;
> + };
> +
> + gr2d {
> + compatible = "nvidia,tegra30-gr2d";
> + reg = <0x54140000 0x00040000>;
> + interrupts = <0 72 0x04>;
> + };
> +
> + gr3d {
> + compatible = "nvidia,tegra30-gr3d";
> + reg = <0x54180000 0x00040000>;
> + };
> +
> + dc@54200000 {
> + compatible = "nvidia,tegra30-dc";
> + reg = <0x54200000 0x00040000>;
> + interrupts = <0 73 0x04>;
> +
> + rgb {
> + status = "disabled";
> + };
> + };
> +
> + dc@54240000 {
> + compatible = "nvidia,tegra30-dc";
> + reg = <0x54240000 0x00040000>;
> + interrupts = <0 74 0x04>;
> +
> + rgb {
> + status = "disabled";
> + };
> + };
> +
> + hdmi {
> + compatible = "nvidia,tegra30-hdmi";
> + reg = <0x54280000 0x00040000>;
> + interrupts = <0 75 0x04>;
> + status = "disabled";
> + };
> +
> + tvo {
> + compatible = "nvidia,tegra30-tvo";
> + reg = <0x542c0000 0x00040000>;
> + interrupts = <0 76 0x04>;
> + status = "disabled";
> + };
> +
> + dsi {
> + compatible = "nvidia,tegra30-dsi";
> + reg = <0x54300000 0x00040000>;
> + status = "disabled";
> + };
> + };
> +
> timer@50004600 {
> compatible = "arm,cortex-a9-twd-timer";
> reg = <0x50040600 0x20>;
> diff --git a/arch/arm/mach-tegra/board-dt-tegra30.c b/arch/arm/mach-tegra/board-dt-tegra30.c
> index cd30338..ff21107 100644
> --- a/arch/arm/mach-tegra/board-dt-tegra30.c
> +++ b/arch/arm/mach-tegra/board-dt-tegra30.c
> @@ -57,6 +57,12 @@ struct of_dev_auxdata tegra30_auxdata_lookup[] __initdata = {
> OF_DEV_AUXDATA("nvidia,tegra30-slink", 0x7000DA00, "spi_tegra.3", NULL),
> OF_DEV_AUXDATA("nvidia,tegra30-slink", 0x7000DC00, "spi_tegra.4", NULL),
> OF_DEV_AUXDATA("nvidia,tegra30-slink", 0x7000DE00, "spi_tegra.5", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra30-host1x", 0x50000000, "tegra-host1x", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra30-dc", 0x54200000, "tegra-dc.0", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra30-dc", 0x54240000, "tegra-dc.1", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra30-hdmi", 0x54280000, "tegra-hdmi", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra30-dsi", 0x54300000, "tegra-dsi", NULL),
> + OF_DEV_AUXDATA("nvidia,tegra30-tvo", 0x542c0000, "tegra-tvo", NULL),
> {}
> };
>
> @@ -82,6 +88,10 @@ static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
> { "sbc4", "pll_p", 100000000, false},
> { "sbc5", "pll_p", 100000000, false},
> { "sbc6", "pll_p", 100000000, false},
> + { "host1x", "pll_c", 144000000, false},
> + { "disp1", "pll_p", 74000000, false},
> + { "disp2", "pll_d2_out0", 148500000, false},
> + { "hdmi", "pll_d2_out0", 148500000, false},
> { NULL, NULL, 0, 0},
> };
>
> diff --git a/arch/arm/mach-tegra/tegra30_clocks_data.c b/arch/arm/mach-tegra/tegra30_clocks_data.c
> index 7bc8b1d..210b8a4 100644
> --- a/arch/arm/mach-tegra/tegra30_clocks_data.c
> +++ b/arch/arm/mach-tegra/tegra30_clocks_data.c
> @@ -1132,14 +1132,14 @@ PERIPH_CLK(2d, "2d", NULL, 21, 0x15c, 520000000, mux_pllm_pllc_pllp_plla, MUX
> PERIPH_CLK(vi_sensor, "tegra_camera", "vi_sensor", 20, 0x1a8, 150000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | PERIPH_NO_RESET);
> PERIPH_CLK(epp, "epp", NULL, 19, 0x16c, 520000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | DIV_U71_INT);
> PERIPH_CLK(mpe, "mpe", NULL, 60, 0x170, 520000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | DIV_U71_INT);
> -PERIPH_CLK(host1x, "host1x", NULL, 28, 0x180, 260000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | DIV_U71_INT);
> +PERIPH_CLK(host1x, "tegra-host1x", NULL, 28, 0x180, 260000000, mux_pllm_pllc_pllp_plla, MUX | DIV_U71 | DIV_U71_INT);
> PERIPH_CLK(cve, "cve", NULL, 49, 0x140, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> PERIPH_CLK(tvo, "tvo", NULL, 49, 0x188, 250000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> PERIPH_CLK(dtv, "dtv", NULL, 79, 0x1dc, 250000000, mux_clk_m, 0);
> -PERIPH_CLK(hdmi, "hdmi", NULL, 51, 0x18c, 148500000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8 | DIV_U71);
> +PERIPH_CLK(hdmi, "tegra-hdmi", NULL, 51, 0x18c, 148500000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8 | DIV_U71);
> PERIPH_CLK(tvdac, "tvdac", NULL, 53, 0x194, 220000000, mux_pllp_plld_pllc_clkm, MUX | DIV_U71); /* requires min voltage */
> -PERIPH_CLK(disp1, "tegradc.0", NULL, 27, 0x138, 600000000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8);
> -PERIPH_CLK(disp2, "tegradc.1", NULL, 26, 0x13c, 600000000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8);
> +PERIPH_CLK(disp1, "tegra-dc.0", NULL, 27, 0x138, 600000000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8);
> +PERIPH_CLK(disp2, "tegra-dc.1", NULL, 26, 0x13c, 600000000, mux_pllp_pllm_plld_plla_pllc_plld2_clkm, MUX | MUX8);
> PERIPH_CLK(usbd, "fsl-tegra-udc", NULL, 22, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
> PERIPH_CLK(usb2, "tegra-ehci.1", NULL, 58, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
> PERIPH_CLK(usb3, "tegra-ehci.2", NULL, 59, 0, 480000000, mux_clk_m, 0); /* requires min voltage */
> @@ -1337,6 +1337,9 @@ struct clk_duplicate tegra_clk_duplicates[] = {
> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.4", "fast-clk"),
> + CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
> + CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
> + CLK_DUPLICATE("pll_d2_out0", "tegra-hdmi", "parent"),
> };

The tegra-dc.1's parent should be pll_d2_out0.

>
> struct clk *tegra_ptr_clks[] = {
> --
> 1.8.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2012-11-13 07:52:54

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On Tue, Nov 13, 2012 at 03:45:00PM +0800, Mark Zhang wrote:
> On 11/09/2012 09:20 PM, Thierry Reding wrote:
> > @@ -116,6 +122,9 @@ static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
> > { "sbc2", "pll_p", 100000000, false },
> > { "sbc3", "pll_p", 100000000, false },
> > { "sbc4", "pll_p", 100000000, false },
> > + { "host1x", "pll_c", 144000000, false },
> > + { "disp1", "pll_p", 600000000, false },
> > + { "disp2", "pll_p", 600000000, false },
>
> I think here the parent of disp2 should be "pll_d_out0", not "pll_p".
> Right now pll_p has not a proper clock setting to make 148.5MHz 1080p
> HDMI happy. In addition, you add the 297MHz in pll_d frequency table
> next and I think this is for disp2 has a proper clock rate to support HDMI.
[...]
> > @@ -1051,6 +1053,9 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
> > CLK_DUPLICATE("pll_p_out3", "tegra-i2c.1", "fast-clk"),
> > CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
> > CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
> > + CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
> > + CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
> > + CLK_DUPLICATE("pll_d_out0", "tegra-hdmi", "parent"),
> > };
>
> The same with my above comments, the tegra-dc.1's parent should be
> pll_d_out0.

The way this works is that for HDMI it is required that the DC and HDMI
blocks have the same parent. So what really happens is that once you
setup one of the DCs to work with HDMI, its clock will automatically be
reparented to the HDMI parent clock, which in this case is "pll_d_out0".

Thierry


Attachments:
(No filename) (1.74 kB)
(No filename) (836.00 B)
Download all attachments

2012-11-13 07:53:56

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 2/2] ARM: tegra: Add Tegra30 host1x support

On Tue, Nov 13, 2012 at 03:45:52PM +0800, Mark Zhang wrote:
> On 11/09/2012 09:20 PM, Thierry Reding wrote:
[...]
> > @@ -1337,6 +1337,9 @@ struct clk_duplicate tegra_clk_duplicates[] = {
> > CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
> > CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
> > CLK_DUPLICATE("pll_p_out3", "tegra-i2c.4", "fast-clk"),
> > + CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
> > + CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
> > + CLK_DUPLICATE("pll_d2_out0", "tegra-hdmi", "parent"),
> > };
>
> The tegra-dc.1's parent should be pll_d2_out0.

I've explained this my reply to the Tegra20 support patch.

Thierry


Attachments:
(No filename) (707.00 B)
(No filename) (836.00 B)
Download all attachments

2012-11-13 08:01:02

by Mark Zhang

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 11/13/2012 03:52 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Tue, Nov 13, 2012 at 03:45:00PM +0800, Mark Zhang wrote:
>> On 11/09/2012 09:20 PM, Thierry Reding wrote:
>>> @@ -116,6 +122,9 @@ static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
>>> { "sbc2", "pll_p", 100000000, false },
>>> { "sbc3", "pll_p", 100000000, false },
>>> { "sbc4", "pll_p", 100000000, false },
>>> + { "host1x", "pll_c", 144000000, false },
>>> + { "disp1", "pll_p", 600000000, false },
>>> + { "disp2", "pll_p", 600000000, false },
>>
>> I think here the parent of disp2 should be "pll_d_out0", not "pll_p".
>> Right now pll_p has not a proper clock setting to make 148.5MHz 1080p
>> HDMI happy. In addition, you add the 297MHz in pll_d frequency table
>> next and I think this is for disp2 has a proper clock rate to support HDMI.
> [...]
>>> @@ -1051,6 +1053,9 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
>>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.1", "fast-clk"),
>>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
>>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
>>> + CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
>>> + CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
>>> + CLK_DUPLICATE("pll_d_out0", "tegra-hdmi", "parent"),
>>> };
>>
>> The same with my above comments, the tegra-dc.1's parent should be
>> pll_d_out0.
>
> The way this works is that for HDMI it is required that the DC and HDMI
> blocks have the same parent. So what really happens is that once you
> setup one of the DCs to work with HDMI, its clock will automatically be
> reparented to the HDMI parent clock, which in this case is "pll_d_out0".
>

Are you sure about this? Is this a hardware feature? I know the dc and
hdmi controller should have the same clock parent but I think this
should be ensured by device driver...

> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>

2012-11-13 08:05:05

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On Tue, Nov 13, 2012 at 04:00:47PM +0800, Mark Zhang wrote:
> On 11/13/2012 03:52 PM, Thierry Reding wrote:
> > * PGP Signed by an unknown key
> >
> > On Tue, Nov 13, 2012 at 03:45:00PM +0800, Mark Zhang wrote:
> >> On 11/09/2012 09:20 PM, Thierry Reding wrote:
> >>> @@ -116,6 +122,9 @@ static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
> >>> { "sbc2", "pll_p", 100000000, false },
> >>> { "sbc3", "pll_p", 100000000, false },
> >>> { "sbc4", "pll_p", 100000000, false },
> >>> + { "host1x", "pll_c", 144000000, false },
> >>> + { "disp1", "pll_p", 600000000, false },
> >>> + { "disp2", "pll_p", 600000000, false },
> >>
> >> I think here the parent of disp2 should be "pll_d_out0", not "pll_p".
> >> Right now pll_p has not a proper clock setting to make 148.5MHz 1080p
> >> HDMI happy. In addition, you add the 297MHz in pll_d frequency table
> >> next and I think this is for disp2 has a proper clock rate to support HDMI.
> > [...]
> >>> @@ -1051,6 +1053,9 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
> >>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.1", "fast-clk"),
> >>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
> >>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
> >>> + CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
> >>> + CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
> >>> + CLK_DUPLICATE("pll_d_out0", "tegra-hdmi", "parent"),
> >>> };
> >>
> >> The same with my above comments, the tegra-dc.1's parent should be
> >> pll_d_out0.
> >
> > The way this works is that for HDMI it is required that the DC and HDMI
> > blocks have the same parent. So what really happens is that once you
> > setup one of the DCs to work with HDMI, its clock will automatically be
> > reparented to the HDMI parent clock, which in this case is "pll_d_out0".
> >
>
> Are you sure about this? Is this a hardware feature? I know the dc and
> hdmi controller should have the same clock parent but I think this
> should be ensured by device driver...

It's implemented in tegra_output_hdmi_setup_clock().

Thierry


Attachments:
(No filename) (2.21 kB)
(No filename) (836.00 B)
Download all attachments

2012-11-13 08:30:41

by Mark Zhang

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 11/13/2012 04:04 PM, Thierry Reding wrote:
> * PGP Signed by an unknown key
>
> On Tue, Nov 13, 2012 at 04:00:47PM +0800, Mark Zhang wrote:
>> On 11/13/2012 03:52 PM, Thierry Reding wrote:
>>>> Old Signed by an unknown key
>>>
>>> On Tue, Nov 13, 2012 at 03:45:00PM +0800, Mark Zhang wrote:
>>>> On 11/09/2012 09:20 PM, Thierry Reding wrote:
>>>>> @@ -116,6 +122,9 @@ static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = {
>>>>> { "sbc2", "pll_p", 100000000, false },
>>>>> { "sbc3", "pll_p", 100000000, false },
>>>>> { "sbc4", "pll_p", 100000000, false },
>>>>> + { "host1x", "pll_c", 144000000, false },
>>>>> + { "disp1", "pll_p", 600000000, false },
>>>>> + { "disp2", "pll_p", 600000000, false },
>>>>
>>>> I think here the parent of disp2 should be "pll_d_out0", not "pll_p".
>>>> Right now pll_p has not a proper clock setting to make 148.5MHz 1080p
>>>> HDMI happy. In addition, you add the 297MHz in pll_d frequency table
>>>> next and I think this is for disp2 has a proper clock rate to support HDMI.
>>> [...]
>>>>> @@ -1051,6 +1053,9 @@ static struct clk_duplicate tegra_clk_duplicates[] = {
>>>>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.1", "fast-clk"),
>>>>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.2", "fast-clk"),
>>>>> CLK_DUPLICATE("pll_p_out3", "tegra-i2c.3", "fast-clk"),
>>>>> + CLK_DUPLICATE("pll_p", "tegra-dc.0", "parent"),
>>>>> + CLK_DUPLICATE("pll_p", "tegra-dc.1", "parent"),
>>>>> + CLK_DUPLICATE("pll_d_out0", "tegra-hdmi", "parent"),
>>>>> };
>>>>
>>>> The same with my above comments, the tegra-dc.1's parent should be
>>>> pll_d_out0.
>>>
>>> The way this works is that for HDMI it is required that the DC and HDMI
>>> blocks have the same parent. So what really happens is that once you
>>> setup one of the DCs to work with HDMI, its clock will automatically be
>>> reparented to the HDMI parent clock, which in this case is "pll_d_out0".
>>>
>>
>> Are you sure about this? Is this a hardware feature? I know the dc and
>> hdmi controller should have the same clock parent but I think this
>> should be ensured by device driver...
>
> It's implemented in tegra_output_hdmi_setup_clock().
>

I see. Okay, thanks a lot.

> Thierry
>
> * Unknown Key
> * 0x7F3EB3A1
>

2012-11-14 08:32:22

by Terje Bergstrom

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 09.11.2012 15:20, Thierry Reding wrote:
> This commit adds the host1x node along with its children to the Tegra20
> DTSI. Furthermore the OF auxiliary data table is updated to have proper
> names assigned to the platform devices instantiated from the device
> tree. Moreover, the clocks required by host1x and the two display
> controllers are initialized and the pll_d frequency table is completed
> with a few entries to support common HDMI and LVDS display modes.

I tried to add nvhost on top of your patches and I noticed a glitch.

> + { "host1x", "pll_c", 144000000, false },

This line causes host1x not to operate correctly. I don't know why this
is so, but when I try to initialize host1x, it hangs with this change,
but everything works without this line.

If you could fix that,

Acked-by: Terje Bergstrom <[email protected]>

Best regards,
Terje

2012-11-14 08:49:38

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On Wed, Nov 14, 2012 at 10:35:31AM +0200, Terje Bergström wrote:
> On 09.11.2012 15:20, Thierry Reding wrote:
> > This commit adds the host1x node along with its children to the Tegra20
> > DTSI. Furthermore the OF auxiliary data table is updated to have proper
> > names assigned to the platform devices instantiated from the device
> > tree. Moreover, the clocks required by host1x and the two display
> > controllers are initialized and the pll_d frequency table is completed
> > with a few entries to support common HDMI and LVDS display modes.
>
> I tried to add nvhost on top of your patches and I noticed a glitch.
>
> > + { "host1x", "pll_c", 144000000, false },
>
> This line causes host1x not to operate correctly. I don't know why this
> is so, but when I try to initialize host1x, it hangs with this change,
> but everything works without this line.

Can you find out how the host1x clock is setup without this change? I
was told that freezes can occur when you try to access the registers
without the host1x clock being enabled. However, the host1x driver
should take care to properly setup the clock.

To find out if the non-running clock is the issue, can you try to patch
that line and make the final element true instead of false? That should
enable the clock on boot so that it should always be running.

Thierry


Attachments:
(No filename) (1.33 kB)
(No filename) (836.00 B)
Download all attachments

2012-11-14 10:20:36

by Terje Bergstrom

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 14.11.2012 10:49, Thierry Reding wrote:
> Can you find out how the host1x clock is setup without this change? I
> was told that freezes can occur when you try to access the registers
> without the host1x clock being enabled. However, the host1x driver
> should take care to properly setup the clock.
>
> To find out if the non-running clock is the issue, can you try to patch
> that line and make the final element true instead of false? That should
> enable the clock on boot so that it should always be running.

I tried with fastboot and U-Boot, and whenever that line is there,
kernel boot will halt at nvhost init. Same happens if I just change the
false to true.

nvhost will enable the clock and disable as it need. Also, part of
host1x initialization did proceed, but it ended up hanging after a few
registers were initialized. So it's not a case of host1x being off, but
host1x hanging after a while.

If I change this line to:

{ "host1x", "pll_p", 216000000, false },

it will also work properly. It looks like we have some problem with
pll_c in Tegra20, or clock configuration with your patch. In Tegra30,
pll_c with 144MHz seems to work fine, but on Tegra20, it doesn't.

In internal kernel, we use pll_c for host1x, so hardware shouldn't be
the problem here.

Best regards,
Terje

2012-11-14 10:54:13

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On Wed, Nov 14, 2012 at 12:23:42PM +0200, Terje Bergström wrote:
> On 14.11.2012 10:49, Thierry Reding wrote:
> > Can you find out how the host1x clock is setup without this change? I
> > was told that freezes can occur when you try to access the registers
> > without the host1x clock being enabled. However, the host1x driver
> > should take care to properly setup the clock.
> >
> > To find out if the non-running clock is the issue, can you try to patch
> > that line and make the final element true instead of false? That should
> > enable the clock on boot so that it should always be running.
>
> I tried with fastboot and U-Boot, and whenever that line is there,
> kernel boot will halt at nvhost init. Same happens if I just change the
> false to true.
>
> nvhost will enable the clock and disable as it need. Also, part of
> host1x initialization did proceed, but it ended up hanging after a few
> registers were initialized. So it's not a case of host1x being off, but
> host1x hanging after a while.
>
> If I change this line to:
>
> { "host1x", "pll_p", 216000000, false },
>
> it will also work properly. It looks like we have some problem with
> pll_c in Tegra20, or clock configuration with your patch. In Tegra30,
> pll_c with 144MHz seems to work fine, but on Tegra20, it doesn't.
>
> In internal kernel, we use pll_c for host1x, so hardware shouldn't be
> the problem here.

I suppose that if things work properly without this line, then we should
probably just drop it. Stephen, any objections?

Thierry


Attachments:
(No filename) (1.52 kB)
(No filename) (836.00 B)
Download all attachments

2012-11-14 15:01:36

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On Wed, Nov 14, 2012 at 10:35:31AM +0200, Terje Bergström wrote:
> On 09.11.2012 15:20, Thierry Reding wrote:
> > This commit adds the host1x node along with its children to the Tegra20
> > DTSI. Furthermore the OF auxiliary data table is updated to have proper
> > names assigned to the platform devices instantiated from the device
> > tree. Moreover, the clocks required by host1x and the two display
> > controllers are initialized and the pll_d frequency table is completed
> > with a few entries to support common HDMI and LVDS display modes.
>
> I tried to add nvhost on top of your patches and I noticed a glitch.
>
> > + { "host1x", "pll_c", 144000000, false },
>
> This line causes host1x not to operate correctly. I don't know why this
> is so, but when I try to initialize host1x, it hangs with this change,
> but everything works without this line.
>
> If you could fix that,

Funny. I just tested with this line removed and I also get the freeze.
With the line I don't get the freeze. Does the freeze only occur with
additional patches on top? If so I think we should keep the line in for
now because it is what most people have tested against and which has
proven to work. We can fix any remaining issues with host1x specific
things when actual patches emerge.

Thierry


Attachments:
(No filename) (1.28 kB)
(No filename) (836.00 B)
Download all attachments

2012-11-14 15:29:44

by Terje Bergstrom

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 14.11.2012 17:01, Thierry Reding wrote:
> Funny. I just tested with this line removed and I also get the freeze.
> With the line I don't get the freeze. Does the freeze only occur with
> additional patches on top? If so I think we should keep the line in for
> now because it is what most people have tested against and which has
> proven to work. We can fix any remaining issues with host1x specific
> things when actual patches emerge.

I was running with 2D acceleration support added on top of your
gitorious tree. It was the nvhost driver, which probes host1x, and
initializes it. In middle of initialization (writing zeros to sync point
registers), host1x stops responding.

I agree, if removing the line causes regression, keep it and let's debug
this issue later.

It might be that we have a difference in bootloader. Does your
bootloader enable display? U-Boot and fastboot do, and it might be that
has an effect. They would need to initialize host1x clocks, and it might
be that kernel initialization somehow clashes with bootloader's.

Best regards,
Terje

2012-11-14 15:33:56

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On Wed, Nov 14, 2012 at 05:29:33PM +0200, Terje Bergström wrote:
> On 14.11.2012 17:01, Thierry Reding wrote:
> > Funny. I just tested with this line removed and I also get the freeze.
> > With the line I don't get the freeze. Does the freeze only occur with
> > additional patches on top? If so I think we should keep the line in for
> > now because it is what most people have tested against and which has
> > proven to work. We can fix any remaining issues with host1x specific
> > things when actual patches emerge.
>
> I was running with 2D acceleration support added on top of your
> gitorious tree. It was the nvhost driver, which probes host1x, and
> initializes it. In middle of initialization (writing zeros to sync point
> registers), host1x stops responding.
>
> I agree, if removing the line causes regression, keep it and let's debug
> this issue later.
>
> It might be that we have a difference in bootloader. Does your
> bootloader enable display? U-Boot and fastboot do, and it might be that
> has an effect. They would need to initialize host1x clocks, and it might
> be that kernel initialization somehow clashes with bootloader's.

I use an upstream U-Boot version that performs no display initialization
on the board that I use. Perhaps that indeed causes your setup to work
properly but not mine.

Thierry


Attachments:
(No filename) (1.30 kB)
(No filename) (836.00 B)
Download all attachments

2012-11-14 16:19:22

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 11/14/2012 03:54 AM, Thierry Reding wrote:
> On Wed, Nov 14, 2012 at 12:23:42PM +0200, Terje Bergström wrote:
>> On 14.11.2012 10:49, Thierry Reding wrote:
>>> Can you find out how the host1x clock is setup without this
>>> change? I was told that freezes can occur when you try to
>>> access the registers without the host1x clock being enabled.
>>> However, the host1x driver should take care to properly setup
>>> the clock.
>>>
>>> To find out if the non-running clock is the issue, can you try
>>> to patch that line and make the final element true instead of
>>> false? That should enable the clock on boot so that it should
>>> always be running.
>>
>> I tried with fastboot and U-Boot, and whenever that line is
>> there, kernel boot will halt at nvhost init. Same happens if I
>> just change the false to true.
>>
>> nvhost will enable the clock and disable as it need. Also, part
>> of host1x initialization did proceed, but it ended up hanging
>> after a few registers were initialized. So it's not a case of
>> host1x being off, but host1x hanging after a while.
>>
>> If I change this line to:
>>
>> { "host1x", "pll_p", 216000000, false },
>>
>> it will also work properly. It looks like we have some problem
>> with pll_c in Tegra20, or clock configuration with your patch. In
>> Tegra30, pll_c with 144MHz seems to work fine, but on Tegra20, it
>> doesn't.
>>
>> In internal kernel, we use pll_c for host1x, so hardware
>> shouldn't be the problem here.
>
> I suppose that if things work properly without this line, then we
> should probably just drop it. Stephen, any objections?

I'd rather initialize it explicitly. If setting it to 216MHz works
fine as Terje indicated, we may as well just do that.

I suspect the issue with the original code:

> { "host1x", "pll_c", 144000000, false },

... is that perhaps the requested 144MHz can't be generated from
pll_c's 600MHz rate, since there's a simple U7.1 divider there (you
could get 120, 133.333, 150), so the clock ends up being programmed to
some incorrect value. In the pll_p/216MHz case, pll_p is programmed to
generate 216MHz anyway, so requesting the same rate for host1x yields
a divider of 1 exactly which works fine.

2012-11-14 16:45:35

by Terje Bergstrom

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 14.11.2012 18:19, Stephen Warren wrote:
> I'd rather initialize it explicitly. If setting it to 216MHz works
> fine as Terje indicated, we may as well just do that.

I'd prefer explicit setting, too.

> I suspect the issue with the original code:
>
>> { "host1x", "pll_c", 144000000, false },
>
> ... is that perhaps the requested 144MHz can't be generated from
> pll_c's 600MHz rate, since there's a simple U7.1 divider there (you
> could get 120, 133.333, 150), so the clock ends up being programmed to
> some incorrect value. In the pll_p/216MHz case, pll_p is programmed to
> generate 216MHz anyway, so requesting the same rate for host1x yields
> a divider of 1 exactly which works fine.

I could try the values you proposed tomorrow when I get back to office.
I believe we've always kept host1x under non-fractional dividers, so I'd
like to try 150MHz on Ventana and 150MHz and 300MHz on Cardhu.

600MHz sounds pretty high for PLLC on Tegra20. For Tegra30 it would be
understandable. In internal kernel I believe we have lower rate for
Tegra20 PLLC. Do we have anything running from PLLC in Tegra20 upstream
kernel?

Terje

2012-11-14 18:13:03

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 11/14/2012 09:45 AM, Terje Bergström wrote:
> On 14.11.2012 18:19, Stephen Warren wrote:
>> I'd rather initialize it explicitly. If setting it to 216MHz works
>> fine as Terje indicated, we may as well just do that.
>
> I'd prefer explicit setting, too.
>
>> I suspect the issue with the original code:
>>
>>> { "host1x", "pll_c", 144000000, false },
>>
>> ... is that perhaps the requested 144MHz can't be generated from
>> pll_c's 600MHz rate, since there's a simple U7.1 divider there (you
>> could get 120, 133.333, 150), so the clock ends up being programmed to
>> some incorrect value. In the pll_p/216MHz case, pll_p is programmed to
>> generate 216MHz anyway, so requesting the same rate for host1x yields
>> a divider of 1 exactly which works fine.
>
> I could try the values you proposed tomorrow when I get back to office.
> I believe we've always kept host1x under non-fractional dividers, so I'd
> like to try 150MHz on Ventana and 150MHz and 300MHz on Cardhu.
>
> 600MHz sounds pretty high for PLLC on Tegra20. For Tegra30 it would be
> understandable. In internal kernel I believe we have lower rate for
> Tegra20 PLLC. Do we have anything running from PLLC in Tegra20 upstream
> kernel?

Yes, sclk/... appear to derive from it:

> { "pll_c", "clk_m", 600000000, true },
> { "pll_c_out1", "pll_c", 120000000, true },
> { "sclk", "pll_c_out1", 120000000, true },
> { "hclk", "sclk", 120000000, true },
> { "pclk", "hclk", 60000000, true },

Git archaeology shows that the following commits are relevant, the first
and last one in particular:

> commit 9abafa021e223f04d6589ee2b977bbaf2e1f1367
> Author: Stephen Warren <[email protected]>
> Date: Thu Apr 12 14:13:05 2012 -0600
>
> ARM: tegra: change pll_p_out4's rate to 24MHz
>
> pll_p_out4 is used on all/most Tegra boards to drive the cdev2 output pin
> to provide a reference clock to a ULPI USB PHY. This reference clock must
> run at 24MHz, and the cdev2 output has no additional dividers.
>
> Remove board-paz00.c's now-duplicate initialization of this clock.
>
> Reported-by: Marc Dietrich <[email protected]>
> Signed-off-by: Stephen Warren <[email protected]>
>
> commit 7ff4db0967bd7d617c77dc5a66c0d95166277817
> Author: Stephen Warren <[email protected]>
> Date: Fri Apr 20 16:58:18 2012 -0600
>
> ARM: tegra: fix pclk rate
>
> Commit 40f9cf0 "ARM: tegra: reparent sclk to pll_c_out1" changed the
> rate of hclk. Since pclk is derived from that, and only has integer
> dividers, the pclk rate needs to change in the same fashion, from 54MHz
> to 60MHz.
>
> Signed-off-by: Stephen Warren <[email protected]>
>
> commit 60f975b98cf41476ba0e156f7523b197b046cf2b
> Author: Stephen Warren <[email protected]>
> Date: Thu Apr 12 14:09:39 2012 -0600
>
> ARM: tegra: reparent sclk to pll_c_out1
>
> pll_p_out4 needs to be used for other purposes. Reparent sclk so that
> it runs from pll_c. Change sclk's rate to 120MHz from 108MHz since this
> is the lowest precise rate that can be achieved by dividing the pll_c
> rate without reducing the sclk rate. (600/5=120, 600/5.5=109.0909...,
> 600/6=100).
>
> Signed-off-by: Stephen Warren <[email protected]>
>
> commit c8b62ab41f76218efca5e4baa5c22ef52a9fe3a5
> Author: Allen Martin <[email protected]>
> Date: Fri Sep 10 09:17:33 2010 -0500
>
> ARM: tegra: Add pllc clock init table
>
> pll_c will be used as a clock source. Fill in tegra_pll_c_freq_table[]
> so that it's possible to explicitly initialize the PLL.
>
> NVIDIA's downstream nv-3.1 kernel and the ChromeOS kernel have different
> pll_c tables. nv-3.1 contains entries for 522MHz and 598MHz output,
> whereas the ChromeOS kernel contains entries for 600MHz output. I chose
> to upstream the ChromeOS values for now, since the 600MHz rate appears
> to match the default rate of this PLL when the HW boots, and it's not
> clear to me why 522 or 598MHz are more useful.
>
> Signed-off-by: Allen Martin <[email protected]>
> Signed-off-by: Olof Johansson <[email protected]>
> Signed-off-by: Stephen Warren <[email protected]>
> [swarren: wrote commit description]

2012-11-14 20:04:24

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On Wed, Nov 14, 2012 at 09:19:17AM -0700, Stephen Warren wrote:
> On 11/14/2012 03:54 AM, Thierry Reding wrote:
> > On Wed, Nov 14, 2012 at 12:23:42PM +0200, Terje Bergström wrote:
> >> On 14.11.2012 10:49, Thierry Reding wrote:
> >>> Can you find out how the host1x clock is setup without this
> >>> change? I was told that freezes can occur when you try to
> >>> access the registers without the host1x clock being enabled.
> >>> However, the host1x driver should take care to properly setup
> >>> the clock.
> >>>
> >>> To find out if the non-running clock is the issue, can you try
> >>> to patch that line and make the final element true instead of
> >>> false? That should enable the clock on boot so that it should
> >>> always be running.
> >>
> >> I tried with fastboot and U-Boot, and whenever that line is
> >> there, kernel boot will halt at nvhost init. Same happens if I
> >> just change the false to true.
> >>
> >> nvhost will enable the clock and disable as it need. Also, part
> >> of host1x initialization did proceed, but it ended up hanging
> >> after a few registers were initialized. So it's not a case of
> >> host1x being off, but host1x hanging after a while.
> >>
> >> If I change this line to:
> >>
> >> { "host1x", "pll_p", 216000000, false },
> >>
> >> it will also work properly. It looks like we have some problem
> >> with pll_c in Tegra20, or clock configuration with your patch. In
> >> Tegra30, pll_c with 144MHz seems to work fine, but on Tegra20, it
> >> doesn't.
> >>
> >> In internal kernel, we use pll_c for host1x, so hardware
> >> shouldn't be the problem here.
> >
> > I suppose that if things work properly without this line, then we
> > should probably just drop it. Stephen, any objections?
>
> I'd rather initialize it explicitly. If setting it to 216MHz works
> fine as Terje indicated, we may as well just do that.
>
> I suspect the issue with the original code:
>
> > { "host1x", "pll_c", 144000000, false },
>
> ... is that perhaps the requested 144MHz can't be generated from
> pll_c's 600MHz rate, since there's a simple U7.1 divider there (you
> could get 120, 133.333, 150), so the clock ends up being programmed to
> some incorrect value. In the pll_p/216MHz case, pll_p is programmed to
> generate 216MHz anyway, so requesting the same rate for host1x yields
> a divider of 1 exactly which works fine.

According to tegra20_clocks_data.c, the maximum clock frequency for
host1x is 166 MHz, so 216 is probably not a good idea. 150 MHz sounds
sensible, though.

I was going to send a new version of the patch set tonight, but I'll
wait until I can test it tomorrow and once Terje has reported back that
things work fine.

Thierry


Attachments:
(No filename) (2.67 kB)
(No filename) (836.00 B)
Download all attachments

2012-11-14 20:15:59

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 11/14/2012 01:04 PM, Thierry Reding wrote:
...
> According to tegra20_clocks_data.c, the maximum clock frequency
> for host1x is 166 MHz, so 216 is probably not a good idea. 150 MHz
> sounds sensible, though.

As a general rule, I wouldn't rely on the upstream clock driver to
specify accurate maximum clock rates at the moment. Our downstream
kernel is more likely to be accurate. Still, upstream might be correct
here already...

2012-11-14 20:22:09

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On Wed, Nov 14, 2012 at 01:15:54PM -0700, Stephen Warren wrote:
> On 11/14/2012 01:04 PM, Thierry Reding wrote:
> ...
> > According to tegra20_clocks_data.c, the maximum clock frequency
> > for host1x is 166 MHz, so 216 is probably not a good idea. 150 MHz
> > sounds sensible, though.
>
> As a general rule, I wouldn't rely on the upstream clock driver to
> specify accurate maximum clock rates at the moment. Our downstream
> kernel is more likely to be accurate. Still, upstream might be correct
> here already...

I just checked and the downstream kernel has the same maximum clock rate
for host1x.

Thierry


Attachments:
(No filename) (613.00 B)
(No filename) (836.00 B)
Download all attachments

2012-11-15 06:53:08

by Terje Bergstrom

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On 14.11.2012 22:04, Thierry Reding wrote:
> According to tegra20_clocks_data.c, the maximum clock frequency for
> host1x is 166 MHz, so 216 is probably not a good idea. 150 MHz sounds
> sensible, though.
>
> I was going to send a new version of the patch set tonight, but I'll
> wait until I can test it tomorrow and once Terje has reported back that
> things work fine.

Hi,

I tried with 150MHz, 133.3MHz, 100MHz and as an act of desperation
300MHz. None of them worked. pll_p with 216MHz seems to be the only
option that works on my board.

But, as said, this seems to affect only the case where nvhost is
integrated, too, so your patch creates a working DRM and frame buffer,
so I'm fine with leaving it as it was in the original patch set.

Meanwhile, we need to figure out if we somehow misprogram PLLC or host1x
clock when it's attached to PLLC on Tegra20.

Terje

2012-11-15 07:11:08

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 1/2] ARM: tegra: Add Tegra20 host1x support

On Thu, Nov 15, 2012 at 08:56:17AM +0200, Terje Bergström wrote:
> On 14.11.2012 22:04, Thierry Reding wrote:
> > According to tegra20_clocks_data.c, the maximum clock frequency for
> > host1x is 166 MHz, so 216 is probably not a good idea. 150 MHz sounds
> > sensible, though.
> >
> > I was going to send a new version of the patch set tonight, but I'll
> > wait until I can test it tomorrow and once Terje has reported back that
> > things work fine.
>
> Hi,
>
> I tried with 150MHz, 133.3MHz, 100MHz and as an act of desperation
> 300MHz. None of them worked. pll_p with 216MHz seems to be the only
> option that works on my board.
>
> But, as said, this seems to affect only the case where nvhost is
> integrated, too, so your patch creates a working DRM and frame buffer,
> so I'm fine with leaving it as it was in the original patch set.

I just tried with 150 MHz as well and it seems to work properly. So as
Stephen pointed out, 144 MHz is not a supported frequency for host1x,
and therefore I'll just leave the valid (and working) 150 MHz in.

> Meanwhile, we need to figure out if we somehow misprogram PLLC or host1x
> clock when it's attached to PLLC on Tegra20.

Yes, absolutely. Among other things there's still the upcoming and much
needed rework of the PLL frequency table code, so while at that we could
look at the issue you're seeing as well.

Thierry


Attachments:
(No filename) (1.34 kB)
(No filename) (836.00 B)
Download all attachments

2012-11-15 08:21:53

by Prashant Gaikwad

[permalink] [raw]
Subject: Re: [PATCH 0/2] Device tree updates for host1x support

On Saturday 10 November 2012 02:40 AM, Stephen Warren wrote:
> On 11/09/2012 11:44 AM, Thierry Reding wrote:
>> On Fri, Nov 09, 2012 at 10:34:41AM -0700, Stephen Warren wrote:
> ...
>>> I really hope soon that Tegra will support DT bindings for clocks
>>> so we can get rid of the AUXDATA, but unfortunately I haven't
>>> seen any movement towards this:-(
>> Oh yes, please. We wouldn't have any of the above problem if we
>> had proper DT bindings for this. Who needs to be prodded to speed
>> this up?
> I believe one of Peter De Schrijver or Prashant Gaikwad are supposed
> to be working on this.
I am working on it, will send patches soon.

Thanks& Regards,
PrashantG

2012-11-15 08:51:02

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 0/2] Device tree updates for host1x support

On Thu, Nov 15, 2012 at 01:51:46PM +0530, Prashant Gaikwad wrote:
> On Saturday 10 November 2012 02:40 AM, Stephen Warren wrote:
> >On 11/09/2012 11:44 AM, Thierry Reding wrote:
> >>On Fri, Nov 09, 2012 at 10:34:41AM -0700, Stephen Warren wrote:
> >...
> >>>I really hope soon that Tegra will support DT bindings for clocks
> >>>so we can get rid of the AUXDATA, but unfortunately I haven't
> >>>seen any movement towards this:-(
> >>Oh yes, please. We wouldn't have any of the above problem if we
> >>had proper DT bindings for this. Who needs to be prodded to speed
> >>this up?
> >I believe one of Peter De Schrijver or Prashant Gaikwad are supposed
> >to be working on this.
> I am working on it, will send patches soon.

Excellent. I'm very much looking forward to this.

Thierry


Attachments:
(No filename) (785.00 B)
(No filename) (836.00 B)
Download all attachments